| Message ID | 20240127092034.887085-3-andyshrk@163.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] arm64: dts: rockchip: aliase sdmmc as mmc1 for Cool Pi 4B | expand |
Hi Andy, Am Samstag, 27. Januar 2024, 10:20:33 CET schrieb Andy Yan: > According to the schematic, this regulator is used both for USB30 and > USB20, so give it a more appropriate name. I don't have the schematics, so I'll need you to answer this, but what is the regulator called _in_ the schematics? I.e. we want regulators to really be named the same as in the schematic so people can look up thing from the dts in the schematics and the other way around too. Thanks Heiko > Fixes: 791c154c3982 ("arm64: dts: rockchip: Add support for rk3588 based board Cool Pi CM5 EVB") > Signed-off-by: Andy Yan <andyshrk@163.com> > --- > > arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-evb.dts | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-evb.dts b/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-evb.dts > index 1b5681fe0471..5f42f1065d73 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-evb.dts > +++ b/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-evb.dts > @@ -84,7 +84,7 @@ vcc3v3_lcd: vcc3v3-lcd-regulator { > vin-supply = <&vcc3v3_sys>; > }; > > - vcc5v0_usb30_host: vcc5v0-usb30-host-regulator { > + vcc5v0_usb_host: vcc5v0-usb-host-regulator { > compatible = "regulator-fixed"; > regulator-name = "vcc5v0_host"; > regulator-boot-on; > @@ -200,12 +200,12 @@ &u2phy3 { > }; > > &u2phy2_host { > - phy-supply = <&vcc5v0_usb30_host>; > + phy-supply = <&vcc5v0_usb_host>; > status = "okay"; > }; > > &u2phy3_host { > - phy-supply = <&vcc5v0_usb30_host>; > + phy-supply = <&vcc5v0_usb_host>; > status = "okay"; > }; > >
Hi Heiko： At 2024-01-27 18:36:40, "Heiko Stübner" <heiko@sntech.de> wrote: >Hi Andy, > >Am Samstag, 27. Januar 2024, 10:20:33 CET schrieb Andy Yan: >> According to the schematic, this regulator is used both for USB30 and >> USB20, so give it a more appropriate name. > >I don't have the schematics, so I'll need you to answer this, but what >is the regulator called _in_ the schematics? There are two regulators called VCC50_USB_HOST1 and VCC50_USB_HOST2, and they are both controlled by GPIO1_D5 They both for two usb 2.0 hosts, not usb 30, the schematics make me a bit confused. > >I.e. we want regulators to really be named the same as in the schematic >so people can look up thing from the dts in the schematics and the other >way around too. > > >Thanks >Heiko > > >> Fixes: 791c154c3982 ("arm64: dts: rockchip: Add support for rk3588 based board Cool Pi CM5 EVB") >> Signed-off-by: Andy Yan <andyshrk@163.com> >> --- >> >> arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-evb.dts | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-evb.dts b/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-evb.dts >> index 1b5681fe0471..5f42f1065d73 100644 >> --- a/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-evb.dts >> +++ b/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-evb.dts >> @@ -84,7 +84,7 @@ vcc3v3_lcd: vcc3v3-lcd-regulator { >> vin-supply = <&vcc3v3_sys>; >> }; >> >> - vcc5v0_usb30_host: vcc5v0-usb30-host-regulator { >> + vcc5v0_usb_host: vcc5v0-usb-host-regulator { >> compatible = "regulator-fixed"; >> regulator-name = "vcc5v0_host"; >> regulator-boot-on; >> @@ -200,12 +200,12 @@ &u2phy3 { >> }; >> >> &u2phy2_host { >> - phy-supply = <&vcc5v0_usb30_host>; >> + phy-supply = <&vcc5v0_usb_host>; >> status = "okay"; >> }; >> >> &u2phy3_host { >> - phy-supply = <&vcc5v0_usb30_host>; >> + phy-supply = <&vcc5v0_usb_host>; >> status = "okay"; >> }; >> >> > > > > > >_______________________________________________ >linux-arm-kernel mailing list >linux-arm-kernel@lists.infradead.org >http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 2024-01-27 13:15, Andy Yan wrote: > At 2024-01-27 18:36:40, "Heiko Stübner" <heiko@sntech.de> wrote: >> Am Samstag, 27. Januar 2024, 10:20:33 CET schrieb Andy Yan: >>> According to the schematic, this regulator is used both for USB30 and >>> USB20, so give it a more appropriate name. >> >> I don't have the schematics, so I'll need you to answer this, but what >> is the regulator called _in_ the schematics? > > There are two regulators called VCC50_USB_HOST1 and VCC50_USB_HOST2, > and they are both controlled by GPIO1_D5 > They both for two usb 2.0 hosts, not usb 30, the schematics make > me a bit confused. In that case, I'd say that renaming the regulator to vcc5v0_usb_host is fine, but there should also be a comment in the board dts file that it's actually two separate regulators. >> I.e. we want regulators to really be named the same as in the >> schematic >> so people can look up thing from the dts in the schematics and the >> other >> way around too. Ah, that's very helpful. >>> Fixes: 791c154c3982 ("arm64: dts: rockchip: Add support for rk3588 >>> based board Cool Pi CM5 EVB") >>> Signed-off-by: Andy Yan <andyshrk@163.com> >>> --- >>> >>> arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-evb.dts | 6 +++--- >>> 1 file changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-evb.dts >>> b/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-evb.dts >>> index 1b5681fe0471..5f42f1065d73 100644 >>> --- a/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-evb.dts >>> +++ b/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-evb.dts >>> @@ -84,7 +84,7 @@ vcc3v3_lcd: vcc3v3-lcd-regulator { >>> vin-supply = <&vcc3v3_sys>; >>> }; >>> >>> - vcc5v0_usb30_host: vcc5v0-usb30-host-regulator { >>> + vcc5v0_usb_host: vcc5v0-usb-host-regulator { >>> compatible = "regulator-fixed"; >>> regulator-name = "vcc5v0_host"; >>> regulator-boot-on; >>> @@ -200,12 +200,12 @@ &u2phy3 { >>> }; >>> >>> &u2phy2_host { >>> - phy-supply = <&vcc5v0_usb30_host>; >>> + phy-supply = <&vcc5v0_usb_host>; >>> status = "okay"; >>> }; >>> >>> &u2phy3_host { >>> - phy-supply = <&vcc5v0_usb30_host>; >>> + phy-supply = <&vcc5v0_usb_host>; >>> status = "okay"; >>> };
Hi Heiko： 在 2024-01-27 21:09:46，"Dragan Simic" <dsimic@manjaro.org> 写道： >On 2024-01-27 13:15, Andy Yan wrote: >> At 2024-01-27 18:36:40, "Heiko Stübner" <heiko@sntech.de> wrote: >>> Am Samstag, 27. Januar 2024, 10:20:33 CET schrieb Andy Yan: >>>> According to the schematic, this regulator is used both for USB30 and >>>> USB20, so give it a more appropriate name. >>> >>> I don't have the schematics, so I'll need you to answer this, but what >>> is the regulator called _in_ the schematics? >> >> There are two regulators called VCC50_USB_HOST1 and VCC50_USB_HOST2, >> and they are both controlled by GPIO1_D5 >> They both for two usb 2.0 hosts, not usb 30, the schematics make >> me a bit confused. > >In that case, I'd say that renaming the regulator to vcc5v0_usb_host is >fine, but there should also be a comment in the board dts file that it's >actually two separate regulators. How do you feel about this ？ Or some other style like： vcc5v0_usb_host1: vcc5v0_usb_host2：vcc5v0-usb-host-regulator { > >>> I.e. we want regulators to really be named the same as in the >>> schematic >>> so people can look up thing from the dts in the schematics and the >>> other >>> way around too. > >Ah, that's very helpful. > >>>> Fixes: 791c154c3982 ("arm64: dts: rockchip: Add support for rk3588 >>>> based board Cool Pi CM5 EVB") >>>> Signed-off-by: Andy Yan <andyshrk@163.com> >>>> --- >>>> >>>> arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-evb.dts | 6 +++--- >>>> 1 file changed, 3 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-evb.dts >>>> b/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-evb.dts >>>> index 1b5681fe0471..5f42f1065d73 100644 >>>> --- a/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-evb.dts >>>> +++ b/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-evb.dts >>>> @@ -84,7 +84,7 @@ vcc3v3_lcd: vcc3v3-lcd-regulator { >>>> vin-supply = <&vcc3v3_sys>; >>>> }; >>>> >>>> - vcc5v0_usb30_host: vcc5v0-usb30-host-regulator { >>>> + vcc5v0_usb_host: vcc5v0-usb-host-regulator { >>>> compatible = "regulator-fixed"; >>>> regulator-name = "vcc5v0_host"; >>>> regulator-boot-on; >>>> @@ -200,12 +200,12 @@ &u2phy3 { >>>> }; >>>> >>>> &u2phy2_host { >>>> - phy-supply = <&vcc5v0_usb30_host>; >>>> + phy-supply = <&vcc5v0_usb_host>; >>>> status = "okay"; >>>> }; >>>> >>>> &u2phy3_host { >>>> - phy-supply = <&vcc5v0_usb30_host>; >>>> + phy-supply = <&vcc5v0_usb_host>; >>>> status = "okay"; >>>> }; > >_______________________________________________ >linux-arm-kernel mailing list >linux-arm-kernel@lists.infradead.org >http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Am Mittwoch, 31. Januar 2024, 13:21:56 CET schrieb Andy Yan: > > Hi Heiko： > > 在 2024-01-27 21:09:46，"Dragan Simic" <dsimic@manjaro.org> 写道： > >On 2024-01-27 13:15, Andy Yan wrote: > >> At 2024-01-27 18:36:40, "Heiko Stübner" <heiko@sntech.de> wrote: > >>> Am Samstag, 27. Januar 2024, 10:20:33 CET schrieb Andy Yan: > >>>> According to the schematic, this regulator is used both for USB30 and > >>>> USB20, so give it a more appropriate name. > >>> > >>> I don't have the schematics, so I'll need you to answer this, but what > >>> is the regulator called _in_ the schematics? > >> > >> There are two regulators called VCC50_USB_HOST1 and VCC50_USB_HOST2, > >> and they are both controlled by GPIO1_D5 > >> They both for two usb 2.0 hosts, not usb 30, the schematics make > >> me a bit confused. > > > >In that case, I'd say that renaming the regulator to vcc5v0_usb_host is > >fine, but there should also be a comment in the board dts file that it's > >actually two separate regulators. > > How do you feel about this ？ Or some other style like： > vcc5v0_usb_host1: vcc5v0_usb_host2：vcc5v0-usb-host-regulator { I think we're using such a scheme in some places already, and yes I really like going this way. So that the phandles follow the schematic names and we can still grep for things. Heiko
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-evb.dts b/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-evb.dts index 1b5681fe0471..5f42f1065d73 100644 --- a/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-evb.dts +++ b/arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-evb.dts @@ -84,7 +84,7 @@ vcc3v3_lcd: vcc3v3-lcd-regulator { vin-supply = <&vcc3v3_sys>; }; - vcc5v0_usb30_host: vcc5v0-usb30-host-regulator { + vcc5v0_usb_host: vcc5v0-usb-host-regulator { compatible = "regulator-fixed"; regulator-name = "vcc5v0_host"; regulator-boot-on; @@ -200,12 +200,12 @@ &u2phy3 { }; &u2phy2_host { - phy-supply = <&vcc5v0_usb30_host>; + phy-supply = <&vcc5v0_usb_host>; status = "okay"; }; &u2phy3_host { - phy-supply = <&vcc5v0_usb30_host>; + phy-supply = <&vcc5v0_usb_host>; status = "okay"; };
According to the schematic, this regulator is used both for USB30 and USB20, so give it a more appropriate name. Fixes: 791c154c3982 ("arm64: dts: rockchip: Add support for rk3588 based board Cool Pi CM5 EVB") Signed-off-by: Andy Yan <andyshrk@163.com> --- arch/arm64/boot/dts/rockchip/rk3588-coolpi-cm5-evb.dts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)