Gerhard Firstly thanks for your comments.
> -----Original Message----- > From: Gerhard Sittig [mailto:[email protected]] > Sent: 2014年1月9日 18:46 > To: Neil Zhang > Cc: [email protected]; [email protected]; [email protected]; > [email protected] > Subject: Re: [PATCH] gpio: pxa: fix bug when get gpio value > > On Thu, Jan 09, 2014 at 17:25 +0800, Neil Zhang wrote: > > > > gpio_get_value should return 0 or 1. > > > > I have checked mach-mmp / mach-pxa / plat-pxa / plat-orion / mach-orion5x. > > It's OK for all of them to change this function to return 0 and 1. > > > > [ ... ] > > > > static int pxa_gpio_get(struct gpio_chip *chip, unsigned offset) { > > - return readl_relaxed(gpio_chip_base(chip) + GPLR_OFFSET) & (1 << > offset); > > + u32 gplr = readl_relaxed(gpio_chip_base(chip) + GPLR_OFFSET); > > + return !!(gplr & (1 << offset)); > > } > > This may be a stupid question, but isn't the "!!value" syntax just replacing > one > kind of "zero or non-zero" with another kind of "zero or non-zero"? This > phrase is used to normalize booleans, but ISTR there is no guarantee that true > needs to equal the value of 1. > > Here is why I'm asking: Is there a need from GPIO get_value() routines to > return normalized values, and if so should not more drivers receive an update? > Or need get_value() routines just return the usual integer zero/non-zero > values > (as is the convention in the C programming language) and GPIO using callers > should know that they are not exactly 0 and 1? > > The latter would make this change for PXA unneeded (nice to have but not > essential), and both the commit message as well as the subject line at least > need > to get re-phrased, as the "bug" is in the caller's expectation and not in the > GPIO > chip driver. > Yes, the API doesn't mandatory expect 0 and 1 as I can see. But actually many driver will return a normalized value. So people may think it's return value is 0 or 1. It would be convenient to normalized the return value for gpio-pxa too. You are right, it's not appropriate to use "bug" in the patch title, thanks. > Please note that I'm not questioning the patches' being applicable, but am > trying > to find out what else needs to be done which previously may have gone > unnoticed. A doc update maybe, to make GPIO users aware that the return > value may not be exactly 1, and to clear up where such an assumption is > wrongly > made. > > If the GPIO subsystem's API wants to guarantee values of 0 and 1 (which I > think it > doesn't), then I feel the adjustment should be done in the gpio_get_value() > routines (in all its public variants, or a common routine which all of them > pass > through), and certainly not in individual chip drivers. > > > virtually yours > Gerhard Sittig > -- > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel > HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany > Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: [email protected] Best Regards, Neil Zhang
