diff mbox series

[v5,04/12] arm64: dts: rockchip: Rename regulator for pcie2x1l2 for Radxa ROCK 5C

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

Commit Message

FUKAUMI Naoki Dec. 16, 2024, 11:30 a.m. UTC
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(-)

Comments

Krzysztof Kozlowski Dec. 16, 2024, 1:38 p.m. UTC | #1
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
FUKAUMI Naoki Dec. 16, 2024, 1:55 p.m. UTC | #2
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
>
FUKAUMI Naoki Dec. 17, 2024, 1:11 a.m. UTC | #3
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
>
diff mbox series

Patch

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";
 };