lkml.org 
[lkml]   [2008]   [Dec]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [RFC 2.6.27 1/1] gpiolib: add batch set/get
    Date
    I'm a bit surprised to see patches against 2.6.27, rather
    than a 2.6.28 (or 2.6.28-rc) kernel. ;)


    On Sunday 28 December 2008, Eric Miao wrote:
    > > @@ -200,8 +203,12 @@ static void am300_set_hdb(struct broadsheetfb_par *par, u16 data)
    > > {
    > > int i;
    > >
    > > +#ifdef CONFIG_GPIOLIB_BATCH
    > > + gpio_set_batch(DB0_GPIO_PIN, data, 0xFFFF, 16);
    > > +#else
    > > for (i = 0; i <= (DB15_GPIO_PIN - DB0_GPIO_PIN) ; i++)
    > > gpio_set_value(DB0_GPIO_PIN + i, (data >> i) & 0x01);
    > > +#endif
    >
    > Well, if AM300 selects GPIOLIB_BATCH, I don't think we need the
    > gpio_set_value() stuffs, and get rid of this #ifdef completely.

    Right ... although we don't *have* a GPIOLIB_BATCH,
    so that's not (yet?) an option.


    > > @@ -1056,6 +1056,128 @@ void __gpio_set_value(unsigned gpio, int value)
    > > }
    > > EXPORT_SYMBOL_GPL(__gpio_set_value);
    > >
    > > +#ifdef CONFIG_GPIOLIB_BATCH
    > > +/**
    > > + * __gpio_set_batch() - assign a batch of gpio pins together
    > > + * @gpio: starting gpio pin
    > > + * @values: values to assign, sequential and including masked bits
    > > + * @bitmask: bitmask to be applied to values
    > > + * @bitwidth: width inclusive of masked-off bits
    > > + * Context: any
    > > + *
    > > + * This is used directly or indirectly to implement gpio_set_value().
    > > + * It invokes the associated gpio_chip.set_batch() method. If that
    > > + * method does not exist for any segment that is involved, then it drops
    > > + * back down to standard gpio_chip.set()
    > > + */
    > > +void __gpio_set_batch(unsigned gpio, u32 values, u32 bitmask, int bitwidth)
    > > +{
    > > + struct gpio_chip *chip;
    > > + int i = 0;
    > > + int value, width, remwidth;
    > > + u32 mask;
    > > +
    > > + do {
    > > + chip = gpio_to_chip(gpio + i);
    > > + WARN_ON(extra_checks && chip->can_sleep);
    > > +
    > > + if (!chip->set_batch) {
    > > + while (((gpio + i) < (chip->base + chip->ngpio))
    > > + && bitwidth) {
    > > + mask = 1 << i;
    > > + value = values & mask;
    > > + if (bitmask & mask)
    > > + chip->set(chip, gpio + i - chip->base,
    > > + value);
    > > + i++;
    > > + bitwidth--;
    >
    > I recommend this being put into something like 'default_gpio_set_batch', and
    > assign this to 'chip->set_batch' when the gpio chip is being registered and
    > found 'chip->set_batch == NULL', so to keep this block consistent.
    >
    > Same comment to the 'get_batch' implementation below.

    Right ... this is something I had suggested earlier: make
    sure that the (renamed) "batch" interfaces don't depend on
    some TBD extension to gpio_chip.

    Those extensions should be just an optimization.



    \
     
     \ /
      Last update: 2008-12-29 21:39    [W:2.365 / U:0.084 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site