Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
Date:   Tue, 17 Oct 2023 15:54:33 -0700
From:   Bjorn Andersson <quic_bjorande@quicinc.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
CC:     Bjorn Andersson <andersson@kernel.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        Wesley Cheng <quic_wcheng@quicinc.com>,
        Thinh Nguyen <Thinh.Nguyen@synopsys.com>,
        Felipe Balbi <balbi@kernel.org>,
        Philipp Zabel <p.zabel@pengutronix.de>,
        <linux-arm-msm@vger.kernel.org>, <linux-usb@vger.kernel.org>,
        <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        Johan Hovold <johan@kernel.org>,
        "Krishna Kurapati PSSNV" <quic_kriskura@quicinc.com>
Subject: Re: [PATCH 10/12] dt-bindings: usb: qcom,dwc3: Introduce flattened
 qcom,dwc3 binding
Message-ID: <20231017225433.GW3553829@hu-bjorande-lv.qualcomm.com>
References: <20231016-dwc3-refactor-v1-0-ab4a84165470@quicinc.com>
 <20231016-dwc3-refactor-v1-10-ab4a84165470@quicinc.com>
 <9f53e647-7c38-435f-bc74-e4f417445830@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <9f53e647-7c38-435f-bc74-e4f417445830@linaro.org>
Precedence: bulk

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.

Thanks,
Bjorn