| Message ID | 20241213-gpio74-v1-4-fa2c089caf41@posteo.net |
|---|---|
| State | New |
| Headers | show |
| Series | gpio: 74HC595 / 74x164 shift register improvements | expand |
On Fri, Dec 13, 2024 at 6:32 PM J. Neuschäfer via B4 Relay <devnull+j.ne.posteo.net@kernel.org> wrote: > From: "J. Neuschäfer" <j.ne@posteo.net> > > The Fairchild MM74HC595 and other compatible parts have a latch clock > input (also known as storage register clock input), which must be > clocked once in order to apply any value that was serially shifted in. > > This patch adds driver support for using a GPIO that connects to the > latch clock. > > Signed-off-by: J. Neuschäfer <j.ne@posteo.net> This looks completely reasonable to me as far as 2/4 gets merged: Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
On Sun, Dec 22, 2024 at 09:58:39AM +0100, Linus Walleij wrote: > On Fri, Dec 13, 2024 at 6:32 PM J. Neuschäfer via B4 Relay > <devnull+j.ne.posteo.net@kernel.org> wrote: > > > From: "J. Neuschäfer" <j.ne@posteo.net> > > > > The Fairchild MM74HC595 and other compatible parts have a latch clock > > input (also known as storage register clock input), which must be > > clocked once in order to apply any value that was serially shifted in. > > > > This patch adds driver support for using a GPIO that connects to the > > latch clock. > > > > Signed-off-by: J. Neuschäfer <j.ne@posteo.net> > > This looks completely reasonable to me as far as 2/4 gets merged: > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> I think I prefer the other option, of documenting that the latch clock pin pretty much behaves as a chip select. Having a separately described latch clock would mean no CS for these chips, and the SPI bindings and drivers don't expect devices without CS. -- jn
diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c index 187032efa5b5cd1aa7aea7b2d55f6c06df4ccac4..8e87eeb7a1c7a8c71079c8d837dc5c426db8b65b 100644 --- a/drivers/gpio/gpio-74x164.c +++ b/drivers/gpio/gpio-74x164.c @@ -7,6 +7,7 @@ */ #include <linux/bitops.h> +#include <linux/delay.h> #include <linux/gpio/consumer.h> #include <linux/gpio/driver.h> #include <linux/module.h> @@ -21,6 +22,7 @@ struct gen_74x164_chip { struct gpio_chip gpio_chip; struct mutex lock; struct gpio_desc *gpiod_oe; + struct gpio_desc *gpiod_latch; u32 registers; /* * Since the registers are chained, every byte sent will make @@ -34,8 +36,20 @@ struct gen_74x164_chip { static int __gen_74x164_write_config(struct gen_74x164_chip *chip) { - return spi_write(to_spi_device(chip->gpio_chip.parent), chip->buffer, + int ret; + + ret = spi_write(to_spi_device(chip->gpio_chip.parent), chip->buffer, chip->registers); + if (ret) + return ret; + + if (chip->gpiod_latch) { + gpiod_set_value_cansleep(chip->gpiod_latch, 1); + udelay(1); + gpiod_set_value_cansleep(chip->gpiod_latch, 0); + } + + return 0; } static int gen_74x164_get_value(struct gpio_chip *gc, unsigned offset) @@ -127,6 +141,11 @@ static int gen_74x164_probe(struct spi_device *spi) if (IS_ERR(chip->gpiod_oe)) return PTR_ERR(chip->gpiod_oe); + chip->gpiod_latch = devm_gpiod_get_optional(&spi->dev, "latch", + GPIOD_OUT_LOW); + if (IS_ERR(chip->gpiod_latch)) + return PTR_ERR(chip->gpiod_latch); + spi_set_drvdata(spi, chip); chip->gpio_chip.label = spi->modalias;