linux-kernel - Re: [PATCH 10/12] dt-bindings: usb: qcom,dwc3: Introduce flattened qcom,dwc3 binding

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: <b52d0de7-e320-4c4c-876e-346281de23a1@linaro.org>
Date:   Wed, 18 Oct 2023 08:00:35 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Bjorn Andersson <quic_bjorande@...cinc.com>
Cc:     Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        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>,
        Wesley Cheng <quic_wcheng@...cinc.com>,
        Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        Felipe Balbi <balbi@...nel.org>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        linux-arm-msm@...r.kernel.org, linux-usb@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        Johan Hovold <johan@...nel.org>,
        Krishna Kurapati PSSNV <quic_kriskura@...cinc.com>
Subject: Re: [PATCH 10/12] dt-bindings: usb: qcom,dwc3: Introduce flattened
 qcom,dwc3 binding

On 18/10/2023 00:54, Bjorn Andersson wrote:
> On Tue, Oct 17, 2023 at 08:11:45AM +0200, Krzysztof Kozlowski wrote:
>> On 17/10/2023 05:11, Bjorn Andersson wrote:
>>> diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> [..]
>>> +select:
>>> +  properties:
>>> +    compatible:
>>> +      items:
>>> +        - enum:
>>> +            - qcom,ipq4019-dwc3
> [..]
>>> +            - qcom,sm8550-dwc3
>>
>> This enum could be replaced with '{}'. Alternatively, drop enum entire
>> select replaced with:
>> - contains
>>   - items:
>>       - const: qcom,dwc3
>>       - const: snps,dwc3
>>
> 
> I thought this would be what I needed as well, but unfortunately this
> select matches either qcom,dwc3, snps,dwc3, or both. With the result
> that e.g. the example in the snps,dwc3 binding matches against this and
> as expected fails when validated against this binding.
> 
> Taking yet another look at this, and reading more about json validation
> I figured out that the following matches nodes with both the
> compatibles:
> 
> select:
>   properties:
>     compatible:
>       items:
>         - const: qcom,dwc3
>         - const: snps,dwc3
>   required:
>     - compatible
> 
> [..]
>>> +
>>> +# Required child node:
>>
>> Drop
>>
> 
> Of course.
> 
>>
>> ...
>>
>>> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>>> index d81c2e849ca9..d6914b8cef6a 100644
>>> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>>> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>>> @@ -44,14 +44,18 @@ properties:
>>>        It's either a single common DWC3 interrupt (dwc_usb3) or individual
>>>        interrupts for the host, gadget and DRD modes.
>>>      minItems: 1
>>> -    maxItems: 4
>>> +    maxItems: 5
>>>  
>>>    interrupt-names:
>>> -    minItems: 1
>>> -    maxItems: 4
>>>      oneOf:
>>> -      - const: dwc_usb3
>>> -      - items:
>>> +      - minItems: 1
>>> +        maxItems: 5
>>> +        items:
>>> +          - const: dwc_usb3
>>> +        additionalItems: true
>>
>> This is not correct change. Before, one dwc_usb3 interrupt was combined
>> allowed, or a set of host+peripheral+otg+wakeup. Now, you allow combined
>> dwc_usb3 with anything.
>>
> 
> My intention here is to make below list of 5 strings be valid according
> to the snps,dwc3 (i.e. dwc_usb3 being the first item), and valid
> according to the qcom,dwc3 binding with all 5 defined.
> 
>   interrupt-names = "dwc_usb3", "hs_phy_irq", "ss_phy_irq",
> 		    "dm_hs_phy_irq", "dp_hs_phy_irq";
> 
> When I express this as:
> 
>   interrupt-names:
>     minItems: 1
>     maxItems: 5
>     oneOf:
>       - const: dwc_usb3
>       - items:
>           enum: [host, peripheral, otg, wakeup]
> 
> I get:
> 
> /local/mnt/workspace/bjorande/linux/Documentation/devicetree/bindings/usb/qcom,dwc3.example.dtb: usb@...0000: interrupt-names: 'oneOf' conditional failed, one must be fixed:
>         ['dwc_usb3', 'hs_phy_irq', 'ss_phy_irq', 'dm_hs_phy_irq', 'dp_hs_phy_irq'] is too long
>         'dwc_usb3' is not one of ['host', 'peripheral', 'otg', 'wakeup']
>         'hs_phy_irq' is not one of ['host', 'peripheral', 'otg', 'wakeup']
>         'ss_phy_irq' is not one of ['host', 'peripheral', 'otg', 'wakeup']
>         'dm_hs_phy_irq' is not one of ['host', 'peripheral', 'otg', 'wakeup']
>         'dp_hs_phy_irq' is not one of ['host', 'peripheral', 'otg', 'wakeup']
>         from schema $id: http://devicetree.org/schemas/usb/snps,dwc3.yaml#
> 
> Which to me sounds like the two oneOf branches allow me a single entry,
> or items from the set given here. In contrast, I believe that my
> proposal allow 1-5 items, where the first needs to be dwc_usb3.
> 
> But the proposal does look messy, so I'd appreciate some guidance on
> this one.

Then you need three branches I guess, with your branch listing the Qcom
specific interrupts. The point is that dwc_usb3 should not be allowed
with host/peripheral/otg/wakeup.

Best regards,
Krzysztof