| Message ID | 20230104205434.867219-1-lorenz@brun.one (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: dts: rockchip: fix USB regulator on ROCK64 | expand |
On Wed, Jan 4, 2023 at 3:55 PM Lorenz Brun <lorenz@brun.one> wrote: > > Currently the ROCK64 device tree specifies two regulators, vcc_host_5v > and vcc_host1_5v for USB VBUS on the device. Both of those are however > specified with RK_PA2 as the GPIO enabling them, causing the following > error when booting: > > rockchip-pinctrl pinctrl: pin gpio0-2 already requested by vcc-host-5v-regulator; cannot claim for vcc-host1-5v-regulator > rockchip-pinctrl pinctrl: pin-2 (vcc-host1-5v-regulator) status -22 > rockchip-pinctrl pinctrl: could not request pin 2 (gpio0-2) from group usb20-host-drv on device rockchip-pinctrl > reg-fixed-voltage vcc-host1-5v-regulator: Error applying setting, reverse things back > > Looking at the schematic, there are in fact three USB regulators, > vcc_host_5v, vcc_host1_5v and vcc_otg_v5. But the enable signal for all > three is driven by Q2604 which is in turn driven by GPIO_A2/PA2. > > Since these three regulators are not controllable separately, I removed > the second one which was causing the error and left a comment explaining > that this regulator actually controls multiple rails. > > Signed-off-by: Lorenz Brun <lorenz@brun.one> > --- > arch/arm64/boot/dts/rockchip/rk3328-rock64.dts | 14 +++----------- > 1 file changed, 3 insertions(+), 11 deletions(-) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > index f69a38f42d2d..bd82bc80444d 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > +++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > @@ -37,6 +37,9 @@ vcc_sd: sdmmc-regulator { > vin-supply = <&vcc_io>; > }; > > + // vcc_host_5v also controls the vcc_host1_5v and vcc_otg_5v rails > + // but there is only one common control signal (USB20_HOST_DRV) at > + // GPIO_A2 > vcc_host_5v: vcc-host-5v-regulator { > compatible = "regulator-fixed"; > gpio = <&gpio0 RK_PA2 GPIO_ACTIVE_LOW>; > @@ -48,17 +51,6 @@ vcc_host_5v: vcc-host-5v-regulator { > vin-supply = <&vcc_sys>; > }; > > - vcc_host1_5v: vcc_otg_5v: vcc-host1-5v-regulator { > - compatible = "regulator-fixed"; > - gpio = <&gpio0 RK_PA2 GPIO_ACTIVE_LOW>; > - pinctrl-names = "default"; > - pinctrl-0 = <&usb20_host_drv>; > - regulator-name = "vcc_host1_5v"; > - regulator-always-on; > - regulator-boot-on; > - vin-supply = <&vcc_sys>; > - }; Fixed-regulator supports multiple regulators sharing a gpio, the issue is you have the pinctrl assigned multiple times which is not supported. Simply removing the pinctrl from all but one of the regulators will solve this issue. > - > vcc_sys: vcc-sys { > compatible = "regulator-fixed"; > regulator-name = "vcc_sys"; > -- > 2.38.1 > > > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip
Am Donnerstag, 5. Januar 2023, 00:46:25 CET schrieb Peter Geis: > On Wed, Jan 4, 2023 at 3:55 PM Lorenz Brun <lorenz@brun.one> wrote: > > > > Currently the ROCK64 device tree specifies two regulators, vcc_host_5v > > and vcc_host1_5v for USB VBUS on the device. Both of those are however > > specified with RK_PA2 as the GPIO enabling them, causing the following > > error when booting: > > > > rockchip-pinctrl pinctrl: pin gpio0-2 already requested by vcc-host-5v-regulator; cannot claim for vcc-host1-5v-regulator > > rockchip-pinctrl pinctrl: pin-2 (vcc-host1-5v-regulator) status -22 > > rockchip-pinctrl pinctrl: could not request pin 2 (gpio0-2) from group usb20-host-drv on device rockchip-pinctrl > > reg-fixed-voltage vcc-host1-5v-regulator: Error applying setting, reverse things back > > > > Looking at the schematic, there are in fact three USB regulators, > > vcc_host_5v, vcc_host1_5v and vcc_otg_v5. But the enable signal for all > > three is driven by Q2604 which is in turn driven by GPIO_A2/PA2. > > > > Since these three regulators are not controllable separately, I removed > > the second one which was causing the error and left a comment explaining > > that this regulator actually controls multiple rails. > > > > Signed-off-by: Lorenz Brun <lorenz@brun.one> > > --- > > arch/arm64/boot/dts/rockchip/rk3328-rock64.dts | 14 +++----------- > > 1 file changed, 3 insertions(+), 11 deletions(-) > > > > diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > > index f69a38f42d2d..bd82bc80444d 100644 > > --- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > > +++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > > @@ -37,6 +37,9 @@ vcc_sd: sdmmc-regulator { > > vin-supply = <&vcc_io>; > > }; > > > > + // vcc_host_5v also controls the vcc_host1_5v and vcc_otg_5v rails > > + // but there is only one common control signal (USB20_HOST_DRV) at > > + // GPIO_A2 > > vcc_host_5v: vcc-host-5v-regulator { > > compatible = "regulator-fixed"; > > gpio = <&gpio0 RK_PA2 GPIO_ACTIVE_LOW>; > > @@ -48,17 +51,6 @@ vcc_host_5v: vcc-host-5v-regulator { > > vin-supply = <&vcc_sys>; > > }; > > > > - vcc_host1_5v: vcc_otg_5v: vcc-host1-5v-regulator { > > - compatible = "regulator-fixed"; > > - gpio = <&gpio0 RK_PA2 GPIO_ACTIVE_LOW>; > > - pinctrl-names = "default"; > > - pinctrl-0 = <&usb20_host_drv>; > > - regulator-name = "vcc_host1_5v"; > > - regulator-always-on; > > - regulator-boot-on; > > - vin-supply = <&vcc_sys>; > > - }; > > Fixed-regulator supports multiple regulators sharing a gpio, the issue > is you have the pinctrl assigned multiple times which is not > supported. Simply removing the pinctrl from all but one of the > regulators will solve this issue. Hmm, but where is the advantage in that? I.e. fully representing the hardware would mean not only sharing the gpio but also the pinctrl on all fixed regulators. And with only one of them having the pinctrl, are we sure the regulator probed first will always be the same, or does the pinctrl setting may only come into effect once the 2nd or 3rd one probes?
On Wed, Jan 4 2023 at 18:46:25 -05:00:00, Peter Geis <pgwipeout@gmail.com> wrote: > On Wed, Jan 4, 2023 at 3:55 PM Lorenz Brun <lorenz@brun.one> wrote: >> >> Currently the ROCK64 device tree specifies two regulators, >> vcc_host_5v >> and vcc_host1_5v for USB VBUS on the device. Both of those are >> however >> specified with RK_PA2 as the GPIO enabling them, causing the >> following >> error when booting: >> >> rockchip-pinctrl pinctrl: pin gpio0-2 already requested by >> vcc-host-5v-regulator; cannot claim for vcc-host1-5v-regulator >> rockchip-pinctrl pinctrl: pin-2 (vcc-host1-5v-regulator) status >> -22 >> rockchip-pinctrl pinctrl: could not request pin 2 (gpio0-2) from >> group usb20-host-drv on device rockchip-pinctrl >> reg-fixed-voltage vcc-host1-5v-regulator: Error applying setting, >> reverse things back >> >> Looking at the schematic, there are in fact three USB regulators, >> vcc_host_5v, vcc_host1_5v and vcc_otg_v5. But the enable signal for >> all >> three is driven by Q2604 which is in turn driven by GPIO_A2/PA2. >> >> Since these three regulators are not controllable separately, I >> removed >> the second one which was causing the error and left a comment >> explaining >> that this regulator actually controls multiple rails. >> >> Signed-off-by: Lorenz Brun <lorenz@brun.one> >> --- >> arch/arm64/boot/dts/rockchip/rk3328-rock64.dts | 14 +++----------- >> 1 file changed, 3 insertions(+), 11 deletions(-) >> >> diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts >> b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts >> index f69a38f42d2d..bd82bc80444d 100644 >> --- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts >> +++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts >> @@ -37,6 +37,9 @@ vcc_sd: sdmmc-regulator { >> vin-supply = <&vcc_io>; >> }; >> >> + // vcc_host_5v also controls the vcc_host1_5v and >> vcc_otg_5v rails >> + // but there is only one common control signal >> (USB20_HOST_DRV) at >> + // GPIO_A2 >> vcc_host_5v: vcc-host-5v-regulator { >> compatible = "regulator-fixed"; >> gpio = <&gpio0 RK_PA2 GPIO_ACTIVE_LOW>; >> @@ -48,17 +51,6 @@ vcc_host_5v: vcc-host-5v-regulator { >> vin-supply = <&vcc_sys>; >> }; >> >> - vcc_host1_5v: vcc_otg_5v: vcc-host1-5v-regulator { >> - compatible = "regulator-fixed"; >> - gpio = <&gpio0 RK_PA2 GPIO_ACTIVE_LOW>; >> - pinctrl-names = "default"; >> - pinctrl-0 = <&usb20_host_drv>; >> - regulator-name = "vcc_host1_5v"; >> - regulator-always-on; >> - regulator-boot-on; >> - vin-supply = <&vcc_sys>; >> - }; > > Fixed-regulator supports multiple regulators sharing a gpio, the issue > is you have the pinctrl assigned multiple times which is not > supported. Simply removing the pinctrl from all but one of the > regulators will solve this issue. Sure, I can just remove the pinctrl. Should I do anything about the fact that there are three USB switches on that GPIO, but only two of them are described as regulators here? Seems a bit inconsistent to me. > >> - >> vcc_sys: vcc-sys { >> compatible = "regulator-fixed"; >> regulator-name = "vcc_sys"; >> -- >> 2.38.1 >> >> >> _______________________________________________ >> Linux-rockchip mailing list >> Linux-rockchip@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-rockchip
On Wed, Jan 4, 2023 at 6:52 PM Heiko Stübner <heiko@sntech.de> wrote: > > Am Donnerstag, 5. Januar 2023, 00:46:25 CET schrieb Peter Geis: > > On Wed, Jan 4, 2023 at 3:55 PM Lorenz Brun <lorenz@brun.one> wrote: > > > > > > Currently the ROCK64 device tree specifies two regulators, vcc_host_5v > > > and vcc_host1_5v for USB VBUS on the device. Both of those are however > > > specified with RK_PA2 as the GPIO enabling them, causing the following > > > error when booting: > > > > > > rockchip-pinctrl pinctrl: pin gpio0-2 already requested by vcc-host-5v-regulator; cannot claim for vcc-host1-5v-regulator > > > rockchip-pinctrl pinctrl: pin-2 (vcc-host1-5v-regulator) status -22 > > > rockchip-pinctrl pinctrl: could not request pin 2 (gpio0-2) from group usb20-host-drv on device rockchip-pinctrl > > > reg-fixed-voltage vcc-host1-5v-regulator: Error applying setting, reverse things back > > > > > > Looking at the schematic, there are in fact three USB regulators, > > > vcc_host_5v, vcc_host1_5v and vcc_otg_v5. But the enable signal for all > > > three is driven by Q2604 which is in turn driven by GPIO_A2/PA2. > > > > > > Since these three regulators are not controllable separately, I removed > > > the second one which was causing the error and left a comment explaining > > > that this regulator actually controls multiple rails. > > > > > > Signed-off-by: Lorenz Brun <lorenz@brun.one> > > > --- > > > arch/arm64/boot/dts/rockchip/rk3328-rock64.dts | 14 +++----------- > > > 1 file changed, 3 insertions(+), 11 deletions(-) > > > > > > diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > > > index f69a38f42d2d..bd82bc80444d 100644 > > > --- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > > > +++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > > > @@ -37,6 +37,9 @@ vcc_sd: sdmmc-regulator { > > > vin-supply = <&vcc_io>; > > > }; > > > > > > + // vcc_host_5v also controls the vcc_host1_5v and vcc_otg_5v rails > > > + // but there is only one common control signal (USB20_HOST_DRV) at > > > + // GPIO_A2 > > > vcc_host_5v: vcc-host-5v-regulator { > > > compatible = "regulator-fixed"; > > > gpio = <&gpio0 RK_PA2 GPIO_ACTIVE_LOW>; > > > @@ -48,17 +51,6 @@ vcc_host_5v: vcc-host-5v-regulator { > > > vin-supply = <&vcc_sys>; > > > }; > > > > > > - vcc_host1_5v: vcc_otg_5v: vcc-host1-5v-regulator { > > > - compatible = "regulator-fixed"; > > > - gpio = <&gpio0 RK_PA2 GPIO_ACTIVE_LOW>; > > > - pinctrl-names = "default"; > > > - pinctrl-0 = <&usb20_host_drv>; > > > - regulator-name = "vcc_host1_5v"; > > > - regulator-always-on; > > > - regulator-boot-on; > > > - vin-supply = <&vcc_sys>; > > > - }; > > > > Fixed-regulator supports multiple regulators sharing a gpio, the issue > > is you have the pinctrl assigned multiple times which is not > > supported. Simply removing the pinctrl from all but one of the > > regulators will solve this issue. > > Hmm, but where is the advantage in that? > I.e. fully representing the hardware would mean not only sharing the gpio > but also the pinctrl on all fixed regulators. > > And with only one of them having the pinctrl, are we sure the regulator > probed first will always be the same, or does the pinctrl setting may only > come into effect once the 2nd or 3rd one probes? It's a current limitation of the pinctrl driver unfortunately. On the off chance the regulator that owns the pinctrl doesn't probe it never gets applied, so it's important to assign it carefully. For instance the best scenario is assigning it to a fixed-regulator that is only fed by other fixed-regulators. That way a dependency issue doesn't cause the pinctrl to not assign. > >
On Wed, Jan 4, 2023 at 6:55 PM Lorenz Brun <lorenz@brun.one> wrote: > > > On Wed, Jan 4 2023 at 18:46:25 -05:00:00, Peter Geis > <pgwipeout@gmail.com> wrote: > > On Wed, Jan 4, 2023 at 3:55 PM Lorenz Brun <lorenz@brun.one> wrote: > >> > >> Currently the ROCK64 device tree specifies two regulators, > >> vcc_host_5v > >> and vcc_host1_5v for USB VBUS on the device. Both of those are > >> however > >> specified with RK_PA2 as the GPIO enabling them, causing the > >> following > >> error when booting: > >> > >> rockchip-pinctrl pinctrl: pin gpio0-2 already requested by > >> vcc-host-5v-regulator; cannot claim for vcc-host1-5v-regulator > >> rockchip-pinctrl pinctrl: pin-2 (vcc-host1-5v-regulator) status > >> -22 > >> rockchip-pinctrl pinctrl: could not request pin 2 (gpio0-2) from > >> group usb20-host-drv on device rockchip-pinctrl > >> reg-fixed-voltage vcc-host1-5v-regulator: Error applying setting, > >> reverse things back > >> > >> Looking at the schematic, there are in fact three USB regulators, > >> vcc_host_5v, vcc_host1_5v and vcc_otg_v5. But the enable signal for > >> all > >> three is driven by Q2604 which is in turn driven by GPIO_A2/PA2. > >> > >> Since these three regulators are not controllable separately, I > >> removed > >> the second one which was causing the error and left a comment > >> explaining > >> that this regulator actually controls multiple rails. > >> > >> Signed-off-by: Lorenz Brun <lorenz@brun.one> > >> --- > >> arch/arm64/boot/dts/rockchip/rk3328-rock64.dts | 14 +++----------- > >> 1 file changed, 3 insertions(+), 11 deletions(-) > >> > >> diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > >> b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > >> index f69a38f42d2d..bd82bc80444d 100644 > >> --- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > >> +++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > >> @@ -37,6 +37,9 @@ vcc_sd: sdmmc-regulator { > >> vin-supply = <&vcc_io>; > >> }; > >> > >> + // vcc_host_5v also controls the vcc_host1_5v and > >> vcc_otg_5v rails > >> + // but there is only one common control signal > >> (USB20_HOST_DRV) at > >> + // GPIO_A2 > >> vcc_host_5v: vcc-host-5v-regulator { > >> compatible = "regulator-fixed"; > >> gpio = <&gpio0 RK_PA2 GPIO_ACTIVE_LOW>; > >> @@ -48,17 +51,6 @@ vcc_host_5v: vcc-host-5v-regulator { > >> vin-supply = <&vcc_sys>; > >> }; > >> > >> - vcc_host1_5v: vcc_otg_5v: vcc-host1-5v-regulator { > >> - compatible = "regulator-fixed"; > >> - gpio = <&gpio0 RK_PA2 GPIO_ACTIVE_LOW>; > >> - pinctrl-names = "default"; > >> - pinctrl-0 = <&usb20_host_drv>; > >> - regulator-name = "vcc_host1_5v"; > >> - regulator-always-on; > >> - regulator-boot-on; > >> - vin-supply = <&vcc_sys>; > >> - }; > > > > Fixed-regulator supports multiple regulators sharing a gpio, the issue > > is you have the pinctrl assigned multiple times which is not > > supported. Simply removing the pinctrl from all but one of the > > regulators will solve this issue. > Sure, I can just remove the pinctrl. Should I do anything about the > fact that there are three USB switches on that GPIO, but only two of > them are described as regulators here? Seems a bit inconsistent to me. All hardware *should* be described, though it isn't uncommon to see a single fixed-regulator describe several individual switches that are all fed from a common source and controlled by the same gpio. If they are not fed by a common source (for example, the otg port is often fed from a separate regulator), they must be modeled separately. > > > > >> - > >> vcc_sys: vcc-sys { > >> compatible = "regulator-fixed"; > >> regulator-name = "vcc_sys"; > >> -- > >> 2.38.1 > >> > >> > >> _______________________________________________ > >> Linux-rockchip mailing list > >> Linux-rockchip@lists.infradead.org > >> http://lists.infradead.org/mailman/listinfo/linux-rockchip > >
Am Donnerstag, 5. Januar 2023, 01:29:47 CET schrieb Peter Geis: > On Wed, Jan 4, 2023 at 6:55 PM Lorenz Brun <lorenz@brun.one> wrote: > > > > > > On Wed, Jan 4 2023 at 18:46:25 -05:00:00, Peter Geis > > <pgwipeout@gmail.com> wrote: > > > On Wed, Jan 4, 2023 at 3:55 PM Lorenz Brun <lorenz@brun.one> wrote: > > >> > > >> Currently the ROCK64 device tree specifies two regulators, > > >> vcc_host_5v > > >> and vcc_host1_5v for USB VBUS on the device. Both of those are > > >> however > > >> specified with RK_PA2 as the GPIO enabling them, causing the > > >> following > > >> error when booting: > > >> > > >> rockchip-pinctrl pinctrl: pin gpio0-2 already requested by > > >> vcc-host-5v-regulator; cannot claim for vcc-host1-5v-regulator > > >> rockchip-pinctrl pinctrl: pin-2 (vcc-host1-5v-regulator) status > > >> -22 > > >> rockchip-pinctrl pinctrl: could not request pin 2 (gpio0-2) from > > >> group usb20-host-drv on device rockchip-pinctrl > > >> reg-fixed-voltage vcc-host1-5v-regulator: Error applying setting, > > >> reverse things back > > >> > > >> Looking at the schematic, there are in fact three USB regulators, > > >> vcc_host_5v, vcc_host1_5v and vcc_otg_v5. But the enable signal for > > >> all > > >> three is driven by Q2604 which is in turn driven by GPIO_A2/PA2. > > >> > > >> Since these three regulators are not controllable separately, I > > >> removed > > >> the second one which was causing the error and left a comment > > >> explaining > > >> that this regulator actually controls multiple rails. > > >> > > >> Signed-off-by: Lorenz Brun <lorenz@brun.one> > > >> --- > > >> arch/arm64/boot/dts/rockchip/rk3328-rock64.dts | 14 +++----------- > > >> 1 file changed, 3 insertions(+), 11 deletions(-) > > >> > > >> diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > > >> b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > > >> index f69a38f42d2d..bd82bc80444d 100644 > > >> --- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > > >> +++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > > >> @@ -37,6 +37,9 @@ vcc_sd: sdmmc-regulator { > > >> vin-supply = <&vcc_io>; > > >> }; > > >> > > >> + // vcc_host_5v also controls the vcc_host1_5v and > > >> vcc_otg_5v rails > > >> + // but there is only one common control signal > > >> (USB20_HOST_DRV) at > > >> + // GPIO_A2 > > >> vcc_host_5v: vcc-host-5v-regulator { > > >> compatible = "regulator-fixed"; > > >> gpio = <&gpio0 RK_PA2 GPIO_ACTIVE_LOW>; > > >> @@ -48,17 +51,6 @@ vcc_host_5v: vcc-host-5v-regulator { > > >> vin-supply = <&vcc_sys>; > > >> }; > > >> > > >> - vcc_host1_5v: vcc_otg_5v: vcc-host1-5v-regulator { > > >> - compatible = "regulator-fixed"; > > >> - gpio = <&gpio0 RK_PA2 GPIO_ACTIVE_LOW>; > > >> - pinctrl-names = "default"; > > >> - pinctrl-0 = <&usb20_host_drv>; > > >> - regulator-name = "vcc_host1_5v"; > > >> - regulator-always-on; > > >> - regulator-boot-on; > > >> - vin-supply = <&vcc_sys>; > > >> - }; > > > > > > Fixed-regulator supports multiple regulators sharing a gpio, the issue > > > is you have the pinctrl assigned multiple times which is not > > > supported. Simply removing the pinctrl from all but one of the > > > regulators will solve this issue. > > Sure, I can just remove the pinctrl. Should I do anything about the > > fact that there are three USB switches on that GPIO, but only two of > > them are described as regulators here? Seems a bit inconsistent to me. > > All hardware *should* be described, though it isn't uncommon to see a > single fixed-regulator describe several individual switches that are > all fed from a common source and controlled by the same gpio. If they > are not fed by a common source (for example, the otg port is often fed > from a separate regulator), they must be modeled separately. Which is essentially what Lorenz' patch already did - moving the 3 switches into one regulator. So it essentially comes down to where does the not-yet modeled otg-regulator get its current from - also vcc_sys?
On Tue, Jan 10 2023 at 14:02:47 +01:00:00, Heiko Stübner <heiko@sntech.de> wrote: > Am Donnerstag, 5. Januar 2023, 01:29:47 CET schrieb Peter Geis: >> On Wed, Jan 4, 2023 at 6:55 PM Lorenz Brun <lorenz@brun.one> wrote: >> > >> > >> > On Wed, Jan 4 2023 at 18:46:25 -05:00:00, Peter Geis >> > <pgwipeout@gmail.com> wrote: >> > > On Wed, Jan 4, 2023 at 3:55 PM Lorenz Brun <lorenz@brun.one> >> wrote: >> > >> >> > >> Currently the ROCK64 device tree specifies two regulators, >> > >> vcc_host_5v >> > >> and vcc_host1_5v for USB VBUS on the device. Both of those are >> > >> however >> > >> specified with RK_PA2 as the GPIO enabling them, causing the >> > >> following >> > >> error when booting: >> > >> >> > >> rockchip-pinctrl pinctrl: pin gpio0-2 already requested by >> > >> vcc-host-5v-regulator; cannot claim for vcc-host1-5v-regulator >> > >> rockchip-pinctrl pinctrl: pin-2 (vcc-host1-5v-regulator) >> status >> > >> -22 >> > >> rockchip-pinctrl pinctrl: could not request pin 2 (gpio0-2) >> from >> > >> group usb20-host-drv on device rockchip-pinctrl >> > >> reg-fixed-voltage vcc-host1-5v-regulator: Error applying >> setting, >> > >> reverse things back >> > >> >> > >> Looking at the schematic, there are in fact three USB >> regulators, >> > >> vcc_host_5v, vcc_host1_5v and vcc_otg_v5. But the enable >> signal for >> > >> all >> > >> three is driven by Q2604 which is in turn driven by >> GPIO_A2/PA2. >> > >> >> > >> Since these three regulators are not controllable separately, >> I >> > >> removed >> > >> the second one which was causing the error and left a comment >> > >> explaining >> > >> that this regulator actually controls multiple rails. >> > >> >> > >> Signed-off-by: Lorenz Brun <lorenz@brun.one> >> > >> --- >> > >> arch/arm64/boot/dts/rockchip/rk3328-rock64.dts | 14 >> +++----------- >> > >> 1 file changed, 3 insertions(+), 11 deletions(-) >> > >> >> > >> diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts >> > >> b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts >> > >> index f69a38f42d2d..bd82bc80444d 100644 >> > >> --- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts >> > >> +++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts >> > >> @@ -37,6 +37,9 @@ vcc_sd: sdmmc-regulator { >> > >> vin-supply = <&vcc_io>; >> > >> }; >> > >> >> > >> + // vcc_host_5v also controls the vcc_host1_5v and >> > >> vcc_otg_5v rails >> > >> + // but there is only one common control signal >> > >> (USB20_HOST_DRV) at >> > >> + // GPIO_A2 >> > >> vcc_host_5v: vcc-host-5v-regulator { >> > >> compatible = "regulator-fixed"; >> > >> gpio = <&gpio0 RK_PA2 GPIO_ACTIVE_LOW>; >> > >> @@ -48,17 +51,6 @@ vcc_host_5v: vcc-host-5v-regulator { >> > >> vin-supply = <&vcc_sys>; >> > >> }; >> > >> >> > >> - vcc_host1_5v: vcc_otg_5v: vcc-host1-5v-regulator { >> > >> - compatible = "regulator-fixed"; >> > >> - gpio = <&gpio0 RK_PA2 GPIO_ACTIVE_LOW>; >> > >> - pinctrl-names = "default"; >> > >> - pinctrl-0 = <&usb20_host_drv>; >> > >> - regulator-name = "vcc_host1_5v"; >> > >> - regulator-always-on; >> > >> - regulator-boot-on; >> > >> - vin-supply = <&vcc_sys>; >> > >> - }; >> > > >> > > Fixed-regulator supports multiple regulators sharing a gpio, >> the issue >> > > is you have the pinctrl assigned multiple times which is not >> > > supported. Simply removing the pinctrl from all but one of the >> > > regulators will solve this issue. >> > Sure, I can just remove the pinctrl. Should I do anything about >> the >> > fact that there are three USB switches on that GPIO, but only two >> of >> > them are described as regulators here? Seems a bit inconsistent >> to me. >> >> All hardware *should* be described, though it isn't uncommon to see >> a >> single fixed-regulator describe several individual switches that are >> all fed from a common source and controlled by the same gpio. If >> they >> are not fed by a common source (for example, the otg port is often >> fed >> from a separate regulator), they must be modeled separately. > > Which is essentially what Lorenz' patch already did - moving the 3 > switches > into one regulator. So it essentially comes down to where does the > not-yet > modeled otg-regulator get its current from - also vcc_sys? > Yeah, all regulators (they are just USB load/overload switches) are powered from VCC_SYS, there aren't that many power rails on the ROCK64. The alternative to my first patch would essentially be this: --- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts +++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts @@ -48,11 +48,18 @@ vcc_host_5v: vcc-host-5v-regulator { vin-supply = <&vcc_sys>; }; - vcc_host1_5v: vcc_otg_5v: vcc-host1-5v-regulator { + vcc_host1_5v: vcc-host1-5v-regulator { + compatible = "regulator-fixed"; + gpio = <&gpio0 RK_PA2 GPIO_ACTIVE_LOW>; + regulator-name = "vcc_host1_5v"; + regulator-always-on; + regulator-boot-on; + vin-supply = <&vcc_sys>; + }; + + vcc_otg_5v: vcc-otg-5v-regulator { compatible = "regulator-fixed"; gpio = <&gpio0 RK_PA2 GPIO_ACTIVE_LOW>; - pinctrl-names = "default"; - pinctrl-0 = <&usb20_host_drv>; regulator-name = "vcc_host1_5v"; regulator-always-on; regulator-boot-on; This drops the pinctrl from all but the first regulator (which is a bit weird, the first one is not really special) and then describes three separate regulators driven by PA2. I have no real preference, the one-regulator solution has the downside of not accurately describing the three separate load switches, the three-regulator solution has the downside of the pinctrl definitions being weird. Regards, Lorenz
Am Di, 10. Jan 2023 um 22:38:26 +01:00:00 schrieb Lorenz Brun <lorenz@brun.one>: > On Tue, Jan 10 2023 at 14:02:47 +01:00:00, Heiko Stübner > <heiko@sntech.de> wrote: >> Am Donnerstag, 5. Januar 2023, 01:29:47 CET schrieb Peter Geis: >>> On Wed, Jan 4, 2023 at 6:55 PM Lorenz Brun <lorenz@brun.one> wrote: >>> > >>> > >>> > On Wed, Jan 4 2023 at 18:46:25 -05:00:00, Peter Geis >>> > <pgwipeout@gmail.com> wrote: >>> > > On Wed, Jan 4, 2023 at 3:55 PM Lorenz Brun <lorenz@brun.one> >>> wrote: >>> > >> >>> > >> Currently the ROCK64 device tree specifies two regulators, >>> > >> vcc_host_5v >>> > >> and vcc_host1_5v for USB VBUS on the device. Both of those >>> are >>> > >> however >>> > >> specified with RK_PA2 as the GPIO enabling them, causing the >>> > >> following >>> > >> error when booting: >>> > >> >>> > >> rockchip-pinctrl pinctrl: pin gpio0-2 already requested by >>> > >> vcc-host-5v-regulator; cannot claim for vcc-host1-5v-regulator >>> > >> rockchip-pinctrl pinctrl: pin-2 (vcc-host1-5v-regulator) >>> status >>> > >> -22 >>> > >> rockchip-pinctrl pinctrl: could not request pin 2 >>> (gpio0-2) from >>> > >> group usb20-host-drv on device rockchip-pinctrl >>> > >> reg-fixed-voltage vcc-host1-5v-regulator: Error applying >>> setting, >>> > >> reverse things back >>> > >> >>> > >> Looking at the schematic, there are in fact three USB >>> regulators, >>> > >> vcc_host_5v, vcc_host1_5v and vcc_otg_v5. But the enable >>> signal for >>> > >> all >>> > >> three is driven by Q2604 which is in turn driven by >>> GPIO_A2/PA2. >>> > >> >>> > >> Since these three regulators are not controllable >>> separately, I >>> > >> removed >>> > >> the second one which was causing the error and left a comment >>> > >> explaining >>> > >> that this regulator actually controls multiple rails. >>> > >> >>> > >> Signed-off-by: Lorenz Brun <lorenz@brun.one> >>> > >> --- >>> > >> arch/arm64/boot/dts/rockchip/rk3328-rock64.dts | 14 >>> +++----------- >>> > >> 1 file changed, 3 insertions(+), 11 deletions(-) >>> > >> >>> > >> diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts >>> > >> b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts >>> > >> index f69a38f42d2d..bd82bc80444d 100644 >>> > >> --- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts >>> > >> +++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts >>> > >> @@ -37,6 +37,9 @@ vcc_sd: sdmmc-regulator { >>> > >> vin-supply = <&vcc_io>; >>> > >> }; >>> > >> >>> > >> + // vcc_host_5v also controls the vcc_host1_5v and >>> > >> vcc_otg_5v rails >>> > >> + // but there is only one common control signal >>> > >> (USB20_HOST_DRV) at >>> > >> + // GPIO_A2 >>> > >> vcc_host_5v: vcc-host-5v-regulator { >>> > >> compatible = "regulator-fixed"; >>> > >> gpio = <&gpio0 RK_PA2 GPIO_ACTIVE_LOW>; >>> > >> @@ -48,17 +51,6 @@ vcc_host_5v: vcc-host-5v-regulator { >>> > >> vin-supply = <&vcc_sys>; >>> > >> }; >>> > >> >>> > >> - vcc_host1_5v: vcc_otg_5v: vcc-host1-5v-regulator { >>> > >> - compatible = "regulator-fixed"; >>> > >> - gpio = <&gpio0 RK_PA2 GPIO_ACTIVE_LOW>; >>> > >> - pinctrl-names = "default"; >>> > >> - pinctrl-0 = <&usb20_host_drv>; >>> > >> - regulator-name = "vcc_host1_5v"; >>> > >> - regulator-always-on; >>> > >> - regulator-boot-on; >>> > >> - vin-supply = <&vcc_sys>; >>> > >> - }; >>> > > >>> > > Fixed-regulator supports multiple regulators sharing a gpio, >>> the issue >>> > > is you have the pinctrl assigned multiple times which is not >>> > > supported. Simply removing the pinctrl from all but one of the >>> > > regulators will solve this issue. >>> > Sure, I can just remove the pinctrl. Should I do anything about >>> the >>> > fact that there are three USB switches on that GPIO, but only >>> two of >>> > them are described as regulators here? Seems a bit inconsistent >>> to me. >>> >>> All hardware *should* be described, though it isn't uncommon to >>> see a >>> single fixed-regulator describe several individual switches that >>> are >>> all fed from a common source and controlled by the same gpio. If >>> they >>> are not fed by a common source (for example, the otg port is often >>> fed >>> from a separate regulator), they must be modeled separately. >> >> Which is essentially what Lorenz' patch already did - moving the 3 >> switches >> into one regulator. So it essentially comes down to where does the >> not-yet >> modeled otg-regulator get its current from - also vcc_sys? >> > Yeah, all regulators (they are just USB load/overload switches) are > powered from VCC_SYS, there aren't that many power rails on the > ROCK64. > > The alternative to my first patch would essentially be this: > > --- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > +++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > @@ -48,11 +48,18 @@ vcc_host_5v: vcc-host-5v-regulator { > vin-supply = <&vcc_sys>; > }; > > - vcc_host1_5v: vcc_otg_5v: vcc-host1-5v-regulator { > + vcc_host1_5v: vcc-host1-5v-regulator { > + compatible = "regulator-fixed"; > + gpio = <&gpio0 RK_PA2 GPIO_ACTIVE_LOW>; > + regulator-name = "vcc_host1_5v"; > + regulator-always-on; > + regulator-boot-on; > + vin-supply = <&vcc_sys>; > + }; > + > + vcc_otg_5v: vcc-otg-5v-regulator { > compatible = "regulator-fixed"; > gpio = <&gpio0 RK_PA2 GPIO_ACTIVE_LOW>; > - pinctrl-names = "default"; > - pinctrl-0 = <&usb20_host_drv>; > regulator-name = "vcc_host1_5v"; > regulator-always-on; > regulator-boot-on; > > This drops the pinctrl from all but the first regulator (which is a > bit weird, the first one is not really special) and then describes > three separate regulators driven by PA2. > > I have no real preference, the one-regulator solution has the > downside of not accurately describing the three separate load > switches, the three-regulator solution has the downside of the > pinctrl definitions being weird. > > Regards, > Lorenz I never got a response to this, I am fine with both approaches but would prefer if one of them goes in so I no longer have this error on every boot. If the approach above with three regulators is chosen I can send out a proper patch. Regards, Lorenz
Hi, Am Montag, 10. April 2023, 18:01:57 CEST schrieb Lorenz Brun: > > Am Di, 10. Jan 2023 um 22:38:26 +01:00:00 schrieb Lorenz Brun > <lorenz@brun.one>: > > On Tue, Jan 10 2023 at 14:02:47 +01:00:00, Heiko Stübner > > <heiko@sntech.de> wrote: > >> Am Donnerstag, 5. Januar 2023, 01:29:47 CET schrieb Peter Geis: > >>> On Wed, Jan 4, 2023 at 6:55 PM Lorenz Brun <lorenz@brun.one> wrote: > >>> > > >>> > > >>> > On Wed, Jan 4 2023 at 18:46:25 -05:00:00, Peter Geis > >>> > <pgwipeout@gmail.com> wrote: > >>> > > On Wed, Jan 4, 2023 at 3:55 PM Lorenz Brun <lorenz@brun.one> > >>> wrote: > >>> > >> > >>> > >> Currently the ROCK64 device tree specifies two regulators, > >>> > >> vcc_host_5v > >>> > >> and vcc_host1_5v for USB VBUS on the device. Both of those > >>> are > >>> > >> however > >>> > >> specified with RK_PA2 as the GPIO enabling them, causing the > >>> > >> following > >>> > >> error when booting: > >>> > >> > >>> > >> rockchip-pinctrl pinctrl: pin gpio0-2 already requested by > >>> > >> vcc-host-5v-regulator; cannot claim for vcc-host1-5v-regulator > >>> > >> rockchip-pinctrl pinctrl: pin-2 (vcc-host1-5v-regulator) > >>> status > >>> > >> -22 > >>> > >> rockchip-pinctrl pinctrl: could not request pin 2 > >>> (gpio0-2) from > >>> > >> group usb20-host-drv on device rockchip-pinctrl > >>> > >> reg-fixed-voltage vcc-host1-5v-regulator: Error applying > >>> setting, > >>> > >> reverse things back > >>> > >> > >>> > >> Looking at the schematic, there are in fact three USB > >>> regulators, > >>> > >> vcc_host_5v, vcc_host1_5v and vcc_otg_v5. But the enable > >>> signal for > >>> > >> all > >>> > >> three is driven by Q2604 which is in turn driven by > >>> GPIO_A2/PA2. > >>> > >> > >>> > >> Since these three regulators are not controllable > >>> separately, I > >>> > >> removed > >>> > >> the second one which was causing the error and left a comment > >>> > >> explaining > >>> > >> that this regulator actually controls multiple rails. > >>> > >> > >>> > >> Signed-off-by: Lorenz Brun <lorenz@brun.one> > >>> > >> --- > >>> > >> arch/arm64/boot/dts/rockchip/rk3328-rock64.dts | 14 > >>> +++----------- > >>> > >> 1 file changed, 3 insertions(+), 11 deletions(-) > >>> > >> > >>> > >> diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > >>> > >> b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > >>> > >> index f69a38f42d2d..bd82bc80444d 100644 > >>> > >> --- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > >>> > >> +++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > >>> > >> @@ -37,6 +37,9 @@ vcc_sd: sdmmc-regulator { > >>> > >> vin-supply = <&vcc_io>; > >>> > >> }; > >>> > >> > >>> > >> + // vcc_host_5v also controls the vcc_host1_5v and > >>> > >> vcc_otg_5v rails > >>> > >> + // but there is only one common control signal > >>> > >> (USB20_HOST_DRV) at > >>> > >> + // GPIO_A2 > >>> > >> vcc_host_5v: vcc-host-5v-regulator { > >>> > >> compatible = "regulator-fixed"; > >>> > >> gpio = <&gpio0 RK_PA2 GPIO_ACTIVE_LOW>; > >>> > >> @@ -48,17 +51,6 @@ vcc_host_5v: vcc-host-5v-regulator { > >>> > >> vin-supply = <&vcc_sys>; > >>> > >> }; > >>> > >> > >>> > >> - vcc_host1_5v: vcc_otg_5v: vcc-host1-5v-regulator { > >>> > >> - compatible = "regulator-fixed"; > >>> > >> - gpio = <&gpio0 RK_PA2 GPIO_ACTIVE_LOW>; > >>> > >> - pinctrl-names = "default"; > >>> > >> - pinctrl-0 = <&usb20_host_drv>; > >>> > >> - regulator-name = "vcc_host1_5v"; > >>> > >> - regulator-always-on; > >>> > >> - regulator-boot-on; > >>> > >> - vin-supply = <&vcc_sys>; > >>> > >> - }; > >>> > > > >>> > > Fixed-regulator supports multiple regulators sharing a gpio, > >>> the issue > >>> > > is you have the pinctrl assigned multiple times which is not > >>> > > supported. Simply removing the pinctrl from all but one of the > >>> > > regulators will solve this issue. > >>> > Sure, I can just remove the pinctrl. Should I do anything about > >>> the > >>> > fact that there are three USB switches on that GPIO, but only > >>> two of > >>> > them are described as regulators here? Seems a bit inconsistent > >>> to me. > >>> > >>> All hardware *should* be described, though it isn't uncommon to > >>> see a > >>> single fixed-regulator describe several individual switches that > >>> are > >>> all fed from a common source and controlled by the same gpio. If > >>> they > >>> are not fed by a common source (for example, the otg port is often > >>> fed > >>> from a separate regulator), they must be modeled separately. > >> > >> Which is essentially what Lorenz' patch already did - moving the 3 > >> switches > >> into one regulator. So it essentially comes down to where does the > >> not-yet > >> modeled otg-regulator get its current from - also vcc_sys? > >> > > Yeah, all regulators (they are just USB load/overload switches) are > > powered from VCC_SYS, there aren't that many power rails on the > > ROCK64. > > > > The alternative to my first patch would essentially be this: > > > > --- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > > +++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts > > @@ -48,11 +48,18 @@ vcc_host_5v: vcc-host-5v-regulator { > > vin-supply = <&vcc_sys>; > > }; > > > > - vcc_host1_5v: vcc_otg_5v: vcc-host1-5v-regulator { > > + vcc_host1_5v: vcc-host1-5v-regulator { > > + compatible = "regulator-fixed"; > > + gpio = <&gpio0 RK_PA2 GPIO_ACTIVE_LOW>; > > + regulator-name = "vcc_host1_5v"; > > + regulator-always-on; > > + regulator-boot-on; > > + vin-supply = <&vcc_sys>; > > + }; > > + > > + vcc_otg_5v: vcc-otg-5v-regulator { > > compatible = "regulator-fixed"; > > gpio = <&gpio0 RK_PA2 GPIO_ACTIVE_LOW>; > > - pinctrl-names = "default"; > > - pinctrl-0 = <&usb20_host_drv>; > > regulator-name = "vcc_host1_5v"; > > regulator-always-on; > > regulator-boot-on; > > > > This drops the pinctrl from all but the first regulator (which is a > > bit weird, the first one is not really special) and then describes > > three separate regulators driven by PA2. > > > > I have no real preference, the one-regulator solution has the > > downside of not accurately describing the three separate load > > switches, the three-regulator solution has the downside of the > > pinctrl definitions being weird. > > > > Regards, > > Lorenz > > I never got a response to this, I am fine with both approaches but > would prefer if one of them goes in so I no longer have this error on > every boot. If the approach above with three regulators is chosen I can > send out a proper patch. Sorry about that. I guess the solution I'd prefer is the original patch, but with a twist ;-) . vcc_host_5v: vcc_host1_5v: vcc_otg_5v: vcc-host-5v-regulator { ... } A node can have multiple phandles (see for example [0]), so we'd do the one node for all regulators (with one gpio), but use the correct phandles to identify the actual line to the consumer. This way we still keep somewhat close to the schematics so people can follow along, but also have the benefit that the regulator gets turned of when none of the 3 users need it. Heiko [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi#n129
diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts index f69a38f42d2d..bd82bc80444d 100644 --- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts +++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts @@ -37,6 +37,9 @@ vcc_sd: sdmmc-regulator { vin-supply = <&vcc_io>; }; + // vcc_host_5v also controls the vcc_host1_5v and vcc_otg_5v rails + // but there is only one common control signal (USB20_HOST_DRV) at + // GPIO_A2 vcc_host_5v: vcc-host-5v-regulator { compatible = "regulator-fixed"; gpio = <&gpio0 RK_PA2 GPIO_ACTIVE_LOW>; @@ -48,17 +51,6 @@ vcc_host_5v: vcc-host-5v-regulator { vin-supply = <&vcc_sys>; }; - vcc_host1_5v: vcc_otg_5v: vcc-host1-5v-regulator { - compatible = "regulator-fixed"; - gpio = <&gpio0 RK_PA2 GPIO_ACTIVE_LOW>; - pinctrl-names = "default"; - pinctrl-0 = <&usb20_host_drv>; - regulator-name = "vcc_host1_5v"; - regulator-always-on; - regulator-boot-on; - vin-supply = <&vcc_sys>; - }; - vcc_sys: vcc-sys { compatible = "regulator-fixed"; regulator-name = "vcc_sys";
Currently the ROCK64 device tree specifies two regulators, vcc_host_5v and vcc_host1_5v for USB VBUS on the device. Both of those are however specified with RK_PA2 as the GPIO enabling them, causing the following error when booting: rockchip-pinctrl pinctrl: pin gpio0-2 already requested by vcc-host-5v-regulator; cannot claim for vcc-host1-5v-regulator rockchip-pinctrl pinctrl: pin-2 (vcc-host1-5v-regulator) status -22 rockchip-pinctrl pinctrl: could not request pin 2 (gpio0-2) from group usb20-host-drv on device rockchip-pinctrl reg-fixed-voltage vcc-host1-5v-regulator: Error applying setting, reverse things back Looking at the schematic, there are in fact three USB regulators, vcc_host_5v, vcc_host1_5v and vcc_otg_v5. But the enable signal for all three is driven by Q2604 which is in turn driven by GPIO_A2/PA2. Since these three regulators are not controllable separately, I removed the second one which was causing the error and left a comment explaining that this regulator actually controls multiple rails. Signed-off-by: Lorenz Brun <lorenz@brun.one> --- arch/arm64/boot/dts/rockchip/rk3328-rock64.dts | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-)