diff mbox series

[v2,3/4] dt-bindings: usb: Add Microchip USB47xx/USB49xx support

Message ID 20200723192901.26661-1-ceggers@arri.de (mailing list archive)
State New, archived
Headers show
Series None | expand

Commit Message

Christian Eggers July 23, 2020, 7:29 p.m. UTC
Add DT bindings for Microchip USB47xx/USB49xx driver.

Signed-off-by: Christian Eggers <ceggers@arri.de>
---
> 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

Comments

gregkh@linuxfoundation.org July 26, 2020, 8:41 a.m. UTC | #1
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 <ceggers@arri.de>
> ---
> > 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
Christian Eggers July 27, 2020, 8:33 a.m. UTC | #2
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
Richard Leitner Aug. 17, 2020, 7:54 a.m. UTC | #3
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
>
Christian Eggers Aug. 24, 2020, 10:31 a.m. UTC | #4
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
diff mbox series

Patch

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 <ceggers@arri.de>
+
+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>;
+      };
+    };
+
+...