Re: [PATCH v1] dts: rockpro64: Remove usb regulator-always-on

[Dragan Simic <dsimic@manjaro.org>]

On 2023-12-14 15:24, Shantur Rathore wrote:
> On Sun, Dec 10, 2023 at 12:08 AM Dragan Simic <dsimic@manjaro.org> 
> wrote:
>> On 2023-12-10 01:03, Shantur Rathore wrote:
>> > On Sat, Dec 9, 2023 at 11:43 PM Dragan Simic <dsimic@manjaro.org>
>> > wrote:
>> >> On 2023-12-10 00:35, Shantur Rathore wrote:
>> >> > USB port regulators should be controlled by PHYs
>> >> > so we remove always-on property and let PHYs manage the
>> >> > regulator.
>> >> >
>> >> > Typec port has misconfugred phy-supply and now that we are
>> >> > removing regulator-always-on, we need to fix the phy-supply
>> >> > so the PHYs are able to turn power to type-c port.
>> >> >
>> >> > Signed-off-by: Shantur Rathore <i@shantur.com>
>> >> > ---
>> >> > + devicetree
>> >> >
>> >> > After this patch the ports were confirmed to power up and down
>> >> > in u-boot when doing usb start and usb stop.
>> >> > At boot the regulators were off, the devices connected weren't
>> >> > powered up, on usb start the PHYs are able to power on the ports
>> >> > and on usb stop they were powered down.
>> >> >
>> >> > At the boot, the ports were powered down which was again powered
>> >> > up by Linux kernel when booting up.
>> >> >
>> >> >
>> >> >  arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi | 4 +---
>> >> >  1 file changed, 1 insertion(+), 3 deletions(-)
>> >> >
>> >> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
>> >> > b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
>> >> > index bca2b50e0a..bd2824aa48 100644
>> >> > --- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
>> >> > +++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
>> >> > @@ -192,7 +192,6 @@ vcc5v0_host: vcc5v0-host-regulator {
>> >> >               pinctrl-names = "default";
>> >> >               pinctrl-0 = <&vcc5v0_host_en>;
>> >> >               regulator-name = "vcc5v0_host";
>> >> > -             regulator-always-on;
>> >> >               vin-supply = <&vcc5v0_usb>;
>> >> >       };
>> >> >
>> >> > @@ -203,7 +202,6 @@ vcc5v0_typec: vcc5v0-typec-regulator {
>> >> >               pinctrl-names = "default";
>> >> >               pinctrl-0 = <&vcc5v0_typec_en>;
>> >> >               regulator-name = "vcc5v0_typec";
>> >> > -             regulator-always-on;
>> >> >               vin-supply = <&vcc5v0_usb>;
>> >> >       };
>> >> >
>> >> > @@ -863,7 +861,7 @@ u2phy0_otg: otg-port {
>> >> >       };
>> >> >
>> >> >       u2phy0_host: host-port {
>> >> > -             phy-supply = <&vcc5v0_host>;
>> >> > +             phy-supply = <&vcc5v0_typec>;
>> >> >               status = "okay";
>> >> >       };
>> >> >  };
>> >>
>> >> Quite frankly, something doesn't feel right there.  Would you mind,
>> >> please, to place this patch on hold until next week or so, at which
>> >> point I should have enough time to go through the RockPro64 schematic
>> >> thoroughly once again, and test the patch in detail?
>> >
>> > Sure, no worries.
>> 
>> Great, thanks.
>> 
>> > Would you mind letting me know which part doesn't feel right to you?
>> 
>> Sure, it was about the last change in the patch.
> 
> The TypeC port VBUS is VBUS_TYPEC on Page 20 of 33 marked for
> VBUS_[1-4] in schematic here - [0]
> 
> [0] - https://files.pine64.org/doc/rockpro64/rockpro64_v21-SCH.pdf

I see, thanks, and I already went once again through the RockPro64 
schematic, but only briefly.  I need to do that again, but in detail, 
and compare it thoroughly with the dts(i).