2021-10-13 16:56:08

by Fabrice Gasnier

[permalink] [raw]
Subject: [PATCH] dt-bindings: usb: dwc2: document the port when usb-role-switch is used

Document the "port" property, which is used with "usb-role-switch"
to describe the bus connector.
Definition is inspired from mediatek,mtu3.yaml.

This fixes some errors seen when running "make dtbs_check":
... 'port' does not match any of the regexes: 'pinctrl-[0-9]+'
From schema: ... Documentation/devicetree/bindings/usb/dwc2.yaml

Signed-off-by: Fabrice Gasnier <[email protected]>
---
Documentation/devicetree/bindings/usb/dwc2.yaml | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc2.yaml b/Documentation/devicetree/bindings/usb/dwc2.yaml
index 10c7d9b..7d1aa53 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.yaml
+++ b/Documentation/devicetree/bindings/usb/dwc2.yaml
@@ -130,6 +130,16 @@ properties:
description: If present indicates that we need to reset the PHY when we
detect a wakeup. This is due to a hardware errata.

+ port:
+ description:
+ Any connector to the data bus of this controller should be modelled
+ using the OF graph bindings specified, if the "usb-role-switch"
+ property is used. See graph.txt
+ $ref: /schemas/graph.yaml#/properties/port
+
+dependencies:
+ port: [ 'usb-role-switch' ]
+
required:
- compatible
- reg
--
2.7.4


2021-10-27 10:24:41

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: usb: dwc2: document the port when usb-role-switch is used

On Wed, Oct 13, 2021 at 06:53:46PM +0200, Fabrice Gasnier wrote:
> Document the "port" property, which is used with "usb-role-switch"
> to describe the bus connector.
> Definition is inspired from mediatek,mtu3.yaml.
>
> This fixes some errors seen when running "make dtbs_check":
> ... 'port' does not match any of the regexes: 'pinctrl-[0-9]+'
> From schema: ... Documentation/devicetree/bindings/usb/dwc2.yaml
>
> Signed-off-by: Fabrice Gasnier <[email protected]>
> ---
> Documentation/devicetree/bindings/usb/dwc2.yaml | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/usb/dwc2.yaml b/Documentation/devicetree/bindings/usb/dwc2.yaml
> index 10c7d9b..7d1aa53 100644
> --- a/Documentation/devicetree/bindings/usb/dwc2.yaml
> +++ b/Documentation/devicetree/bindings/usb/dwc2.yaml
> @@ -130,6 +130,16 @@ properties:
> description: If present indicates that we need to reset the PHY when we
> detect a wakeup. This is due to a hardware errata.
>
> + port:
> + description:
> + Any connector to the data bus of this controller should be modelled
> + using the OF graph bindings specified, if the "usb-role-switch"
> + property is used. See graph.txt

Drop 'See graph.txt'

> + $ref: /schemas/graph.yaml#/properties/port
> +
> +dependencies:
> + port: [ 'usb-role-switch' ]

usb-role-switch without port is valid or both must be present. In case
of the latter, you need to add:

usb-role-switch: [ port ]

Also, you don't need quotes.

> +
> required:
> - compatible
> - reg
> --
> 2.7.4
>
>

2021-10-27 21:27:15

by Fabrice Gasnier

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: usb: dwc2: document the port when usb-role-switch is used

On 10/26/21 10:38 PM, Rob Herring wrote:
> On Wed, Oct 13, 2021 at 06:53:46PM +0200, Fabrice Gasnier wrote:
>> Document the "port" property, which is used with "usb-role-switch"
>> to describe the bus connector.
>> Definition is inspired from mediatek,mtu3.yaml.
>>
>> This fixes some errors seen when running "make dtbs_check":
>> ... 'port' does not match any of the regexes: 'pinctrl-[0-9]+'
>> From schema: ... Documentation/devicetree/bindings/usb/dwc2.yaml
>>
>> Signed-off-by: Fabrice Gasnier <[email protected]>
>> ---
>> Documentation/devicetree/bindings/usb/dwc2.yaml | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/dwc2.yaml b/Documentation/devicetree/bindings/usb/dwc2.yaml
>> index 10c7d9b..7d1aa53 100644
>> --- a/Documentation/devicetree/bindings/usb/dwc2.yaml
>> +++ b/Documentation/devicetree/bindings/usb/dwc2.yaml
>> @@ -130,6 +130,16 @@ properties:
>> description: If present indicates that we need to reset the PHY when we
>> detect a wakeup. This is due to a hardware errata.
>>
>> + port:
>> + description:
>> + Any connector to the data bus of this controller should be modelled
>> + using the OF graph bindings specified, if the "usb-role-switch"
>> + property is used. See graph.txt
>
> Drop 'See graph.txt'
>
>> + $ref: /schemas/graph.yaml#/properties/port
>> +
>> +dependencies:
>> + port: [ 'usb-role-switch' ]
>
> usb-role-switch without port is valid or both must be present. In case
> of the latter, you need to add:

Hi Rob,

Thanks for your review.
I agree with your statement. But I miss something in this proposal:

>
> usb-role-switch: [ port ]

I tried this without success, when running dt_binding_check.

For testing, I added locally in the example successively:

1 - usb-role-switch (alone)
-----
+ usb-role-switch;
"dwc2.example.dt.yaml: usb@101c0000: 'port' is a dependency of
'usb-role-switch'

2 - port without usb-role-switch
-----
+ //usb-role-switch;
+
+ port {
+ usb_role_sw: endpoint {
+ remote-endpoint = <&hs_ep>;
+ };
+ };
dt_binding_check is happy here. But it shouldn't, right ?

3 - both (port + usb-role-switch)
-----
dt_binding_check is ok.


But it seems functional, with
+ dependencies:
+ port: [ usb-role-switch ]
Shall I keep (without quotes) ?
With this, above statement, (changes in example) seems correctly
validated with dt_binding_check.

1 - dt_binding_check is happy
2 - usb@101c0000: 'usb-role-switch' is a dependency of 'port'
(as expected ?)
3 - dt_binding_check is happy

I'll send a v2 with the other changes you propose.
Please review it there.

Best Regards,
Fabrice

>
> Also, you don't need quotes.
>
>> +
>> required:
>> - compatible
>> - reg
>> --
>> 2.7.4
>>
>>

2021-10-28 22:53:11

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: usb: dwc2: document the port when usb-role-switch is used

On Wed, Oct 27, 2021 at 7:05 AM Fabrice Gasnier
<[email protected]> wrote:
>
> On 10/26/21 10:38 PM, Rob Herring wrote:
> > On Wed, Oct 13, 2021 at 06:53:46PM +0200, Fabrice Gasnier wrote:
> >> Document the "port" property, which is used with "usb-role-switch"
> >> to describe the bus connector.
> >> Definition is inspired from mediatek,mtu3.yaml.
> >>
> >> This fixes some errors seen when running "make dtbs_check":
> >> ... 'port' does not match any of the regexes: 'pinctrl-[0-9]+'
> >> From schema: ... Documentation/devicetree/bindings/usb/dwc2.yaml
> >>
> >> Signed-off-by: Fabrice Gasnier <[email protected]>
> >> ---
> >> Documentation/devicetree/bindings/usb/dwc2.yaml | 10 ++++++++++
> >> 1 file changed, 10 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/usb/dwc2.yaml b/Documentation/devicetree/bindings/usb/dwc2.yaml
> >> index 10c7d9b..7d1aa53 100644
> >> --- a/Documentation/devicetree/bindings/usb/dwc2.yaml
> >> +++ b/Documentation/devicetree/bindings/usb/dwc2.yaml
> >> @@ -130,6 +130,16 @@ properties:
> >> description: If present indicates that we need to reset the PHY when we
> >> detect a wakeup. This is due to a hardware errata.
> >>
> >> + port:
> >> + description:
> >> + Any connector to the data bus of this controller should be modelled
> >> + using the OF graph bindings specified, if the "usb-role-switch"
> >> + property is used. See graph.txt
> >
> > Drop 'See graph.txt'
> >
> >> + $ref: /schemas/graph.yaml#/properties/port
> >> +
> >> +dependencies:
> >> + port: [ 'usb-role-switch' ]
> >
> > usb-role-switch without port is valid or both must be present. In case
> > of the latter, you need to add:

If I wasn't clear, that was a question. Is usb-role-switch without
port is valid?

>
> Hi Rob,
>
> Thanks for your review.
> I agree with your statement. But I miss something in this proposal:
>
> >
> > usb-role-switch: [ port ]
>
> I tried this without success, when running dt_binding_check.
>
> For testing, I added locally in the example successively:
>
> 1 - usb-role-switch (alone)
> -----
> + usb-role-switch;
> "dwc2.example.dt.yaml: usb@101c0000: 'port' is a dependency of
> 'usb-role-switch'
>
> 2 - port without usb-role-switch
> -----
> + //usb-role-switch;
> +
> + port {
> + usb_role_sw: endpoint {
> + remote-endpoint = <&hs_ep>;
> + };
> + };
> dt_binding_check is happy here. But it shouldn't, right ?

You had this?:

dependencies:
usb-role-switch: [ port ]
port: [ usb-role-switch ]


If both properties have to be present together, then you need the above.

>
> 3 - both (port + usb-role-switch)
> -----
> dt_binding_check is ok.

2021-10-29 14:53:50

by Fabrice Gasnier

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: usb: dwc2: document the port when usb-role-switch is used

On 10/29/21 12:50 AM, Rob Herring wrote:
> On Wed, Oct 27, 2021 at 7:05 AM Fabrice Gasnier
> <[email protected]> wrote:
>>
>> On 10/26/21 10:38 PM, Rob Herring wrote:
>>> On Wed, Oct 13, 2021 at 06:53:46PM +0200, Fabrice Gasnier wrote:
>>>> Document the "port" property, which is used with "usb-role-switch"
>>>> to describe the bus connector.
>>>> Definition is inspired from mediatek,mtu3.yaml.
>>>>
>>>> This fixes some errors seen when running "make dtbs_check":
>>>> ... 'port' does not match any of the regexes: 'pinctrl-[0-9]+'
>>>> From schema: ... Documentation/devicetree/bindings/usb/dwc2.yaml
>>>>
>>>> Signed-off-by: Fabrice Gasnier <[email protected]>
>>>> ---
>>>> Documentation/devicetree/bindings/usb/dwc2.yaml | 10 ++++++++++
>>>> 1 file changed, 10 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/usb/dwc2.yaml b/Documentation/devicetree/bindings/usb/dwc2.yaml
>>>> index 10c7d9b..7d1aa53 100644
>>>> --- a/Documentation/devicetree/bindings/usb/dwc2.yaml
>>>> +++ b/Documentation/devicetree/bindings/usb/dwc2.yaml
>>>> @@ -130,6 +130,16 @@ properties:
>>>> description: If present indicates that we need to reset the PHY when we
>>>> detect a wakeup. This is due to a hardware errata.
>>>>
>>>> + port:
>>>> + description:
>>>> + Any connector to the data bus of this controller should be modelled
>>>> + using the OF graph bindings specified, if the "usb-role-switch"
>>>> + property is used. See graph.txt
>>>
>>> Drop 'See graph.txt'
>>>
>>>> + $ref: /schemas/graph.yaml#/properties/port
>>>> +
>>>> +dependencies:
>>>> + port: [ 'usb-role-switch' ]
>>>
>>> usb-role-switch without port is valid or both must be present. In case
>>> of the latter, you need to add:
>
> If I wasn't clear, that was a question. Is usb-role-switch without
> port is valid?

Hi Rob,

Ah ok... I didn't get it was a question.
So, usb-role-switch without port is valid.

>
>>
>> Hi Rob,
>>
>> Thanks for your review.
>> I agree with your statement. But I miss something in this proposal:
>>
>>>
>>> usb-role-switch: [ port ]
>>
>> I tried this without success, when running dt_binding_check.
>>
>> For testing, I added locally in the example successively:
>>
>> 1 - usb-role-switch (alone)
>> -----
>> + usb-role-switch;
>> "dwc2.example.dt.yaml: usb@101c0000: 'port' is a dependency of
>> 'usb-role-switch'
>>
>> 2 - port without usb-role-switch
>> -----
>> + //usb-role-switch;
>> +
>> + port {
>> + usb_role_sw: endpoint {
>> + remote-endpoint = <&hs_ep>;
>> + };
>> + };
>> dt_binding_check is happy here. But it shouldn't, right ?
>
> You had this?:
>
> dependencies:
> usb-role-switch: [ port ]
> port: [ usb-role-switch ]

no, above test was only with "usb-role-switch: [ port ]"

>
>
> If both properties have to be present together, then you need the above.

This isn't the case. The only constraints here is, if port is used, then
usb-role-switch must be set.

dependencies:
port: [ usb-role-switch ]

V2 is under review, with the above.

BR,
Fabrice
>
>>
>> 3 - both (port + usb-role-switch)
>> -----
>> dt_binding_check is ok.