El Wed, Mar 08, 2023 at 01:59:54PM +0200, Eugen Hristev deia: > On 3/8/23 13:30, Xavier Drudis Ferran wrote: > > 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) > Hi, > > I have nothing against it, the regulator should be all the way optional IMO >
Well, at least if it is always-on for whatever reason, then it is not an error that it cannot be turned off. > > 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; /*****/ > > Pretty weird that a regulator that can be turned on/off via a GPIO is > 'regulator-always-on'. I find this odd and i think it's not correctly > described at DT level. > I don't know enough to tell. I've just looked a little and it seems to be used for USB only (on rock pi 4, firefly, eaidk-610, khadas-edge, leez-p710, nanopc-t4, orangepi, puma, rock960, rockpro64) Curiously rk3399-evb does NOT have regulator-always-on in vcc5v0_host and roc-pc seems to add it in u-boot.dtsi only, since it was preserved at some u-boot - linux sync. pinebook-pro has regulator-always-on, but then has regulator-state-mem, regulator-off-in-suspend... > > Anyway, maybe we should move on even if we can't disable the regulator in > any case ? We should just dev_err and continue ? > dev_err or not dev_err depends on whether always-on is always a bug there or may be a feature, I don't know. But moving on would be nice, yes. > Kever, do you have any preference ? > > Eugen Thanks