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@a600000: 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
