Re: [PATCH] dt-bindings: usb: usb-device: allow multiple compatibles

[Quentin Schulz]

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@xxxxxxxxx>
> >
> > 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@xxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxx/
> > ---
> >   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@xxxxxxxxxxxxx/ 
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