linux-kernel - Re: [PATCH v2 1/7] dt-bindings: usb: ti,j721e-usb: add ti,j7200-usb compatible

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: <d0cc33d4-2b1a-43cd-8cd9-6b58d6c71c85@linaro.org>
Date:   Tue, 21 Nov 2023 18:11:28 +0100
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Théo Lebrun <theo.lebrun@...tlin.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Roger Quadros <rogerq@...nel.org>,
        Peter Chen <peter.chen@...nel.org>,
        Pawel Laszczak <pawell@...ence.com>,
        Nishanth Menon <nm@...com>,
        Vignesh Raghavendra <vigneshr@...com>,
        Tero Kristo <kristo@...nel.org>
Cc:     linux-usb@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        Grégory Clement <gregory.clement@...tlin.com>,
        Conor Dooley <conor.dooley@...rochip.com>
Subject: Re: [PATCH v2 1/7] dt-bindings: usb: ti,j721e-usb: add ti,j7200-usb
 compatible

On 21/11/2023 17:53, Théo Lebrun wrote:
> Hello,
> 
> On Mon Nov 20, 2023 at 6:32 PM CET, Krzysztof Kozlowski wrote:
>> On 20/11/2023 18:06, Théo Lebrun wrote:
>>> On this platform, the controller & its wrapper are reset on resume. This
>>> makes it have a different behavior from other platforms.
>>>
>>> We allow using the new compatible with a fallback onto the original
>>> ti,j721e-usb compatible. We therefore allow using an older kernel with
>>
>> Where is fallback ti,j721e-usb used? Please point me to the code.
> 
> No fallback is implemented in code. Using a kernel that doesn't have
> this patch series but a more recent devicetree: DT has both
> devicetrees & the kernel will know which driver to use.

I meant your bindings. You said - with fallback to ti,j721e-usb. I do
not see it. To me the commit description is not accurate.

> 
> That is opposed to having only compatible = "ti,j7200-usb". If using an
> old kernel, it would not know what driver to match it to.
> 
> [...]
> 
>>> --- a/Documentation/devicetree/bindings/usb/ti,j721e-usb.yaml
>>> +++ b/Documentation/devicetree/bindings/usb/ti,j721e-usb.yaml
>>> @@ -12,11 +12,15 @@ maintainers:
>>>  properties:
>>>    compatible:
>>>      oneOf:
>>> +      - const: ti,j7200-usb
>>>        - const: ti,j721e-usb
>>>        - const: ti,am64-usb
>>>        - items:
>>>            - const: ti,j721e-usb
>>>            - const: ti,am64-usb
>>> +      - items:
>>> +          - const: ti,j721e-usb
>>
>> This makes little sense. It's already on the list. Twice! Don't add it
>> third time.
>>
>> I am sorry, but this binding makes no sense. I mean, existing binding
>> makes no sense, but your change is not making it anyhow better.
> 
> The goal of the DT schema pre-patch was to allow all three:
> 
>    compatible = "ti,j721e-usb";
>    compatible = "ti,am64-usb";
>    compatible = "ti,j721e-usb", "ti,am64-usb";

Which does not make sense.

How ti,j721e-usb can be and cannot be compatible with am64 in the same time?

> 
> I've followed the same scheme & added both of those:
> 
>    compatible = "ti,j7200-usb";
>    compatible = "ti,j7200-usb", "ti,j721e-usb";
> 
> I messed up the ordering in the added 'items' options, but the logic
> seems right to me. And dtbs_check agrees. Am I missing something?
> 

Logic is wrong. Device either is or is not compatible with something. At
least usually. We have some exceptions like SMMU for Adreno. Is this the
case? Why the device is and is not compatible with some other variant?

Best regards,
Krzysztof