linux-kernel - RE: [PATCH] gpio: pxa: fix bug when get gpio value

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 9 Jan 2014 18:04:18 -0800
From:	Neil Zhang <zhangwm@...vell.com>
To:	Gerhard Sittig <gsi@...x.de>
CC:	"linus.walleij@...aro.org" <linus.walleij@...aro.org>,
	"gnurou@...il.com" <gnurou@...il.com>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] gpio: pxa: fix bug when get gpio value

Gerhard

Firstly thanks for your comments.

> -----Original Message-----
> From: Gerhard Sittig [mailto:gsi@...x.de]
> Sent: 2014年1月9日 18:46
> To: Neil Zhang
> Cc: linus.walleij@...aro.org; gnurou@...il.com; linux-gpio@...r.kernel.org;
> linux-kernel@...r.kernel.org
> 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: office@...x.de

Best Regards,
Neil Zhang