diff mbox series

[v1,1/3] riscv: dts: starfive: Enable axp15060 pmic for cpufreq

Message ID 20230411083257.16155-2-mason.huo@starfivetech.com (mailing list archive)
State Superseded
Headers show
Series Add JH7110 cpufreq support | expand

Checks

Context Check Description
conchuod/tree_selection fail Failed to apply to next/pending-fixes or riscv/for-next

Commit Message

Mason Huo April 11, 2023, 8:32 a.m. UTC
The VisionFive 2 board has an embedded pmic axp15060,
which supports the cpu DVFS through the dcdc2 regulator.
This patch enables axp15060 pmic and configs the dcdc2.

Signed-off-by: Mason Huo <mason.huo@starfivetech.com>
---
 .../starfive/jh7110-starfive-visionfive-2.dtsi    | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

Conor Dooley April 11, 2023, 9:13 a.m. UTC | #1
On Tue, Apr 11, 2023 at 04:32:55PM +0800, Mason Huo wrote:
> The VisionFive 2 board has an embedded pmic axp15060,
> which supports the cpu DVFS through the dcdc2 regulator.
> This patch enables axp15060 pmic and configs the dcdc2.
> 
> Signed-off-by: Mason Huo <mason.huo@starfivetech.com>
> ---
>  .../starfive/jh7110-starfive-visionfive-2.dtsi    | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> index 2a6d81609284..df582bddae4b 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> @@ -114,6 +114,21 @@ &i2c5 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&i2c5_pins>;
>  	status = "okay";
> +
> +	pmic: axp15060_reg@36 {

No underscores in node names please & "pmic" is the generic node name
for pmics.

Cheers,
Conor.

> +		compatible = "x-powers,axp15060";
> +		reg = <0x36>;
> +
> +		regulators {
> +			reg_dcdc2: dcdc2 {
> +				regulator-boot-on;
> +				regulator-always-on;
> +				regulator-min-microvolt = <500000>;
> +				regulator-max-microvolt = <1540000>;
> +				regulator-name = "vdd-cpu";
> +			};
> +		};
> +	};
>  };
>  
>  &i2c6 {
> -- 
> 2.39.2
>
Qu Shengyu April 11, 2023, 2:49 p.m. UTC | #2
> On Tue, Apr 11, 2023 at 04:32:55PM +0800, Mason Huo wrote:
>> The VisionFive 2 board has an embedded pmic axp15060,
>> which supports the cpu DVFS through the dcdc2 regulator.
>> This patch enables axp15060 pmic and configs the dcdc2.
>>
>> Signed-off-by: Mason Huo <mason.huo@starfivetech.com>
>> ---
>>   .../starfive/jh7110-starfive-visionfive-2.dtsi    | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>> index 2a6d81609284..df582bddae4b 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>> @@ -114,6 +114,21 @@ &i2c5 {
>>   	pinctrl-names = "default";
>>   	pinctrl-0 = <&i2c5_pins>;
>>   	status = "okay";
>> +
>> +	pmic: axp15060_reg@36 {
> No underscores in node names please & "pmic" is the generic node name
> for pmics.
>
> Cheers,
> Conor.
>
>> +		compatible = "x-powers,axp15060";
>> +		reg = <0x36>;
>> +
>> +		regulators {
>> +			reg_dcdc2: dcdc2 {

reg_dcdc2 seems not a good name, too generic for identification. In most

cases, it's same as regulator-name but using "_" rather than "-".

>> +				regulator-boot-on;

It should not be used,  in Documentation/devicetree/bindings/regulator

/regulator.yaml, it is described as follows:

"This property is intended to only be used for regulators where software

cannot read the state of the regulator."

In this case, regulator state is completely able to be read by driver.

Best regards,

Shengyu
Mason Huo April 12, 2023, 2:36 a.m. UTC | #3
On 2023/4/11 17:13, Conor Dooley wrote:
> On Tue, Apr 11, 2023 at 04:32:55PM +0800, Mason Huo wrote:
>> The VisionFive 2 board has an embedded pmic axp15060,
>> which supports the cpu DVFS through the dcdc2 regulator.
>> This patch enables axp15060 pmic and configs the dcdc2.
>> 
>> Signed-off-by: Mason Huo <mason.huo@starfivetech.com>
>> ---
>>  .../starfive/jh7110-starfive-visionfive-2.dtsi    | 15 +++++++++++++++
>>  1 file changed, 15 insertions(+)
>> 
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>> index 2a6d81609284..df582bddae4b 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>> @@ -114,6 +114,21 @@ &i2c5 {
>>  	pinctrl-names = "default";
>>  	pinctrl-0 = <&i2c5_pins>;
>>  	status = "okay";
>> +
>> +	pmic: axp15060_reg@36 {
> 
> No underscores in node names please & "pmic" is the generic node name
> for pmics.
> 
> Cheers,
> Conor.
> 
Address it in next version.

Thanks
Mason
>> +		compatible = "x-powers,axp15060";
>> +		reg = <0x36>;
>> +
>> +		regulators {
>> +			reg_dcdc2: dcdc2 {
>> +				regulator-boot-on;
>> +				regulator-always-on;
>> +				regulator-min-microvolt = <500000>;
>> +				regulator-max-microvolt = <1540000>;
>> +				regulator-name = "vdd-cpu";
>> +			};
>> +		};
>> +	};
>>  };
>>  
>>  &i2c6 {
>> -- 
>> 2.39.2
>>
Mason Huo April 12, 2023, 2:36 a.m. UTC | #4
On 2023/4/11 22:49, Shengyu Qu wrote:
>> On Tue, Apr 11, 2023 at 04:32:55PM +0800, Mason Huo wrote:
>>> The VisionFive 2 board has an embedded pmic axp15060,
>>> which supports the cpu DVFS through the dcdc2 regulator.
>>> This patch enables axp15060 pmic and configs the dcdc2.
>>>
>>> Signed-off-by: Mason Huo <mason.huo@starfivetech.com>
>>> ---
>>>   .../starfive/jh7110-starfive-visionfive-2.dtsi    | 15 +++++++++++++++
>>>   1 file changed, 15 insertions(+)
>>>
>>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>>> index 2a6d81609284..df582bddae4b 100644
>>> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>>> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>>> @@ -114,6 +114,21 @@ &i2c5 {
>>>       pinctrl-names = "default";
>>>       pinctrl-0 = <&i2c5_pins>;
>>>       status = "okay";
>>> +
>>> +    pmic: axp15060_reg@36 {
>> No underscores in node names please & "pmic" is the generic node name
>> for pmics.
>>
>> Cheers,
>> Conor.
>>
>>> +        compatible = "x-powers,axp15060";
>>> +        reg = <0x36>;
>>> +
>>> +        regulators {
>>> +            reg_dcdc2: dcdc2 {
> 
> reg_dcdc2 seems not a good name, too generic for identification. In most
> 
> cases, it's same as regulator-name but using "_" rather than "-".
> 
Hi Shengyu,

Thanks for your review.
Will change to "vdd_cpu".

>>> +                regulator-boot-on;
> 
> It should not be used,  in Documentation/devicetree/bindings/regulator
> 
> /regulator.yaml, it is described as follows:
> 
> "This property is intended to only be used for regulators where software
> 
> cannot read the state of the regulator."
> 
> In this case, regulator state is completely able to be read by driver.
> 
> Best regards,
> 
> Shengyu
>Will remove it.

Thanks
Mason
diff mbox series

Patch

diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
index 2a6d81609284..df582bddae4b 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
@@ -114,6 +114,21 @@  &i2c5 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&i2c5_pins>;
 	status = "okay";
+
+	pmic: axp15060_reg@36 {
+		compatible = "x-powers,axp15060";
+		reg = <0x36>;
+
+		regulators {
+			reg_dcdc2: dcdc2 {
+				regulator-boot-on;
+				regulator-always-on;
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <1540000>;
+				regulator-name = "vdd-cpu";
+			};
+		};
+	};
 };
 
 &i2c6 {