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

From: H. Nikolaus Schaller
Date: Wed Dec 09 2020 - 17:08:49 EST



> 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.