LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
To: Rob Herring <robh@kernel.org>
Cc: linuxarm@huawei.com, mauro.chehab@huawei.com,
	Yu Chen <chenyu56@huawei.com>,
	John Stultz <john.stultz@linaro.org>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] dt-bindings: misc: add schema for USB hub on Kirin devices
Date: Sat, 14 Aug 2021 08:58:46 +0200	[thread overview]
Message-ID: <20210814085846.15119702@coco.lan> (raw)
In-Reply-To: <YRa/fURTp8QncIEZ@robh.at.kernel.org>

Em Fri, 13 Aug 2021 13:52:45 -0500
Rob Herring <robh@kernel.org> escreveu:

> On Thu, Aug 05, 2021 at 12:31:00PM +0200, Mauro Carvalho Chehab wrote:
> > From: Yu Chen <chenyu56@huawei.com>
> > 
> > This patch adds binding documentation to support USB HUB and
> > USB data role switch of HiSilicon HiKey960 and HiKey970 boards.  
> 
> Like PCIe, there's a standard way to describe USB devices in DT. Though 
> PCI is easy compared to USB. :(
> 
> Also like PCIe on Hikey, I'm less than thrilled to define how this 
> should look for a board that's generally not widely available or well 
> supported.

This board is widely available. Mine was bought at Amazon. 96boards has
a list of other places with also sell it:
	https://www.96boards.org/product/hikey970/

> > [mchehab: updated OF names and added support for HiKey970]
> > Signed-off-by: Yu Chen <chenyu56@huawei.com>
> > Signed-off-by: John Stultz <john.stultz@linaro.org>
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > ---
> >  .../bindings/misc/hisilicon,hikey-usb.yaml    | 95 +++++++++++++++++++
> >  1 file changed, 95 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/misc/hisilicon,hikey-usb.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/misc/hisilicon,hikey-usb.yaml b/Documentation/devicetree/bindings/misc/hisilicon,hikey-usb.yaml
> > new file mode 100644
> > index 000000000000..857f9bd802fe
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/misc/hisilicon,hikey-usb.yaml
> > @@ -0,0 +1,95 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +# Copyright 2019 Linaro Ltd.
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/misc/hisilicon,hikey-usb.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: HiKey960/970 onboard USB GPIO Hub
> > +
> > +maintainers:
> > +  - John Stultz <john.stultz@linaro.org>
> > +
> > +description: |
> > +  Supports the onboard USB GPIO hub found on HiKey960/970.
> > +  The HUB, which acts as a role-switch intermediary to detect the state of
> > +  the USB-C port, to switch the hub into dual-role USB-C or host mode,
> > +  which enables the onboard USB-A host ports.
> > +
> > +  Schematics about the hub can be found here:
> > +    https://github.com/96boards/documentation/raw/master/consumer/hikey/hikey960/hardware-docs/HiKey960_Schematics.pdf
> > +    https://www.96boards.org/documentation/consumer/hikey/hikey970/hardware-docs/files/hikey970-schematics.pdf
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - hisilicon,gpio_hubv1
> > +      - hisilicon,kirin970_hikey_usbhub  
> 
> s/_/-/
> 
> Why is one compatible pretty generic and the other very specific?

Historical reasons. I'll change them to:

	  compatible:
	    enum:
	      - hisilicon,hikey960-usbhub
	      - hisilicon,hikey970-usbhub

> 
> > +
> > +  typec-vbus:
> > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> > +    description: phandle to the typec-vbus gpio  
> 
> If a GPIO, why is it not using a GPIO DT property?

Not sure what you meant. Do you want append "-gpios" to each of the gpio
property, like:

	typec-vbus-gpios
	otg-switch-gpios

Or do you mean something else?

> 
> > +
> > +  otg-switch:
> > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> > +    description: phandle to the otg-switch gpio  
> 
> Ditto?
> 
> > +
> > +  hub-vdd33-en:
> > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> > +    description: phandle to the hub 3.3v power enablement gpio  
> 
> GPIOs controlling a power rail should use a gpio-regulator.

Ok. I'll change and do some tests with Kirin960.

> 
> > +
> > +  hub_reset_en_gpio:  
> 
> s/_/-/
> 
> And still, not a GPIO DT property...
> 
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description: phandle to the hub reset gpio
> > +
> > +  usb-role-switch:
> > +    $ref: /schemas/types.yaml#/definitions/flag
> > +    description: Support role switch.
> > +
> > +  hub-vdd-supply:
> > +    description: regulator for hub power
> > +
> > +  port:
> > +    description: |
> > +      any connector to the data bus of this controller should be modelled
> > +      using the OF graph bindings specified, if the "usb-role-switch"
> > +      property is used. Note for this driver, two ports are supported,
> > +      the first being the endpoint that will be notified by this driver,
> > +      and the second being the endpoint that notifies this driver of a
> > +      role switch.  
> 
> You're describing this in terms of driver connections rather than h/w 
> connections.
> 
> But we've already got ways to describe the data connections. For 
> starters, it should be a child of the USB host.
> 
> And how does all this tie in with the USB connector binding?

The HiKey960/970 hardware has those components:

	- a DWC3 IP at the Kirin960/970 SoC;
	- an I2C-controlled host/budget PHY (rt1711h);
	- an in-board USB hub chip. Either one of those chips:
		- On HiKey960: USB5743
		- On HiKey970: TUSB8041
	- USB3 and Type-C ports:
		- On HiKey960: one Type-C port, two USB3 ports
		- On HiKey970: two Type-C ports, two USB3 ports

That makes the port connections more complex, as they are between
the PHY and the HUB.

This is at Kirin970 SoC DT schema (I omitted some properties, to make it
shorter):

	soc {
		gpio0: gpio@e8a0b000 {
			compatible = "arm,pl061", "arm,primecell";
			gpio-controller;
			#gpio-cells = <2>;
			gpio-ranges = <&pmx0 1 0 1 &pmx0 3 1 5>;
		};
		gpio4: gpio@e8a0f000 {
			compatible = "arm,pl061", "arm,primecell";
			gpio-controller;
			#gpio-cells = <2>;
			gpio-ranges = <&pmx0 0 18 8>;
		};
		gpio26: gpio@fff0f000 {
			compatible = "arm,pl061", "arm,primecell";
			gpio-controller;
			#gpio-cells = <2>;
			gpio-ranges = <&pmx1 0 30 1>;
		};

		i2c1: i2c@ffd72000 {
			compatible = "snps,designware-i2c";
		};

		usb3_otg_bc: usb3_otg_bc@ff200000 {
			compatible = "syscon", "simple-mfd";
			reg = <0x0 0xff200000 0x0 0x1000>;

			usb_phy: usbphy {
				compatible = "hisilicon,hi3670-usb-phy";
			};
		};

		usb31_misc_rst: usb31_misc_rst_controller {
			compatible = "hisilicon,hi3660-reset";
			#reset-cells = <2>;
			hisi,rst-syscon = <&usb3_otg_bc>;
		};

		usb3: hisi_dwc3 {
			compatible = "hisilicon,hi3670-dwc3";

			resets = <&crg_rst 0x90 6>,
				 <&crg_rst 0x90 7>,
				 <&usb31_misc_rst 0xA0 8>,
				 <&usb31_misc_rst 0xA0 9>;

			dwc3: usb@ff100000 {
				compatible = "snps,dwc3";
			};
		};
	};

This is the HiKey970 DT schema:

       hikey_usbhub: hikey_usbhub {
               compatible = "hisilicon,kirin970_hikey_usbhub";

               typec-vbus-gpios = <&gpio26 1 0>;
               otg-switch-gpios = <&gpio4 2 0>;
               hub_reset_en_gpio = <&gpio0 3 0>;
               hub-vdd-supply = <&ldo17>;
               usb-role-switch;

               port {
                       #address-cells = <1>;
                       #size-cells = <0>;

                       hikey_usb_ep0: endpoint@0 {
                               reg = <0>;
                               remote-endpoint = <&dwc3_role_switch>;
                       };
                       hikey_usb_ep1: endpoint@1 {
                               reg = <1>;
                               remote-endpoint = <&rt1711h_ep>;
                       };
               };
       };

	&usb_phy {
		phy-supply = <&ldo17>;
	};

	&i2c1 {
		status = "okay";

		rt1711h: rt1711h@4e {
			compatible = "richtek,rt1711h";

			usb_con: connector {
				compatible = "usb-c-connector";
				label = "USB-C";
				data-role = "dual";
				power-role = "dual";
				ports {
					#address-cells = <1>;
					#size-cells = <0>;
					port@1 {
						reg = <1>;
						usb_con_ss: endpoint {
							remote-endpoint = <&dwc3_ss>;
						};
					};
				};
			};
			port {
				#address-cells = <1>;
				#size-cells = <0>;

				rt1711h_ep: endpoint@0 {
					reg = <0>;
					remote-endpoint = <&hikey_usb_ep1>;
				};
			};
		};
	};



> 
> > +
> > +
> > +required:
> > +  - compatible
> > +  - typec-vbus
> > +  - otg-switch
> > +  - usb-role-switch
> > +  - port
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +
> > +    hisi_hikey_usb: hisi_hikey_usb {
> > +        compatible = "hisilicon,gpio_hubv1";
> > +        typec-vbus = <&gpio25 2 GPIO_ACTIVE_HIGH>;
> > +        otg-switch = <&gpio25 6 GPIO_ACTIVE_HIGH>;
> > +        hub-vdd33-en = <&gpio5 6 GPIO_ACTIVE_HIGH>;
> > +        usb-role-switch;
> > +
> > +        port {
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +
> > +            hikey_usb_ep0: endpoint@0 {
> > +                reg = <0>;
> > +                remote-endpoint = <&dwc3_role_switch>;
> > +            };
> > +            hikey_usb_ep1: endpoint@1 {
> > +                reg = <1>;
> > +                remote-endpoint = <&rt1711h_ep>;
> > +            };
> > +        };
> > +    };
> > -- 
> > 2.31.1
> > 
> >   



Thanks,
Mauro

      reply	other threads:[~2021-08-14  6:58 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-05 10:31 Mauro Carvalho Chehab
2021-08-13 18:52 ` Rob Herring
2021-08-14  6:58   ` Mauro Carvalho Chehab [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210814085846.15119702@coco.lan \
    --to=mchehab+huawei@kernel.org \
    --cc=chenyu56@huawei.com \
    --cc=devicetree@vger.kernel.org \
    --cc=john.stultz@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxarm@huawei.com \
    --cc=mauro.chehab@huawei.com \
    --cc=robh@kernel.org \
    --subject='Re: [PATCH] dt-bindings: misc: add schema for USB hub on Kirin devices' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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