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 |
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.
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
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 --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