| Message ID | 1457968758-22670-1-git-send-email-geert+renesas@glider.be |
|---|---|
| State | New |
| Headers | show |
On 14/03/2016 11:19 PM, Geert Uytterhoeven wrote: > This allows to set multiple outputs using a single SPI transfer. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> Reviewed-by: Phil Reid <preid@electromag.com.au> I do have a general question about GPIO drivers. pca953x does not update the cached data unless the write operation was successful. Which I folowed with the implement of set_multiple. However a number of other drivers update regardless. eg chip->buffer is updated even if the write_config fails. What is the preferred approach? > --- > drivers/gpio/gpio-74x164.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c > index c81224ff2dca988b..62291a81c97f7140 100644 > --- a/drivers/gpio/gpio-74x164.c > +++ b/drivers/gpio/gpio-74x164.c > @@ -75,6 +75,29 @@ static void gen_74x164_set_value(struct gpio_chip *gc, > mutex_unlock(&chip->lock); > } > > +static void gen_74x164_set_multiple(struct gpio_chip *gc, unsigned long *mask, > + unsigned long *bits) > +{ > + struct gen_74x164_chip *chip = gpiochip_get_data(gc); > + unsigned int i, idx, shift; > + u8 bank, bankmask; > + > + mutex_lock(&chip->lock); > + for (i = 0, bank = chip->registers - 1; i < chip->registers; > + i++, bank--) { > + idx = i / sizeof(*mask); > + shift = i % sizeof(*mask) * BITS_PER_BYTE; > + bankmask = mask[idx] >> shift; > + if (!bankmask) > + continue; > + > + chip->buffer[bank] &= ~bankmask; > + chip->buffer[bank] |= bankmask & (bits[idx] >> shift); > + } > + __gen_74x164_write_config(chip); > + mutex_unlock(&chip->lock); > +} > + > static int gen_74x164_direction_output(struct gpio_chip *gc, > unsigned offset, int val) > { > @@ -114,6 +137,7 @@ static int gen_74x164_probe(struct spi_device *spi) > chip->gpio_chip.direction_output = gen_74x164_direction_output; > chip->gpio_chip.get = gen_74x164_get_value; > chip->gpio_chip.set = gen_74x164_set_value; > + chip->gpio_chip.set_multiple = gen_74x164_set_multiple; > chip->gpio_chip.base = -1; > > chip->registers = nregs; >
On Mon, Mar 14, 2016 at 4:19 PM, Geert Uytterhoeven <geert+renesas@glider.be> wrote: > This allows to set multiple outputs using a single SPI transfer. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> Patch applied for v4.7 with Phil's Review tag. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 15, 2016 at 5:04 AM, Phil Reid <preid@electromag.com.au> wrote: > pca953x does not update the cached data unless the write operation > was successful. Which I folowed with the implement of set_multiple. > However a number of other drivers update regardless. > eg chip->buffer is updated even if the write_config fails. > > What is the preferred approach? Obviously the other drivers are buggy and need to be fixed. It can't be too many since the number of drivers with failable write operations aren's so many. (And I guess they seldom fail so it's not a big real world problem.) But it should be fixed. Add Axel Lin to the thread, he's awesome at finding semantic bugs. Patches accepted :) Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c index c81224ff2dca988b..62291a81c97f7140 100644 --- a/drivers/gpio/gpio-74x164.c +++ b/drivers/gpio/gpio-74x164.c @@ -75,6 +75,29 @@ static void gen_74x164_set_value(struct gpio_chip *gc, mutex_unlock(&chip->lock); } +static void gen_74x164_set_multiple(struct gpio_chip *gc, unsigned long *mask, + unsigned long *bits) +{ + struct gen_74x164_chip *chip = gpiochip_get_data(gc); + unsigned int i, idx, shift; + u8 bank, bankmask; + + mutex_lock(&chip->lock); + for (i = 0, bank = chip->registers - 1; i < chip->registers; + i++, bank--) { + idx = i / sizeof(*mask); + shift = i % sizeof(*mask) * BITS_PER_BYTE; + bankmask = mask[idx] >> shift; + if (!bankmask) + continue; + + chip->buffer[bank] &= ~bankmask; + chip->buffer[bank] |= bankmask & (bits[idx] >> shift); + } + __gen_74x164_write_config(chip); + mutex_unlock(&chip->lock); +} + static int gen_74x164_direction_output(struct gpio_chip *gc, unsigned offset, int val) { @@ -114,6 +137,7 @@ static int gen_74x164_probe(struct spi_device *spi) chip->gpio_chip.direction_output = gen_74x164_direction_output; chip->gpio_chip.get = gen_74x164_get_value; chip->gpio_chip.set = gen_74x164_set_value; + chip->gpio_chip.set_multiple = gen_74x164_set_multiple; chip->gpio_chip.base = -1; chip->registers = nregs;
This allows to set multiple outputs using a single SPI transfer. Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- drivers/gpio/gpio-74x164.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)