[PATCH] arm64: dts: rockchip: fix USB regulator on ROCK64

[Heiko Stübner]

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?