| Message ID | 20241216113052.15696-5-naoki@radxa.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: dts: rockchip: Refine dts for Radxa ROCK 5C | expand |
On 16/12/2024 12:30, FUKAUMI Naoki wrote: > Use consistent name with other regulators. No functional change. > > Fixes: 3ddf5cdb77e6 ("arm64: dts: rockchip: add Radxa ROCK 5C") > Signed-off-by: FUKAUMI Naoki <naoki@radxa.com> > --- > Changes in v5: > - Reword commit message > Changes in v4: > - reword commit message > Changes in v3: > - none > Changes in v2: > - new > --- > arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts > index 85589d1a6d3b..61d75ab503b2 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts > +++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts > @@ -76,13 +76,13 @@ pwm-fan { > pwms = <&pwm3 0 60000 0>; > }; > > - pcie2x1l2_3v3: regulator-pcie2x1l2-3v3 { > + vcc3v3_pcie2x1l2: regulator-vcc3v3_pcie2x1l2 { No, neither explained, nor correct. See DTS coding style. Please use name for all fixed regulators which matches current format recommendation: 'regulator-[0-9]v[0-9]' https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml?h=v6.11-rc1#n46 Best regards, Krzysztof
On 12/16/24 22:38, Krzysztof Kozlowski wrote: > On 16/12/2024 12:30, FUKAUMI Naoki wrote: >> Use consistent name with other regulators. No functional change. >> >> Fixes: 3ddf5cdb77e6 ("arm64: dts: rockchip: add Radxa ROCK 5C") >> Signed-off-by: FUKAUMI Naoki <naoki@radxa.com> >> --- >> Changes in v5: >> - Reword commit message >> Changes in v4: >> - reword commit message >> Changes in v3: >> - none >> Changes in v2: >> - new >> --- >> arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts >> index 85589d1a6d3b..61d75ab503b2 100644 >> --- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts >> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts >> @@ -76,13 +76,13 @@ pwm-fan { >> pwms = <&pwm3 0 60000 0>; >> }; >> >> - pcie2x1l2_3v3: regulator-pcie2x1l2-3v3 { >> + vcc3v3_pcie2x1l2: regulator-vcc3v3_pcie2x1l2 { > > No, neither explained, nor correct. See DTS coding style. > > Please use name for all fixed regulators which matches current format > recommendation: 'regulator-[0-9]v[0-9]' > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml?h=v6.11-rc1#n46 Thanks for pointing. (Please blame other dts too) Best regards, -- FUKAUMI Naoki Radxa Computer (Shenzhen) Co., Ltd. > Best regards, > Krzysztof >
On 12/16/24 22:38, Krzysztof Kozlowski wrote: > On 16/12/2024 12:30, FUKAUMI Naoki wrote: >> Use consistent name with other regulators. No functional change. >> >> Fixes: 3ddf5cdb77e6 ("arm64: dts: rockchip: add Radxa ROCK 5C") >> Signed-off-by: FUKAUMI Naoki <naoki@radxa.com> >> --- >> Changes in v5: >> - Reword commit message >> Changes in v4: >> - reword commit message >> Changes in v3: >> - none >> Changes in v2: >> - new >> --- >> arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts >> index 85589d1a6d3b..61d75ab503b2 100644 >> --- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts >> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts >> @@ -76,13 +76,13 @@ pwm-fan { >> pwms = <&pwm3 0 60000 0>; >> }; >> >> - pcie2x1l2_3v3: regulator-pcie2x1l2-3v3 { >> + vcc3v3_pcie2x1l2: regulator-vcc3v3_pcie2x1l2 { > > No, neither explained, nor correct. See DTS coding style. > > Please use name for all fixed regulators which matches current format > recommendation: 'regulator-[0-9]v[0-9]' > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml?h=v6.11-rc1#n46 'regulator-[0-9]v[0-9]' is preferred, but 'regulator-[0-9a-z-]+' is also permitted, right? i.e. regulator-vcc3v3_pcie2x1l2 should be regulator-vcc3v3-pcie2x1l2 Or, should we revert below patch and use 'regulator-[0-9]v[0-9]'? https://lore.kernel.org/all/0ae40493-93e9-40cd-9ca9-990ae064f21a@gmail.com/ Is 'regulator-0v0' valid? Is 'regulator-12v0' invalid? How should we handle multiple 1v8/3v3/5v0 regulators? Are examples in fixed-regulator.yaml valid? 'regulator-name' should be '[0-9]v[0-9]' even if there is same name? Best regards, -- FUKAUMI Naoki Radxa Computer (Shenzhen) Co., Ltd. > Best regards, > Krzysztof >
Hi Krzysztof, Could you please reply to this email? (Not for me, but for everyone) Best regards, -- FUKAUMI Naoki Radxa Computer (Shenzhen) Co., Ltd. On 12/17/24 10:11, FUKAUMI Naoki wrote: > On 12/16/24 22:38, Krzysztof Kozlowski wrote: >> On 16/12/2024 12:30, FUKAUMI Naoki wrote: >>> Use consistent name with other regulators. No functional change. >>> >>> Fixes: 3ddf5cdb77e6 ("arm64: dts: rockchip: add Radxa ROCK 5C") >>> Signed-off-by: FUKAUMI Naoki <naoki@radxa.com> >>> --- >>> Changes in v5: >>> - Reword commit message >>> Changes in v4: >>> - reword commit message >>> Changes in v3: >>> - none >>> Changes in v2: >>> - new >>> --- >>> arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts | 6 +++--- >>> 1 file changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts b/arch/ >>> arm64/boot/dts/rockchip/rk3588s-rock-5c.dts >>> index 85589d1a6d3b..61d75ab503b2 100644 >>> --- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts >>> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts >>> @@ -76,13 +76,13 @@ pwm-fan { >>> pwms = <&pwm3 0 60000 0>; >>> }; >>> - pcie2x1l2_3v3: regulator-pcie2x1l2-3v3 { >>> + vcc3v3_pcie2x1l2: regulator-vcc3v3_pcie2x1l2 { >> >> No, neither explained, nor correct. See DTS coding style. >> >> Please use name for all fixed regulators which matches current format >> recommendation: 'regulator-[0-9]v[0-9]' >> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/ >> tree/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml? >> h=v6.11-rc1#n46 > > 'regulator-[0-9]v[0-9]' is preferred, but 'regulator-[0-9a-z-]+' is also > permitted, right? > > i.e. regulator-vcc3v3_pcie2x1l2 should be regulator-vcc3v3-pcie2x1l2 > > > Or, should we revert below patch and use 'regulator-[0-9]v[0-9]'? > > https://lore.kernel.org/ > all/0ae40493-93e9-40cd-9ca9-990ae064f21a@gmail.com/ > > Is 'regulator-0v0' valid? > Is 'regulator-12v0' invalid? > > How should we handle multiple 1v8/3v3/5v0 regulators? > > Are examples in fixed-regulator.yaml valid? > 'regulator-name' should be '[0-9]v[0-9]' even if there is same name? > > Best regards, > > -- > FUKAUMI Naoki > Radxa Computer (Shenzhen) Co., Ltd. > >> Best regards, >> Krzysztof >> >
On 20/12/2024 07:51, FUKAUMI Naoki wrote: > Hi Krzysztof, > > Could you please reply to this email? > (Not for me, but for everyone) You have me how much time... 3 days to reply? > > Best regards, > > -- > FUKAUMI Naoki > Radxa Computer (Shenzhen) Co., Ltd. > > On 12/17/24 10:11, FUKAUMI Naoki wrote: >> On 12/16/24 22:38, Krzysztof Kozlowski wrote: >>> On 16/12/2024 12:30, FUKAUMI Naoki wrote: >>>> Use consistent name with other regulators. No functional change. >>>> >>>> Fixes: 3ddf5cdb77e6 ("arm64: dts: rockchip: add Radxa ROCK 5C") >>>> Signed-off-by: FUKAUMI Naoki <naoki@radxa.com> >>>> --- >>>> Changes in v5: >>>> - Reword commit message >>>> Changes in v4: >>>> - reword commit message >>>> Changes in v3: >>>> - none >>>> Changes in v2: >>>> - new >>>> --- >>>> arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts | 6 +++--- >>>> 1 file changed, 3 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts b/arch/ >>>> arm64/boot/dts/rockchip/rk3588s-rock-5c.dts >>>> index 85589d1a6d3b..61d75ab503b2 100644 >>>> --- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts >>>> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts >>>> @@ -76,13 +76,13 @@ pwm-fan { >>>> pwms = <&pwm3 0 60000 0>; >>>> }; >>>> - pcie2x1l2_3v3: regulator-pcie2x1l2-3v3 { >>>> + vcc3v3_pcie2x1l2: regulator-vcc3v3_pcie2x1l2 { >>> >>> No, neither explained, nor correct. See DTS coding style. >>> >>> Please use name for all fixed regulators which matches current format >>> recommendation: 'regulator-[0-9]v[0-9]' >>> >>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/ >>> tree/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml? >>> h=v6.11-rc1#n46 >> >> 'regulator-[0-9]v[0-9]' is preferred, but 'regulator-[0-9a-z-]+' is also >> permitted, right? >> >> i.e. regulator-vcc3v3_pcie2x1l2 should be regulator-vcc3v3-pcie2x1l2 >> >> >> Or, should we revert below patch and use 'regulator-[0-9]v[0-9]'? >> >> https://lore.kernel.org/ >> all/0ae40493-93e9-40cd-9ca9-990ae064f21a@gmail.com/ >> >> Is 'regulator-0v0' valid? Why would it be valid? Can you have regulator with 0 volts? >> Is 'regulator-12v0' invalid? Read the binding. I gave you very specific link. >> >> How should we handle multiple 1v8/3v3/5v0 regulators? Just add suffix. But usually more than one suffix, vcc+3v3+pcie_2x1l2, means you created a very specific name. Best regards, Krzysztof
On 12/22/24 05:15, Krzysztof Kozlowski wrote: > On 20/12/2024 07:51, FUKAUMI Naoki wrote: >> Hi Krzysztof, >> >> Could you please reply to this email? >> (Not for me, but for everyone) > > You have me how much time... 3 days to reply? sorry. >> Best regards, >> >> -- >> FUKAUMI Naoki >> Radxa Computer (Shenzhen) Co., Ltd. >> >> On 12/17/24 10:11, FUKAUMI Naoki wrote: >>> On 12/16/24 22:38, Krzysztof Kozlowski wrote: >>>> On 16/12/2024 12:30, FUKAUMI Naoki wrote: >>>>> Use consistent name with other regulators. No functional change. >>>>> >>>>> Fixes: 3ddf5cdb77e6 ("arm64: dts: rockchip: add Radxa ROCK 5C") >>>>> Signed-off-by: FUKAUMI Naoki <naoki@radxa.com> >>>>> --- >>>>> Changes in v5: >>>>> - Reword commit message >>>>> Changes in v4: >>>>> - reword commit message >>>>> Changes in v3: >>>>> - none >>>>> Changes in v2: >>>>> - new >>>>> --- >>>>> arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts | 6 +++--- >>>>> 1 file changed, 3 insertions(+), 3 deletions(-) >>>>> >>>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts b/arch/ >>>>> arm64/boot/dts/rockchip/rk3588s-rock-5c.dts >>>>> index 85589d1a6d3b..61d75ab503b2 100644 >>>>> --- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts >>>>> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts >>>>> @@ -76,13 +76,13 @@ pwm-fan { >>>>> pwms = <&pwm3 0 60000 0>; >>>>> }; >>>>> - pcie2x1l2_3v3: regulator-pcie2x1l2-3v3 { >>>>> + vcc3v3_pcie2x1l2: regulator-vcc3v3_pcie2x1l2 { >>>> >>>> No, neither explained, nor correct. See DTS coding style. >>>> >>>> Please use name for all fixed regulators which matches current format >>>> recommendation: 'regulator-[0-9]v[0-9]' >>>> >>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/ >>>> tree/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml? >>>> h=v6.11-rc1#n46 >>> >>> 'regulator-[0-9]v[0-9]' is preferred, but 'regulator-[0-9a-z-]+' is also >>> permitted, right? >>> >>> i.e. regulator-vcc3v3_pcie2x1l2 should be regulator-vcc3v3-pcie2x1l2 >>> >>> >>> Or, should we revert below patch and use 'regulator-[0-9]v[0-9]'? >>> >>> https://lore.kernel.org/ >>> all/0ae40493-93e9-40cd-9ca9-990ae064f21a@gmail.com/ >>> >>> Is 'regulator-0v0' valid? > > Why would it be valid? Can you have regulator with 0 volts? There may be a .dtsi that is shared across multiple .dts, so some regulators may not be able to set a specific voltage in the .dtsi. How should I describe it? >>> Is 'regulator-12v0' invalid? > > Read the binding. I gave you very specific link. 46| - description: Preferred name is 'regulator-[0-9]v[0-9]' 47| pattern: '^regulator(-[0-9]+v[0-9]+|-[0-9a-z-]+)?$' The "description" and "pattern" don't match. What you provided is a link to the "description". >>> How should we handle multiple 1v8/3v3/5v0 regulators? > > Just add suffix. But usually more than one suffix, vcc+3v3+pcie_2x1l2, > means you created a very specific name. So shouldn't we refer to the schematic? Best regards, -- FUKAUMI Naoki Radxa Computer (Shenzhen) Co., Ltd. > Best regards, > Krzysztof >
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts index 85589d1a6d3b..61d75ab503b2 100644 --- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts +++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts @@ -76,13 +76,13 @@ pwm-fan { pwms = <&pwm3 0 60000 0>; }; - pcie2x1l2_3v3: regulator-pcie2x1l2-3v3 { + vcc3v3_pcie2x1l2: regulator-vcc3v3_pcie2x1l2 { compatible = "regulator-fixed"; enable-active-high; gpio = <&gpio0 RK_PC5 GPIO_ACTIVE_HIGH>; pinctrl-names = "default"; pinctrl-0 = <&pow_en>; - regulator-name = "pcie2x1l2_3v3"; + regulator-name = "vcc3v3_pcie2x1l2"; regulator-min-microvolt = <3300000>; regulator-max-microvolt = <3300000>; vin-supply = <&vcc_sysin>; @@ -421,7 +421,7 @@ &pcie2x1l2 { pinctrl-names = "default"; pinctrl-0 = <&pcie20x1_2_perstn_m0>; reset-gpios = <&gpio3 RK_PD1 GPIO_ACTIVE_HIGH>; - vpcie3v3-supply = <&pcie2x1l2_3v3>; + vpcie3v3-supply = <&vcc3v3_pcie2x1l2>; status = "okay"; };
Use consistent name with other regulators. No functional change. Fixes: 3ddf5cdb77e6 ("arm64: dts: rockchip: add Radxa ROCK 5C") Signed-off-by: FUKAUMI Naoki <naoki@radxa.com> --- Changes in v5: - Reword commit message Changes in v4: - reword commit message Changes in v3: - none Changes in v2: - new --- arch/arm64/boot/dts/rockchip/rk3588s-rock-5c.dts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)