Re: [Letux-kernel] [PATCH 2/2] DTS: ARM: gta04: introduce legacy spi-cs-high to make display work again

From: H. Nikolaus Schaller
Date: Fri Sep 20 2019 - 05:19:21 EST



> Am 20.09.2019 um 10:55 schrieb Linus Walleij <linus.walleij@xxxxxxxxxx>:
>
> On Tue, Sep 17, 2019 at 4:26 PM H. Nikolaus Schaller <hns@xxxxxxxxxxxxx> wrote:
>>> Am 17.09.2019 um 00:52 schrieb Linus Walleij <linus.walleij@xxxxxxxxxx>:
>>> On Mon, Sep 16, 2019 at 12:59 PM H. Nikolaus Schaller <hns@xxxxxxxxxxxxx> wrote:
>>>
>>>> ping.
>>>>
>>>> Device omap3-gta04 is neither working with v5.3 nor linux-next quite a while and we need a solution.
>>>
>>> Can't we just apply the last part of the patch in this thread:
>>>
>>> diff --git a/arch/arm/boot/dts/omap3-gta04.dtsi
>>> b/arch/arm/boot/dts/omap3-gta04.dtsi
>>> index 9a9a29fe88ec..47bab8e1040e 100644
>>> --- a/arch/arm/boot/dts/omap3-gta04.dtsi
>>> +++ b/arch/arm/boot/dts/omap3-gta04.dtsi
>>> @@ -124,6 +124,7 @@
>>> spi-max-frequency = <100000>;
>>> spi-cpol;
>>> spi-cpha;
>>> + spi-cs-high;
>>>
>>> backlight= <&backlight>;
>>> label = "lcd";
>>>
>>>
>>> Surely this fixes the problem?
>>
>> yes, it is a workaround, but appears to violate some policies.
>> E.g. the spi-cs-high; is undocumented but DT bindings maintainer
>> seems to be against documenting it as I had proposed in my
>> other patch.
>
> It is documented as a boolean in
> Documentation/devicetree/bindings/spi/spi-controller.yaml
> with the following description:
>
> spi-cs-high:
> $ref: /schemas/types.yaml#/definitions/flag
> description:
> The device requires the chip select active high.
>
> So I don't think it is about it being undocumented.

Yes, the basic property is documented. But incomplete.

The strange inversion side-effect on the third gpio parameter
is undocumented and not understandable from this description
alone.

>
>> Rather he seems to have proposed a white-list in the driver code.
>> So that the legacy mode is only becoming active for those systems
>> which really need the legacy mode instead of everyone.
>
> Yeah that seems like a plausible way forward if we want to
> move away from the legacy way of specifying polarity.
>
>> Then, we do not need this patch for GTA04.
>
> We don't need to implement the perfect solution up front.
> We can aim for that in the long run. I usually go by the IETF
> motto "rough consensus and running code".
>
>> So its up to you to decide which way to go. We are happy with
>> any one that makes mainline work again asap...
>
> I suggest to go both way:
> apply this oneliner and tag for stable so that GTA04 works
> again.
>
> Then for the next kernel think about a possible more abitious
> whitelist solution and after adding that remove *all* "spi-cs-high"
> flags from all device trees in the kernel after fixing them
> all up.

Ok, that looks like a viable path.

BR and thanks,
Nikolaus