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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 26 Jul 2022 08:54:45 +0200
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     Marcus Folkesson <marcus.folkesson@...il.com>,
        Bartosz Golaszewski <brgl@...ev.pl>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Maxime Ripard <mripard@...nel.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        devicetree <devicetree@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] gpio: gpio-74x164: add support for CDx4HC4094

On Tue, Jul 26, 2022 at 12:22 AM Linus Walleij <linus.walleij@...aro.org> wrote:
>
> On Mon, Jul 25, 2022 at 10:49 PM Andy Shevchenko
> <andy.shevchenko@...il.com> wrote:
> > On Mon, Jul 25, 2022 at 3:54 PM Linus Walleij <linus.walleij@...aro.org> wrote:
> > > On Mon, Jul 25, 2022 at 11:32 AM Andy Shevchenko
> > > <andy.shevchenko@...il.com> wrote:
> > > > On Thu, Jul 21, 2022 at 11:32 AM Marcus Folkesson
> > > > <marcus.folkesson@...il.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