[lkml]   [2009]   [Jan]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [RFC 2.6.28 1/2] gpiolib: add set/get batch v4
    Hi Jaya,

    On Sat, Jan 17, 2009 at 05:57:17PM +0800, Jaya Kumar wrote:
    > Hi friends,
    > This is v4 of batch support for gpiolib. Thanks to David Brownell, Eric Miao,
    > David Hylands, Robin, Ben, Jamie and others for prior feedback. The post for
    > v3 summarized the previous discussion. Since then the changes I've made are:
    > - split the patches into generic and arch specific
    IMHO this should be three patches: "gpiolib", "pxa" and "am300epd".
    Well, ...

    > +[OPTIONAL] Spinlock-Safe GPIO Batch access
    Is it really spinlock safe in general? Or only if gpio_cansleep(gpio)
    if false for each gpio to get or set?

    > +static int __generic_gpio_set_batch(struct gpio_chip *chip, unsigned offset,
    > + u32 values, u32 bitmask, int width)
    IMHO a better name is __gpio_set_batch_generic (or
    __gpiolib_set_batch_generic?), YMMV.

    > +int __gpio_set_batch(unsigned gpio, u32 values, u32 bitmask, int bitwidth)
    Sometimes your width parameter is called bitwidth, sometimes width. I'd
    like to have that consistant.

    While talking about this parameter. I don't really like it, because you
    can calculate it from bitmask. In an earlier mail you write:

    bitwidth (needed to iterate and map to chip ngpios) could be
    calculated from bitmask, but that involves iteratively counting
    bits from the mask, so we would have to do 800*600 bit counts.
    Unless, we do ugly things like cache the previous bitwidth/mask
    and compare against the current caller arguments. Given that the
    bitwidth would typically be a fixed value, I believe it is more
    intuitive for the caller to provide it, ...

    I think it's easier than that. bitwidth is just fls(bitmask) which
    should be efficient enough not to bother the programmer. If bitmask is
    constant it's even the compiler that does the work here.

    BTW, I wonder why the argument to fls has type int and not unsigned.

    Best regards,

    Pengutronix e.K. | Uwe Kleine-König |
    Industrial Linux Solutions | |
    Peiner Strasse 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
    Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2009-01-19 11:07    [W:0.035 / U:67.300 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site