[PATCH 0/4] gpio: 74HC595 / 74x164 shift register improvements

[PATCH 0/4] gpio: 74HC595 / 74x164 shift register improvements

[J. Neuschäfer via B4 Relay]

This patchset adds a compatible string for another part, and adds the
ability to specify and use the latch clock pin on 74x164-compatible
shift registers.

Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
---
J. Neuschäfer (4):
      dt-bindings: gpio: fairchild,74hc595: Add On Semi MC74HC595A compat
      dt-bindings: gpio: fairchild,74hc595: Add latch-gpios property
      gpio: 74x164: Add On Semi MC74HC595A compat
      gpio: 74x164: Add latch GPIO support

 .../bindings/gpio/fairchild,74hc595.yaml           |  5 +++++
 drivers/gpio/gpio-74x164.c                         | 22 +++++++++++++++++++++-
 2 files changed, 26 insertions(+), 1 deletion(-)
---
base-commit: fac04efc5c793dccbd07e2d59af9f90b7fc0dca4
change-id: 20241213-gpio74-66ea33da446d

Best regards,
-- 
J. Neuschäfer <j.ne@posteo.net>

[PATCH 1/4] dt-bindings: gpio: fairchild,74hc595: Add On Semi MC74HC595A compat

[J. Neuschäfer via B4 Relay]

From: "J. Neuschäfer" <j.ne@posteo.net>

The On Semi MC74HC595A/MC74HCT595A is another part that is compatible with
the Fairchild MM74HC595. This patch adds it to the DT binding.

Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
---
 Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml b/Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml
index e8bc9f018edb1253d700945f006e19598efb299a..629cf9b2ab8e4a63fbe17f56792a46d2066d40c3 100644
--- a/Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml
+++ b/Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml
@@ -14,6 +14,7 @@ properties:
     enum:
       - fairchild,74hc595
       - nxp,74lvc594
+      - onnn,74hc595a
 
   reg:
     maxItems: 1

-- 
2.45.2

[PATCH 2/4] dt-bindings: gpio: fairchild,74hc595: Add latch-gpios property

[J. Neuschäfer via B4 Relay]

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 a latch-gpios property to the binding to allow
specifying a GPIO that connects to the latch clock.

Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
---
 Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml b/Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml
index 629cf9b2ab8e4a63fbe17f56792a46d2066d40c3..38d473a3852154e53faec88dc911dc0a4f9cbd1f 100644
--- a/Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml
+++ b/Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml
@@ -34,6 +34,10 @@ properties:
     description: GPIO connected to the OE (Output Enable) pin.
     maxItems: 1
 
+  latch-gpios:
+    description: GPIO connected to the latch clock or storage register clock pin.
+    maxItems: 1
+
 patternProperties:
   "^(hog-[0-9]+|.+-hog(-[0-9]+)?)$":
     type: object

-- 
2.45.2

[PATCH 3/4] gpio: 74x164: Add On Semi MC74HC595A compat

[J. Neuschäfer via B4 Relay]

From: "J. Neuschäfer" <j.ne@posteo.net>

The On Semi MC74HC595A/MC74HCT595A is another part that is compatible
with the Fairchild MM74HC595. This patch adds it to the driver.

Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
---
 drivers/gpio/gpio-74x164.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c
index fca6cd2eb1ddacb3c330111343ebecf9b7c9234d..187032efa5b5cd1aa7aea7b2d55f6c06df4ccac4 100644
--- a/drivers/gpio/gpio-74x164.c
+++ b/drivers/gpio/gpio-74x164.c
@@ -173,6 +173,7 @@ MODULE_DEVICE_TABLE(spi, gen_74x164_spi_ids);
 static const struct of_device_id gen_74x164_dt_ids[] = {
 	{ .compatible = "fairchild,74hc595" },
 	{ .compatible = "nxp,74lvc594" },
+	{ .compatible = "onnn,74hc595a" },
 	{},
 };
 MODULE_DEVICE_TABLE(of, gen_74x164_dt_ids);

-- 
2.45.2

[PATCH 4/4] gpio: 74x164: Add latch GPIO support

[J. Neuschäfer via B4 Relay]

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>
---
 drivers/gpio/gpio-74x164.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

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;

-- 
2.45.2

Re: [PATCH 4/4] gpio: 74x164: Add latch GPIO support

[J. Neuschäfer]

On Fri, Dec 13, 2024 at 06:32:50PM +0100, J. Neuschäfer via B4 Relay 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>

I just noticed that this feature is unnecessary for my use-case:
The 74HC595 doesn't have a chip-select input, but if the rising-edge
triggered latch clock input is reinterpreted as an active-low chip
select, it does the right thing.

                     _   _       _   _
 shift clock    ____| |_| |_..._| |_| |_________

 latch clock                           * trigger
                ___                     ________
 chip select#      |___________________|



 -- jn

Re: [PATCH 2/4] dt-bindings: gpio: fairchild,74hc595: Add latch-gpios property

[Rob Herring]

On Fri, Dec 13, 2024 at 06:32:48PM +0100, J. Neuschäfer wrote:
> 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.

That sounds like all the existing parts have the signal and it is 
required to operate? Or just needed to write settings, but not read GPIO 
input state for example?

If the new parts are usable without latch, then they should have a 
fallback compatible. If they aren't usable, then it should be 1 binding 
patch.

> 
> This patch adds a latch-gpios property to the binding to allow
> specifying a GPIO that connects to the latch clock.
> 
> Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
> ---
>  Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml b/Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml
> index 629cf9b2ab8e4a63fbe17f56792a46d2066d40c3..38d473a3852154e53faec88dc911dc0a4f9cbd1f 100644
> --- a/Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml
> +++ b/Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml
> @@ -34,6 +34,10 @@ properties:
>      description: GPIO connected to the OE (Output Enable) pin.
>      maxItems: 1
>  
> +  latch-gpios:
> +    description: GPIO connected to the latch clock or storage register clock pin.
> +    maxItems: 1
> +
>  patternProperties:
>    "^(hog-[0-9]+|.+-hog(-[0-9]+)?)$":
>      type: object
> 
> -- 
> 2.45.2
> 

Re: [PATCH 2/4] dt-bindings: gpio: fairchild,74hc595: Add latch-gpios property

[J. Neuschäfer]

On Tue, Dec 17, 2024 at 09:25:22AM -0600, Rob Herring wrote:
> On Fri, Dec 13, 2024 at 06:32:48PM +0100, J. Neuschäfer wrote:
> > 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.
> 
> That sounds like all the existing parts have the signal and it is 
> required to operate? Or just needed to write settings, but not read GPIO 
> input state for example?

These parts are output-only (so, "GPO"s, arguably).

The situation with the latch signal is weirder, as I found out in the
meantime: These parts don't have a chip-select built in, but the
rising-edge triggered latch clock can be reinterpreted as an active-low
chip-select, because that would also rise after the appropriate number
of bits has been shifted through the SPI bus.

                     _   _       _   _
 shift clock    ____| |_| |_..._| |_| |_________

 latch clock                           * trigger
                ___                     ________
 chip select#      |___________________|



So, I now think that no additional signal and no binding change is
actually needed, just perhaps an explanatory comment.


> 
> If the new parts are usable without latch, then they should have a 
> fallback compatible. If they aren't usable, then it should be 1 binding 
> patch.

AFAICT, the new part (onnn,74hc595a) behaves the same at the existing
(fairchild,74hc595 and nxp,74lvc594), with regards to the latch signal,
so my two binding patches are independent of other.
In other words, this one can be dropped, but the other still stands.



Best regards
 -- jn

Re: [PATCH 1/4] dt-bindings: gpio: fairchild,74hc595: Add On Semi MC74HC595A compat

[Linus Walleij]

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 On Semi MC74HC595A/MC74HCT595A is another part that is compatible with
> the Fairchild MM74HC595. This patch adds it to the DT binding.
>
> Signed-off-by: J. Neuschäfer <j.ne@posteo.net>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 3/4] gpio: 74x164: Add On Semi MC74HC595A compat

[Linus Walleij]

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 On Semi MC74HC595A/MC74HCT595A is another part that is compatible
> with the Fairchild MM74HC595. This patch adds it to the driver.
>
> Signed-off-by: J. Neuschäfer <j.ne@posteo.net>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 4/4] gpio: 74x164: Add latch GPIO support

[Linus Walleij]

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

Re: [PATCH 4/4] gpio: 74x164: Add latch GPIO support

[J. Neuschäfer]

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