[PATCH 1/2] gpio: gpio-74x164: add support for CDx4HC4094

[PATCH 1/2] gpio: gpio-74x164: add support for CDx4HC4094

[Marcus Folkesson]

74hc4094 and 75hc4094 works similar to 74x164 but has an additional
storage latch associated with each stage for strobing data from the
serial input to parallell buffer tri-state output.

Add support for an optional strobe pin.

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
 drivers/gpio/gpio-74x164.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c
index e00c33310517..4a1c4de358e4 100644
--- a/drivers/gpio/gpio-74x164.c
+++ b/drivers/gpio/gpio-74x164.c
@@ -21,6 +21,7 @@ struct gen_74x164_chip {
 	struct gpio_chip	gpio_chip;
 	struct mutex		lock;
 	struct gpio_desc	*gpiod_oe;
+	struct gpio_desc	*gpiod_strobe;
 	u32			registers;
 	/*
 	 * Since the registers are chained, every byte sent will make
@@ -66,6 +67,10 @@ static void gen_74x164_set_value(struct gpio_chip *gc,
 		chip->buffer[bank] &= ~(1 << pin);
 
 	__gen_74x164_write_config(chip);
+
+	/*  Latch data to output pins*/
+	gpiod_set_value_cansleep(chip->gpiod_strobe, 1);
+	gpiod_set_value_cansleep(chip->gpiod_strobe, 0);
 	mutex_unlock(&chip->lock);
 }
 
@@ -87,6 +92,10 @@ static void gen_74x164_set_multiple(struct gpio_chip *gc, unsigned long *mask,
 		chip->buffer[bank] |= bitmask;
 	}
 	__gen_74x164_write_config(chip);
+
+	/*  Latch data to output pins*/
+	gpiod_set_value_cansleep(chip->gpiod_strobe, 1);
+	gpiod_set_value_cansleep(chip->gpiod_strobe, 0);
 	mutex_unlock(&chip->lock);
 }
 
@@ -129,6 +138,12 @@ static int gen_74x164_probe(struct spi_device *spi)
 
 	gpiod_set_value_cansleep(chip->gpiod_oe, 1);
 
+	chip->gpiod_strobe = devm_gpiod_get_optional(&spi->dev, "strobe",
+			GPIOD_OUT_LOW);
+	if (IS_ERR(chip->gpiod_strobe))
+		return PTR_ERR(chip->gpiod_strobe);
+
+
 	spi_set_drvdata(spi, chip);
 
 	chip->gpio_chip.label = spi->modalias;
@@ -153,6 +168,10 @@ static int gen_74x164_probe(struct spi_device *spi)
 		goto exit_destroy;
 	}
 
+	/*  Latch data to output pins*/
+	gpiod_set_value_cansleep(chip->gpiod_strobe, 1);
+	gpiod_set_value_cansleep(chip->gpiod_strobe, 0);
+
 	ret = gpiochip_add_data(&chip->gpio_chip, chip);
 	if (!ret)
 		return 0;
@@ -182,6 +201,8 @@ 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 = "ti,cd54hc4094" },
+	{ .compatible = "ti,cd74hc4094" },
 	{},
 };
 MODULE_DEVICE_TABLE(of, gen_74x164_dt_ids);
-- 
2.36.1

[PATCH 2/2] dt-bindings: gpio: fairchild,74hc595: add strobe-gpios property

[Marcus Folkesson]

Some shift registers (74hc4094 for example) has a strobe signal to latch
data from the serial input to the parallel output.

Add an optional strobe-gpios property to support those chips.

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
 .../devicetree/bindings/gpio/fairchild,74hc595.yaml         | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml b/Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml
index a99e7842ca17..9893df9ae22c 100644
--- a/Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml
+++ b/Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml
@@ -14,6 +14,8 @@ properties:
     enum:
       - fairchild,74hc595
       - nxp,74lvc594
+      - ti,cd54hc4094
+      - ti,cd74hc4094
 
   reg:
     maxItems: 1
@@ -33,6 +35,10 @@ properties:
     description: GPIO connected to the OE (Output Enable) pin.
     maxItems: 1
 
+  strobe-gpios:
+    description: GPIO connected to the STROBE pin
+    maxItems: 1
+
   spi-max-frequency: true
 
 patternProperties:
-- 
2.36.1

Re: [PATCH 2/2] dt-bindings: gpio: fairchild,74hc595: add strobe-gpios property

[Krzysztof Kozlowski]

On 21/07/2022 11:34, Marcus Folkesson wrote:
> Some shift registers (74hc4094 for example) has a strobe signal to latch
> data from the serial input to the parallel output.
> 
> Add an optional strobe-gpios property to support those chips.
> 
> Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> ---
>  .../devicetree/bindings/gpio/fairchild,74hc595.yaml         | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml b/Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml
> index a99e7842ca17..9893df9ae22c 100644
> --- a/Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml
> +++ b/Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml
> @@ -14,6 +14,8 @@ properties:
>      enum:
>        - fairchild,74hc595
>        - nxp,74lvc594
> +      - ti,cd54hc4094
> +      - ti,cd74hc4094
>  
>    reg:
>      maxItems: 1
> @@ -33,6 +35,10 @@ properties:
>      description: GPIO connected to the OE (Output Enable) pin.
>      maxItems: 1
>  
> +  strobe-gpios:
> +    description: GPIO connected to the STROBE pin
> +    maxItems: 1

This should be disallowed (in allOf:if:then) for variants which do not
support it (or do not have even STROBE pin).


Best regards,
Krzysztof

Re: [PATCH 1/2] gpio: gpio-74x164: add support for CDx4HC4094

[Alexander Dahl]

Hello Marcus,

Am Thu, Jul 21, 2022 at 11:34:21AM +0200 schrieb Marcus Folkesson:
> 74hc4094 and 75hc4094 works similar to 74x164 but has an additional
               ^^

That 75 is probably a typo, isn't it?

Greets
Alex

> storage latch associated with each stage for strobing data from the
> serial input to parallell buffer tri-state output.
> 
> Add support for an optional strobe pin.
> 
> Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> ---
>  drivers/gpio/gpio-74x164.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c
> index e00c33310517..4a1c4de358e4 100644
> --- a/drivers/gpio/gpio-74x164.c
> +++ b/drivers/gpio/gpio-74x164.c
> @@ -21,6 +21,7 @@ struct gen_74x164_chip {
>  	struct gpio_chip	gpio_chip;
>  	struct mutex		lock;
>  	struct gpio_desc	*gpiod_oe;
> +	struct gpio_desc	*gpiod_strobe;
>  	u32			registers;
>  	/*
>  	 * Since the registers are chained, every byte sent will make
> @@ -66,6 +67,10 @@ static void gen_74x164_set_value(struct gpio_chip *gc,
>  		chip->buffer[bank] &= ~(1 << pin);
>  
>  	__gen_74x164_write_config(chip);
> +
> +	/*  Latch data to output pins*/
> +	gpiod_set_value_cansleep(chip->gpiod_strobe, 1);
> +	gpiod_set_value_cansleep(chip->gpiod_strobe, 0);
>  	mutex_unlock(&chip->lock);
>  }
>  
> @@ -87,6 +92,10 @@ static void gen_74x164_set_multiple(struct gpio_chip *gc, unsigned long *mask,
>  		chip->buffer[bank] |= bitmask;
>  	}
>  	__gen_74x164_write_config(chip);
> +
> +	/*  Latch data to output pins*/
> +	gpiod_set_value_cansleep(chip->gpiod_strobe, 1);
> +	gpiod_set_value_cansleep(chip->gpiod_strobe, 0);
>  	mutex_unlock(&chip->lock);
>  }
>  
> @@ -129,6 +138,12 @@ static int gen_74x164_probe(struct spi_device *spi)
>  
>  	gpiod_set_value_cansleep(chip->gpiod_oe, 1);
>  
> +	chip->gpiod_strobe = devm_gpiod_get_optional(&spi->dev, "strobe",
> +			GPIOD_OUT_LOW);
> +	if (IS_ERR(chip->gpiod_strobe))
> +		return PTR_ERR(chip->gpiod_strobe);
> +
> +
>  	spi_set_drvdata(spi, chip);
>  
>  	chip->gpio_chip.label = spi->modalias;
> @@ -153,6 +168,10 @@ static int gen_74x164_probe(struct spi_device *spi)
>  		goto exit_destroy;
>  	}
>  
> +	/*  Latch data to output pins*/
> +	gpiod_set_value_cansleep(chip->gpiod_strobe, 1);
> +	gpiod_set_value_cansleep(chip->gpiod_strobe, 0);
> +
>  	ret = gpiochip_add_data(&chip->gpio_chip, chip);
>  	if (!ret)
>  		return 0;
> @@ -182,6 +201,8 @@ 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 = "ti,cd54hc4094" },
> +	{ .compatible = "ti,cd74hc4094" },
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, gen_74x164_dt_ids);
> -- 
> 2.36.1
> 

Re: [PATCH 1/2] gpio: gpio-74x164: add support for CDx4HC4094

[Andy Shevchenko]

On Thu, Jul 21, 2022 at 11:32 AM Marcus Folkesson
<marcus.folkesson@gmail.com> wrote:
>
> 74hc4094 and 75hc4094 works similar to 74x164 but has an additional
> storage latch associated with each stage for strobing data from the
> serial input to parallell buffer tri-state output.

parallel

> Add support for an optional strobe pin.

Sorry for my absence of understanding, but why?
SPI has MOSI, CLK, CS, where the last one is exactly for that. No?


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] gpio: gpio-74x164: add support for CDx4HC4094

[Andy Shevchenko]

On Mon, Jul 25, 2022 at 11:32 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Thu, Jul 21, 2022 at 11:32 AM Marcus Folkesson
> <marcus.folkesson@gmail.com> wrote:
> >
> > 74hc4094 and 75hc4094 works similar to 74x164 but has an additional
> > storage latch associated with each stage for strobing data from the
> > serial input to parallell buffer tri-state output.
>
> parallel
>
> > Add support for an optional strobe pin.
>
> Sorry for my absence of understanding, but why?
> SPI has MOSI, CLK, CS, where the last one is exactly for that. No?

I have a 595 register which also has a latch. What I expect to see in
this series is just a compatible string addition (but again, why even
that is needed?).


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] gpio: gpio-74x164: add support for CDx4HC4094

[Marcus Folkesson]

[-- Attachment #1: Type: text/plain, Size: 792 bytes --]

On Mon, Jul 25, 2022 at 11:32:16AM +0200, Andy Shevchenko wrote:
> On Thu, Jul 21, 2022 at 11:32 AM Marcus Folkesson
> <marcus.folkesson@gmail.com> wrote:
> >
> > 74hc4094 and 75hc4094 works similar to 74x164 but has an additional
> > storage latch associated with each stage for strobing data from the
> > serial input to parallell buffer tri-state output.
> 
> parallel
> 
> > Add support for an optional strobe pin.
> 
> Sorry for my absence of understanding, but why?
> SPI has MOSI, CLK, CS, where the last one is exactly for that. No?

You're right, I didn't think of that CS could be used for this.
The additional compatible strings is then superfluous, so I will skip those.

> 
> 
> -- 
> With Best Regards,
> Andy Shevchenko

Best regards
Marcus Folkesson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/2] gpio: gpio-74x164: add support for CDx4HC4094

[Linus Walleij]

On Mon, Jul 25, 2022 at 11:32 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Thu, Jul 21, 2022 at 11:32 AM Marcus Folkesson
> <marcus.folkesson@gmail.com> wrote:
> >
> > 74hc4094 and 75hc4094 works similar to 74x164 but has an additional
> > storage latch associated with each stage for strobing data from the
> > serial input to parallell buffer tri-state output.
>
> parallel
>
> > Add support for an optional strobe pin.
>
> Sorry for my absence of understanding, but why?
> SPI has MOSI, CLK, CS, where the last one is exactly for that. No?

Forgive me if I misunderstand, but if you use CS that
way, the way that the SPI framework works is to assert
CS then transfer a few chunks over SPI (MOSI/CLK)
then de-assert CS.

If CS is used for strobe, it is constantly asserted
during transfer and the sequence will be latched
out immediately as you write the SPI transfers and
the data is clocked through the register, making the
whole train of zeroes and ones flash across the
output pins before they stabilize after the SPI
transfer is finished.

If you first do the SPI transfer, then strobe after
finished, this will not happen.

Then it should be a separate pin, so this doesn't
happen, right?

Yours,
Linus Walleij

Re: [PATCH 1/2] gpio: gpio-74x164: add support for CDx4HC4094

[Andy Shevchenko]

On Mon, Jul 25, 2022 at 3:54 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Mon, Jul 25, 2022 at 11:32 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Thu, Jul 21, 2022 at 11:32 AM Marcus Folkesson
> > <marcus.folkesson@gmail.com> wrote:
> > >
> > > 74hc4094 and 75hc4094 works similar to 74x164 but has an additional
> > > storage latch associated with each stage for strobing data from the
> > > serial input to parallell buffer tri-state output.
> >
> > parallel
> >
> > > Add support for an optional strobe pin.
> >
> > Sorry for my absence of understanding, but why?
> > SPI has MOSI, CLK, CS, where the last one is exactly for that. No?
>
> Forgive me if I misunderstand, but if you use CS that
> way, the way that the SPI framework works is to assert
> CS then transfer a few chunks over SPI (MOSI/CLK)
> then de-assert CS.
>
> If CS is used for strobe, it is constantly asserted
> during transfer and the sequence will be latched
> out immediately as you write the SPI transfers and
> the data is clocked through the register, making the
> whole train of zeroes and ones flash across the
> output pins before they stabilize after the SPI
> transfer is finished.
>
> If you first do the SPI transfer, then strobe after
> finished, this will not happen.
>
> Then it should be a separate pin, so this doesn't
> happen, right?
>
> Yours,
> Linus Walleij



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] gpio: gpio-74x164: add support for CDx4HC4094

[Andy Shevchenko]

On Mon, Jul 25, 2022 at 3:54 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, Jul 25, 2022 at 11:32 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Thu, Jul 21, 2022 at 11:32 AM Marcus Folkesson
> > <marcus.folkesson@gmail.com> wrote:

...

> > Sorry for my absence of understanding, but why?
> > SPI has MOSI, CLK, CS, where the last one is exactly for that. No?
>
> Forgive me if I misunderstand, but if you use CS that
> way, the way that the SPI framework works is to assert
> CS then transfer a few chunks over SPI (MOSI/CLK)
> then de-assert CS.

No, CS here is used exactly for what it is designed for ("tell that
this message is *for me*"). Yes, hardware implementation here is a
latch register. Because otherwise ALL messages are "for me" which is
wrong. Is it wrong interpretation of the hardware and SPI?

> If CS is used for strobe, it is constantly asserted
> during transfer and the sequence will be latched
> out immediately as you write the SPI transfers and
> the data is clocked through the register, making the
> whole train of zeroes and ones flash across the
> output pins before they stabilize after the SPI
> transfer is finished.

I'm not sure I understand the stabilization issue here. It's how SPI
normally works and we have a lot of delays here and there related to
the phase of the CS in comparison to clock and data. We have a lot of
time to stabilize the outputs of the shift register before latching
it. Did I miss anything?


> If you first do the SPI transfer, then strobe after
> finished, this will not happen.

I have hardware, I have tested it and I understand what you mean by
"stabilizing", but finishing transfer _is_ CS toggling for _this_
chip. No?

> Then it should be a separate pin, so this doesn't
> happen, right?

I think no, you don't need it. I.o.w. either I'm missing something
very interesting about both this kind of chips and SPI basics (shame
on me in this case) or...?


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] gpio: gpio-74x164: add support for CDx4HC4094

[Linus Walleij]

On Mon, Jul 25, 2022 at 10:49 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Mon, Jul 25, 2022 at 3:54 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Mon, Jul 25, 2022 at 11:32 AM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > > On Thu, Jul 21, 2022 at 11:32 AM Marcus Folkesson
> > > <marcus.folkesson@gmail.com> wrote:
>
> ...
>
> > > Sorry for my absence of understanding, but why?
> > > SPI has MOSI, CLK, CS, where the last one is exactly for that. No?
> >
> > Forgive me if I misunderstand, but if you use CS that
> > way, the way that the SPI framework works is to assert
> > CS then transfer a few chunks over SPI (MOSI/CLK)
> > then de-assert CS.
>
> No, CS here is used exactly for what it is designed for ("tell that
> this message is *for me*"). Yes, hardware implementation here is a
> latch register. Because otherwise ALL messages are "for me" which is
> wrong. Is it wrong interpretation of the hardware and SPI?

I was under the impression that the shift register has no idea
if the message is "for me", and that there can only be one shift register
on the bus if using ordinary SPI to control it.

I look at this data sheet:
https://www.farnell.com/datasheets/2030250.pdf

IIUC what you say is CS == STR?

> > If CS is used for strobe, it is constantly asserted
> > during transfer and the sequence will be latched
> > out immediately as you write the SPI transfers and
> > the data is clocked through the register, making the
> > whole train of zeroes and ones flash across the
> > output pins before they stabilize after the SPI
> > transfer is finished.
>
> I'm not sure I understand the stabilization issue here. It's how SPI
> normally works and we have a lot of delays here and there related to
> the phase of the CS in comparison to clock and data. We have a lot of
> time to stabilize the outputs of the shift register before latching
> it. Did I miss anything?

STR (strobe) is latching out the data, and how is that happening
when you connect it to CS? CS is asserted throughout the whole
transaction...

STR is supposed to be used like in the patch: to be toggled after
the transfer is complete. CS does not behave like this at all.

> > If you first do the SPI transfer, then strobe after
> > finished, this will not happen.
>
> I have hardware, I have tested it and I understand what you mean by
> "stabilizing", but finishing transfer _is_ CS toggling for _this_
> chip. No?

Well it will work, because all values come out, however *during*
the transfer, not *after* the transfer as intended with the
STR signal.

> > Then it should be a separate pin, so this doesn't
> > happen, right?
>
> I think no, you don't need it. I.o.w. either I'm missing something
> very interesting about both this kind of chips and SPI basics (shame
> on me in this case) or...?

SPI will assert CS, then make the transfer i.e. toggle the clock
a few times with new data on MOSI each clock cycle, then
de-assert CS.

STR is supposed to be toggled after all this happened to latch
out the data to the outputs, just like the patch does.

What happens if STR is connected to CS is that you will see
the shift register contents change on the output. It will be
fast so it will look fine if it is e.g. a LED. But if it is something
more sensitive, there will be chaos.

At least how I see it ...?

Yours,
Linus Walleij

Re: [PATCH 1/2] gpio: gpio-74x164: add support for CDx4HC4094

[Andy Shevchenko]

On Tue, Jul 26, 2022 at 12:22 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Mon, Jul 25, 2022 at 10:49 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Mon, Jul 25, 2022 at 3:54 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> > > On Mon, Jul 25, 2022 at 11:32 AM Andy Shevchenko
> > > <andy.shevchenko@gmail.com> wrote:
> > > > On Thu, Jul 21, 2022 at 11:32 AM Marcus Folkesson
> > > > <marcus.folkesson@gmail.com> wrote:
> >
> > ...
> >
> > > > Sorry for my absence of understanding, but why?
> > > > SPI has MOSI, CLK, CS, where the last one is exactly for that. No?
> > >
> > > Forgive me if I misunderstand, but if you use CS that
> > > way, the way that the SPI framework works is to assert
> > > CS then transfer a few chunks over SPI (MOSI/CLK)
> > > then de-assert CS.
> >
> > No, CS here is used exactly for what it is designed for ("tell that
> > this message is *for me*"). Yes, hardware implementation here is a
> > latch register. Because otherwise ALL messages are "for me" which is
> > wrong. Is it wrong interpretation of the hardware and SPI?
>
> I was under the impression that the shift register has no idea
> if the message is "for me", and that there can only be one shift register
> on the bus if using ordinary SPI to control it.

Yes and no. Yes, the shift register in this HW is always 'for all'.
No, there are as many shift registers as you have chip selects on the same bus.

> I look at this data sheet:
> https://www.farnell.com/datasheets/2030250.pdf
>
> IIUC what you say is CS == STR?

Yes.

> > > If CS is used for strobe, it is constantly asserted
> > > during transfer and the sequence will be latched
> > > out immediately as you write the SPI transfers and
> > > the data is clocked through the register, making the
> > > whole train of zeroes and ones flash across the
> > > output pins before they stabilize after the SPI
> > > transfer is finished.
> >
> > I'm not sure I understand the stabilization issue here. It's how SPI
> > normally works and we have a lot of delays here and there related to
> > the phase of the CS in comparison to clock and data. We have a lot of
> > time to stabilize the outputs of the shift register before latching
> > it. Did I miss anything?
>
> STR (strobe) is latching out the data, and how is that happening
> when you connect it to CS? CS is asserted throughout the whole
> transaction...

Is it a problem? I consider it exactly what it's designed for, it
tells the chip: "hey, there is something for you".

> STR is supposed to be used like in the patch: to be toggled after
> the transfer is complete. CS does not behave like this at all.

How is CS different in this sense? Just longer? Who cares that it
starts early and latches exactly at the time it must latch the data.

> > > If you first do the SPI transfer, then strobe after
> > > finished, this will not happen.
> >
> > I have hardware, I have tested it and I understand what you mean by
> > "stabilizing", but finishing transfer _is_ CS toggling for _this_
> > chip. No?
>
> Well it will work, because all values come out, however *during*
> the transfer, not *after* the transfer as intended with the
> STR signal.

No. I think you missed one little but crucial detail. The strobe is
edge, not level, while you consider CS as a level signal (only).

> > > Then it should be a separate pin, so this doesn't
> > > happen, right?
> >
> > I think no, you don't need it. I.o.w. either I'm missing something
> > very interesting about both this kind of chips and SPI basics (shame
> > on me in this case) or...?
>
> SPI will assert CS, then make the transfer i.e. toggle the clock
> a few times with new data on MOSI each clock cycle, then
> de-assert CS.

Right.

> STR is supposed to be toggled after all this happened to latch
> out the data to the outputs, just like the patch does.

So...

> What happens if STR is connected to CS is that you will see
> the shift register contents change on the output. It will be
> fast so it will look fine if it is e.g. a LED. But if it is something
> more sensitive, there will be chaos.

See Figure 7 and Table 9 in the datasheet you linked to. When STR is
low, the data is not changed. Maybe it's not clear in this datasheet,
I found somewhere
in a more clear table.

> At least how I see it ...?

If what you are saying is true, the hardware is totally broken and I
wouldn't use it. I.o.w. if the strobe passes all data when it's at low
level and latches at low-to-high, it's broken by definition in my
point of view.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] gpio: gpio-74x164: add support for CDx4HC4094

[Andy Shevchenko]

On Tue, Jul 26, 2022 at 8:54 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Tue, Jul 26, 2022 at 12:22 AM Linus Walleij <linus.walleij@linaro.org> wrote:

...

> See Figure 7 and Table 9 in the datasheet you linked to. When STR is

Table 9 --> Figure 9

> low, the data is not changed. Maybe it's not clear in this datasheet,
> I found somewhere
> in a more clear table.

If you have hardware and an oscilloscope you may test it. We will know
for sure :-)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] gpio: gpio-74x164: add support for CDx4HC4094

[Andy Shevchenko]

On Tue, Jul 26, 2022 at 9:00 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Tue, Jul 26, 2022 at 8:54 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Tue, Jul 26, 2022 at 12:22 AM Linus Walleij <linus.walleij@linaro.org> wrote:

...

> > See Figure 7 and Table 9 in the datasheet you linked to. When STR is
>
> Table 9 --> Figure 9
>
> > low, the data is not changed. Maybe it's not clear in this datasheet,
> > I found somewhere
> > in a more clear table.

Ah, Table 3 shows that.

> If you have hardware and an oscilloscope you may test it. We will know
> for sure :-)



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] gpio: gpio-74x164: add support for CDx4HC4094

[Linus Walleij]

On Tue, Jul 26, 2022 at 8:55 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Tue, Jul 26, 2022 at 12:22 AM Linus Walleij <linus.walleij@linaro.org> wrote:

> > Well it will work, because all values come out, however *during*
> > the transfer, not *after* the transfer as intended with the
> > STR signal.
>
> No. I think you missed one little but crucial detail. The strobe is
> edge, not level, while you consider CS as a level signal (only).

Aha! Then it works :)

> See Figure 7 and Table 9 in the datasheet you linked to. When STR is
> low, the data is not changed. Maybe it's not clear in this datasheet,
> I found somewhere
> in a more clear table.
>
> > At least how I see it ...?
>
> If what you are saying is true, the hardware is totally broken and I
> wouldn't use it. I.o.w. if the strobe passes all data when it's at low
> level and latches at low-to-high, it's broken by definition in my
> point of view.

I have seen strobes that are level-active in my life, but if you
say this one isn't then all is fine!

Yours,
Linus Walleij

Re: [PATCH 1/2] gpio: gpio-74x164: add support for CDx4HC4094

[Andy Shevchenko]

On Tue, Jul 26, 2022 at 9:23 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Tue, Jul 26, 2022 at 8:55 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Tue, Jul 26, 2022 at 12:22 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> > > Well it will work, because all values come out, however *during*
> > > the transfer, not *after* the transfer as intended with the
> > > STR signal.
> >
> > No. I think you missed one little but crucial detail. The strobe is
> > edge, not level, while you consider CS as a level signal (only).
>
> Aha! Then it works :)
>
> > See Figure 7 and Table 9 in the datasheet you linked to. When STR is
> > low, the data is not changed. Maybe it's not clear in this datasheet,
> > I found somewhere
> > in a more clear table.
> >
> > > At least how I see it ...?
> >
> > If what you are saying is true, the hardware is totally broken and I
> > wouldn't use it. I.o.w. if the strobe passes all data when it's at low
> > level and latches at low-to-high, it's broken by definition in my
> > point of view.
>
> I have seen strobes that are level-active in my life, but if you
> say this one isn't then all is fine!

It's rather my common sense, because we have a clock here to shift a
register. It's kinda obvious *to me* that when we have a latch, that
latch shouldn't pass data on a level. Yes, there are buffers with
level strobe, but it's different. So, as I said, the oscilloscope can
finish our discussion one way or the other, but I believe it will be
my way ;) I will very (badly) surprised if it becomes your way. It
will ruin my beliefs in humanity. Not less.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] dt-bindings: gpio: fairchild,74hc595: add strobe-gpios property

[Linus Walleij]

On Thu, Jul 21, 2022 at 11:30 AM Marcus Folkesson
<marcus.folkesson@gmail.com> wrote:

> Some shift registers (74hc4094 for example) has a strobe signal to latch
> data from the serial input to the parallel output.
>
> Add an optional strobe-gpios property to support those chips.
>
> Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>

As pointed out in discussion with Andy, let's drop this and just
use cs-gpios as-is.

Yours,
Linus Walleij

Re: [PATCH 1/2] gpio: gpio-74x164: add support for CDx4HC4094

[Linus Walleij]

On Tue, Jul 26, 2022 at 9:31 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:

> It's rather my common sense, because we have a clock here to shift a
> register. It's kinda obvious *to me* that when we have a latch, that
> latch shouldn't pass data on a level. Yes, there are buffers with
> level strobe, but it's different. So, as I said, the oscilloscope can
> finish our discussion one way or the other, but I believe it will be
> my way ;) I will very (badly) surprised if it becomes your way. It
> will ruin my beliefs in humanity. Not less.

I think you are right Andy, I trust your judgement better than my own
here for sure!

Yours,
Linus Walleij

Re: [PATCH 1/2] gpio: gpio-74x164: add support for CDx4HC4094

[Andy Shevchenko]

On Tue, Jul 26, 2022 at 10:28 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Tue, Jul 26, 2022 at 9:31 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>
> > It's rather my common sense, because we have a clock here to shift a
> > register. It's kinda obvious *to me* that when we have a latch, that
> > latch shouldn't pass data on a level. Yes, there are buffers with
> > level strobe, but it's different. So, as I said, the oscilloscope can
> > finish our discussion one way or the other, but I believe it will be
> > my way ;) I will very (badly) surprised if it becomes your way. It
> > will ruin my beliefs in humanity. Not less.
>
> I think you are right Andy, I trust your judgement better than my own
> here for sure!

The true science when we put any statement in doubt and then go
step-by-step to prove it again. If this will be done successfully the
statement is correct. That said, thank you to put this in doubt, it's
appreciated!

-- 
With Best Regards,
Andy Shevchenko