Re: [RFC][PATCH v3 11/11] misc: hisi_hikey_usb: Driver to support usb functionality of Hikey960

Re: [RFC][PATCH v3 11/11] misc: hisi_hikey_usb: Driver to support usb functionality of Hikey960

[Mauro Carvalho Chehab]

Hi John,

Em Wed, 16 Oct 2019 03:33:40 +0000
John Stultz <john.stultz@linaro.org> escreveu:

> From: Yu Chen <chenyu56@huawei.com>
> 
> The HiKey960 has a fairly complex USB configuration due to it
> needing to support a USB-C port for host/device mode and multiple
> USB-A ports in host mode using a single USB controller.
> 
> See schematics here:
>   https://github.com/96boards/documentation/raw/master/consumer/hikey/hikey960/hardware-docs/HiKey960_Schematics.pdf
> 
> This driver acts as a usb-role-switch intermediary, intercepting
> the role switch notifications from the tcpm code, and passing
> them on to the dwc3 core.
> 
> In doing so, it also controls the onboard hub and power gpios in
> order to properly route the data lines between the USB-C port
> and the onboard hub to the USB-A ports.
> 
> NOTE: It was noted that controlling the TYPEC_VBUS_POWER_OFF and
> TYPEC_VBUS_POWER_ON values here is not reccomended. I'm looking
> for a way to remove that bit from the logic here, but wanted to
> still get feedback on this approach.

Let me somewhat hijack this thread. I'm trying to add support here
for the Hikey 970 driver. Maybe you might help me finding the remaing
issues over there ;-)

The Hikey 970 has lots of things in common with Hikey 960, but
the USB hub uses a somewhat different approach (based on what I
saw at the Linaro's 4.9 official Hikey kernel tree).

Basically, with the enclosed patch applied, the USB hub needs these
at the DT file:

		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;
...
		}

E.g. when compared with Hikey 960, the USB hub:

- Hikey 970 uses a regulator instead of GPIO for powering on;
- Hikey 970 has a reset pin controlled via GPIO.

It should be simple to add support for it, as done by the
enclosed patch. With this, the phy driver for Hikey 970 and a new
small driver to properly set clocks and reset lines at dwg3[1],
I can now see the hub on my Hikey970:

	$ lsusb
	Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
	Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub

Still, I'm missing something to make it work, as, besides the hub,
right now, it doesn't detect the keyboard/mouse, which are
attached at the USB hub.

Do you have any ideas?

-

[1] Right now, this is needed:
	https://github.com/96boards-hikey/linux/blob/hikey970-v4.9/drivers/usb/dwc3/dwc3-hisi.c

    Placing dwc3 directly under soc at DT causes some weird NMI, with
    either produce an OOPS or hangs the machine at boot time.

Thanks,
Mauro

[PATCH] misc: hisi_hikey_usb: add support for Hikey 970

The HiKey 970 board uses a voltage regulator and GPIO reset pin.

Add support for them.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

diff --git a/drivers/misc/hisi_hikey_usb.c b/drivers/misc/hisi_hikey_usb.c
index 3a98a890757c..76eb38fc6169 100644
--- a/drivers/misc/hisi_hikey_usb.c
+++ b/drivers/misc/hisi_hikey_usb.c
@@ -14,8 +14,10 @@
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/notifier.h>
+#include <linux/of_gpio.h>
 #include <linux/platform_device.h>
 #include <linux/property.h>
+#include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 #include <linux/usb/role.h>
 
@@ -46,18 +48,27 @@ struct hisi_hikey_usb {
 
 static void hub_power_ctrl(struct hisi_hikey_usb *hisi_hikey_usb, int value)
 {
+	if (!hisi_hikey_usb->hub_vbus)
+		return;
+
 	gpiod_set_value_cansleep(hisi_hikey_usb->hub_vbus, value);
 }
 
 static void usb_switch_ctrl(struct hisi_hikey_usb *hisi_hikey_usb,
 			    int switch_to)
 {
+	if (!hisi_hikey_usb->otg_switch)
+		return;
+
 	gpiod_set_value_cansleep(hisi_hikey_usb->otg_switch, switch_to);
 }
 
 static void usb_typec_power_ctrl(struct hisi_hikey_usb *hisi_hikey_usb,
 				 int value)
 {
+	if (!hisi_hikey_usb->typec_vbus)
+		return;
+
 	gpiod_set_value_cansleep(hisi_hikey_usb->typec_vbus, value);
 }
 
@@ -117,31 +128,89 @@ static int hub_usb_role_switch_set(struct usb_role_switch *sw, enum usb_role rol
 	return 0;
 }
 
+static int hisi_hikey_usb_parse_kirin970(struct platform_device *pdev)
+{
+	struct regulator *regulator;
+	int hub_reset_en_gpio;
+	int ret;
+
+	regulator = devm_regulator_get_optional(&pdev->dev, "hub-vdd");
+	if (IS_ERR(regulator)) {
+		if (PTR_ERR(regulator) == -EPROBE_DEFER) {
+			dev_info(&pdev->dev,
+				"waiting for hub-vdd-supply to be probed\n");
+			return PTR_ERR(regulator);
+		} else {
+			/* let it fall back to regulator dummy */
+			regulator = devm_regulator_get(&pdev->dev, "hub-vdd");
+			if (IS_ERR(regulator)) {
+				dev_err(&pdev->dev,
+					"get hub-vdd-supply failed with error %ld\n",
+					PTR_ERR(regulator));
+				return PTR_ERR(regulator);
+			}
+		}
+	}
+
+	ret = regulator_set_voltage(regulator, 3300000, 3300000);
+	if (ret)
+		dev_err(&pdev->dev, "set hub-vdd-supply voltage failed\n");
+
+	hub_reset_en_gpio = of_get_named_gpio(pdev->dev.of_node,
+					      "hub_reset_en_gpio", 0);
+	if (!gpio_is_valid(hub_reset_en_gpio)) {
+		dev_err(&pdev->dev, "Failed to get a valid reset gpio\n");
+		return -ENODEV;
+	}
+
+	ret = devm_gpio_request(&pdev->dev, hub_reset_en_gpio,
+				"hub_reset_en_gpio");
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to request the reset gpio\n");
+		return ret;
+	}
+	ret = gpio_direction_output(hub_reset_en_gpio, 1);
+	if (ret)
+		dev_err(&pdev->dev,
+			"Failed to set the direction of the reset gpio\n");
+
+	return ret;
+}
+
 static int hisi_hikey_usb_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct hisi_hikey_usb *hisi_hikey_usb;
 	struct usb_role_switch_desc hub_role_switch = {NULL};
+	int ret;
 
 	hisi_hikey_usb = devm_kzalloc(dev, sizeof(*hisi_hikey_usb), GFP_KERNEL);
 	if (!hisi_hikey_usb)
 		return -ENOMEM;
 
-	hisi_hikey_usb->typec_vbus = devm_gpiod_get(dev, "typec-vbus",
-						    GPIOD_OUT_LOW);
-	if (IS_ERR(hisi_hikey_usb->typec_vbus))
-		return PTR_ERR(hisi_hikey_usb->typec_vbus);
-
 	hisi_hikey_usb->otg_switch = devm_gpiod_get(dev, "otg-switch",
 						    GPIOD_OUT_HIGH);
 	if (IS_ERR(hisi_hikey_usb->otg_switch))
 		return PTR_ERR(hisi_hikey_usb->otg_switch);
 
-	/* hub-vdd33-en is optional */
-	hisi_hikey_usb->hub_vbus = devm_gpiod_get_optional(dev, "hub-vdd33-en",
-							   GPIOD_OUT_HIGH);
-	if (IS_ERR(hisi_hikey_usb->hub_vbus))
-		return PTR_ERR(hisi_hikey_usb->hub_vbus);
+	/* Parse Kirin 970-specific OF data */
+	if (of_device_is_compatible(pdev->dev.of_node,
+				    "hisilicon,kirin970_hikey_usbhub")) {
+		ret = hisi_hikey_usb_parse_kirin970(pdev);
+		if (ret)
+			return ret;
+	} else {
+		hisi_hikey_usb->typec_vbus = devm_gpiod_get(dev, "typec-vbus",
+							    GPIOD_OUT_LOW);
+		if (IS_ERR(hisi_hikey_usb->typec_vbus))
+			return PTR_ERR(hisi_hikey_usb->typec_vbus);
+
+		/* hub-vdd33-en is optional */
+		hisi_hikey_usb->hub_vbus = devm_gpiod_get_optional(dev, "hub-vdd33-en",
+								GPIOD_OUT_HIGH);
+		if (IS_ERR(hisi_hikey_usb->hub_vbus))
+			return PTR_ERR(hisi_hikey_usb->hub_vbus);
+	}
 
 	hisi_hikey_usb->dev_role_sw = usb_role_switch_get(dev);
 	if (!hisi_hikey_usb->dev_role_sw)
@@ -185,6 +254,7 @@ static int  hisi_hikey_usb_remove(struct platform_device *pdev)
 
 static const struct of_device_id id_table_hisi_hikey_usb[] = {
 	{.compatible = "hisilicon,gpio_hubv1"},
+	{.compatible = "hisilicon,kirin970_hikey_usbhub"},
 	{}
 };
 MODULE_DEVICE_TABLE(of, id_table_hisi_hikey_usb);

Re: [RFC][PATCH v3 11/11] misc: hisi_hikey_usb: Driver to support usb functionality of Hikey960

[John Stultz]

On Mon, Aug 10, 2020 at 9:35 AM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
> Em Wed, 16 Oct 2019 03:33:40 +0000
> John Stultz <john.stultz@linaro.org> escreveu:
>
> > From: Yu Chen <chenyu56@huawei.com>
> >
> > The HiKey960 has a fairly complex USB configuration due to it
> > needing to support a USB-C port for host/device mode and multiple
> > USB-A ports in host mode using a single USB controller.
> >
> > See schematics here:
> >   https://github.com/96boards/documentation/raw/master/consumer/hikey/hikey960/hardware-docs/HiKey960_Schematics.pdf
> >
> > This driver acts as a usb-role-switch intermediary, intercepting
> > the role switch notifications from the tcpm code, and passing
> > them on to the dwc3 core.
> >
> > In doing so, it also controls the onboard hub and power gpios in
> > order to properly route the data lines between the USB-C port
> > and the onboard hub to the USB-A ports.
> >
> > NOTE: It was noted that controlling the TYPEC_VBUS_POWER_OFF and
> > TYPEC_VBUS_POWER_ON values here is not reccomended. I'm looking
> > for a way to remove that bit from the logic here, but wanted to
> > still get feedback on this approach.
>
> Let me somewhat hijack this thread. I'm trying to add support here
> for the Hikey 970 driver. Maybe you might help me finding the remaing
> issues over there ;-)

So.. just as a heads up, this is a fairly old version of this patch. I
have the current version here:
  https://git.linaro.org/people/john.stultz/android-dev.git/commit/?h=dev/hikey960-mainline-WIP&id=1155346a06472177b8a7e7918de052549916f06f

So you may want to rework ontop of that.

That said, the last time I submitted the hub/mux driver, Rob pushed
back suggesting that the vbus, switch and hub power should probably be
DT describable:
  https://lore.kernel.org/lkml/20191218163738.GA12358@bogus/

I'm at the point where I probably don't have additional cycles to
spend to rework all the supporting drivers to support such a DT
binding, so I'm not very optimistic this patch will go upstream (its
much easier to float the current hub/mux driver).  So you may want to
focus on Rob's feedback there rather than any of my feedback here. :)


> The Hikey 970 has lots of things in common with Hikey 960, but
> the USB hub uses a somewhat different approach (based on what I
> saw at the Linaro's 4.9 official Hikey kernel tree).
>
> Basically, with the enclosed patch applied, the USB hub needs these
> at the DT file:
>
>                 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;
> ...
>                 }
>
> E.g. when compared with Hikey 960, the USB hub:
>
> - Hikey 970 uses a regulator instead of GPIO for powering on;

So, it might not be too hard to rework the hikey960 hub power gpio to
a gpio-regulator binding, and then both platforms can use the same
code?

> - Hikey 970 has a reset pin controlled via GPIO.

You might be able to put this reset pin under the dwc3 resets?


> It should be simple to add support for it, as done by the
> enclosed patch. With this, the phy driver for Hikey 970 and a new
> small driver to properly set clocks and reset lines at dwg3[1],
> I can now see the hub on my Hikey970:
>
>         $ lsusb
>         Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
>         Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
>
> Still, I'm missing something to make it work, as, besides the hub,
> right now, it doesn't detect the keyboard/mouse, which are
> attached at the USB hub.
>
> Do you have any ideas?

Not sure about the hub keyboard mouse issue. I worry that may be an
issue with the hub power not being on?
Make sure the mux driver is in the expected state when you boot up and
switch modes.

> [1] Right now, this is needed:
>         https://github.com/96boards-hikey/linux/blob/hikey970-v4.9/drivers/usb/dwc3/dwc3-hisi.c
>
>     Placing dwc3 directly under soc at DT causes some weird NMI, with
>     either produce an OOPS or hangs the machine at boot time.

I suspect you can drop the dwc3-hisi glue code once you move the clks
and resets to the dwc3 node directly, as we did for hikey960.
  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/hisilicon/hi3660.dtsi?id=4bcf69e57063c9b1b15df1a293c969e80a1c97e6#n1169

thanks
-john

Re: [RFC][PATCH v3 11/11] misc: hisi_hikey_usb: Driver to support usb functionality of Hikey960

[Mauro Carvalho Chehab]

Em Mon, 10 Aug 2020 21:36:58 -0700
John Stultz <john.stultz@linaro.org> escreveu:

> On Mon, Aug 10, 2020 at 9:35 AM Mauro Carvalho Chehab
> <mchehab+huawei@kernel.org> wrote:
> > Em Wed, 16 Oct 2019 03:33:40 +0000
> > John Stultz <john.stultz@linaro.org> escreveu:
> >  
> > > From: Yu Chen <chenyu56@huawei.com>
> > >
> > > The HiKey960 has a fairly complex USB configuration due to it
> > > needing to support a USB-C port for host/device mode and multiple
> > > USB-A ports in host mode using a single USB controller.
> > >
> > > See schematics here:
> > >   https://github.com/96boards/documentation/raw/master/consumer/hikey/hikey960/hardware-docs/HiKey960_Schematics.pdf
> > >
> > > This driver acts as a usb-role-switch intermediary, intercepting
> > > the role switch notifications from the tcpm code, and passing
> > > them on to the dwc3 core.
> > >
> > > In doing so, it also controls the onboard hub and power gpios in
> > > order to properly route the data lines between the USB-C port
> > > and the onboard hub to the USB-A ports.
> > >
> > > NOTE: It was noted that controlling the TYPEC_VBUS_POWER_OFF and
> > > TYPEC_VBUS_POWER_ON values here is not reccomended. I'm looking
> > > for a way to remove that bit from the logic here, but wanted to
> > > still get feedback on this approach.  
> >
> > Let me somewhat hijack this thread. I'm trying to add support here
> > for the Hikey 970 driver. Maybe you might help me finding the remaing
> > issues over there ;-)  
> 
> So.. just as a heads up, this is a fairly old version of this patch. I
> have the current version here:
>   https://git.linaro.org/people/john.stultz/android-dev.git/commit/?h=dev/hikey960-mainline-WIP&id=1155346a06472177b8a7e7918de052549916f06f
> 
> So you may want to rework ontop of that.

Yeah, I used the version from your tree.

> 
> That said, the last time I submitted the hub/mux driver, Rob pushed
> back suggesting that the vbus, switch and hub power should probably be
> DT describable:
>   https://lore.kernel.org/lkml/20191218163738.GA12358@bogus/

Yeah, makes sense. After USB starts working, I'll try to write a
patch on the top of yours in order to use the schema he proposed.

For now, I'm trying to understand why the only two devices found
are the hub ones. Maybe the device is still in budget mode.

> I'm at the point where I probably don't have additional cycles to
> spend to rework all the supporting drivers to support such a DT
> binding, so I'm not very optimistic this patch will go upstream (its
> much easier to float the current hub/mux driver).  So you may want to
> focus on Rob's feedback there rather than any of my feedback here. :)

I have some cycles to spend on this. Just got a 960 board on my hands.
I guess I'll try to test your patches on the top of it.

> > The Hikey 970 has lots of things in common with Hikey 960, but
> > the USB hub uses a somewhat different approach (based on what I
> > saw at the Linaro's 4.9 official Hikey kernel tree).
> >
> > Basically, with the enclosed patch applied, the USB hub needs these
> > at the DT file:
> >
> >                 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;
> > ...
> >                 }
> >
> > E.g. when compared with Hikey 960, the USB hub:
> >
> > - Hikey 970 uses a regulator instead of GPIO for powering on;  
> 
> So, it might not be too hard to rework the hikey960 hub power gpio to
> a gpio-regulator binding, and then both platforms can use the same
> code?

Good point. Yeah, it doesn't sound hard to do that.

> > - Hikey 970 has a reset pin controlled via GPIO.  
> 
> You might be able to put this reset pin under the dwc3 resets?

I'll try.

> > It should be simple to add support for it, as done by the
> > enclosed patch. With this, the phy driver for Hikey 970 and a new
> > small driver to properly set clocks and reset lines at dwg3[1],
> > I can now see the hub on my Hikey970:
> >
> >         $ lsusb
> >         Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
> >         Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
> >
> > Still, I'm missing something to make it work, as, besides the hub,
> > right now, it doesn't detect the keyboard/mouse, which are
> > attached at the USB hub.
> >
> > Do you have any ideas?  
> 
> Not sure about the hub keyboard mouse issue. I worry that may be an
> issue with the hub power not being on?
> Make sure the mux driver is in the expected state when you boot up and
> switch modes.

No, it is not power. The power supply for LDO17 (used on Hikey 970) is
enabled before this driver gets called (as there's a logic handling
EPROBE_DEFER on my patch).

It is starting to work, after a couple of hacks:

[    1.503038] JDB: dwc3_core_init DWC3_DSTS: 0xd3037c
[    1.560131] JDB: dwc3_core_init_mode  dr_mode: 3
[    1.583522] dwc3 ff100000.dwc3: JDB: dwc3_drd_init: dwc3_get_extcon returned 0
[    1.595804] JDB: dwc3_set_mode  desired role: 1
[    1.769184] dwc3 ff100000.dwc3: JDB: dwc3_drd_init: dwc3_setup_role_switch returned 0
[    1.777110] JDB: __dwc3_set_mode dr_mode: 3
[    1.781343] JDB: dwc3_set_prtcap  current_dr_role set to: 1
[    1.781348] JDB: dwc3_probe init mode returned 0
...
[    3.829930] platform ldo17: Adding to iommu group 52
...
[    4.760428] JDB: hisi_hikey_usb_probe: usb_role_switch_get returned 0
[    4.766867] JDB: hisi_hikey_usb_probe: initializing USB role switch 
[    4.773252] JDB: hisi_hikey_usb_probe complete!
[    4.848182] usb 2-1: new SuperSpeed Gen 1 USB device number 2 using xhci-hcd
[    4.908727] hub 2-1:1.0: USB hub found
[    4.912551] hub 2-1:1.0: 4 ports detected
[    5.004136] usb 1-1: new high-speed USB device number 2 using xhci-hcd
[    5.556147] usb 1-1.1: new low-speed USB device number 3 using xhci-hcd
[    5.710919] input: PixArt Dell MS116 USB Optical Mouse as /devices/platform/soc/ff100000.dwc3/xhci-hcd.0.auto/usb1/1-1/1-1.1/1-1.1:1.0/0003:413C:301A.0001/input/input0
[    5.732353] hid-generic 0003:413C:301A.0001: input: USB HID v1.11 Mouse [PixArt Dell MS116 USB Optical Mouse] on usb-xhci-hcd.0.auto-1.1/input0
[    5.832141] usb 1-1.2: new low-speed USB device number 4 using xhci-hcd
[    6.032760] input: Dell KB216 Wired Keyboard as /devices/platform/soc/ff100000.dwc3/xhci-hcd.0.auto/usb1/1-1/1-1.2/1-1.2:1.0/0003:413C:2113.0002/input/input1
[    6.104393] hid-generic 0003:413C:2113.0002: input: USB HID v1.11 Keyboard [Dell KB216 Wired Keyboard] on usb-xhci-hcd.0.auto-1.2/input0
[    6.122554] input: Dell KB216 Wired Keyboard System Control as /devices/platform/soc/ff100000.dwc3/xhci-hcd.0.auto/usb1/1-1/1-1.2/1-1.2:1.1/0003:413C:2113.0003/input/input2
[    6.200460] input: Dell KB216 Wired Keyboard Consumer Control as /devices/platform/soc/ff100000.dwc3/xhci-hcd.0.auto/usb1/1-1/1-1.2/1-1.2:1.1/0003:413C:2113.0003/input/input3
[    6.216611] hid-generic 0003:413C:2113.0003: input: USB HID v1.11 Device [Dell KB216 Wired Keyboard] on usb-xhci-hcd.0.auto-1.2/input1

However, when rt1711 is probed, the USB switches to role "none":

[    6.690237] JDB: rt1711h_probe
[    6.740208] JDB: tcpm_init
[    6.744181] JDB: tcpci_init
[    6.744365] JDB: rt1711h_init
[    6.757570] JDB: tcpm_reset_port
[    6.760415] JDB: tcpm_typec_disconnect
[    6.770065] JDB: tcpci_set_vconn
[    6.780626] JDB: rt1711h_set_vconn
[    6.789456] JDB: tcpci_set_polarity
[    6.805052] JDB: tcpm_mux_set
[    6.818989] JDB: hub_usb_role_switch_set role: none!
[    6.958528] JDB: relay_set_role_switch role: none!
[    6.960157] JDB: tcpci_set_roles
[    6.960698] JDB: tcpci_get_vbus
[    6.961116] JDB: tcpm_set_state
[    6.961444] JDB: tcpm_set_state
[    6.961475] JDB: tcpm_reset_port
[    6.961477] JDB: tcpm_typec_disconnect
[    6.961792] pl061_gpio fff10000.gpio: line 5: IRQ on LOW level
[    6.961829] JDB: tcpci_set_vconn
[    6.961831] JDB: rt1711h_set_vconn
[    6.962069] JDB: tcpci_set_polarity
[    6.962402] JDB: tcpm_mux_set
[    6.962514] JDB: hub_usb_role_switch_set role: none!
[    6.968554] JDB: hub_power_ctrl value: 0 (0000000000000000)
[    6.968557] JDB: usb_switch_ctrl value: 0 ((____ptrval____))
[    6.968563] JDB: usb_typec_power_ctrl value: 1 ((____ptrval____))
[    6.978721] JDB: dwc3_set_mode  desired role: 1
[    6.978740] JDB: tcpci_set_roles
[    6.978755] JDB: relay_set_role_switch role: host!
[    6.978759] JDB: hub_power_ctrl value: 0 (0000000000000000)
[    6.978761] JDB: usb_switch_ctrl value: 0 ((____ptrval____))
[    6.978763] JDB: usb_typec_power_ctrl value: 1 ((____ptrval____))
[    6.978766] JDB: dwc3_set_mode  desired role: 1
[    6.978779] JDB: __dwc3_set_mode dr_mode: 3
[    6.986650] JDB: rt1711h_irq
[    6.986836] JDB: tcpci_irq
[    6.987197] JDB: tcpm_set_state
[    7.004582] JDB: tcpm_vbus_change
[    7.012256] JDB: tcpm_pd_event_handler
[    7.012260] JDB: tcpci_get_vbus

And the USB devices got disconnected afterwards:

[    7.020689] usb 1-1.1: reset low-speed USB device number 3 using xhci-hcd
[    7.020695] usb 1-1.1: hub failed to enable device, error -22
[    7.020765] usb 1-1.1: USB disconnect, device number 3
[    7.288840] usb 1-1.2: USB disconnect, device number 4

I tried force it to switch to host mode with:

	sudo su -c 'echo "device" > /sys/kernel/debug/usb/ff100000.dwc3/mode'
	sudo su -c 'echo "host" > /sys/kernel/debug/usb/ff100000.dwc3/mode'

But didn't work. What's weird is that the usb_role_switch_desc->set() ops 
is only called once, just after the USB hub driver is probed. 

Switching the mode later between host/device mode doesn't make any calls to
the USB hub driver.

> 
> > [1] Right now, this is needed:
> >         https://github.com/96boards-hikey/linux/blob/hikey970-v4.9/drivers/usb/dwc3/dwc3-hisi.c
> >
> >     Placing dwc3 directly under soc at DT causes some weird NMI, with
> >     either produce an OOPS or hangs the machine at boot time.  
> 
> I suspect you can drop the dwc3-hisi glue code once you move the clks
> and resets to the dwc3 node directly, as we did for hikey960.
>   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/hisilicon/hi3660.dtsi?id=4bcf69e57063c9b1b15df1a293c969e80a1c97e6#n1169

I was able to drop it, but I had to add this at dwc3 settings:

	regulator-on-in-suspend;

As otherwise the device seems to stop a few seconds after the dwc3
driver gets started.

I suspect it could be related to those calls at the dwg3 driver:

	pm_runtime_use_autosuspend(dev);
	pm_runtime_set_autosuspend_delay(dev, DWC3_DEFAULT_AUTOSUSPEND_DELAY);

As this seems to be the only difference between what the dwc3 core
does and the dwc3-hisi doesn't do.

Thanks,
Mauro

Re: [RFC][PATCH v3 11/11] misc: hisi_hikey_usb: Driver to support usb functionality of Hikey960

[Mauro Carvalho Chehab]

Em Tue, 11 Aug 2020 14:21:06 +0200
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:

> I was able to drop it, but I had to add this at dwc3 settings:
> 
> 	regulator-on-in-suspend;
> 
> As otherwise the device seems to stop a few seconds after the dwc3
> driver gets started.
> 
> I suspect it could be related to those calls at the dwg3 driver:
> 
> 	pm_runtime_use_autosuspend(dev);
> 	pm_runtime_set_autosuspend_delay(dev, DWC3_DEFAULT_AUTOSUSPEND_DELAY);
> 
> As this seems to be the only difference between what the dwc3 core
> does and the dwc3-hisi doesn't do.

Found the issue.

It is due to a quirk that was renamed:

	-                       snps,dis-split-quirk;
	+                       snps,dis_enblslpm_quirk;

I'm now using just those drivers:

	- hikey 970 phy (pulled from OOT);
	- upstream dwc3;
	- upstream rt1711 driver;
	- drivers/misc/hisi_hikey_usb.c (with my changes).

Now, the problem is that the USB devices are getting one reset per 
second (which starts about 10 seconds after dwc3 driver is loaded):

[   14.080666] usb 1-1.1: reset low-speed USB device number 3 using xhci-hcd
[   14.752689] usb 1-1.2: reset low-speed USB device number 4 using xhci-hcd
[   15.712646] usb 1-1.1: reset low-speed USB device number 3 using xhci-hcd
[   16.480683] usb 1-1.2: reset low-speed USB device number 4 using xhci-hcd
[   17.472651] usb 1-1.1: reset low-speed USB device number 3 using xhci-hcd
[   18.208675] usb 1-1.2: reset low-speed USB device number 4 using xhci-hcd

It sounds like an autosuspend thing, but disabling it (using powertop) 
doesn't affect it. Any ideas?

PS.: I'm enclosing the relevant DT data.

Thanks,
Mauro

		i2c1: i2c@ffd72000 {
			compatible = "snps,designware-i2c";
			reg = <0x0 0xffd72000 0x0 0x1000>;
			interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
			#address-cells = <1>;
			#size-cells = <0>;
			clock-frequency = <400000>;
			clocks = <&iomcu HI3670_CLK_GATE_I2C1>;
			resets = <&iomcu_rst 0x20 4>;
			pinctrl-names = "default";
			pinctrl-0 = <&i2c1_pmx_func &i2c1_cfg_func>;

			rt1711h: rt1711h@4e {
				compatible = "richtek,rt1711h";
				reg = <0x4e>;
				status = "ok";
				interrupt-parent = <&gpio27>;
				interrupts = <5 IRQ_TYPE_LEVEL_LOW>;
				pinctrl-names = "default";
				pinctrl-0 = <&pd_cfg_func>;

				usb_con: connector {
					compatible = "usb-c-connector";
					label = "USB-C";
					data-role = "dual";
					power-role = "dual";
					try-power-role = "sink";
					source-pdos = <PDO_FIXED(5000, 500, PDO_FIXED_USB_COMM)>;
					sink-pdos = <PDO_FIXED(5000, 500, PDO_FIXED_USB_COMM)
						     PDO_VAR(5000, 5000, 1000)>;
					op-sink-microwatt = <10000000>;

					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>;
					};
				};
			};
		};

		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>;
				};
			};
		};

		usb31_misc: usb31_misc@ff200000 {
			compatible = "syscon";
			reg = <0x0 0xff200000 0x0 0x1000>;
		};

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

		usb_phy: usbphy {
			compatible = "hisilicon,hi3670-usb-phy";
			#phy-cells = <0>;
			hisilicon,pericrg-syscon = <&crg_ctrl>;
			hisilicon,pctrl-syscon = <&pctrl>;
			hisilicon,sctrl-syscon = <&sctrl>;
			hisilicon,usb31-misc-syscon = <&usb31_misc>;
			eye-diagram-param = <0xFDFEE4>;
			usb3-phy-tx-vboost-lvl = <0x5>;
		};
		dwc3: dwc3@ff100000 {
			compatible = "snps,dwc3";
			reg = <0x0 0xff100000 0x0 0x100000>;
			interrupts = <0 159 IRQ_TYPE_LEVEL_HIGH>,
				    <0 161 IRQ_TYPE_LEVEL_HIGH>;

			clocks = <&crg_ctrl HI3670_CLK_GATE_ABB_USB>,
				    <&crg_ctrl HI3670_HCLK_GATE_USB3OTG>,
				    <&crg_ctrl HI3670_CLK_GATE_USB3OTG_REF>,
				    <&crg_ctrl HI3670_ACLK_GATE_USB3DVFS>;
			clock-names = "clk_gate_abb_usb",
				      "hclk_gate_usb3otg",
				      "clk_gate_usb3otg_ref",
				      "aclk_gate_usb3dvfs";

			assigned-clocks = <&crg_ctrl HI3670_ACLK_GATE_USB3DVFS>;
			assigned-clock-rates = <238000000>;
			resets = <&crg_rst 0x90 6>,
				 <&crg_rst 0x90 7>,
				 <&usb31_misc_rst 0xA0 8>,
				 <&usb31_misc_rst 0xA0 9>;

			phys = <&usb_phy>;
			phy-names = "usb3-phy";
			dr_mode = "otg";
			maximum-speed = "super-speed";
			phy_type = "utmi";
			snps,dis-del-phy-power-chg-quirk;
			snps,lfps_filter_quirk;
			snps,dis_u2_susphy_quirk;
			snps,dis_u3_susphy_quirk;
			snps,tx_de_emphasis_quirk;
			snps,tx_de_emphasis = <1>;
			snps,dis_enblslpm_quirk;
			snps,gctl-reset-quirk;
			usb-role-switch;
			role-switch-default-mode = "host";
			port {
				#address-cells = <1>;
				#size-cells = <0>;
				dwc3_role_switch: endpoint@0 {
					reg = <0>;
					remote-endpoint = <&hikey_usb_ep0>;
				};

				dwc3_ss: endpoint@1 {
					reg = <1>;
					remote-endpoint = <&usb_con_ss>;
				};
			};
		};