linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] dt-bindings: usb: Add binding for Microchip usb5744 hub controller
@ 2023-05-05 13:25 Michal Simek
  2023-05-07  8:07 ` Krzysztof Kozlowski
  0 siblings, 1 reply; 6+ messages in thread
From: Michal Simek @ 2023-05-05 13:25 UTC (permalink / raw)
  To: linux-kernel, monstr, michal.simek, git, ilias.apalodimas
  Cc: Greg Kroah-Hartman, Krzysztof Kozlowski, Piyush Mehta,
	Rob Herring, devicetree, linux-usb

The Microchip usb5744 is a SS/HS USB 3.0 hub controller with 4 ports.
The binding describes USB related aspects of the USB5744 hub, it as
well cover the option of connecting the controller as an i2c slave.
When i2c interface is connected hub needs to be initialized first.
Hub itself has fixed i2c address 0x2D but hardcoding address is not good
idea because address can be shifted by i2c address translator in the
middle.

Signed-off-by: Piyush Mehta <piyush.mehta@amd.com>
Signed-off-by: Michal Simek <michal.simek@amd.com>
---

Changes in v2:
- fix i2c-bus property
- swap usb2.0/3.0 compatible strings
- fix indentation in example (4 spaces)
- add new i2c node with microchip,usb5744 compatible property

It looks like that usb8041 has also an optional i2c interface which is not
covered. But it is mentioned at commit 40e58a8a7ca6 ("dt-bindings: usb:
Add binding for TI USB8041 hub controller").

i2c-bus name property was suggested by Rob at
https://lore.kernel.org/all/CAL_JsqJedhX6typpUKbnzV7CLK6UZVjq3CyG9iY_j5DLPqvVdw@mail.gmail.com/
and
https://lore.kernel.org/all/CAL_JsqJZBbu+UXqUNdZwg-uv0PAsNg55026PTwhKr5wQtxCjVQ@mail.gmail.com/

the question is if adding address like this is acceptable.
But it must be specified.

Driver will follow based on final dt-binding.

$ref: usb-device.yaml# should be also added but have no idea how to wire it
up to be applied only on usb node not i2c one.

---
 .../bindings/usb/microchip,usb5744.yaml       | 110 ++++++++++++++++++
 1 file changed, 110 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/microchip,usb5744.yaml

diff --git a/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml b/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
new file mode 100644
index 000000000000..7e0a3472ea95
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
@@ -0,0 +1,110 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/microchip,usb5744.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip USB5744 4-port Hub Controller
+
+description:
+  Microchip's USB5744 SmartHubTM IC is a 4 port, SuperSpeed (SS)/Hi-Speed (HS),
+  low power, low pin count configurable and fully compliant with the USB 3.1
+  Gen 1 specification. The USB5744 also supports Full Speed (FS) and Low Speed
+  (LS) USB signaling, offering complete coverage of all defined USB operating
+  speeds. The new SuperSpeed hubs operate in parallel with the USB 2.0
+  controller, so 5 Gbps SuperSpeed data transfers are not affected by slower
+  USB 2.0 traffic.
+
+maintainers:
+  - Piyush Mehta <piyush.mehta@amd.com>
+  - Michal Simek <michal.simek@amd.com>
+
+select:
+  properties:
+    compatible:
+      contains:
+        const: microchip,usb5744
+  required:
+    - compatible
+
+properties:
+  compatible:
+    enum:
+      - usb424,2744
+      - usb424,5744
+      - microchip,usb5744
+
+  reg: true
+
+required:
+  - compatible
+  - reg
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - usb424,2744
+              - usb424,5744
+    then:
+      properties:
+        reset-gpios:
+          maxItems: 1
+          description:
+            GPIO controlling the GRST# pin.
+
+        vdd-supply:
+          description:
+            VDD power supply to the hub
+
+        peer-hub:
+          $ref: /schemas/types.yaml#/definitions/phandle
+          description:
+            phandle to the peer hub on the controller.
+
+        i2c-bus:
+          $ref: /schemas/types.yaml#/definitions/phandle
+          description:
+            phandle of an usb hub connected via i2c bus.
+
+      required:
+        - peer-hub
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    i2c: i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        hub: usb-hub@2d {
+            compatible = "microchip,usb5744";
+            reg = <0x2d>;
+        };
+    };
+
+    usb {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        /* 2.0 hub on port 1 */
+        hub_2_0: hub@1 {
+            compatible = "usb424,2744";
+            reg = <1>;
+            peer-hub = <&hub_3_0>;
+            i2c-bus = <&hub>;
+            reset-gpios = <&gpio 3 GPIO_ACTIVE_LOW>;
+        };
+
+        /* 3.0 hub on port 2 */
+        hub_3_0: hub@2 {
+            compatible = "usb424,5744";
+            reg = <2>;
+            peer-hub = <&hub_2_0>;
+            i2c-bus = <&hub>;
+            reset-gpios = <&gpio 3 GPIO_ACTIVE_LOW>;
+        };
+    };
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] dt-bindings: usb: Add binding for Microchip usb5744 hub controller
  2023-05-05 13:25 [PATCH v2] dt-bindings: usb: Add binding for Microchip usb5744 hub controller Michal Simek
@ 2023-05-07  8:07 ` Krzysztof Kozlowski
  2023-05-09 14:19   ` Michal Simek
  0 siblings, 1 reply; 6+ messages in thread
From: Krzysztof Kozlowski @ 2023-05-07  8:07 UTC (permalink / raw)
  To: Michal Simek, linux-kernel, monstr, michal.simek, git, ilias.apalodimas
  Cc: Greg Kroah-Hartman, Krzysztof Kozlowski, Piyush Mehta,
	Rob Herring, devicetree, linux-usb

On 05/05/2023 15:25, Michal Simek wrote:
> The Microchip usb5744 is a SS/HS USB 3.0 hub controller with 4 ports.
> The binding describes USB related aspects of the USB5744 hub, it as
> well cover the option of connecting the controller as an i2c slave.
> When i2c interface is connected hub needs to be initialized first.
> Hub itself has fixed i2c address 0x2D but hardcoding address is not good
> idea because address can be shifted by i2c address translator in the
> middle.
> 
> Signed-off-by: Piyush Mehta <piyush.mehta@amd.com>
> Signed-off-by: Michal Simek <michal.simek@amd.com>
> ---
> 
> Changes in v2:
> - fix i2c-bus property
> - swap usb2.0/3.0 compatible strings
> - fix indentation in example (4 spaces)
> - add new i2c node with microchip,usb5744 compatible property
> 
> It looks like that usb8041 has also an optional i2c interface which is not
> covered. But it is mentioned at commit 40e58a8a7ca6 ("dt-bindings: usb:
> Add binding for TI USB8041 hub controller").
> 
> i2c-bus name property was suggested by Rob at
> https://lore.kernel.org/all/CAL_JsqJedhX6typpUKbnzV7CLK6UZVjq3CyG9iY_j5DLPqvVdw@mail.gmail.com/
> and
> https://lore.kernel.org/all/CAL_JsqJZBbu+UXqUNdZwg-uv0PAsNg55026PTwhKr5wQtxCjVQ@mail.gmail.com/
> 
> the question is if adding address like this is acceptable.
> But it must be specified.
> 
> Driver will follow based on final dt-binding.
> 
> $ref: usb-device.yaml# should be also added but have no idea how to wire it
> up to be applied only on usb node not i2c one.
> 
> ---
>  .../bindings/usb/microchip,usb5744.yaml       | 110 ++++++++++++++++++
>  1 file changed, 110 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
> 
> diff --git a/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml b/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
> new file mode 100644
> index 000000000000..7e0a3472ea95
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
> @@ -0,0 +1,110 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/microchip,usb5744.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip USB5744 4-port Hub Controller
> +
> +description:
> +  Microchip's USB5744 SmartHubTM IC is a 4 port, SuperSpeed (SS)/Hi-Speed (HS),
> +  low power, low pin count configurable and fully compliant with the USB 3.1
> +  Gen 1 specification. The USB5744 also supports Full Speed (FS) and Low Speed
> +  (LS) USB signaling, offering complete coverage of all defined USB operating
> +  speeds. The new SuperSpeed hubs operate in parallel with the USB 2.0
> +  controller, so 5 Gbps SuperSpeed data transfers are not affected by slower
> +  USB 2.0 traffic.
> +
> +maintainers:
> +  - Piyush Mehta <piyush.mehta@amd.com>
> +  - Michal Simek <michal.simek@amd.com>
> +
> +select:
> +  properties:
> +    compatible:
> +      contains:
> +        const: microchip,usb5744
> +  required:
> +    - compatible

I don't understand why do you need this select. It basically disables
schema matching for other ones.

> +
> +properties:
> +  compatible:
> +    enum:
> +      - usb424,2744
> +      - usb424,5744
> +      - microchip,usb5744
> +
> +  reg: true

maxItems: 1

> +
> +required:
> +  - compatible
> +  - reg
> +


Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] dt-bindings: usb: Add binding for Microchip usb5744 hub controller
  2023-05-07  8:07 ` Krzysztof Kozlowski
@ 2023-05-09 14:19   ` Michal Simek
  2023-05-09 16:04     ` Krzysztof Kozlowski
  0 siblings, 1 reply; 6+ messages in thread
From: Michal Simek @ 2023-05-09 14:19 UTC (permalink / raw)
  To: Krzysztof Kozlowski, linux-kernel, monstr, michal.simek, git,
	ilias.apalodimas
  Cc: Greg Kroah-Hartman, Krzysztof Kozlowski, Piyush Mehta,
	Rob Herring, devicetree, linux-usb



On 5/7/23 10:07, Krzysztof Kozlowski wrote:
> On 05/05/2023 15:25, Michal Simek wrote:
>> The Microchip usb5744 is a SS/HS USB 3.0 hub controller with 4 ports.
>> The binding describes USB related aspects of the USB5744 hub, it as
>> well cover the option of connecting the controller as an i2c slave.
>> When i2c interface is connected hub needs to be initialized first.
>> Hub itself has fixed i2c address 0x2D but hardcoding address is not good
>> idea because address can be shifted by i2c address translator in the
>> middle.
>>
>> Signed-off-by: Piyush Mehta <piyush.mehta@amd.com>
>> Signed-off-by: Michal Simek <michal.simek@amd.com>
>> ---
>>
>> Changes in v2:
>> - fix i2c-bus property
>> - swap usb2.0/3.0 compatible strings
>> - fix indentation in example (4 spaces)
>> - add new i2c node with microchip,usb5744 compatible property
>>
>> It looks like that usb8041 has also an optional i2c interface which is not
>> covered. But it is mentioned at commit 40e58a8a7ca6 ("dt-bindings: usb:
>> Add binding for TI USB8041 hub controller").
>>
>> i2c-bus name property was suggested by Rob at
>> https://lore.kernel.org/all/CAL_JsqJedhX6typpUKbnzV7CLK6UZVjq3CyG9iY_j5DLPqvVdw@mail.gmail.com/
>> and
>> https://lore.kernel.org/all/CAL_JsqJZBbu+UXqUNdZwg-uv0PAsNg55026PTwhKr5wQtxCjVQ@mail.gmail.com/
>>
>> the question is if adding address like this is acceptable.
>> But it must be specified.
>>
>> Driver will follow based on final dt-binding.
>>
>> $ref: usb-device.yaml# should be also added but have no idea how to wire it
>> up to be applied only on usb node not i2c one.
>>
>> ---
>>   .../bindings/usb/microchip,usb5744.yaml       | 110 ++++++++++++++++++
>>   1 file changed, 110 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml b/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
>> new file mode 100644
>> index 000000000000..7e0a3472ea95
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
>> @@ -0,0 +1,110 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/usb/microchip,usb5744.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Microchip USB5744 4-port Hub Controller
>> +
>> +description:
>> +  Microchip's USB5744 SmartHubTM IC is a 4 port, SuperSpeed (SS)/Hi-Speed (HS),
>> +  low power, low pin count configurable and fully compliant with the USB 3.1
>> +  Gen 1 specification. The USB5744 also supports Full Speed (FS) and Low Speed
>> +  (LS) USB signaling, offering complete coverage of all defined USB operating
>> +  speeds. The new SuperSpeed hubs operate in parallel with the USB 2.0
>> +  controller, so 5 Gbps SuperSpeed data transfers are not affected by slower
>> +  USB 2.0 traffic.
>> +
>> +maintainers:
>> +  - Piyush Mehta <piyush.mehta@amd.com>
>> +  - Michal Simek <michal.simek@amd.com>
>> +
>> +select:
>> +  properties:
>> +    compatible:
>> +      contains:
>> +        const: microchip,usb5744
>> +  required:
>> +    - compatible
> 
> I don't understand why do you need this select. It basically disables
> schema matching for other ones.

I didn't find a way how to have usbXXX,XXXX compatible strings and 
microchip,usb5744 compatible in the same file. I am definitely lacking knowledge 
how to write it properly that's why any advise is welcome.

Thanks,
Michal

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] dt-bindings: usb: Add binding for Microchip usb5744 hub controller
  2023-05-09 14:19   ` Michal Simek
@ 2023-05-09 16:04     ` Krzysztof Kozlowski
  2023-05-10 11:00       ` Michal Simek
  0 siblings, 1 reply; 6+ messages in thread
From: Krzysztof Kozlowski @ 2023-05-09 16:04 UTC (permalink / raw)
  To: Michal Simek, linux-kernel, monstr, michal.simek, git, ilias.apalodimas
  Cc: Greg Kroah-Hartman, Krzysztof Kozlowski, Piyush Mehta,
	Rob Herring, devicetree, linux-usb

On 09/05/2023 16:19, Michal Simek wrote:
> 
> 
> On 5/7/23 10:07, Krzysztof Kozlowski wrote:
>> On 05/05/2023 15:25, Michal Simek wrote:
>>> The Microchip usb5744 is a SS/HS USB 3.0 hub controller with 4 ports.
>>> The binding describes USB related aspects of the USB5744 hub, it as
>>> well cover the option of connecting the controller as an i2c slave.
>>> When i2c interface is connected hub needs to be initialized first.
>>> Hub itself has fixed i2c address 0x2D but hardcoding address is not good
>>> idea because address can be shifted by i2c address translator in the
>>> middle.
>>>
>>> Signed-off-by: Piyush Mehta <piyush.mehta@amd.com>
>>> Signed-off-by: Michal Simek <michal.simek@amd.com>
>>> ---
>>>
>>> Changes in v2:
>>> - fix i2c-bus property
>>> - swap usb2.0/3.0 compatible strings
>>> - fix indentation in example (4 spaces)
>>> - add new i2c node with microchip,usb5744 compatible property
>>>
>>> It looks like that usb8041 has also an optional i2c interface which is not
>>> covered. But it is mentioned at commit 40e58a8a7ca6 ("dt-bindings: usb:
>>> Add binding for TI USB8041 hub controller").
>>>
>>> i2c-bus name property was suggested by Rob at
>>> https://lore.kernel.org/all/CAL_JsqJedhX6typpUKbnzV7CLK6UZVjq3CyG9iY_j5DLPqvVdw@mail.gmail.com/
>>> and
>>> https://lore.kernel.org/all/CAL_JsqJZBbu+UXqUNdZwg-uv0PAsNg55026PTwhKr5wQtxCjVQ@mail.gmail.com/
>>>
>>> the question is if adding address like this is acceptable.
>>> But it must be specified.
>>>
>>> Driver will follow based on final dt-binding.
>>>
>>> $ref: usb-device.yaml# should be also added but have no idea how to wire it
>>> up to be applied only on usb node not i2c one.
>>>
>>> ---
>>>   .../bindings/usb/microchip,usb5744.yaml       | 110 ++++++++++++++++++
>>>   1 file changed, 110 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml b/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
>>> new file mode 100644
>>> index 000000000000..7e0a3472ea95
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
>>> @@ -0,0 +1,110 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/usb/microchip,usb5744.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Microchip USB5744 4-port Hub Controller
>>> +
>>> +description:
>>> +  Microchip's USB5744 SmartHubTM IC is a 4 port, SuperSpeed (SS)/Hi-Speed (HS),
>>> +  low power, low pin count configurable and fully compliant with the USB 3.1
>>> +  Gen 1 specification. The USB5744 also supports Full Speed (FS) and Low Speed
>>> +  (LS) USB signaling, offering complete coverage of all defined USB operating
>>> +  speeds. The new SuperSpeed hubs operate in parallel with the USB 2.0
>>> +  controller, so 5 Gbps SuperSpeed data transfers are not affected by slower
>>> +  USB 2.0 traffic.
>>> +
>>> +maintainers:
>>> +  - Piyush Mehta <piyush.mehta@amd.com>
>>> +  - Michal Simek <michal.simek@amd.com>
>>> +
>>> +select:
>>> +  properties:
>>> +    compatible:
>>> +      contains:
>>> +        const: microchip,usb5744
>>> +  required:
>>> +    - compatible
>>
>> I don't understand why do you need this select. It basically disables
>> schema matching for other ones.
> 
> I didn't find a way how to have usbXXX,XXXX compatible strings and 
> microchip,usb5744 compatible in the same file. I am definitely lacking knowledge 
> how to write it properly that's why any advise is welcome.

Hm, if you just have both of them like you have now, what happens?

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] dt-bindings: usb: Add binding for Microchip usb5744 hub controller
  2023-05-09 16:04     ` Krzysztof Kozlowski
@ 2023-05-10 11:00       ` Michal Simek
  2023-05-10 13:17         ` Krzysztof Kozlowski
  0 siblings, 1 reply; 6+ messages in thread
From: Michal Simek @ 2023-05-10 11:00 UTC (permalink / raw)
  To: Krzysztof Kozlowski, linux-kernel, monstr, michal.simek, git,
	ilias.apalodimas
  Cc: Greg Kroah-Hartman, Krzysztof Kozlowski, Piyush Mehta,
	Rob Herring, devicetree, linux-usb



On 5/9/23 18:04, Krzysztof Kozlowski wrote:
> On 09/05/2023 16:19, Michal Simek wrote:
>>
>>
>> On 5/7/23 10:07, Krzysztof Kozlowski wrote:
>>> On 05/05/2023 15:25, Michal Simek wrote:
>>>> The Microchip usb5744 is a SS/HS USB 3.0 hub controller with 4 ports.
>>>> The binding describes USB related aspects of the USB5744 hub, it as
>>>> well cover the option of connecting the controller as an i2c slave.
>>>> When i2c interface is connected hub needs to be initialized first.
>>>> Hub itself has fixed i2c address 0x2D but hardcoding address is not good
>>>> idea because address can be shifted by i2c address translator in the
>>>> middle.
>>>>
>>>> Signed-off-by: Piyush Mehta <piyush.mehta@amd.com>
>>>> Signed-off-by: Michal Simek <michal.simek@amd.com>
>>>> ---
>>>>
>>>> Changes in v2:
>>>> - fix i2c-bus property
>>>> - swap usb2.0/3.0 compatible strings
>>>> - fix indentation in example (4 spaces)
>>>> - add new i2c node with microchip,usb5744 compatible property
>>>>
>>>> It looks like that usb8041 has also an optional i2c interface which is not
>>>> covered. But it is mentioned at commit 40e58a8a7ca6 ("dt-bindings: usb:
>>>> Add binding for TI USB8041 hub controller").
>>>>
>>>> i2c-bus name property was suggested by Rob at
>>>> https://lore.kernel.org/all/CAL_JsqJedhX6typpUKbnzV7CLK6UZVjq3CyG9iY_j5DLPqvVdw@mail.gmail.com/
>>>> and
>>>> https://lore.kernel.org/all/CAL_JsqJZBbu+UXqUNdZwg-uv0PAsNg55026PTwhKr5wQtxCjVQ@mail.gmail.com/
>>>>
>>>> the question is if adding address like this is acceptable.
>>>> But it must be specified.
>>>>
>>>> Driver will follow based on final dt-binding.
>>>>
>>>> $ref: usb-device.yaml# should be also added but have no idea how to wire it
>>>> up to be applied only on usb node not i2c one.
>>>>
>>>> ---
>>>>    .../bindings/usb/microchip,usb5744.yaml       | 110 ++++++++++++++++++
>>>>    1 file changed, 110 insertions(+)
>>>>    create mode 100644 Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml b/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
>>>> new file mode 100644
>>>> index 000000000000..7e0a3472ea95
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
>>>> @@ -0,0 +1,110 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/usb/microchip,usb5744.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Microchip USB5744 4-port Hub Controller
>>>> +
>>>> +description:
>>>> +  Microchip's USB5744 SmartHubTM IC is a 4 port, SuperSpeed (SS)/Hi-Speed (HS),
>>>> +  low power, low pin count configurable and fully compliant with the USB 3.1
>>>> +  Gen 1 specification. The USB5744 also supports Full Speed (FS) and Low Speed
>>>> +  (LS) USB signaling, offering complete coverage of all defined USB operating
>>>> +  speeds. The new SuperSpeed hubs operate in parallel with the USB 2.0
>>>> +  controller, so 5 Gbps SuperSpeed data transfers are not affected by slower
>>>> +  USB 2.0 traffic.
>>>> +
>>>> +maintainers:
>>>> +  - Piyush Mehta <piyush.mehta@amd.com>
>>>> +  - Michal Simek <michal.simek@amd.com>
>>>> +
>>>> +select:
>>>> +  properties:
>>>> +    compatible:
>>>> +      contains:
>>>> +        const: microchip,usb5744
>>>> +  required:
>>>> +    - compatible
>>>
>>> I don't understand why do you need this select. It basically disables
>>> schema matching for other ones.
>>
>> I didn't find a way how to have usbXXX,XXXX compatible strings and
>> microchip,usb5744 compatible in the same file. I am definitely lacking knowledge
>> how to write it properly that's why any advise is welcome.
> 
> Hm, if you just have both of them like you have now, what happens?


make 
DT_SCHEMA_FILES=Documentation/devicetree/bindings/usb/microchip,usb5744.yaml 
dt_binding_check
   DTEX    Documentation/devicetree/bindings/usb/microchip,usb5744.example.dts
   LINT    Documentation/devicetree/bindings
   CHKDT   Documentation/devicetree/bindings/processed-schema.json
   SCHEMA  Documentation/devicetree/bindings/processed-schema.json
   DTC_CHK Documentation/devicetree/bindings/usb/microchip,usb5744.example.dtb
/home/monstr/data/disk/linux/Documentation/devicetree/bindings/usb/microchip,usb5744.example.dtb: 
hub@1: 'i2c-bus', 'peer-hub', 'reset-gpios' do not match any of the regexes: 
'pinctrl-[0-9]+'
	From schema: 
/home/monstr/data/disk/linux/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
/home/monstr/data/disk/linux/Documentation/devicetree/bindings/usb/microchip,usb5744.example.dtb: 
hub@2: 'i2c-bus', 'peer-hub', 'reset-gpios' do not match any of the regexes: 
'pinctrl-[0-9]+'
	From schema: 
/home/monstr/data/disk/linux/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml

And this is even without usb-device.yaml wired.

Thanks,
Michal



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] dt-bindings: usb: Add binding for Microchip usb5744 hub controller
  2023-05-10 11:00       ` Michal Simek
@ 2023-05-10 13:17         ` Krzysztof Kozlowski
  0 siblings, 0 replies; 6+ messages in thread
From: Krzysztof Kozlowski @ 2023-05-10 13:17 UTC (permalink / raw)
  To: Michal Simek, linux-kernel, monstr, michal.simek, git, ilias.apalodimas
  Cc: Greg Kroah-Hartman, Krzysztof Kozlowski, Piyush Mehta,
	Rob Herring, devicetree, linux-usb

On 10/05/2023 13:00, Michal Simek wrote:
> 
> 
> On 5/9/23 18:04, Krzysztof Kozlowski wrote:
>> On 09/05/2023 16:19, Michal Simek wrote:
>>>
>>>
>>> On 5/7/23 10:07, Krzysztof Kozlowski wrote:
>>>> On 05/05/2023 15:25, Michal Simek wrote:
>>>>> The Microchip usb5744 is a SS/HS USB 3.0 hub controller with 4 ports.
>>>>> The binding describes USB related aspects of the USB5744 hub, it as
>>>>> well cover the option of connecting the controller as an i2c slave.
>>>>> When i2c interface is connected hub needs to be initialized first.
>>>>> Hub itself has fixed i2c address 0x2D but hardcoding address is not good
>>>>> idea because address can be shifted by i2c address translator in the
>>>>> middle.
>>>>>
>>>>> Signed-off-by: Piyush Mehta <piyush.mehta@amd.com>
>>>>> Signed-off-by: Michal Simek <michal.simek@amd.com>
>>>>> ---
>>>>>
>>>>> Changes in v2:
>>>>> - fix i2c-bus property
>>>>> - swap usb2.0/3.0 compatible strings
>>>>> - fix indentation in example (4 spaces)
>>>>> - add new i2c node with microchip,usb5744 compatible property
>>>>>
>>>>> It looks like that usb8041 has also an optional i2c interface which is not
>>>>> covered. But it is mentioned at commit 40e58a8a7ca6 ("dt-bindings: usb:
>>>>> Add binding for TI USB8041 hub controller").
>>>>>
>>>>> i2c-bus name property was suggested by Rob at
>>>>> https://lore.kernel.org/all/CAL_JsqJedhX6typpUKbnzV7CLK6UZVjq3CyG9iY_j5DLPqvVdw@mail.gmail.com/
>>>>> and
>>>>> https://lore.kernel.org/all/CAL_JsqJZBbu+UXqUNdZwg-uv0PAsNg55026PTwhKr5wQtxCjVQ@mail.gmail.com/
>>>>>
>>>>> the question is if adding address like this is acceptable.
>>>>> But it must be specified.
>>>>>
>>>>> Driver will follow based on final dt-binding.
>>>>>
>>>>> $ref: usb-device.yaml# should be also added but have no idea how to wire it
>>>>> up to be applied only on usb node not i2c one.
>>>>>
>>>>> ---
>>>>>    .../bindings/usb/microchip,usb5744.yaml       | 110 ++++++++++++++++++
>>>>>    1 file changed, 110 insertions(+)
>>>>>    create mode 100644 Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml b/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..7e0a3472ea95
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
>>>>> @@ -0,0 +1,110 @@
>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: http://devicetree.org/schemas/usb/microchip,usb5744.yaml#
>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>> +
>>>>> +title: Microchip USB5744 4-port Hub Controller
>>>>> +
>>>>> +description:
>>>>> +  Microchip's USB5744 SmartHubTM IC is a 4 port, SuperSpeed (SS)/Hi-Speed (HS),
>>>>> +  low power, low pin count configurable and fully compliant with the USB 3.1
>>>>> +  Gen 1 specification. The USB5744 also supports Full Speed (FS) and Low Speed
>>>>> +  (LS) USB signaling, offering complete coverage of all defined USB operating
>>>>> +  speeds. The new SuperSpeed hubs operate in parallel with the USB 2.0
>>>>> +  controller, so 5 Gbps SuperSpeed data transfers are not affected by slower
>>>>> +  USB 2.0 traffic.
>>>>> +
>>>>> +maintainers:
>>>>> +  - Piyush Mehta <piyush.mehta@amd.com>
>>>>> +  - Michal Simek <michal.simek@amd.com>
>>>>> +
>>>>> +select:
>>>>> +  properties:
>>>>> +    compatible:
>>>>> +      contains:
>>>>> +        const: microchip,usb5744
>>>>> +  required:
>>>>> +    - compatible
>>>>
>>>> I don't understand why do you need this select. It basically disables
>>>> schema matching for other ones.
>>>
>>> I didn't find a way how to have usbXXX,XXXX compatible strings and
>>> microchip,usb5744 compatible in the same file. I am definitely lacking knowledge
>>> how to write it properly that's why any advise is welcome.
>>
>> Hm, if you just have both of them like you have now, what happens?
> 
> 
> make 
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/usb/microchip,usb5744.yaml 
> dt_binding_check
>    DTEX    Documentation/devicetree/bindings/usb/microchip,usb5744.example.dts
>    LINT    Documentation/devicetree/bindings
>    CHKDT   Documentation/devicetree/bindings/processed-schema.json
>    SCHEMA  Documentation/devicetree/bindings/processed-schema.json
>    DTC_CHK Documentation/devicetree/bindings/usb/microchip,usb5744.example.dtb
> /home/monstr/data/disk/linux/Documentation/devicetree/bindings/usb/microchip,usb5744.example.dtb: 
> hub@1: 'i2c-bus', 'peer-hub', 'reset-gpios' do not match any of the regexes: 
> 'pinctrl-[0-9]+'
> 	From schema: 
> /home/monstr/data/disk/linux/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
> /home/monstr/data/disk/linux/Documentation/devicetree/bindings/usb/microchip,usb5744.example.dtb: 
> hub@2: 'i2c-bus', 'peer-hub', 'reset-gpios' do not match any of the regexes: 
> 'pinctrl-[0-9]+'
> 	From schema: 
> /home/monstr/data/disk/linux/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml
> 
> And this is even without usb-device.yaml wired.

Yeah, you cannot define properties in allOf:if:then. Your select just
makes the schema not selected thus it is a NOOP and error is hidden.

I gave you examples how these should be expressed and the examples *do
not* define properties in allOf:if:then, right?

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-05-10 13:17 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-05 13:25 [PATCH v2] dt-bindings: usb: Add binding for Microchip usb5744 hub controller Michal Simek
2023-05-07  8:07 ` Krzysztof Kozlowski
2023-05-09 14:19   ` Michal Simek
2023-05-09 16:04     ` Krzysztof Kozlowski
2023-05-10 11:00       ` Michal Simek
2023-05-10 13:17         ` Krzysztof Kozlowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).