Re: [PATCH 2/5] dt-bindings: usb: cypress,hx3: Add support for all variants

[Quentin Schulz <quentin.schulz@cherry.de>]

Hi Conor,

On 4/9/25 6:18 PM, Conor Dooley wrote:
> On Tue, Apr 08, 2025 at 06:36:04PM +0200, Łukasz Czechowski wrote:
>> Hello,
>>
>> śr., 26 mar 2025 o 18:58 Conor Dooley <conor@kernel.org> napisał(a):
>>>
>>> On Wed, Mar 26, 2025 at 05:22:57PM +0100, Lukasz Czechowski wrote:
>>>> The Cypress HX3 hubs use different default PID value depending
>>>> on the variant. Update compatibles list.
>>>>
>>>> Fixes: 1eca51f58a10 ("dt-bindings: usb: Add binding for Cypress HX3 USB 3.0 family")
>>>> Cc: stable@vger.kernel.org # 6.6
>>>> Cc: stable@vger.kernel.org # Backport of the patch in this series fixing product ID in onboard_dev_id_table and onboard_dev_match in drivers/usb/misc/onboard_usb_dev.{c,h} driver
>>>> Signed-off-by: Lukasz Czechowski <lukasz.czechowski@thaumatec.com>
>>>> ---
>>>>   Documentation/devicetree/bindings/usb/cypress,hx3.yaml | 6 ++++++
>>>>   1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/usb/cypress,hx3.yaml b/Documentation/devicetree/bindings/usb/cypress,hx3.yaml
>>>> index 1033b7a4b8f9..f0b93002bd02 100644
>>>> --- a/Documentation/devicetree/bindings/usb/cypress,hx3.yaml
>>>> +++ b/Documentation/devicetree/bindings/usb/cypress,hx3.yaml
>>>> @@ -15,8 +15,14 @@ allOf:
>>>>   properties:
>>>>     compatible:
>>>>       enum:
>>>> +      - usb4b4,6500
>>>> +      - usb4b4,6502
>>>> +      - usb4b4,6503
>>>>         - usb4b4,6504
>>>>         - usb4b4,6506
>>>> +      - usb4b4,6507
>>>> +      - usb4b4,6508
>>>> +      - usb4b4,650a
>>>
>>> All these devices seem to have the same match data, why is a fallback
>>> not suitable?
>>>
>>
>> Thank you for the suggestion. Indeed the fallback compatible appears
>> to work fine in this case,
>> and I am able to avoid additional entries in onboard_dev_match table
>> as added in the first patch in this series.
>>
>> However, after I've updated the cypress,hx3.yaml schema file and
>> rk3399-puma.dtsi file,
>> I get the following warnings, when running "make dtbs_check":
>>
>> linux/arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dtb: hub@1:
>> compatible: ['usb4b4,6502', 'usb4b4,6506'] is too long
>>   from schema $id: http://devicetree.org/schemas/usb/cypress,hx3.yaml#
>> linux/arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dtb: hub@2:
>> compatible: ['usb4b4,6500', 'usb4b4,6504'] is too long
>>   from schema $id: http://devicetree.org/schemas/usb/cypress,hx3.yaml#
>>
>> Below is the diff of my changes:
>>
>> diff --git a/Documentation/devicetree/bindings/usb/cypress,hx3.yaml
>> b/Documentation/devicetree/bindings/usb/cypress,hx3.yaml
>> index f0b93002bd02..d6eac1213228 100644
>> --- a/Documentation/devicetree/bindings/usb/cypress,hx3.yaml
>> +++ b/Documentation/devicetree/bindings/usb/cypress,hx3.yaml
>> @@ -14,15 +14,22 @@ allOf:
>>
>>   properties:
>>     compatible:
>> -    enum:
>> -      - usb4b4,6500
>> -      - usb4b4,6502
>> -      - usb4b4,6503
>> -      - usb4b4,6504
>> -      - usb4b4,6506
>> -      - usb4b4,6507
>> -      - usb4b4,6508
>> -      - usb4b4,650a
>> +    oneOf:
>> +      - enum:
>> +          - usb4b4,6504
>> +          - usb4b4,6506
>> +      - items:
>> +          - enum:
>> +              - usb4b4,6500
>> +              - usb4b4,6508
>> +          - const: usb4b4,6504
>> +      - items:
>> +          - enum:
>> +              - usb4b4,6502
>> +              - usb4b4,6503
>> +              - usb4b4,6507
>> +              - usb4b4,650a
>> +          - const: usb4b4,6506
>>
>>     reg: true
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi
>> b/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi
>> index d0d867374b3f..7fac61f95fc6 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi
>> @@ -594,14 +594,14 @@ &usbdrd_dwc3_1 {
>>          #size-cells = <0>;
>>
>>          hub_2_0: hub@1 {
>> -               compatible = "usb4b4,6502";
>> +               compatible = "usb4b4,6502", "usb4b4,6506";
>>                  reg = <1>;
>>                  peer-hub = <&hub_3_0>;
>>                  reset-gpios = <&gpio4 RK_PA3 GPIO_ACTIVE_HIGH>;
>>          };
>>
>>          hub_3_0: hub@2 {
>> -               compatible = "usb4b4,6500";
>> +               compatible = "usb4b4,6500", "usb4b4,6504";
>>                  reg = <2>;
>>                  peer-hub = <&hub_2_0>;
>>                  reset-gpios = <&gpio4 RK_PA3 GPIO_ACTIVE_HIGH>;
>>
>>
>> Do you have any suggestions on how I can properly update the schema
>> files to avoid the above warnings?
> 
> The diffs you have here look okay, not really sure what you're asking
> for.

It fails dtbs_check:

linux/arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dtb: hub@1: 
compatible: ['usb4b4,6502', 'usb4b4,6506'] is too long
  from schema $id: http://devicetree.org/schemas/usb/cypress,hx3.yaml#
linux/arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dtb: hub@2: 
compatible: ['usb4b4,6500', 'usb4b4,6504'] is too long
  from schema $id: http://devicetree.org/schemas/usb/cypress,hx3.yaml#

I'm not sure we're allowed to add new errors with dtbs_check (and would 
like to avoid it in any case).

Cheers,
Quentin