On 27/08/2022 11:50, Frank Wunderlich wrote:
> Hi
>
>> Gesendet: Freitag, 26. August 2022 um 08:50 Uhr
>> Von: "Krzysztof Kozlowski" <[email protected]>
>> On 25/08/2022 22:38, Frank Wunderlich wrote:
>>> From: Frank Wunderlich <[email protected]>
>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts b/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts
>>> index 93d383b8be87..40b90c052634 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts
>>> @@ -86,6 +86,66 @@ vcc5v0_sys: vcc5v0-sys {
>>> vin-supply = <&dc_12v>;
>>> };
>>>
>>> + pcie30_avdd0v9: pcie30-avdd0v9 {
>>
>> Use consistent naming, so if other nodes have "regulator" suffix, use it
>> here as well.
>
> only these 3 new have the suffix:
>
> vcc3v3_pi6c_05: vcc3v3-pi6c-05-regulator
> vcc3v3_minipcie: vcc3v3-minipcie-regulator
> vcc3v3_ngff: vcc3v3-ngff-regulator
>
> so i would drop it there...
>
> so i end up with (including existing ones to compare):
>
> vcc3v3_sys: vcc3v3-sys
> vcc5v0_sys: vcc5v0-sys
> pcie30_avdd0v9: pcie30-avdd0v9
> pcie30_avdd1v8: pcie30-avdd1v8
> vcc3v3_pi6c_05: vcc3v3-pi6c-05
> vcc3v3_minipcie: vcc3v3-minipcie
> vcc3v3_ngff: vcc3v3-ngff
> vcc5v0_usb: vcc5v0_usb
> vcc5v0_usb_host: vcc5v0-usb-host
> vcc5v0_usb_otg: vcc5v0-usb-otg
>
> is this ok?
>
> maybe swap avdd* and pcie30 part to have voltage in front of function.
>
I prefer all of them have regulator suffix. I think reasonable is also
to rename the old ones and then add new ones with suffix.
Best regards,
Krzysztof
> Gesendet: Samstag, 27. August 2022 um 10:56 Uhr
> Von: "Krzysztof Kozlowski" <[email protected]>
> On 27/08/2022 11:50, Frank Wunderlich wrote:
> > Hi
> >
> >> Gesendet: Freitag, 26. August 2022 um 08:50 Uhr
> >> Von: "Krzysztof Kozlowski" <[email protected]>
> >> On 25/08/2022 22:38, Frank Wunderlich wrote:
> >>> From: Frank Wunderlich <[email protected]>
> >
> >>> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts b/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts
> >>> index 93d383b8be87..40b90c052634 100644
> >>> --- a/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts
> >>> +++ b/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts
> >>> @@ -86,6 +86,66 @@ vcc5v0_sys: vcc5v0-sys {
> >>> vin-supply = <&dc_12v>;
> >>> };
> >>>
> >>> + pcie30_avdd0v9: pcie30-avdd0v9 {
> >>
> >> Use consistent naming, so if other nodes have "regulator" suffix, use it
> >> here as well.
> >
> > only these 3 new have the suffix:
> >
> > vcc3v3_pi6c_05: vcc3v3-pi6c-05-regulator
> > vcc3v3_minipcie: vcc3v3-minipcie-regulator
> > vcc3v3_ngff: vcc3v3-ngff-regulator
> >
> > so i would drop it there...
> >
> > so i end up with (including existing ones to compare):
> >
> > vcc3v3_sys: vcc3v3-sys
> > vcc5v0_sys: vcc5v0-sys
> > pcie30_avdd0v9: pcie30-avdd0v9
> > pcie30_avdd1v8: pcie30-avdd1v8
> > vcc3v3_pi6c_05: vcc3v3-pi6c-05
> > vcc3v3_minipcie: vcc3v3-minipcie
> > vcc3v3_ngff: vcc3v3-ngff
> > vcc5v0_usb: vcc5v0_usb
> > vcc5v0_usb_host: vcc5v0-usb-host
> > vcc5v0_usb_otg: vcc5v0-usb-otg
> >
> > is this ok?
> >
> > maybe swap avdd* and pcie30 part to have voltage in front of function.
> >
>
> I prefer all of them have regulator suffix. I think reasonable is also
> to rename the old ones and then add new ones with suffix.
ok, will change these to add -regulator in name (not label). and then rename the others in separate Patch outside of the series.
so basicly here
- pcie30_avdd0v9: pcie30-avdd0v9 {
+ pcie30_avdd0v9: pcie30-avdd0v9-regulator {
- pcie30_avdd1v8: pcie30-avdd1v8 {
+ pcie30_avdd1v8: pcie30-avdd1v8-regulator {
how about the swapping of pcie30 and the avddXvY? In Schematic they are named PCIE30_AVDD_0V9 / PCIE30_AVDD_1V8, so better leave this?
avdd0v9-pcie30 will be more similar to the other regulators, but inconsistent with Schematic.
regards Frank
On 27/08/2022 12:14, Frank Wunderlich wrote:
>> Gesendet: Samstag, 27. August 2022 um 10:56 Uhr
>> Von: "Krzysztof Kozlowski" <[email protected]>
>
>> On 27/08/2022 11:50, Frank Wunderlich wrote:
>>> Hi
>>>
>>>> Gesendet: Freitag, 26. August 2022 um 08:50 Uhr
>>>> Von: "Krzysztof Kozlowski" <[email protected]>
>>>> On 25/08/2022 22:38, Frank Wunderlich wrote:
>>>>> From: Frank Wunderlich <[email protected]>
>>>
>>>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts b/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts
>>>>> index 93d383b8be87..40b90c052634 100644
>>>>> --- a/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts
>>>>> +++ b/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts
>>>>> @@ -86,6 +86,66 @@ vcc5v0_sys: vcc5v0-sys {
>>>>> vin-supply = <&dc_12v>;
>>>>> };
>>>>>
>>>>> + pcie30_avdd0v9: pcie30-avdd0v9 {
>>>>
>>>> Use consistent naming, so if other nodes have "regulator" suffix, use it
>>>> here as well.
>>>
>>> only these 3 new have the suffix:
>>>
>>> vcc3v3_pi6c_05: vcc3v3-pi6c-05-regulator
>>> vcc3v3_minipcie: vcc3v3-minipcie-regulator
>>> vcc3v3_ngff: vcc3v3-ngff-regulator
>>>
>>> so i would drop it there...
>>>
>>> so i end up with (including existing ones to compare):
>>>
>>> vcc3v3_sys: vcc3v3-sys
>>> vcc5v0_sys: vcc5v0-sys
>>> pcie30_avdd0v9: pcie30-avdd0v9
>>> pcie30_avdd1v8: pcie30-avdd1v8
>>> vcc3v3_pi6c_05: vcc3v3-pi6c-05
>>> vcc3v3_minipcie: vcc3v3-minipcie
>>> vcc3v3_ngff: vcc3v3-ngff
>>> vcc5v0_usb: vcc5v0_usb
>>> vcc5v0_usb_host: vcc5v0-usb-host
>>> vcc5v0_usb_otg: vcc5v0-usb-otg
>>>
>>> is this ok?
>>>
>>> maybe swap avdd* and pcie30 part to have voltage in front of function.
>>>
>>
>> I prefer all of them have regulator suffix. I think reasonable is also
>> to rename the old ones and then add new ones with suffix.
>
> ok, will change these to add -regulator in name (not label). and then rename the others in separate Patch outside of the series.
>
> so basicly here
> - pcie30_avdd0v9: pcie30-avdd0v9 {
> + pcie30_avdd0v9: pcie30-avdd0v9-regulator {
> - pcie30_avdd1v8: pcie30-avdd1v8 {
> + pcie30_avdd1v8: pcie30-avdd1v8-regulator {
>
> how about the swapping of pcie30 and the avddXvY? In Schematic they are named PCIE30_AVDD_0V9 / PCIE30_AVDD_1V8, so better leave this?
>
> avdd0v9-pcie30 will be more similar to the other regulators, but inconsistent with Schematic.
Does not matter to me - it is still a specific prefix, so whatever you
put there it's for you, not for me. Keeping something aligned to
schematic - even if not consistently named - makes sense to me.
Best regards,
Krzysztof
Hi,
Am Samstag, 27. August 2022, 11:14:28 CEST schrieb Frank Wunderlich:
> > Gesendet: Samstag, 27. August 2022 um 10:56 Uhr
> > Von: "Krzysztof Kozlowski" <[email protected]>
>
> > On 27/08/2022 11:50, Frank Wunderlich wrote:
> > > Hi
> > >
> > >> Gesendet: Freitag, 26. August 2022 um 08:50 Uhr
> > >> Von: "Krzysztof Kozlowski" <[email protected]>
> > >> On 25/08/2022 22:38, Frank Wunderlich wrote:
> > >>> From: Frank Wunderlich <[email protected]>
> > >
> > >>> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts b/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts
> > >>> index 93d383b8be87..40b90c052634 100644
> > >>> --- a/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts
> > >>> +++ b/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts
> > >>> @@ -86,6 +86,66 @@ vcc5v0_sys: vcc5v0-sys {
> > >>> vin-supply = <&dc_12v>;
> > >>> };
> > >>>
> > >>> + pcie30_avdd0v9: pcie30-avdd0v9 {
> > >>
> > >> Use consistent naming, so if other nodes have "regulator" suffix, use it
> > >> here as well.
> > >
> > > only these 3 new have the suffix:
> > >
> > > vcc3v3_pi6c_05: vcc3v3-pi6c-05-regulator
> > > vcc3v3_minipcie: vcc3v3-minipcie-regulator
> > > vcc3v3_ngff: vcc3v3-ngff-regulator
> > >
> > > so i would drop it there...
> > >
> > > so i end up with (including existing ones to compare):
> > >
> > > vcc3v3_sys: vcc3v3-sys
> > > vcc5v0_sys: vcc5v0-sys
> > > pcie30_avdd0v9: pcie30-avdd0v9
> > > pcie30_avdd1v8: pcie30-avdd1v8
> > > vcc3v3_pi6c_05: vcc3v3-pi6c-05
> > > vcc3v3_minipcie: vcc3v3-minipcie
> > > vcc3v3_ngff: vcc3v3-ngff
> > > vcc5v0_usb: vcc5v0_usb
> > > vcc5v0_usb_host: vcc5v0-usb-host
> > > vcc5v0_usb_otg: vcc5v0-usb-otg
> > >
> > > is this ok?
> > >
> > > maybe swap avdd* and pcie30 part to have voltage in front of function.
> > >
> >
> > I prefer all of them have regulator suffix. I think reasonable is also
> > to rename the old ones and then add new ones with suffix.
>
> ok, will change these to add -regulator in name (not label). and then rename the others in separate Patch outside of the series.
>
> so basicly here
> - pcie30_avdd0v9: pcie30-avdd0v9 {
> + pcie30_avdd0v9: pcie30-avdd0v9-regulator {
> - pcie30_avdd1v8: pcie30-avdd1v8 {
> + pcie30_avdd1v8: pcie30-avdd1v8-regulator {
>
> how about the swapping of pcie30 and the avddXvY? In Schematic they are named PCIE30_AVDD_0V9 / PCIE30_AVDD_1V8, so better leave this?
>
> avdd0v9-pcie30 will be more similar to the other regulators, but inconsistent with Schematic.
now that the phy-driver changes got applied I'll just pick up the remaining
patches and do the node-name conversion while applying, so no need
to send another revision for it.
But of course feel free so send patches for converting the other regulator
names.
And I'm definitly preferring keeping close to schematic names :-)
Heiko