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

Message ID 20231016-dwc3-refactor-v1-10-ab4a84165470@quicinc.com
State New
Headers
Series usb: dwc3: qcom: Flatten dwc3 structure |

Commit Message

Bjorn Andersson Oct. 17, 2023, 3:11 a.m. UTC
  The Qualcomm USB block consists of three intertwined parts, the XHCI,
the DWC3 core and the Qualcomm DWC3 glue. The three parts can not be
operated independently, but the binding was for historical reasons split
to mimic the Linux driver implementation.

The split binding also makes it hard to alter the implementation, as
properties and resources are split between the two nodes, in some cases
with some duplication.

Introduce a new binding, with a single representation of the whole USB
block in one node.

Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
 .../devicetree/bindings/usb/qcom,dwc3.yaml         | 482 +++++++++++++++++++++
 .../devicetree/bindings/usb/snps,dwc3.yaml         |  14 +-
 2 files changed, 491 insertions(+), 5 deletions(-)
  

Comments

Krzysztof Kozlowski Oct. 17, 2023, 6:11 a.m. UTC | #1
On 17/10/2023 05:11, Bjorn Andersson wrote:
> The Qualcomm USB block consists of three intertwined parts, the XHCI,
> the DWC3 core and the Qualcomm DWC3 glue. The three parts can not be
> operated independently, but the binding was for historical reasons split
> to mimic the Linux driver implementation.
> 
> The split binding also makes it hard to alter the implementation, as
> properties and resources are split between the two nodes, in some cases
> with some duplication.
> 
> Introduce a new binding, with a single representation of the whole USB
> block in one node.
> 
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
>  .../devicetree/bindings/usb/qcom,dwc3.yaml         | 482 +++++++++++++++++++++
>  .../devicetree/bindings/usb/snps,dwc3.yaml         |  14 +-
>  2 files changed, 491 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> new file mode 100644
> index 000000000000..cb50261c6a36
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> @@ -0,0 +1,482 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/qcom,dwc3.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm SuperSpeed DWC3 USB SoC controller
> +
> +maintainers:
> +  - Wesley Cheng <quic_wcheng@quicinc.com>
> +
> +select:
> +  properties:
> +    compatible:
> +      items:
> +        - enum:
> +            - qcom,ipq4019-dwc3
> +            - qcom,ipq5018-dwc3
> +            - qcom,ipq5332-dwc3
> +            - qcom,ipq6018-dwc3
> +            - qcom,ipq8064-dwc3
> +            - qcom,ipq8074-dwc3
> +            - qcom,ipq9574-dwc3
> +            - qcom,msm8953-dwc3
> +            - qcom,msm8994-dwc3
> +            - qcom,msm8996-dwc3
> +            - qcom,msm8998-dwc3
> +            - qcom,qcm2290-dwc3
> +            - qcom,qcs404-dwc3
> +            - qcom,sa8775p-dwc3
> +            - qcom,sc7180-dwc3
> +            - qcom,sc7280-dwc3
> +            - qcom,sc8180x-dwc3
> +            - qcom,sc8280xp-dwc3
> +            - qcom,sc8280xp-dwc3-mp
> +            - qcom,sdm660-dwc3
> +            - qcom,sdm670-dwc3
> +            - qcom,sdm845-dwc3
> +            - qcom,sdx55-dwc3
> +            - qcom,sdx65-dwc3
> +            - qcom,sdx75-dwc3
> +            - qcom,sm4250-dwc3
> +            - qcom,sm6115-dwc3
> +            - qcom,sm6125-dwc3
> +            - qcom,sm6350-dwc3
> +            - qcom,sm6375-dwc3
> +            - qcom,sm8150-dwc3
> +            - qcom,sm8250-dwc3
> +            - qcom,sm8350-dwc3
> +            - qcom,sm8450-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



> +        - const: qcom,dwc3
> +        - const: snps,dwc3
> +  required:
> +    - compatible
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - qcom,ipq4019-dwc3
> +          - qcom,ipq5018-dwc3
> +          - qcom,ipq5332-dwc3
> +          - qcom,ipq6018-dwc3
> +          - qcom,ipq8064-dwc3
> +          - qcom,ipq8074-dwc3
> +          - qcom,ipq9574-dwc3
> +          - qcom,msm8953-dwc3
> +          - qcom,msm8994-dwc3
> +          - qcom,msm8996-dwc3
> +          - qcom,msm8998-dwc3
> +          - qcom,qcm2290-dwc3
> +          - qcom,qcs404-dwc3
> +          - qcom,sa8775p-dwc3
> +          - qcom,sc7180-dwc3
> +          - qcom,sc7280-dwc3
> +          - qcom,sc8180x-dwc3
> +          - qcom,sc8280xp-dwc3
> +          - qcom,sc8280xp-dwc3-mp
> +          - qcom,sdm660-dwc3
> +          - qcom,sdm670-dwc3
> +          - qcom,sdm845-dwc3
> +          - qcom,sdx55-dwc3
> +          - qcom,sdx65-dwc3
> +          - qcom,sdx75-dwc3
> +          - qcom,sm4250-dwc3
> +          - qcom,sm6115-dwc3
> +          - qcom,sm6125-dwc3
> +          - qcom,sm6350-dwc3
> +          - qcom,sm6375-dwc3
> +          - qcom,sm8150-dwc3
> +          - qcom,sm8250-dwc3
> +          - qcom,sm8350-dwc3
> +          - qcom,sm8450-dwc3
> +          - qcom,sm8550-dwc3
> +      - const: qcom,dwc3
> +      - const: snps,dwc3
> +
> +  reg:
> +    description: Offset and length of register set for QSCRATCH wrapper
> +    maxItems: 1
> +
> +  power-domains:
> +    description: specifies a phandle to PM domain provider node

Drop description

> +    maxItems: 1
> +
> +  required-opps:
> +    maxItems: 1
> +
> +  clocks:
> +    description: |
> +      Several clocks are used, depending on the variant. Typical ones are::
> +       - cfg_noc:: System Config NOC clock.
> +       - core:: Master/Core clock, has to be >= 125 MHz for SS operation and >=
> +                60MHz for HS operation.
> +       - iface:: System bus AXI clock.
> +       - sleep:: Sleep clock, used for wakeup when USB3 core goes into low
> +                 power mode (U3).
> +       - mock_utmi:: Mock utmi clock needed for ITP/SOF generation in host
> +                     mode. Its frequency should be 19.2MHz.
> +    minItems: 1
> +    maxItems: 9
> +
> +  clock-names:
> +    minItems: 1
> +    maxItems: 9
> +
> +  resets:
> +    maxItems: 1
> +
> +  interconnects:
> +    maxItems: 2
> +
> +  interconnect-names:
> +    items:
> +      - const: usb-ddr
> +      - const: apps-usb
> +
> +  interrupts:
> +    minItems: 2
> +    maxItems: 5
> +
> +  interrupt-names:
> +    minItems: 2
> +    maxItems: 5
> +
> +  qcom,select-utmi-as-pipe-clk:
> +    description:
> +      If present, disable USB3 pipe_clk requirement.
> +      Used when dwc3 operates without SSPHY and only
> +      HS/FS/LS modes are supported.
> +    type: boolean
> +
> +  wakeup-source: true
> +
> +# Required child node:

Drop


...

> 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.



Best regards,
Krzysztof
  
Rob Herring Oct. 17, 2023, 6:31 p.m. UTC | #2
On Mon, Oct 16, 2023 at 08:11:18PM -0700, Bjorn Andersson wrote:
> The Qualcomm USB block consists of three intertwined parts, the XHCI,
> the DWC3 core and the Qualcomm DWC3 glue. The three parts can not be
> operated independently, but the binding was for historical reasons split
> to mimic the Linux driver implementation.
> 
> The split binding also makes it hard to alter the implementation, as
> properties and resources are split between the two nodes, in some cases
> with some duplication.
> 
> Introduce a new binding, with a single representation of the whole USB
> block in one node.
> 
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
>  .../devicetree/bindings/usb/qcom,dwc3.yaml         | 482 +++++++++++++++++++++
>  .../devicetree/bindings/usb/snps,dwc3.yaml         |  14 +-
>  2 files changed, 491 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> new file mode 100644
> index 000000000000..cb50261c6a36
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> @@ -0,0 +1,482 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/qcom,dwc3.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm SuperSpeed DWC3 USB SoC controller
> +
> +maintainers:
> +  - Wesley Cheng <quic_wcheng@quicinc.com>
> +
> +select:
> +  properties:
> +    compatible:
> +      items:
> +        - enum:
> +            - qcom,ipq4019-dwc3
> +            - qcom,ipq5018-dwc3
> +            - qcom,ipq5332-dwc3
> +            - qcom,ipq6018-dwc3
> +            - qcom,ipq8064-dwc3
> +            - qcom,ipq8074-dwc3
> +            - qcom,ipq9574-dwc3
> +            - qcom,msm8953-dwc3
> +            - qcom,msm8994-dwc3
> +            - qcom,msm8996-dwc3
> +            - qcom,msm8998-dwc3
> +            - qcom,qcm2290-dwc3
> +            - qcom,qcs404-dwc3
> +            - qcom,sa8775p-dwc3
> +            - qcom,sc7180-dwc3
> +            - qcom,sc7280-dwc3
> +            - qcom,sc8180x-dwc3
> +            - qcom,sc8280xp-dwc3
> +            - qcom,sc8280xp-dwc3-mp
> +            - qcom,sdm660-dwc3
> +            - qcom,sdm670-dwc3
> +            - qcom,sdm845-dwc3
> +            - qcom,sdx55-dwc3
> +            - qcom,sdx65-dwc3
> +            - qcom,sdx75-dwc3
> +            - qcom,sm4250-dwc3
> +            - qcom,sm6115-dwc3
> +            - qcom,sm6125-dwc3
> +            - qcom,sm6350-dwc3
> +            - qcom,sm6375-dwc3
> +            - qcom,sm8150-dwc3
> +            - qcom,sm8250-dwc3
> +            - qcom,sm8350-dwc3
> +            - qcom,sm8450-dwc3
> +            - qcom,sm8550-dwc3
> +        - const: qcom,dwc3
> +        - const: snps,dwc3
> +  required:
> +    - compatible
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - qcom,ipq4019-dwc3
> +          - qcom,ipq5018-dwc3
> +          - qcom,ipq5332-dwc3
> +          - qcom,ipq6018-dwc3
> +          - qcom,ipq8064-dwc3
> +          - qcom,ipq8074-dwc3
> +          - qcom,ipq9574-dwc3
> +          - qcom,msm8953-dwc3
> +          - qcom,msm8994-dwc3
> +          - qcom,msm8996-dwc3
> +          - qcom,msm8998-dwc3
> +          - qcom,qcm2290-dwc3
> +          - qcom,qcs404-dwc3
> +          - qcom,sa8775p-dwc3
> +          - qcom,sc7180-dwc3
> +          - qcom,sc7280-dwc3
> +          - qcom,sc8180x-dwc3
> +          - qcom,sc8280xp-dwc3
> +          - qcom,sc8280xp-dwc3-mp
> +          - qcom,sdm660-dwc3
> +          - qcom,sdm670-dwc3
> +          - qcom,sdm845-dwc3
> +          - qcom,sdx55-dwc3
> +          - qcom,sdx65-dwc3
> +          - qcom,sdx75-dwc3
> +          - qcom,sm4250-dwc3
> +          - qcom,sm6115-dwc3
> +          - qcom,sm6125-dwc3
> +          - qcom,sm6350-dwc3
> +          - qcom,sm6375-dwc3
> +          - qcom,sm8150-dwc3
> +          - qcom,sm8250-dwc3
> +          - qcom,sm8350-dwc3
> +          - qcom,sm8450-dwc3
> +          - qcom,sm8550-dwc3
> +      - const: qcom,dwc3
> +      - const: snps,dwc3
> +
> +  reg:
> +    description: Offset and length of register set for QSCRATCH wrapper
> +    maxItems: 1

Isn't this more things now? Or the description is wrong.

Rob
  
Bjorn Andersson Oct. 17, 2023, 9:02 p.m. UTC | #3
On Tue, Oct 17, 2023 at 01:31:36PM -0500, Rob Herring wrote:
> On Mon, Oct 16, 2023 at 08:11:18PM -0700, Bjorn Andersson wrote:
> > diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
[..]
> > +  reg:
> > +    description: Offset and length of register set for QSCRATCH wrapper
> > +    maxItems: 1
> 
> Isn't this more things now? Or the description is wrong.
> 

The description is wrong, the single cell is now intended to cover the
whole USB block, which spans XHCI, DWC3 and Qualcomm glue parts.

Regards,
Bjorn
  
Bjorn Andersson Oct. 17, 2023, 10:54 p.m. UTC | #4
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
  
Krzysztof Kozlowski Oct. 18, 2023, 6 a.m. UTC | #5
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
  
Johan Hovold Nov. 22, 2023, 12:40 p.m. UTC | #6
On Mon, Oct 16, 2023 at 08:11:18PM -0700, Bjorn Andersson wrote:
> The Qualcomm USB block consists of three intertwined parts, the XHCI,
> the DWC3 core and the Qualcomm DWC3 glue. The three parts can not be
> operated independently, but the binding was for historical reasons split
> to mimic the Linux driver implementation.
> 
> The split binding also makes it hard to alter the implementation, as
> properties and resources are split between the two nodes, in some cases
> with some duplication.
> 
> Introduce a new binding, with a single representation of the whole USB
> block in one node.
> 
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,sc8280xp-dwc3
> +              - qcom,sc8280xp-dwc3-mp

The multiport implementation is not ready yet and this part of the
binding has been reverted (similar for the multiport interrupts below).

> +    then:
> +      properties:
> +        clocks:
> +          maxItems: 9
> +        clock-names:
> +          items:
> +            - const: cfg_noc
> +            - const: core
> +            - const: iface
> +            - const: sleep
> +            - const: mock_utmi
> +            - const: noc_aggr
> +            - const: noc_aggr_north
> +            - const: noc_aggr_south
> +            - const: noc_sys

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,sc8280xp-dwc3-mp
> +    then:
> +      properties:
> +        interrupts:
> +          maxItems: 14
> +        interrupt-names:
> +          items:
> +            - const: pwr_event_1
> +            - const: pwr_event_2
> +            - const: pwr_event_3
> +            - const: pwr_event_4
> +            - const: dp_hs_phy_1
> +            - const: dm_hs_phy_1
> +            - const: dp_hs_phy_2
> +            - const: dm_hs_phy_2
> +            - const: dp_hs_phy_3
> +            - const: dm_hs_phy_3
> +            - const: dp_hs_phy_4
> +            - const: dm_hs_phy_4
> +            - const: ss_phy_1
> +            - const: ss_phy_2

So same here.

> +    else:
> +      properties:
> +        interrupts:
> +          minItems: 1
> +          items:
> +            - description: Common DWC3 interrupt
> +            - description: The interrupt that is asserted
> +                when a wakeup event is received on USB2 bus.
> +            - description: The interrupt that is asserted
> +                when a wakeup event is received on USB3 bus.
> +            - description: Wakeup event on DM line.
> +            - description: Wakeup event on DP line.

I guess you may have copied this from the current binding but the
descriptions here are not correct. The HS/SS interrupt comes from the
PHYs in case the corresponding events have been enabled. I assume it can
be used for connect/disconnect events as well as remote wakeup and
whether to actually wake the system up on those is an implementation
detail.

Similar for DM/DP which represents the state of the data lines and that
can be used to detect all sorts of events, not just remote wakeup.

> +
> +        interrupt-names:
> +          minItems: 1
> +          items:
> +            - const: dwc_usb3
> +            - const: hs_phy_irq
> +            - const: ss_phy_irq
> +            - const: dm_hs_phy_irq
> +            - const: dp_hs_phy_irq

And here you are now defining all of these interrupts for all the
current SoCs it seems, despite not all of them actually having all of
these at once. (The order also does not match the current devicetrees.)

Some only have HS/SS, and it's not clear whether the HS interrupts are
actually functional when a SoC is also using DP/DM.

We're currently discussing this here:

	https://lore.kernel.org/lkml/ZVYTFi3Jnnljl48L@hovoldconsulting.com/

> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/qcom,gcc-sdm845.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    usb@a600000 {
> +        compatible = "qcom,sdm845-dwc3", "qcom,dwc3", "snps,dwc3";
> +        reg = <0x0a600000 0x200000>;

> +        snps,dis_u2_susphy_quirk;
> +        snps,dis_enblslpm_quirk;
> +        phys = <&usb_1_hsphy>, <&usb_1_ssphy>;
> +        phy-names = "usb2-phy", "usb3-phy";
> +

Stray newline.

> +    };
> +...

Johan
  

Patch

diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
new file mode 100644
index 000000000000..cb50261c6a36
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
@@ -0,0 +1,482 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/qcom,dwc3.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm SuperSpeed DWC3 USB SoC controller
+
+maintainers:
+  - Wesley Cheng <quic_wcheng@quicinc.com>
+
+select:
+  properties:
+    compatible:
+      items:
+        - enum:
+            - qcom,ipq4019-dwc3
+            - qcom,ipq5018-dwc3
+            - qcom,ipq5332-dwc3
+            - qcom,ipq6018-dwc3
+            - qcom,ipq8064-dwc3
+            - qcom,ipq8074-dwc3
+            - qcom,ipq9574-dwc3
+            - qcom,msm8953-dwc3
+            - qcom,msm8994-dwc3
+            - qcom,msm8996-dwc3
+            - qcom,msm8998-dwc3
+            - qcom,qcm2290-dwc3
+            - qcom,qcs404-dwc3
+            - qcom,sa8775p-dwc3
+            - qcom,sc7180-dwc3
+            - qcom,sc7280-dwc3
+            - qcom,sc8180x-dwc3
+            - qcom,sc8280xp-dwc3
+            - qcom,sc8280xp-dwc3-mp
+            - qcom,sdm660-dwc3
+            - qcom,sdm670-dwc3
+            - qcom,sdm845-dwc3
+            - qcom,sdx55-dwc3
+            - qcom,sdx65-dwc3
+            - qcom,sdx75-dwc3
+            - qcom,sm4250-dwc3
+            - qcom,sm6115-dwc3
+            - qcom,sm6125-dwc3
+            - qcom,sm6350-dwc3
+            - qcom,sm6375-dwc3
+            - qcom,sm8150-dwc3
+            - qcom,sm8250-dwc3
+            - qcom,sm8350-dwc3
+            - qcom,sm8450-dwc3
+            - qcom,sm8550-dwc3
+        - const: qcom,dwc3
+        - const: snps,dwc3
+  required:
+    - compatible
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - qcom,ipq4019-dwc3
+          - qcom,ipq5018-dwc3
+          - qcom,ipq5332-dwc3
+          - qcom,ipq6018-dwc3
+          - qcom,ipq8064-dwc3
+          - qcom,ipq8074-dwc3
+          - qcom,ipq9574-dwc3
+          - qcom,msm8953-dwc3
+          - qcom,msm8994-dwc3
+          - qcom,msm8996-dwc3
+          - qcom,msm8998-dwc3
+          - qcom,qcm2290-dwc3
+          - qcom,qcs404-dwc3
+          - qcom,sa8775p-dwc3
+          - qcom,sc7180-dwc3
+          - qcom,sc7280-dwc3
+          - qcom,sc8180x-dwc3
+          - qcom,sc8280xp-dwc3
+          - qcom,sc8280xp-dwc3-mp
+          - qcom,sdm660-dwc3
+          - qcom,sdm670-dwc3
+          - qcom,sdm845-dwc3
+          - qcom,sdx55-dwc3
+          - qcom,sdx65-dwc3
+          - qcom,sdx75-dwc3
+          - qcom,sm4250-dwc3
+          - qcom,sm6115-dwc3
+          - qcom,sm6125-dwc3
+          - qcom,sm6350-dwc3
+          - qcom,sm6375-dwc3
+          - qcom,sm8150-dwc3
+          - qcom,sm8250-dwc3
+          - qcom,sm8350-dwc3
+          - qcom,sm8450-dwc3
+          - qcom,sm8550-dwc3
+      - const: qcom,dwc3
+      - const: snps,dwc3
+
+  reg:
+    description: Offset and length of register set for QSCRATCH wrapper
+    maxItems: 1
+
+  power-domains:
+    description: specifies a phandle to PM domain provider node
+    maxItems: 1
+
+  required-opps:
+    maxItems: 1
+
+  clocks:
+    description: |
+      Several clocks are used, depending on the variant. Typical ones are::
+       - cfg_noc:: System Config NOC clock.
+       - core:: Master/Core clock, has to be >= 125 MHz for SS operation and >=
+                60MHz for HS operation.
+       - iface:: System bus AXI clock.
+       - sleep:: Sleep clock, used for wakeup when USB3 core goes into low
+                 power mode (U3).
+       - mock_utmi:: Mock utmi clock needed for ITP/SOF generation in host
+                     mode. Its frequency should be 19.2MHz.
+    minItems: 1
+    maxItems: 9
+
+  clock-names:
+    minItems: 1
+    maxItems: 9
+
+  resets:
+    maxItems: 1
+
+  interconnects:
+    maxItems: 2
+
+  interconnect-names:
+    items:
+      - const: usb-ddr
+      - const: apps-usb
+
+  interrupts:
+    minItems: 2
+    maxItems: 5
+
+  interrupt-names:
+    minItems: 2
+    maxItems: 5
+
+  qcom,select-utmi-as-pipe-clk:
+    description:
+      If present, disable USB3 pipe_clk requirement.
+      Used when dwc3 operates without SSPHY and only
+      HS/FS/LS modes are supported.
+    type: boolean
+
+  wakeup-source: true
+
+# Required child node:
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - interrupts
+  - interrupt-names
+
+allOf:
+  - $ref: snps,dwc3.yaml#
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,ipq4019-dwc3
+    then:
+      properties:
+        clocks:
+          maxItems: 3
+        clock-names:
+          items:
+            - const: core
+            - const: sleep
+            - const: mock_utmi
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,ipq8064-dwc3
+    then:
+      properties:
+        clocks:
+          items:
+            - description: Master/Core clock, has to be >= 125 MHz
+                for SS operation and >= 60MHz for HS operation.
+        clock-names:
+          items:
+            - const: core
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,ipq9574-dwc3
+              - qcom,msm8953-dwc3
+              - qcom,msm8996-dwc3
+              - qcom,msm8998-dwc3
+              - qcom,sa8775p-dwc3
+              - qcom,sc7180-dwc3
+              - qcom,sc7280-dwc3
+              - qcom,sdm670-dwc3
+              - qcom,sdm845-dwc3
+              - qcom,sdx55-dwc3
+              - qcom,sdx65-dwc3
+              - qcom,sdx75-dwc3
+              - qcom,sm6350-dwc3
+    then:
+      properties:
+        clocks:
+          maxItems: 5
+        clock-names:
+          items:
+            - const: cfg_noc
+            - const: core
+            - const: iface
+            - const: sleep
+            - const: mock_utmi
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,ipq6018-dwc3
+    then:
+      properties:
+        clocks:
+          minItems: 3
+          maxItems: 4
+        clock-names:
+          oneOf:
+            - items:
+                - const: core
+                - const: sleep
+                - const: mock_utmi
+            - items:
+                - const: cfg_noc
+                - const: core
+                - const: sleep
+                - const: mock_utmi
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,ipq8074-dwc3
+    then:
+      properties:
+        clocks:
+          maxItems: 4
+        clock-names:
+          items:
+            - const: cfg_noc
+            - const: core
+            - const: sleep
+            - const: mock_utmi
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,ipq5018-dwc3
+              - qcom,ipq5332-dwc3
+              - qcom,msm8994-dwc3
+              - qcom,qcs404-dwc3
+    then:
+      properties:
+        clocks:
+          maxItems: 4
+        clock-names:
+          items:
+            - const: core
+            - const: iface
+            - const: sleep
+            - const: mock_utmi
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,sc8280xp-dwc3
+              - qcom,sc8280xp-dwc3-mp
+    then:
+      properties:
+        clocks:
+          maxItems: 9
+        clock-names:
+          items:
+            - const: cfg_noc
+            - const: core
+            - const: iface
+            - const: sleep
+            - const: mock_utmi
+            - const: noc_aggr
+            - const: noc_aggr_north
+            - const: noc_aggr_south
+            - const: noc_sys
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,sdm660-dwc3
+    then:
+      properties:
+        clocks:
+          minItems: 5
+          maxItems: 6
+        clock-names:
+          oneOf:
+            - items:
+                - const: cfg_noc
+                - const: core
+                - const: iface
+                - const: sleep
+                - const: mock_utmi
+                - const: bus
+            - items:
+                - const: cfg_noc
+                - const: core
+                - const: sleep
+                - const: mock_utmi
+                - const: bus
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,qcm2290-dwc3
+              - qcom,sc8180x-dwc3
+              - qcom,sm6115-dwc3
+              - qcom,sm6125-dwc3
+              - qcom,sm8150-dwc3
+              - qcom,sm8250-dwc3
+              - qcom,sm8450-dwc3
+              - qcom,sm8550-dwc3
+    then:
+      properties:
+        clocks:
+          minItems: 6
+        clock-names:
+          items:
+            - const: cfg_noc
+            - const: core
+            - const: iface
+            - const: sleep
+            - const: mock_utmi
+            - const: xo
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,sm8350-dwc3
+    then:
+      properties:
+        clocks:
+          minItems: 5
+          maxItems: 6
+        clock-names:
+          minItems: 5
+          items:
+            - const: cfg_noc
+            - const: core
+            - const: iface
+            - const: sleep
+            - const: mock_utmi
+            - const: xo
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,sc8280xp-dwc3-mp
+    then:
+      properties:
+        interrupts:
+          maxItems: 14
+        interrupt-names:
+          items:
+            - const: pwr_event_1
+            - const: pwr_event_2
+            - const: pwr_event_3
+            - const: pwr_event_4
+            - const: dp_hs_phy_1
+            - const: dm_hs_phy_1
+            - const: dp_hs_phy_2
+            - const: dm_hs_phy_2
+            - const: dp_hs_phy_3
+            - const: dm_hs_phy_3
+            - const: dp_hs_phy_4
+            - const: dm_hs_phy_4
+            - const: ss_phy_1
+            - const: ss_phy_2
+    else:
+      properties:
+        interrupts:
+          minItems: 1
+          items:
+            - description: Common DWC3 interrupt
+            - description: The interrupt that is asserted
+                when a wakeup event is received on USB2 bus.
+            - description: The interrupt that is asserted
+                when a wakeup event is received on USB3 bus.
+            - description: Wakeup event on DM line.
+            - description: Wakeup event on DP line.
+
+        interrupt-names:
+          minItems: 1
+          items:
+            - const: dwc_usb3
+            - const: hs_phy_irq
+            - const: ss_phy_irq
+            - const: dm_hs_phy_irq
+            - const: dp_hs_phy_irq
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,gcc-sdm845.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    usb@a600000 {
+        compatible = "qcom,sdm845-dwc3", "qcom,dwc3", "snps,dwc3";
+        reg = <0x0a600000 0x200000>;
+
+        clocks = <&gcc GCC_CFG_NOC_USB3_PRIM_AXI_CLK>,
+                 <&gcc GCC_USB30_PRIM_MASTER_CLK>,
+                 <&gcc GCC_AGGRE_USB3_PRIM_AXI_CLK>,
+                 <&gcc GCC_USB30_PRIM_SLEEP_CLK>,
+                 <&gcc GCC_USB30_PRIM_MOCK_UTMI_CLK>;
+        clock-names = "cfg_noc",
+                      "core",
+                      "iface",
+                      "sleep",
+                      "mock_utmi";
+
+        assigned-clocks = <&gcc GCC_USB30_PRIM_MOCK_UTMI_CLK>,
+                      <&gcc GCC_USB30_PRIM_MASTER_CLK>;
+        assigned-clock-rates = <19200000>, <150000000>;
+
+        interrupts = <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 486 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 488 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 489 IRQ_TYPE_LEVEL_HIGH>;
+        interrupt-names = "dwc_usb3", "hs_phy_irq", "ss_phy_irq",
+                      "dm_hs_phy_irq", "dp_hs_phy_irq";
+
+        power-domains = <&gcc USB30_PRIM_GDSC>;
+
+        iommus = <&apps_smmu 0x740 0>;
+
+        resets = <&gcc GCC_USB30_PRIM_BCR>;
+
+        snps,dis_u2_susphy_quirk;
+        snps,dis_enblslpm_quirk;
+        phys = <&usb_1_hsphy>, <&usb_1_ssphy>;
+        phy-names = "usb2-phy", "usb3-phy";
+
+    };
+...
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
+      - minItems: 1
+        maxItems: 4
+        items:
           enum: [host, peripheral, otg, wakeup]
 
   clocks: