| Message ID | 20201113152856.3758-1-a.fatoum@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] ARM: dts: imx6qdl: specify vcc-supply for NOP USB PHYs | expand |
Hello, did this maybe slip through the cracks? On 13.11.20 16:28, Ahmad Fatoum wrote: > The SoC dtsi lists a NOP USB PHY for each of the two HSIC-only USB > controllers. Their device tree node doesn't indicate a vcc-supply > resulting in: > > usb_phy_generic usbphynop1: supply vcc not found, using dummy regulator > usb_phy_generic usbphynop2: supply vcc not found, using dummy regulator > > warnings on boot up. The USB IP vcc-supply - separate from the vusb - is > hardwired to LDO_2P5[1], which we already have a device tree node for. > Reference it for the dummy "phy" as well. > > This will lead to breakage (probe deferment) for kernels that: > - Use a HSIC USB controller > - Use this new device tree > - but have CONFIG_REGULATOR_ANATOP disabled > > Because while the regulator is always-on, it can't be resolved when > there is no driver for it. > > As there are > > - no affected upstream device trees > - existing device trees are unaffected without recompilation > - disabling CONFIG_REGULATOR_ANATOP is explicitly a non-recommended > configuration per symbol help text > > this potential breakage is deemed acceptable. > > [1]: i.MX 6Dual/6Quad Reference Manual, Rev. C, > Figure 53-1. Power System Overview > > Cc: Frieder Schrempf <frieder.schrempf@kontron.de> > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> > --- > arch/arm/boot/dts/imx6qdl.dtsi | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi > index 43edbf1156c7..22e4c142de13 100644 > --- a/arch/arm/boot/dts/imx6qdl.dtsi > +++ b/arch/arm/boot/dts/imx6qdl.dtsi > @@ -131,11 +131,13 @@ pmu: pmu { > > usbphynop1: usbphynop1 { > compatible = "usb-nop-xceiv"; > + vcc-supply = <®_vdd2p5>; > #phy-cells = <0>; > }; > > usbphynop2: usbphynop2 { > compatible = "usb-nop-xceiv"; > + vcc-supply = <®_vdd2p5>; > #phy-cells = <0>; > }; > >
On Fri, Nov 13, 2020 at 04:28:55PM +0100, Ahmad Fatoum wrote: > The SoC dtsi lists a NOP USB PHY for each of the two HSIC-only USB > controllers. Their device tree node doesn't indicate a vcc-supply > resulting in: > > usb_phy_generic usbphynop1: supply vcc not found, using dummy regulator > usb_phy_generic usbphynop2: supply vcc not found, using dummy regulator So the patch is all for removing this kernel message, with introducing the following potential breakage? Shawn > > warnings on boot up. The USB IP vcc-supply - separate from the vusb - is > hardwired to LDO_2P5[1], which we already have a device tree node for. > Reference it for the dummy "phy" as well. > > This will lead to breakage (probe deferment) for kernels that: > - Use a HSIC USB controller > - Use this new device tree > - but have CONFIG_REGULATOR_ANATOP disabled > > Because while the regulator is always-on, it can't be resolved when > there is no driver for it. > > As there are > > - no affected upstream device trees > - existing device trees are unaffected without recompilation > - disabling CONFIG_REGULATOR_ANATOP is explicitly a non-recommended > configuration per symbol help text > > this potential breakage is deemed acceptable. > > [1]: i.MX 6Dual/6Quad Reference Manual, Rev. C, > Figure 53-1. Power System Overview > > Cc: Frieder Schrempf <frieder.schrempf@kontron.de> > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> > --- > arch/arm/boot/dts/imx6qdl.dtsi | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi > index 43edbf1156c7..22e4c142de13 100644 > --- a/arch/arm/boot/dts/imx6qdl.dtsi > +++ b/arch/arm/boot/dts/imx6qdl.dtsi > @@ -131,11 +131,13 @@ pmu: pmu { > > usbphynop1: usbphynop1 { > compatible = "usb-nop-xceiv"; > + vcc-supply = <®_vdd2p5>; > #phy-cells = <0>; > }; > > usbphynop2: usbphynop2 { > compatible = "usb-nop-xceiv"; > + vcc-supply = <®_vdd2p5>; > #phy-cells = <0>; > }; > > -- > 2.28.0 >
Hello Shawn, On 19.01.21 09:57, Shawn Guo wrote: > On Fri, Nov 13, 2020 at 04:28:55PM +0100, Ahmad Fatoum wrote: >> The SoC dtsi lists a NOP USB PHY for each of the two HSIC-only USB >> controllers. Their device tree node doesn't indicate a vcc-supply >> resulting in: >> >> usb_phy_generic usbphynop1: supply vcc not found, using dummy regulator >> usb_phy_generic usbphynop2: supply vcc not found, using dummy regulator > > So the patch is all for removing this kernel message, with introducing > the following potential breakage? Pretty much. Either: - We never fix the warning - We add an explicit dummy fixed-regulator, which doesn't reflect the HW - We reference the correct regulator and have the low risk of a non-upstream device tree breaking when they update both kernel, device tree and have an non-recommended CONFIG_REGULATOR_ANATOP=n What do you think? Cheers, Ahmad > > Shawn > >> >> warnings on boot up. The USB IP vcc-supply - separate from the vusb - is >> hardwired to LDO_2P5[1], which we already have a device tree node for. >> Reference it for the dummy "phy" as well. >> >> This will lead to breakage (probe deferment) for kernels that: >> - Use a HSIC USB controller >> - Use this new device tree >> - but have CONFIG_REGULATOR_ANATOP disabled >> >> Because while the regulator is always-on, it can't be resolved when >> there is no driver for it. >> >> As there are >> >> - no affected upstream device trees >> - existing device trees are unaffected without recompilation >> - disabling CONFIG_REGULATOR_ANATOP is explicitly a non-recommended >> configuration per symbol help text >> >> this potential breakage is deemed acceptable. >> >> [1]: i.MX 6Dual/6Quad Reference Manual, Rev. C, >> Figure 53-1. Power System Overview >> >> Cc: Frieder Schrempf <frieder.schrempf@kontron.de> >> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> >> --- >> arch/arm/boot/dts/imx6qdl.dtsi | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi >> index 43edbf1156c7..22e4c142de13 100644 >> --- a/arch/arm/boot/dts/imx6qdl.dtsi >> +++ b/arch/arm/boot/dts/imx6qdl.dtsi >> @@ -131,11 +131,13 @@ pmu: pmu { >> >> usbphynop1: usbphynop1 { >> compatible = "usb-nop-xceiv"; >> + vcc-supply = <®_vdd2p5>; >> #phy-cells = <0>; >> }; >> >> usbphynop2: usbphynop2 { >> compatible = "usb-nop-xceiv"; >> + vcc-supply = <®_vdd2p5>; >> #phy-cells = <0>; >> }; >> >> -- >> 2.28.0 >> >
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi index 43edbf1156c7..22e4c142de13 100644 --- a/arch/arm/boot/dts/imx6qdl.dtsi +++ b/arch/arm/boot/dts/imx6qdl.dtsi @@ -131,11 +131,13 @@ pmu: pmu { usbphynop1: usbphynop1 { compatible = "usb-nop-xceiv"; + vcc-supply = <®_vdd2p5>; #phy-cells = <0>; }; usbphynop2: usbphynop2 { compatible = "usb-nop-xceiv"; + vcc-supply = <®_vdd2p5>; #phy-cells = <0>; };
The SoC dtsi lists a NOP USB PHY for each of the two HSIC-only USB controllers. Their device tree node doesn't indicate a vcc-supply resulting in: usb_phy_generic usbphynop1: supply vcc not found, using dummy regulator usb_phy_generic usbphynop2: supply vcc not found, using dummy regulator warnings on boot up. The USB IP vcc-supply - separate from the vusb - is hardwired to LDO_2P5[1], which we already have a device tree node for. Reference it for the dummy "phy" as well. This will lead to breakage (probe deferment) for kernels that: - Use a HSIC USB controller - Use this new device tree - but have CONFIG_REGULATOR_ANATOP disabled Because while the regulator is always-on, it can't be resolved when there is no driver for it. As there are - no affected upstream device trees - existing device trees are unaffected without recompilation - disabling CONFIG_REGULATOR_ANATOP is explicitly a non-recommended configuration per symbol help text this potential breakage is deemed acceptable. [1]: i.MX 6Dual/6Quad Reference Manual, Rev. C, Figure 53-1. Power System Overview Cc: Frieder Schrempf <frieder.schrempf@kontron.de> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> --- arch/arm/boot/dts/imx6qdl.dtsi | 2 ++ 1 file changed, 2 insertions(+)