Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Fabrice Gasnier <fabrice.gasnier@foss.st.com>
Subject: Re: [PATCH] dt-bindings: usb: dwc2: document the port when
 usb-role-switch is used
To:     Rob Herring <robh@kernel.org>
CC:     <hminas@synopsys.com>, <gregkh@linuxfoundation.org>,
        <linux-usb@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <linux-stm32@st-md-mailman.stormreply.com>,
        <devicetree@vger.kernel.org>, <amelie.delaunay@foss.st.com>,
        <alexandre.torgue@foss.st.com>
References: <1634144026-3326-1-git-send-email-fabrice.gasnier@foss.st.com>
 <YXhnLh9OYxUz8dIC@robh.at.kernel.org>
Message-ID: <be749cbe-c029-7a24-da0e-94ab363ea07e@foss.st.com>
Date:   Wed, 27 Oct 2021 14:05:15 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <YXhnLh9OYxUz8dIC@robh.at.kernel.org>
Content-Type: text/plain; charset="utf-8"
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

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 <fabrice.gasnier@foss.st.com>
>> ---
>>  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
>>
>>