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

[H. Nikolaus Schaller]

> Am 09.12.2020 um 22:28 schrieb Sven Van Asbroeck <thesven73@xxxxxxxxx>:
> 
> On Wed, Dec 9, 2020 at 3:08 PM H. Nikolaus Schaller <hns@xxxxxxxxxxxxx> 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.