El Fri, Mar 03, 2023 at 09:31:33AM +0200, Eugen Hristev deia: > @@ -105,6 +130,17 @@ static int rockchip_usb2phy_power_off(struct phy *phy) > struct udevice *parent = dev_get_parent(phy->dev); > struct rockchip_usb2phy *priv = dev_get_priv(parent); > const struct rockchip_usb2phy_port_cfg *port_cfg = us2phy_get_port(phy); > + struct udevice *vbus = NULL; > + int ret; > + > + vbus = rockchip_usb2phy_check_vbus(phy); > + if (vbus) { > + ret = regulator_set_enable(vbus, false); > + if (ret) {
Could we have if (ret && ret != -EACCES ) { instead here ? (reason below) > + dev_err(phy->dev, "vbus disable failed: %d\n", ret); > + return ret; > + } > + } > > property_enable(priv->reg_base, &port_cfg->phy_sus, true); > Thank you. I've tested a little your patch (on an older master, 4eb7c5030d3f3c70 (2023-02-19) and some minor config changes that don't seem to matter). I could try on current master if needed, but I thought I'd ask already. In a Rock-pi 4B the ehci ports weren't working without your patch and they still don't work with your patch. With my patch (v5, [1]) they worked, but with my patch and yours usb reset fails: resetting USB... rockchip_usb2phy_port host-port: vbus disable failed: -13 rockchip_usb2phy_port host-port: PHY: Failed to power off host-port: -13. device_remove: Device 'usb@fe38 ' failed to remove, but children are gone rockchip_usb2phy_port host-port: vbus disable failed: -13 rockchip_usb2phy_port host-port: PHY: Failed to power off host-port: -13. device_remove: Device 'usb@fe3c0000' failed to remove, but children are gone Bus usb@fe380000: Bus usb@fe3c0000: Bus usb@fe800000: Register 2000140 NbrPorts 2 Starting the controller USB XHCI 1.10 Bus usb@fe900000: Register 2000140 NbrPorts 2 Starting the controller USB XHCI 1.10 scanning bus usb@fe380000 for devices... "Synchronous Abort" handler, esr 0x96000010 elr: 000000000021de20 lr : 000000000021de20 (reloc) elr: 00000000f3f56e20 lr : 00000000f3f56e20 x0 : 0000000000000000 x1 : 00000000f1f19fef x2 : 00000000f1f19fee x3 : 00000000f1f53bc0 x4 : 0000000000000000 x5 : 0000000000000000 x6 : 0000000000000000 x7 : 00000000f1f60000 x8 : 0000000000000100 x9 : 0000000000000008 x10: 0000000000000006 x11: 0000000000000080 x12: 000000000001869f x13: 000000000000869c x14: 0000000000000000 x15: 0000000000000002 x16: 000000007e4f2113 x17: 000000009a11f13e x18: 00000000f1f30d90 x19: 00000000f1f323c0 x20: 00000000f1f1a680 x21: 00000000f1f19fee x22: 00000000f1f19fef x23: 0000000088404000 x24: 00000000f1f1a280 x25: 0000000000000001 x26: 00000000f1f1a680 x27: 00000000f1f1a0c0 x28: 0000000008400000 x29: 00000000f1f19f90 Code: aa0203f5 f944a413 aa1303e0 94003d8f (b9400401) Resetting CPU ... resetting ... The apparent reason is that arch/arm/dts/rk3399-rock-pi-4.dtsi says vcc5v0_host: vcc5v0-host-regulator { compatible = "regulator-fixed"; enable-active-high; gpio = <&gpio4 RK_PD1 GPIO_ACTIVE_HIGH>; pinctrl-names = "default"; pinctrl-0 = <&vcc5v0_host_en>; regulator-name = "vcc5v0_host"; /*****/ regulator-always-on; /*****/ vin-supply = <&vcc5v0_sys>; }; [...] &u2phy0 { status = "okay"; u2phy0_otg: otg-port { status = "okay"; }; u2phy0_host: host-port { phy-supply = <&vcc5v0_host>; status = "okay"; }; }; &u2phy1 { status = "okay"; u2phy1_otg: otg-port { status = "okay"; }; u2phy1_host: host-port { phy-supply = <&vcc5v0_host>; status = "okay"; }; }; and the regulator_set_enable() in regulator-uclass.c when passed enable==0 will return -EACCES (meaning you can't turn off an always-on regulator). Now my patch is not accepted, so EHCI isn't working on Rock-pi-4, so the error won't show. But I suspect it may bite if v5 is ever accepted. And I think it would fail if EHCI is fixed in any other way. I may try my v1, v2 or v3 with your patch if you think it's worth it. Now I don't know what the right solution is : - ignore -EACCES in rockchip_usb2phy_power_off() (as above, seems easiest?) - check always_on in rockchip_usb2phy_power_off() before calling regulator_set_enable() - change .dts (but they come from linux, don't they?) - make sure somehow that things don't break even if rockchip_usb2phy_power_off() exits early with error. Btw, my v5 seems not to be correctly in patchwork, should I resend ? I guess I messed up subject lines ? I think there are other rk3399 boards that have similar .dtsi files, but I haven't investigated thoroughly. Do they all have broken EHCI ? If someone has a rk3399 board with working EHCI maybe they should test your patch and try usb start ; usb stop and usb start ; usb reset ? [1] https://lists.denx.de/pipermail/u-boot/2023-February/510672.html Thanks, -- Xavier Drudis Ferran