linux-kernel - Re: [patch 2.6.20-rc1 4/6] PXA GPIO wrappers

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, 21 Dec 2006 15:10:42 -0500 (EST)
From:	Nicolas Pitre <nico@....org>
To:	pHilipp Zabel <philipp.zabel@...il.com>
Cc:	David Brownell <david-b@...bell.net>,
	Andrew Morton <akpm@...l.org>,
	Linux Kernel list <linux-kernel@...r.kernel.org>,
	Andrew Victor <andrew@...people.com>,
	Bill Gatliff <bgat@...lgatliff.com>,
	Haavard Skinnemoen <hskinnemoen@...el.com>,
	Kevin Hilman <khilman@...sta.com>,
	Russell King <rmk@....linux.org.uk>,
	Tony Lindgren <tony@...mide.com>
Subject: Re: [patch 2.6.20-rc1 4/6] PXA GPIO wrappers

On Thu, 21 Dec 2006, pHilipp Zabel wrote:

> > > --- linux-2.6.orig/arch/arm/mach-pxa/generic.c        2006-12-16
> > > +++ linux-2.6/arch/arm/mach-pxa/generic.c     2006-12-16
> > > 16:47:45.000000000
> > > @@ -129,6 +129,29 @@
> > > EXPORT_SYMBOL(pxa_gpio_mode);
> > >
> > > /*
> > > + * Return GPIO level, nonzero means high, zero is low
> > > + */
> > > +int pxa_gpio_get_value(unsigned gpio)
> > > +{
> > > +     return GPLR(gpio) & GPIO_bit(gpio);
> > > +}
> > > +
> > > +EXPORT_SYMBOL(pxa_gpio_get_value);
> > > +
> > > +/*
> > > + * Set output GPIO level
> > > + */
> > > +void pxa_gpio_set_value(unsigned gpio, int value)
> > > +{
> > > +     if (value)
> > > +             GPSR(gpio) = GPIO_bit(gpio);
> > > +     else
> > > +             GPCR(gpio) = GPIO_bit(gpio);
> > > +}
> > > +
> > > +EXPORT_SYMBOL(pxa_gpio_set_value);
> >
> > Instead of duplicating code here, you probably should just reuse
> > __gpio_set_value() and __gpio_get_value() inside those functions.
> 
> Probably? What I am wondering is this: can the compiler
> optimize away the range check that is duplicated in GPSR/GPCR
> and  GPIO_bit for __gpio_set/get_value? Or could we optimize
> this case by expanding the macros in place (which would mean
> duplicating code from pxa-regs.h)...

Sorry I don't quite follow you here.  Why would you expand the macro in 
place?

My suggestion is only about not duplicating the source code.  The 
generated assembly will be the same.

And your patch looks fine to me now, except for this:

+int pxa_gpio_get_value(unsigned gpio)
+{
+       __gpio_get_value(gpio);
+}

You certainly meant to add a "return" in there, right?


Nicolas
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/