[PATCH 08/11] dt-bindings: usb: ci-hdrc-usb2: Fix handling pinctrl properties

Konrad Dybcio posted 11 patches 2 years, 7 months ago
[PATCH 08/11] dt-bindings: usb: ci-hdrc-usb2: Fix handling pinctrl properties
Posted by Konrad Dybcio 2 years, 7 months ago
Untangle the bit messy oneOf trees and add the missing pinctrl-2 mention
to handle the different pinctrl combinations.

Fixes: 4c8375d35f72 ("dt-bindings: usb: ci-hdrc-usb2: convert to DT schema format")
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 .../devicetree/bindings/usb/ci-hdrc-usb2.yaml      | 27 ++++++----------------
 1 file changed, 7 insertions(+), 20 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
index 782402800d4a..24431a7adf3e 100644
--- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
+++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
@@ -199,17 +199,6 @@ properties:
       In case of HSIC-mode, "idle" and "active" pin modes are mandatory.
       In this case, the "idle" state needs to pull down the data and
       strobe pin and the "active" state needs to pull up the strobe pin.
-    oneOf:
-      - items:
-          - const: idle
-          - const: active
-      - items:
-          - const: default
-          - enum:
-              - host
-              - device
-      - items:
-          - const: default
 
   pinctrl-0:
     maxItems: 1
@@ -357,17 +346,15 @@ allOf:
             - const: active
     else:
       properties:
+        pinctrl-2:
+          maxItems: 1
+
         pinctrl-names:
           minItems: 1
-          maxItems: 2
-          oneOf:
-            - items:
-                - const: default
-                - enum:
-                    - host
-                    - device
-            - items:
-                - const: default
+          items:
+            - const: default
+            - const: host
+            - const: device
   - if:
       properties:
         compatible:

-- 
2.41.0
Re: [PATCH 08/11] dt-bindings: usb: ci-hdrc-usb2: Fix handling pinctrl properties
Posted by Rob Herring 2 years, 7 months ago
On Tue, Jun 27, 2023 at 06:24:24PM +0200, Konrad Dybcio wrote:
> Untangle the bit messy oneOf trees and add the missing pinctrl-2 mention
> to handle the different pinctrl combinations.
> 
> Fixes: 4c8375d35f72 ("dt-bindings: usb: ci-hdrc-usb2: convert to DT schema format")
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>  .../devicetree/bindings/usb/ci-hdrc-usb2.yaml      | 27 ++++++----------------
>  1 file changed, 7 insertions(+), 20 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> index 782402800d4a..24431a7adf3e 100644
> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> @@ -199,17 +199,6 @@ properties:
>        In case of HSIC-mode, "idle" and "active" pin modes are mandatory.
>        In this case, the "idle" state needs to pull down the data and
>        strobe pin and the "active" state needs to pull up the strobe pin.
> -    oneOf:
> -      - items:
> -          - const: idle
> -          - const: active

These are no longer valid values? The description still mentions them.

> -      - items:
> -          - const: default
> -          - enum:
> -              - host
> -              - device
> -      - items:
> -          - const: default
>  
>    pinctrl-0:
>      maxItems: 1
> @@ -357,17 +346,15 @@ allOf:
>              - const: active
>      else:
>        properties:
> +        pinctrl-2:

This should be implicitly allowed. Is it not?

I'm reallly at a loss as to what problem this patch solves.

> +          maxItems: 1
> +
>          pinctrl-names:
>            minItems: 1
> -          maxItems: 2
> -          oneOf:
> -            - items:
> -                - const: default
> -                - enum:
> -                    - host
> -                    - device
> -            - items:
> -                - const: default
> +          items:
> +            - const: default
> +            - const: host
> +            - const: device
>    - if:
>        properties:
>          compatible:
> 
> -- 
> 2.41.0
>
Re: [PATCH 08/11] dt-bindings: usb: ci-hdrc-usb2: Fix handling pinctrl properties
Posted by Konrad Dybcio 2 years, 7 months ago
On 29.06.2023 17:23, Rob Herring wrote:
> On Tue, Jun 27, 2023 at 06:24:24PM +0200, Konrad Dybcio wrote:
>> Untangle the bit messy oneOf trees and add the missing pinctrl-2 mention
>> to handle the different pinctrl combinations.
>>
>> Fixes: 4c8375d35f72 ("dt-bindings: usb: ci-hdrc-usb2: convert to DT schema format")
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>> ---
>>  .../devicetree/bindings/usb/ci-hdrc-usb2.yaml      | 27 ++++++----------------
>>  1 file changed, 7 insertions(+), 20 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
>> index 782402800d4a..24431a7adf3e 100644
>> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
>> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
>> @@ -199,17 +199,6 @@ properties:
>>        In case of HSIC-mode, "idle" and "active" pin modes are mandatory.
>>        In this case, the "idle" state needs to pull down the data and
>>        strobe pin and the "active" state needs to pull up the strobe pin.
>> -    oneOf:
>> -      - items:
>> -          - const: idle
>> -          - const: active
> 
> These are no longer valid values? The description still mentions them.
I believe allOf: now covers them all?

> 
>> -      - items:
>> -          - const: default
>> -          - enum:
>> -              - host
>> -              - device
>> -      - items:
>> -          - const: default
>>  
>>    pinctrl-0:
>>      maxItems: 1
>> @@ -357,17 +346,15 @@ allOf:
>>              - const: active
>>      else:
>>        properties:
>> +        pinctrl-2:
> 
> This should be implicitly allowed. Is it not?
No, it errored out for me.

> 
> I'm reallly at a loss as to what problem this patch solves.
Specifying all 3 pin states is impossible with the current state of
this binding, even though it's a supported configuration (check
qcom/apq8039-t2.dtb). I should have been more clear in the commit
message.

Konrad

> 
>> +          maxItems: 1
>> +
>>          pinctrl-names:
>>            minItems: 1
>> -          maxItems: 2
>> -          oneOf:
>> -            - items:
>> -                - const: default
>> -                - enum:
>> -                    - host
>> -                    - device
>> -            - items:
>> -                - const: default
>> +          items:
>> +            - const: default
>> +            - const: host
>> +            - const: device
>>    - if:
>>        properties:
>>          compatible:
>>
>> -- 
>> 2.41.0
>>