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

Lorenz Brun lorenz at brun.one
Mon Apr 10 09:01:57 PDT 2023




Am Di, 10. Jan 2023 um 22:38:26 +01:00:00 schrieb Lorenz Brun 
<lorenz at brun.one>:
> 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

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







More information about the Linux-rockchip mailing list