linux-kernel - Re: [PATCH] spi: dt-bindings: clarify CS behavior for spi-cs-high and gpio descriptors

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]

Message-Id: <BC1AFCEC-DE7D-4135-81DC-1B20CC662917@goldelico.com>
Date:   Wed, 9 Dec 2020 23:04:36 +0100
From:   "H. Nikolaus Schaller" <hns@...delico.com>
To:     Sven Van Asbroeck <thesven73@...il.com>
Cc:     Andreas Kemnade <andreas@...nade.info>,
        Mark Brown <broonie@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        linux-spi <linux-spi@...r.kernel.org>,
        linux-gpio@...r.kernel.org,
        devicetree <devicetree@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Discussions about the Letux Kernel 
        <letux-kernel@...nphoenux.org>, kernel@...a-handheld.com,
        Lukas Wunner <lukas@...ner.de>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Maxime Ripard <maxime@...no.tech>
Subject: Re: [PATCH] spi: dt-bindings: clarify CS behavior for spi-cs-high and gpio descriptors

> Am 09.12.2020 um 22:28 schrieb Sven Van Asbroeck <thesven73@...il.com>:
> 
> On Wed, Dec 9, 2020 at 3:08 PM H. Nikolaus Schaller <hns@...delico.com> wrote:
>> 
>> But I have tested with
>> 
>>> spi->mode |= SPI_MODE_3;
>> 
>> which should keep the mode intact. Right? That did not work either.
>> 
> 
> - make sure ("spi: fix client driver breakages when using GPIO descriptors")
>  is in your tree

Well, if you remember, the panel did work *before* this patch was in my tree
and I found this patch as the reason of the break...

> - your panel's CS is active-low, so 'spi-cs-high' should be removed from its
>  devicetree entry. In accordance with the rules as explained in commit
>  message of 6953c57ab172. Also in accordance with the table you posted
>  in this patch.

It could not have been different because the table was the result of
experimentally checking all possible combinations...

> 
> When these two changes in place, your panel should work. I have tested this
> by mirroring your setup on my board:
> 
> spi5-gpio {
>       compatible = "spi-gpio";
>       #address-cells = <0x1>;
>       #size-cells = <0x0>;
>       pinctrl-names = "default";
>       pinctrl-0 = <&...>;
> 
>       sck-gpios = <&gpio... GPIO_ACTIVE_HIGH>;
>       miso-gpios = <&gpio... GPIO_ACTIVE_HIGH>;
>       mosi-gpios = <&gpio... GPIO_ACTIVE_HIGH>;
>       cs-gpios = <&gpio... GPIO_ACTIVE_HIGH>;

BTW: exactly this choice is questionable ^^^ if you have an active low CS
and it needs an explanation.

>       num-chipselects = <1>;
> 
>       ethernet-switch@0 { /* active low cs */
>               compatible = "micrel,ksz8795";
>               spi-max-frequency = <1000000>;
>               reg = <0>;
>       };
> };
> 
> If this does not work for you, then what are we missing?

I am missing that you notice that we are not discussing what I should
do with the panel driver or my device tree. I have these patches laying around
for a while (which exactly do what you try to convince me about - except that
I would apply an GPIO_ACTIVE_LOW). Just not submitted because I want to
have a clear definition agreed on first. For a simple reason: reviewers
of my patch should know what to check for.

In this thread we discuss a patch for the SPI bindings documentation which
is something different. See subject and the file the patch affects.

And I am looking for an ack and merge by maintainers of the affected subsystems
that the table is ok. Nothing else.

Please let's stay on topic and please cooperate.