linux-kernel - Re: [RFC 2.6.27 1/1] gpiolib: add support for batch set of pins

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:	Sat, 29 Nov 2008 14:47:28 -0800
From:	David Brownell <david-b@...bell.net>
To:	"Jaya Kumar" <jayakumar.lkml@...il.com>,
	"Paulius Zaleckas" <paulius.zaleckas@...tonika.lt>
Cc:	"Sam Ravnborg" <sam@...nborg.org>,
	"Eric Miao" <eric.miao@...vell.com>,
	"Haavard Skinnemoen" <hskinnemoen@...el.com>,
	"Philipp Zabel" <philipp.zabel@...il.com>,
	"Russell King" <rmk@....linux.org.uk>,
	"Ben Gardner" <bgardner@...tec.com>, "Greg KH" <greg@...ah.com>,
	linux-arm-kernel@...ts.arm.linux.org.uk,
	linux-fbdev-devel@...ts.sourceforge.net,
	linux-kernel@...r.kernel.org
Subject: Re: [RFC 2.6.27 1/1] gpiolib: add support for batch set of pins

On Wednesday 26 November 2008, Jaya Kumar wrote:
> On Wed, Nov 26, 2008 at 4:09 AM, Paulius Zaleckas wrote:
> > Jaya Kumar wrote:
> >>       void                    (*set)(struct gpio_chip *chip,
> >>                                               unsigned offset, int value);
> >> +     void                    (*set_bus)(struct gpio_chip *chip,
> >> +                                             unsigned offset, int values,
> >
> > I think values should be unsigned
> 
> Okay, can do but it is unusual no?

For bitmasks, anything except "unsigned long" is unusual.
In fact, <linux/bitmask.h> uses them in arrays...

If this goes through, I suspect it would be fair to expect
a gpio_chip to work with only one word at at time.  SOC based
GPIO controllers stick to their natural word sizes (32 or 16
bits in most cases), in arrays, and I've yet to come across
external controllers that are any different.  So passing in
arrays of "unsigned long" would seem to be overkill.

However, "set_bus" is seems misleading to me:  "set" because
it makes me think it's writing to the set_bits register,
which won't clear things; "bus" because this is doing ganged
operations, which aren't only for busses ... and in fact a
bus operation probably needs multiple ganged operations,
e.g. first write the address, handshake, then write data and
handshake again.

Maybe words like "assign" and "bitmask" would get past those
particular issues... though I don't hugely like "assign".

In terms of low level primitives, I've already commented
that the "read" side is missing.  An additional issue came
to mind:  the policy of using contiguous bits should not
be mandated here.  It doesn't need to be, either ... just
pass a mask of valid bits, along with a mask of values.

> since set uses int value, i figured set_bus should be similar right?

It doesn't take a bitmask; that's a single zero/nonzero value,
for which the sign (bit) is irrelevant.  It's signed mostly to
distinguish the two parameters by type, so it's less easy to
confuse them; sometimes the compiler will point out goofage.
(Plus, the offsets can ever be negative.)

- Dave
--
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/