diff mbox series

dt-bindings: usb: usb-device: allow multiple compatibles

Message ID 20250415-dt-binding-usb-device-compatibles-v1-1-90f3cff32aa0@cherry.de (mailing list archive)
State New
Headers show
Series dt-bindings: usb: usb-device: allow multiple compatibles | expand

Commit Message

Quentin Schulz April 15, 2025, 2:34 p.m. UTC
From: Quentin Schulz <quentin.schulz@cherry.de>

The dt-core typically allows multiple compatibles[1] but usb-device
currently forces a single compatible.

This is an issue when multiple devices with slightly different productID
all behave the same. This would require the driver to keep updating its
compatible matching table and the bindings to include this new productID
instead of doing what is usually done: have two compatibles, the
leftmost which matches exactly the HW device definition, and the
rightmost one as a fallback which is assumed to be 100% compatible with
the device at hand. If this assumption turns out to be wrong, it is easy
to work around this without having to modify the device tree by handling
the leftmost compatible in the driver.

[1] https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/dt-core.yaml#L21-L25

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
This came up while working on fixing USB on an RK3399 Puma which has an
onboard USB hub whose productID isn't in any driver compatible list
but which can be supported by a driver with a slightly different
productID matching another variant of the same IC, from the same
datasheet.

See https://lore.kernel.org/linux-rockchip/20250326-onboard_usb_dev-v1-0-a4b0a5d1b32c@thaumatec.com/
---
 Documentation/devicetree/bindings/usb/usb-device.yaml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


---
base-commit: 834a4a689699090a406d1662b03affa8b155d025
change-id: 20250415-dt-binding-usb-device-compatibles-188f7b0a81b4

Best regards,

Comments

Rob Herring (Arm) April 15, 2025, 3:34 p.m. UTC | #1
On Tue, 15 Apr 2025 16:34:27 +0200, Quentin Schulz wrote:
> From: Quentin Schulz <quentin.schulz@cherry.de>
> 
> The dt-core typically allows multiple compatibles[1] but usb-device
> currently forces a single compatible.
> 
> This is an issue when multiple devices with slightly different productID
> all behave the same. This would require the driver to keep updating its
> compatible matching table and the bindings to include this new productID
> instead of doing what is usually done: have two compatibles, the
> leftmost which matches exactly the HW device definition, and the
> rightmost one as a fallback which is assumed to be 100% compatible with
> the device at hand. If this assumption turns out to be wrong, it is easy
> to work around this without having to modify the device tree by handling
> the leftmost compatible in the driver.
> 
> [1] https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/dt-core.yaml#L21-L25
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
> ---
> This came up while working on fixing USB on an RK3399 Puma which has an
> onboard USB hub whose productID isn't in any driver compatible list
> but which can be supported by a driver with a slightly different
> productID matching another variant of the same IC, from the same
> datasheet.
> 
> See https://lore.kernel.org/linux-rockchip/20250326-onboard_usb_dev-v1-0-a4b0a5d1b32c@thaumatec.com/
> ---
>  Documentation/devicetree/bindings/usb/usb-device.yaml | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/usb-device.yaml: properties:compatible:items: {'pattern': '^usb[0-9a-f]{1,4},[0-9a-f]{1,4}$'} is not of type 'array'
	from schema $id: http://devicetree.org/meta-schemas/string-array.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250415-dt-binding-usb-device-compatibles-v1-1-90f3cff32aa0@cherry.de

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Rob Herring (Arm) April 15, 2025, 7:01 p.m. UTC | #2
On Tue, Apr 15, 2025 at 04:34:27PM +0200, Quentin Schulz wrote:
> From: Quentin Schulz <quentin.schulz@cherry.de>
> 
> The dt-core typically allows multiple compatibles[1] but usb-device
> currently forces a single compatible.
> 
> This is an issue when multiple devices with slightly different productID
> all behave the same. This would require the driver to keep updating its
> compatible matching table and the bindings to include this new productID
> instead of doing what is usually done: have two compatibles, the
> leftmost which matches exactly the HW device definition, and the
> rightmost one as a fallback which is assumed to be 100% compatible with
> the device at hand. If this assumption turns out to be wrong, it is easy
> to work around this without having to modify the device tree by handling
> the leftmost compatible in the driver.
> 
> [1] https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/dt-core.yaml#L21-L25
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
> ---
> This came up while working on fixing USB on an RK3399 Puma which has an
> onboard USB hub whose productID isn't in any driver compatible list
> but which can be supported by a driver with a slightly different
> productID matching another variant of the same IC, from the same
> datasheet.
> 
> See https://lore.kernel.org/linux-rockchip/20250326-onboard_usb_dev-v1-0-a4b0a5d1b32c@thaumatec.com/
> ---
>  Documentation/devicetree/bindings/usb/usb-device.yaml | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/usb-device.yaml b/Documentation/devicetree/bindings/usb/usb-device.yaml
> index c676956810331b81f11f3624340fc3e612c98315..9d55be4fb5981164cca969dbda5ba70ab3a87773 100644
> --- a/Documentation/devicetree/bindings/usb/usb-device.yaml
> +++ b/Documentation/devicetree/bindings/usb/usb-device.yaml
> @@ -28,7 +28,8 @@ description: |
>  
>  properties:
>    compatible:
> -    pattern: "^usb[0-9a-f]{1,4},[0-9a-f]{1,4}$"
> +    items:
> +      pattern: "^usb[0-9a-f]{1,4},[0-9a-f]{1,4}$"

I would use 'contains' here rather than 'items'. That's even more 
relaxed in allowing "normal" compatible strings, but is aligned with 
what we have for PCI device.

Rob
Quentin Schulz April 16, 2025, 9:14 a.m. UTC | #3
Hi Rob,

On 4/15/25 9:01 PM, Rob Herring wrote:
> On Tue, Apr 15, 2025 at 04:34:27PM +0200, Quentin Schulz wrote:
>> From: Quentin Schulz <quentin.schulz@cherry.de>
>>
>> The dt-core typically allows multiple compatibles[1] but usb-device
>> currently forces a single compatible.
>>
>> This is an issue when multiple devices with slightly different productID
>> all behave the same. This would require the driver to keep updating its
>> compatible matching table and the bindings to include this new productID
>> instead of doing what is usually done: have two compatibles, the
>> leftmost which matches exactly the HW device definition, and the
>> rightmost one as a fallback which is assumed to be 100% compatible with
>> the device at hand. If this assumption turns out to be wrong, it is easy
>> to work around this without having to modify the device tree by handling
>> the leftmost compatible in the driver.
>>
>> [1] https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/dt-core.yaml#L21-L25
>>
>> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
>> ---
>> This came up while working on fixing USB on an RK3399 Puma which has an
>> onboard USB hub whose productID isn't in any driver compatible list
>> but which can be supported by a driver with a slightly different
>> productID matching another variant of the same IC, from the same
>> datasheet.
>>
>> See https://lore.kernel.org/linux-rockchip/20250326-onboard_usb_dev-v1-0-a4b0a5d1b32c@thaumatec.com/
>> ---
>>   Documentation/devicetree/bindings/usb/usb-device.yaml | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/usb-device.yaml b/Documentation/devicetree/bindings/usb/usb-device.yaml
>> index c676956810331b81f11f3624340fc3e612c98315..9d55be4fb5981164cca969dbda5ba70ab3a87773 100644
>> --- a/Documentation/devicetree/bindings/usb/usb-device.yaml
>> +++ b/Documentation/devicetree/bindings/usb/usb-device.yaml
>> @@ -28,7 +28,8 @@ description: |
>>   
>>   properties:
>>     compatible:
>> -    pattern: "^usb[0-9a-f]{1,4},[0-9a-f]{1,4}$"
>> +    items:
>> +      pattern: "^usb[0-9a-f]{1,4},[0-9a-f]{1,4}$"
> 
> I would use 'contains' here rather than 'items'. That's even more
> relaxed in allowing "normal" compatible strings, but is aligned with
> what we have for PCI device.
> 

Thanks for the suggestion, makes sense.

Now, I'm wondering how to handle that on the actual device binding?

For example I tried the following:

diff --git a/Documentation/devicetree/bindings/usb/usb-device.yaml 
b/Documentation/devicetree/bindings/usb/usb-device.yaml
index 09fceb469f105..20a6c021ebdba 100644
--- a/Documentation/devicetree/bindings/usb/usb-device.yaml
+++ b/Documentation/devicetree/bindings/usb/usb-device.yaml
@@ -124,3 +124,15 @@ examples:
              };
          };
      };
+  - |
+    usb@11270000 {
+        reg = <0x11270000 0x1000>;
+        interrupts = <0x0 0x4e 0x0>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        hub@1 {
+            compatible = "usb5e3,609", "usb5e3,608";
+            reg = <1>;
+        };
+    };


And I got:

/home/qschulz/work/upstream/linux/build/puma/Documentation/devicetree/bindings/usb/usb-device.example.dtb: 
hub@1: compatible:0: 'usb5e3,609' is not one of ['usb5e3,608', 
'usb5e3,610', 'usb5e3,620', 'usb5e3,626']
	from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#

Fair enough, this means the DT binding currently always needs all 
compatibles to be defined.

Then I modified usb5e3,609 to be usb5e3,610, and I got:

/home/qschulz/work/upstream/linux/build/puma/Documentation/devicetree/bindings/usb/usb-device.example.dtb: 
hub@1: compatible: ['usb5e3,610', 'usb5e3,608'] is too long
	from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#

So it seems like we need the driver DT binding to handle multiple 
compatibles too. I needed to do:

diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml 
b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
index 6fe2d356dcbde..e8e5f78356334 100644
--- a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
+++ b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
@@ -11,11 +11,12 @@ maintainers:

  properties:
    compatible:
-    enum:
-      - usb5e3,608
-      - usb5e3,610
-      - usb5e3,620
-      - usb5e3,626
+    items:
+      enum:
+        - usb5e3,608
+        - usb5e3,610
+        - usb5e3,620
+        - usb5e3,626

    reg: true

for it to pass the dt_binding_check. I assume we do not want to use 
contains: in the event that the leftmost compatible is handled by one dt 
binding and the rightmost by another one, in which case they would both 
match and apply their own requirements/constraints?

In short, is it correct that when we want to add a 
"just-in-case-we-discover-quirks-later"-compatible to a device tree, we 
anyway want it explicitly listed in the dt binding matching the 
rightmost compatible?

I'm basically trying to understand what we want/need to do for the 
cypress,hx3 binding in RK3399 Puma series: 
https://lore.kernel.org/linux-rockchip/20250326-onboard_usb_dev-v1-0-a4b0a5d1b32c@thaumatec.com/ 
to be able to use a fallback compatible in the driver and still pass the 
dt checks. (Considering we will want to backport the patches to stable 
releases too).

Cheers,
Quentin
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/usb/usb-device.yaml b/Documentation/devicetree/bindings/usb/usb-device.yaml
index c676956810331b81f11f3624340fc3e612c98315..9d55be4fb5981164cca969dbda5ba70ab3a87773 100644
--- a/Documentation/devicetree/bindings/usb/usb-device.yaml
+++ b/Documentation/devicetree/bindings/usb/usb-device.yaml
@@ -28,7 +28,8 @@  description: |
 
 properties:
   compatible:
-    pattern: "^usb[0-9a-f]{1,4},[0-9a-f]{1,4}$"
+    items:
+      pattern: "^usb[0-9a-f]{1,4},[0-9a-f]{1,4}$"
     description: Device nodes or combined nodes.
       "usbVID,PID", where VID is the vendor id and PID the product id.
       The textual representation of VID and PID shall be in lower case