2020-07-22 19:14:08

by Christian Eggers

[permalink] [raw]
Subject: [PATCH 3/4] dt-bindings: usb: Add Microchip USB47xx/USB49xx support

Add DT bindings for Microchip USB47xx/USB49xx driver.

Signed-off-by: Christian Eggers <[email protected]>
---
.../devicetree/bindings/usb/usb49xx.yaml | 230 ++++++++++++++++++
1 file changed, 230 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/usb49xx.yaml

diff --git a/Documentation/devicetree/bindings/usb/usb49xx.yaml b/Documentation/devicetree/bindings/usb/usb49xx.yaml
new file mode 100644
index 000000000000..20c7b64c6b7d
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/usb49xx.yaml
@@ -0,0 +1,230 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/usb49xx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip USB47xx/USB49xx USB 2.0 Hi-Speed Hub Controller
+
+maintainers:
+ - Christian Eggers <[email protected]>
+
+description: |
+ http://ww1.microchip.com/downloads/en/Appnotes/AN2651-Configuration-of-Microchip-USB47xx-USB49xx-Application-Note-00002651B.pdf
+
+properties:
+ compatible:
+ enum:
+ - microchip,usb4712
+ - microchip,usb4712i
+ - microchip,usb4715
+ - microchip,usb4715i
+ - microchip,usb4912
+ - microchip,usb4912i
+ - microchip,usb4914
+ - microchip,usb4914i
+ - microchip,usb4916
+ - microchip,usb4916i
+ - microchip,usb4925
+ - microchip,usb4925i
+ - microchip,usb4927
+ - microchip,usb4927i
+
+ reg:
+ maxItems: 1
+ description:
+ I2C address on the selected bus (usually <0x2D>).
+
+ reset-gpios:
+ maxItems: 1
+ description:
+ Specify the gpio for hub reset.
+
+ vdd-supply:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ Specify the regulator supplying vdd.
+
+ skip-config:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Skip Hub configuration, but only send the USB-Attach command.
+
+ vendor-id:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ maximum: 65535
+ description:
+ Set USB Vendor ID of the hub.
+
+ product-id:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ maximum: 65535
+ description:
+ Set USB Product ID of the hub.
+
+ device-id:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ maximum: 65535
+ description:
+ Set USB Device ID of the hub.
+
+ language-id:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ maximum: 65535
+ description:
+ Set USB Language ID.
+
+ manufacturer:
+ $ref: /schemas/types.yaml#/definitions/string
+ description:
+ Set USB Manufacturer string (max. 62 characters long).
+
+ product:
+ $ref: /schemas/types.yaml#/definitions/string
+ description:
+ Set USB Product string (max. 62 characters long).
+
+ bus-powered:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Selects bus powered operation.
+
+ self-powered:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Selects self powered operation (default).
+
+ disable-hi-speed:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Disable USB Hi-Speed support.
+
+ multi-tt:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Selects multi-transaction-translator (default).
+
+ single-tt:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Selects single-transaction-translator.
+
+ disable-eop:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Disable End of Packet generation in full-speed mode.
+
+ ganged-sensing:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Select ganged over-current sense type in self-powered mode.
+
+ individual-sensing:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Select individual over-current sense type in self-powered mode (default).
+
+ ganged-port-switching:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Select ganged port power switching mode.
+
+ individual-port-switching:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Select individual port power switching mode (default).
+
+ dynamic-power-switching:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Enable auto-switching from self- to bus-powered operation if the local
+ power source is removed or unavailable.
+
+ oc-delay-us:
+ enum:
+ - 50
+ - 100
+ - 200
+ - 400
+ default: 200
+ description:
+ Delay time (in microseconds) for filtering the over-current sense inputs.
+ If an invalid value is given, the default is used instead.
+
+ compound-device:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Indicate the hub is part of a compound device.
+
+ port-mapping-mode:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Enable port mapping mode.
+
+ non-removable-ports:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Should specify the ports which have a non-removable device connected.
+
+ sp-disabled-ports:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Specifies the ports which will be self-power disabled.
+
+ bp-disabled-ports:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Specifies the ports which will be bus-power disabled.
+
+ power-on-time-ms:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ default: 100
+ minimum: 0
+ maximum: 510
+ description:
+ Specifies the time (in milliseconds) it takes from the time the host
+ initiates the power-on sequence to a port until the port has adequate
+ power.
+
+ hub-controller-port:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Hub port where the internal hub controller shall be connected. Usually
+ <number of ports>+1.
+
+additionalProperties: false
+
+required:
+ - compatible
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ i2c0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ usb4916i@2d {
+ compatible = "microchip,usb4916i";
+ reg = <0x2d>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_usb_hub>;
+ /* usb49xx.c already assumes low-active, don't negate twice */
+ reset-gpios = <&gpio3 19 GPIO_ACTIVE_HIGH>;
+ //skip-config;
+ //self-powered; /* power on default */
+ //individual-sensing; /* power on default */
+ //multi-tt; /* power on default */
+ //disable-eop; /* power on default */
+ //individual-port-switching; /* power on default */
+ //oc-delay-ns = <200>; /* power on default */
+ power-on-time-ms = <4>; /* T_ON,max = 4 ms for NCP380 */
+ ocs-min-width-ms = <0>; /* MIC2005 only outputs 2us FAULT pulses */
+ manufacturer = "Foo";
+ product = "Foo-Bar";
+ /* port 5 is connected to an internal SD-Card reader */
+ non-removable-ports = <5>;
+ };
+ };
+
+...
--
Christian Eggers
Embedded software developer

Arnold & Richter Cine Technik GmbH & Co. Betriebs KG
Sitz: Muenchen - Registergericht: Amtsgericht Muenchen - Handelsregisternummer: HRA 57918
Persoenlich haftender Gesellschafter: Arnold & Richter Cine Technik GmbH
Sitz: Muenchen - Registergericht: Amtsgericht Muenchen - Handelsregisternummer: HRB 54477
Geschaeftsfuehrer: Dr. Michael Neuhaeuser; Stephan Schenk; Walter Trauninger; Markus Zeiler


2020-07-23 15:36:07

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 3/4] dt-bindings: usb: Add Microchip USB47xx/USB49xx support

On Wed, 22 Jul 2020 20:38:58 +0200, Christian Eggers wrote:
> Add DT bindings for Microchip USB47xx/USB49xx driver.
>
> Signed-off-by: Christian Eggers <[email protected]>
> ---
> .../devicetree/bindings/usb/usb49xx.yaml | 230 ++++++++++++++++++
> 1 file changed, 230 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/usb/usb49xx.yaml
>


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

/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/usb49xx.example.dt.yaml: usb4916i@2d: 'ocs-min-width-ms' does not match any of the regexes: 'pinctrl-[0-9]+'


See https://patchwork.ozlabs.org/patch/1334134

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

pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade

Please check and re-submit.

2020-07-23 19:32:32

by Christian Eggers

[permalink] [raw]
Subject: [PATCH v2 3/4] dt-bindings: usb: Add Microchip USB47xx/USB49xx support

Add DT bindings for Microchip USB47xx/USB49xx driver.

Signed-off-by: Christian Eggers <[email protected]>
---
> My bot found errors running 'make dt_binding_check' on your patch:

> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/usb49xx.example.dt.yaml: usb4916i@2d: 'ocs-min-width-ms' does not match any of the regexes: 'pinctrl-[0-9]+'
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure dt-schema is up to date:
The mistake was sitting in front of the computer. I simply overlooked this message.

Changes in v2:
- added property description for ocs-min-width-ms
- fixed property description for oc-delay-ns

.../devicetree/bindings/usb/usb49xx.yaml | 238 ++++++++++++++++++
1 file changed, 238 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/usb49xx.yaml

diff --git a/Documentation/devicetree/bindings/usb/usb49xx.yaml b/Documentation/devicetree/bindings/usb/usb49xx.yaml
new file mode 100644
index 000000000000..a4843f2cbefa
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/usb49xx.yaml
@@ -0,0 +1,238 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/usb49xx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip USB47xx/USB49xx USB 2.0 Hi-Speed Hub Controller
+
+maintainers:
+ - Christian Eggers <[email protected]>
+
+description: |
+ http://ww1.microchip.com/downloads/en/Appnotes/AN2651-Configuration-of-Microchip-USB47xx-USB49xx-Application-Note-00002651B.pdf
+
+properties:
+ compatible:
+ enum:
+ - microchip,usb4712
+ - microchip,usb4712i
+ - microchip,usb4715
+ - microchip,usb4715i
+ - microchip,usb4912
+ - microchip,usb4912i
+ - microchip,usb4914
+ - microchip,usb4914i
+ - microchip,usb4916
+ - microchip,usb4916i
+ - microchip,usb4925
+ - microchip,usb4925i
+ - microchip,usb4927
+ - microchip,usb4927i
+
+ reg:
+ maxItems: 1
+ description:
+ I2C address on the selected bus (usually <0x2D>).
+
+ reset-gpios:
+ maxItems: 1
+ description:
+ Specify the gpio for hub reset.
+
+ vdd-supply:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ Specify the regulator supplying vdd.
+
+ skip-config:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Skip Hub configuration, but only send the USB-Attach command.
+
+ vendor-id:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ maximum: 65535
+ description:
+ Set USB Vendor ID of the hub.
+
+ product-id:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ maximum: 65535
+ description:
+ Set USB Product ID of the hub.
+
+ device-id:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ maximum: 65535
+ description:
+ Set USB Device ID of the hub.
+
+ language-id:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ maximum: 65535
+ description:
+ Set USB Language ID.
+
+ manufacturer:
+ $ref: /schemas/types.yaml#/definitions/string
+ description:
+ Set USB Manufacturer string (max. 62 characters long).
+
+ product:
+ $ref: /schemas/types.yaml#/definitions/string
+ description:
+ Set USB Product string (max. 62 characters long).
+
+ bus-powered:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Selects bus powered operation.
+
+ self-powered:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Selects self powered operation (default).
+
+ disable-hi-speed:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Disable USB Hi-Speed support.
+
+ multi-tt:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Selects multi-transaction-translator (default).
+
+ single-tt:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Selects single-transaction-translator.
+
+ disable-eop:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Disable End of Packet generation in full-speed mode.
+
+ ganged-sensing:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Select ganged over-current sense type in self-powered mode.
+
+ individual-sensing:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Select individual over-current sense type in self-powered mode (default).
+
+ ganged-port-switching:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Select ganged port power switching mode.
+
+ individual-port-switching:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Select individual port power switching mode (default).
+
+ dynamic-power-switching:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Enable auto-switching from self- to bus-powered operation if the local
+ power source is removed or unavailable.
+
+ oc-delay-ns:
+ enum:
+ - 50
+ - 100
+ - 200
+ - 400
+ default: 200
+ description:
+ Delay time (in nanoseconds) for filtering the over-current sense inputs.
+ If an invalid value is given, the default is used instead.
+
+ compound-device:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Indicate the hub is part of a compound device.
+
+ port-mapping-mode:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Enable port mapping mode.
+
+ non-removable-ports:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Should specify the ports which have a non-removable device connected.
+
+ sp-disabled-ports:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Specifies the ports which will be self-power disabled.
+
+ bp-disabled-ports:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Specifies the ports which will be bus-power disabled.
+
+ power-on-time-ms:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ default: 100
+ minimum: 0
+ maximum: 510
+ description:
+ Specifies the time (in milliseconds) it takes from the time the host
+ initiates the power-on sequence to a port until the port has adequate
+ power.
+
+ ocs-min-width-ms:
+ default: 5
+ minimum: 0
+ maximum: 5
+ description:
+ Minimum OCS pulse width (in milliseconds) required to detect an OCS
+ event.
+
+ hub-controller-port:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Hub port where the internal hub controller shall be connected. Usually
+ <number of ports>+1.
+
+additionalProperties: false
+
+required:
+ - compatible
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ i2c0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ usb4916i@2d {
+ compatible = "microchip,usb4916i";
+ reg = <0x2d>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_usb_hub>;
+ /* usb49xx.c already assumes low-active, don't negate twice */
+ reset-gpios = <&gpio3 19 GPIO_ACTIVE_HIGH>;
+ //skip-config;
+ //self-powered; /* power on default */
+ //individual-sensing; /* power on default */
+ //multi-tt; /* power on default */
+ //disable-eop; /* power on default */
+ //individual-port-switching; /* power on default */
+ //oc-delay-ns = <200>; /* power on default */
+ power-on-time-ms = <4>; /* T_ON,max = 4 ms for NCP380 */
+ ocs-min-width-ms = <0>; /* MIC2005 only outputs 2us FAULT pulses */
+ manufacturer = "Foo";
+ product = "Foo-Bar";
+ /* port 5 is connected to an internal SD-Card reader */
+ non-removable-ports = <5>;
+ };
+ };
+
+...
--
Christian Eggers
Embedded software developer

Arnold & Richter Cine Technik GmbH & Co. Betriebs KG
Sitz: Muenchen - Registergericht: Amtsgericht Muenchen - Handelsregisternummer: HRA 57918
Persoenlich haftender Gesellschafter: Arnold & Richter Cine Technik GmbH
Sitz: Muenchen - Registergericht: Amtsgericht Muenchen - Handelsregisternummer: HRB 54477
Geschaeftsfuehrer: Dr. Michael Neuhaeuser; Stephan Schenk; Walter Trauninger; Markus Zeiler

2020-07-26 08:41:54

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v2 3/4] dt-bindings: usb: Add Microchip USB47xx/USB49xx support

On Thu, Jul 23, 2020 at 09:29:01PM +0200, Christian Eggers wrote:
> Add DT bindings for Microchip USB47xx/USB49xx driver.
>
> Signed-off-by: Christian Eggers <[email protected]>
> ---
> > My bot found errors running 'make dt_binding_check' on your patch:
>
> > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/usb49xx.example.dt.yaml: usb4916i@2d: 'ocs-min-width-ms' does not match any of the regexes: 'pinctrl-[0-9]+'
> > If you already ran 'make dt_binding_check' and didn't see the above
> > error(s), then make sure dt-schema is up to date:
> The mistake was sitting in front of the computer. I simply overlooked this message.
>
> Changes in v2:
> - added property description for ocs-min-width-ms
> - fixed property description for oc-delay-ns

Please resend the whole series, not just a single patch, as it makes it
very difficult to pick the "correct" patches to be applied...

thanks,

greg k-h

2020-07-27 09:08:57

by Christian Eggers

[permalink] [raw]
Subject: [PATCH v3] Add two new configuration drivers for Microchip USB hubs

On Sonday, greg k-h wrote:
> Please resend the whole series, not just a single patch, as it makes it
> very difficult to pick the "correct" patches to be applied...

Changes in v3:
- none (only resend the whole series)

Changes in v2:
- added property description for ocs-min-width-ms
- fixed property description for oc-delay-ns



2020-08-17 07:57:38

by Richard Leitner

[permalink] [raw]
Subject: Re: [PATCH v3] Add two new configuration drivers for Microchip USB hubs

On Mon, Jul 27, 2020 at 10:33:29AM +0200, Christian Eggers wrote:
> On Sonday, greg k-h wrote:
> > Please resend the whole series, not just a single patch, as it makes it
> > very difficult to pick the "correct" patches to be applied...

Hi Christian,
sorry for the late reply. My MUA somehow didn't show me that series
earlier...

I haven't looked into the patches in detail, but at a first glance it
looks like a lot copy-n-paste.
Have you thought about merging the (after your series) 3 hub drivers
into one? Something like a "microchip i2c usb hub driver"?
Would that be feasible for your point of view?

regards;rl

>
> Changes in v3:
> - none (only resend the whole series)
>
> Changes in v2:
> - added property description for ocs-min-width-ms
> - fixed property description for oc-delay-ns
>

2020-08-24 10:33:43

by Christian Eggers

[permalink] [raw]
Subject: Re: [PATCH v3] Add two new configuration drivers for Microchip USB hubs

Hi Richard,

On Monday, 17 August 2020, 09:54:26 CEST, Richard Leitner wrote:
> Hi Christian,
> sorry for the late reply. My MUA somehow didn't show me that series
> earlier...
likewise... I was on holiday last week.

> I haven't looked into the patches in detail, but at a first glance it
> looks like a lot copy-n-paste.
> Have you thought about merging the (after your series) 3 hub drivers
> into one? Something like a "microchip i2c usb hub driver"?
> Would that be feasible for your point of view?
I'm not sure about the criteria for having separate drivers vs. a combined
one.
As changing the driver usually requires testing with real hardware, keeping
them separate may be easier. On the other hand, I already synced some changes
from usb251xb into "my" drivers, so it is likely that such work will also have
to done in future.

Rob Herring already suggested to create a common yaml document for the
device tree bindings. It would probably make sense to share the device tree
code between our drivers. But I would like to keep the "hardware" side of the
drivers independent, as there are subtle differences between the different hub
series.

Compared to usb251x, the new drivers don't write the full configuration memory
of the hub (the configuration memory space is much bigger than 8 bit and has
many holes). They rely on the defaults after hardware reset on perform read-
modify-write for the properties set in the device tree.

If the usb251xb hubs could be programmed in a similar way, merging all drivers
should be possible using a mic_usb_hub_ops struct with function pointers for
applying the (common) device tree properties (e.g. set_product_id(),
set_device_id(), ..., set_oc_delay(), set_manufacturer_string(), ...).

Best regards
Christian