re: port-arm/52163: support for gpio for Xscale

[matthew green <mrg%eterna.com.au@localhost>]

The following reply was made to PR port-arm/52163; it has been noted by GNATS.

From: matthew green <mrg%eterna.com.au@localhost>
To: gnats-bugs%NetBSD.org@localhost, smesgr%gmail.com@localhost
Cc: port-arm-maintainer%netbsd.org@localhost, gnats-admin%netbsd.org@localhost,
    netbsd-bugs%netbsd.org@localhost
Subject: re: port-arm/52163: support for gpio for Xscale
Date: Sun, 04 Jun 2017 06:03:34 +1000

 > +.Cd "pxagpio0 at pxaip?"
 > +.Cd "gpio* at gpiobus?"
 
 this part (forcing to unit zero) and below ..
 
 > +#if NGPIO > 0
 > +/* GPIO support functions */
 > +static int
 > +pxa2x0_gpio_pin_read(void *arg, int pin)
 > +{
 > +	struct pxagpio_softc *sc = arg;
 > +
 > +	if (device_unit(sc->sc_dev) > 0) {
 > +		return 0;
 > +	}
 > +
 > +	return pxa2x0_gpio_get_bit(pin);
 > +}
 
 this is what we call "cf_unit abuse".  the driver code hard-codes
 the definition of a device unit, where as this should not be the
 case in almost every case.
 
 i *think* you can simply remove all these checks for device_unit()
 and mark the device as "pxagpio* at pxaip?" in the manual.  on this
 hardware, only one will be found and the cf_unit doesn't matter.
 
 > +	if(flags & GPIO_PIN_OUTPUT) {
 > +		pxa2x0_gpio_set_function(pin, GPIO_OUT);
 
 nit:  "if (" not "if(".
 
 > +	} else if(flags & GPIO_PIN_INPUT) {
 
 and here too..
 
 another nit: the new pin loop in attach should be formatted properly,
 
 
 can you make these changes, test and re-send?  thanks.
 
 
 .mrg.