[PATCH] arm64: dts: rockchip: fix USB regulator on ROCK64
Lorenz Brun
lorenz at brun.one
Tue Jan 10 13:38:26 PST 2023
On Tue, Jan 10 2023 at 14:02:47 +01:00:00, Heiko Stübner
<heiko at 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 at brun.one> wrote:
>> >
>> >
>> > On Wed, Jan 4 2023 at 18:46:25 -05:00:00, Peter Geis
>> > <pgwipeout at gmail.com> wrote:
>> > > On Wed, Jan 4, 2023 at 3:55 PM Lorenz Brun <lorenz at 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 at 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
More information about the Linux-rockchip
mailing list