diff mbox series

[v4,2/5] MIPS: DTS: CI20: fix PMU definitions for ACT8600

Message ID af70bb34d95746cdbc468e91e531c4576a1855a6.1582912972.git.hns@goldelico.com (mailing list archive)
State Superseded
Headers show
Series MIPS: Fixes and improvements for CI20 board (JZ4780) | expand

Commit Message

H. Nikolaus Schaller Feb. 28, 2020, 6:02 p.m. UTC
There is a ACT8600 on the CI20 board and the bindings of the
ACT8865 driver have changed without updating the CI20 device
tree. Therefore the PMU can not be probed successfully and
is running in power-on reset state.

Fix DT to match the latest act8865-regulator bindings.

Fixes: 73f2b940474d ("MIPS: CI20: DTS: Add I2C nodes")
Cc: stable@vger.kernel.org
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 arch/mips/boot/dts/ingenic/ci20.dts | 48 ++++++++++++++++++++---------
 1 file changed, 33 insertions(+), 15 deletions(-)

Comments

Paul Cercueil Feb. 29, 2020, 4:09 p.m. UTC | #1
Hi Nikolaus,


Le ven., févr. 28, 2020 at 19:02, H. Nikolaus Schaller 
<hns@goldelico.com> a écrit :
> There is a ACT8600 on the CI20 board and the bindings of the
> ACT8865 driver have changed without updating the CI20 device
> tree. Therefore the PMU can not be probed successfully and
> is running in power-on reset state.
> 
> Fix DT to match the latest act8865-regulator bindings.
> 
> Fixes: 73f2b940474d ("MIPS: CI20: DTS: Add I2C nodes")
> Cc: stable@vger.kernel.org
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
>  arch/mips/boot/dts/ingenic/ci20.dts | 48 
> ++++++++++++++++++++---------
>  1 file changed, 33 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/mips/boot/dts/ingenic/ci20.dts 
> b/arch/mips/boot/dts/ingenic/ci20.dts
> index 59c104289ece..44741e927d2b 100644
> --- a/arch/mips/boot/dts/ingenic/ci20.dts
> +++ b/arch/mips/boot/dts/ingenic/ci20.dts
> @@ -4,6 +4,8 @@
>  #include "jz4780.dtsi"
>  #include <dt-bindings/clock/ingenic,tcu.h>
>  #include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/interrupt-controller/irq.h>

This include should be in patch 3/5 where it's first used.

With that fixed:
Reviewed-by: Paul Cercueil <paul@crapouillou.net>

for the whole series.

Cheers,
-Paul

> +#include <dt-bindings/regulator/active-semi,8865-regulator.h>
> 
>  / {
>  	compatible = "img,ci20", "ingenic,jz4780";
> @@ -166,65 +168,81 @@
>  		reg = <0x5a>;
>  		status = "okay";
> 
> +/*
> +Optional input supply properties:
> +- for act8600:
> +  - vp1-supply: The input supply for DCDC_REG1
> +  - vp2-supply: The input supply for DCDC_REG2
> +  - vp3-supply: The input supply for DCDC_REG3
> +  - inl-supply: The input supply for LDO_REG5, LDO_REG6, LDO_REG7 
> and LDO_REG8
> +  SUDCDC_REG4, LDO_REG9 and LDO_REG10 do not have separate supplies.
> +*/
> +
>  		regulators {
>  			vddcore: SUDCDC1 {
> -				regulator-name = "VDDCORE";
> +				regulator-name = "DCDC_REG1";
>  				regulator-min-microvolt = <1100000>;
>  				regulator-max-microvolt = <1100000>;
>  				regulator-always-on;
>  			};
>  			vddmem: SUDCDC2 {
> -				regulator-name = "VDDMEM";
> +				regulator-name = "DCDC_REG2";
>  				regulator-min-microvolt = <1500000>;
>  				regulator-max-microvolt = <1500000>;
>  				regulator-always-on;
>  			};
>  			vcc_33: SUDCDC3 {
> -				regulator-name = "VCC33";
> +				regulator-name = "DCDC_REG3";
>  				regulator-min-microvolt = <3300000>;
>  				regulator-max-microvolt = <3300000>;
>  				regulator-always-on;
>  			};
>  			vcc_50: SUDCDC4 {
> -				regulator-name = "VCC50";
> +				regulator-name = "SUDCDC_REG4";
>  				regulator-min-microvolt = <5000000>;
>  				regulator-max-microvolt = <5000000>;
>  				regulator-always-on;
>  			};
>  			vcc_25: LDO_REG5 {
> -				regulator-name = "VCC25";
> +				regulator-name = "LDO_REG5";
>  				regulator-min-microvolt = <2500000>;
>  				regulator-max-microvolt = <2500000>;
>  				regulator-always-on;
>  			};
>  			wifi_io: LDO_REG6 {
> -				regulator-name = "WIFIIO";
> +				regulator-name = "LDO_REG6";
>  				regulator-min-microvolt = <2500000>;
>  				regulator-max-microvolt = <2500000>;
>  				regulator-always-on;
>  			};
>  			vcc_28: LDO_REG7 {
> -				regulator-name = "VCC28";
> +				regulator-name = "LDO_REG7";
>  				regulator-min-microvolt = <2800000>;
>  				regulator-max-microvolt = <2800000>;
>  				regulator-always-on;
>  			};
>  			vcc_15: LDO_REG8 {
> -				regulator-name = "VCC15";
> +				regulator-name = "LDO_REG8";
>  				regulator-min-microvolt = <1500000>;
>  				regulator-max-microvolt = <1500000>;
>  				regulator-always-on;
>  			};
> -			vcc_18: LDO_REG9 {
> -				regulator-name = "VCC18";
> -				regulator-min-microvolt = <1800000>;
> -				regulator-max-microvolt = <1800000>;
> +			vrtc_18: LDO_REG9 {
> +				regulator-name = "LDO_REG9";
> +				/* Despite the datasheet stating 3.3V for REG9 and
> +				   driver expecting that, REG9 outputs 1.8V.
> +				   Likely the CI20 uses a chip variant.
> +				   Since it is a simple on/off LDO the exact values
> +				   do not matter.
> +				*/
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;
>  				regulator-always-on;
>  			};
>  			vcc_11: LDO_REG10 {
> -				regulator-name = "VCC11";
> -				regulator-min-microvolt = <1100000>;
> -				regulator-max-microvolt = <1100000>;
> +				regulator-name = "LDO_REG10";
> +				regulator-min-microvolt = <1200000>;
> +				regulator-max-microvolt = <1200000>;
>  				regulator-always-on;
>  			};
>  		};
> --
> 2.23.0
>
H. Nikolaus Schaller Feb. 29, 2020, 6:58 p.m. UTC | #2
Hi Paul,

> Am 29.02.2020 um 17:09 schrieb Paul Cercueil <paul@crapouillou.net>:
> 
> Hi Nikolaus,
> 
> 
> Le ven., févr. 28, 2020 at 19:02, H. Nikolaus Schaller <hns@goldelico.com> a écrit :
>> There is a ACT8600 on the CI20 board and the bindings of the
>> ACT8865 driver have changed without updating the CI20 device
>> tree. Therefore the PMU can not be probed successfully and
>> is running in power-on reset state.
>> Fix DT to match the latest act8865-regulator bindings.
>> Fixes: 73f2b940474d ("MIPS: CI20: DTS: Add I2C nodes")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>> ---
>> arch/mips/boot/dts/ingenic/ci20.dts | 48 ++++++++++++++++++++---------
>> 1 file changed, 33 insertions(+), 15 deletions(-)
>> diff --git a/arch/mips/boot/dts/ingenic/ci20.dts b/arch/mips/boot/dts/ingenic/ci20.dts
>> index 59c104289ece..44741e927d2b 100644
>> --- a/arch/mips/boot/dts/ingenic/ci20.dts
>> +++ b/arch/mips/boot/dts/ingenic/ci20.dts
>> @@ -4,6 +4,8 @@
>> #include "jz4780.dtsi"
>> #include <dt-bindings/clock/ingenic,tcu.h>
>> #include <dt-bindings/gpio/gpio.h>
>> +#include <dt-bindings/interrupt-controller/irq.h>
> 
> This include should be in patch 3/5 where it's first used.

Yes. That is much better.

> 
> With that fixed:
> Reviewed-by: Paul Cercueil <paul@crapouillou.net>
> 
> for the whole series.

What is easier: that you fix it during applying somewhere
or should I send a v5?

BR,
Nikolaus
Paul Cercueil Feb. 29, 2020, 7:33 p.m. UTC | #3
Hi Nikolaus,


Le sam., févr. 29, 2020 at 19:58, H. Nikolaus Schaller 
<hns@goldelico.com> a écrit :
> Hi Paul,
> 
>>  Am 29.02.2020 um 17:09 schrieb Paul Cercueil <paul@crapouillou.net>:
>> 
>>  Hi Nikolaus,
>> 
>> 
>>  Le ven., févr. 28, 2020 at 19:02, H. Nikolaus Schaller 
>> <hns@goldelico.com> a écrit :
>>>  There is a ACT8600 on the CI20 board and the bindings of the
>>>  ACT8865 driver have changed without updating the CI20 device
>>>  tree. Therefore the PMU can not be probed successfully and
>>>  is running in power-on reset state.
>>>  Fix DT to match the latest act8865-regulator bindings.
>>>  Fixes: 73f2b940474d ("MIPS: CI20: DTS: Add I2C nodes")
>>>  Cc: stable@vger.kernel.org
>>>  Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>>>  ---
>>>  arch/mips/boot/dts/ingenic/ci20.dts | 48 
>>> ++++++++++++++++++++---------
>>>  1 file changed, 33 insertions(+), 15 deletions(-)
>>>  diff --git a/arch/mips/boot/dts/ingenic/ci20.dts 
>>> b/arch/mips/boot/dts/ingenic/ci20.dts
>>>  index 59c104289ece..44741e927d2b 100644
>>>  --- a/arch/mips/boot/dts/ingenic/ci20.dts
>>>  +++ b/arch/mips/boot/dts/ingenic/ci20.dts
>>>  @@ -4,6 +4,8 @@
>>>  #include "jz4780.dtsi"
>>>  #include <dt-bindings/clock/ingenic,tcu.h>
>>>  #include <dt-bindings/gpio/gpio.h>
>>>  +#include <dt-bindings/interrupt-controller/irq.h>
>> 
>>  This include should be in patch 3/5 where it's first used.
> 
> Yes. That is much better.
> 
>> 
>>  With that fixed:
>>  Reviewed-by: Paul Cercueil <paul@crapouillou.net>
>> 
>>  for the whole series.
> 
> What is easier: that you fix it during applying somewhere
> or should I send a v5?

I guess send a V5.

- Paul

> 
> BR,
> Nikolaus
>
diff mbox series

Patch

diff --git a/arch/mips/boot/dts/ingenic/ci20.dts b/arch/mips/boot/dts/ingenic/ci20.dts
index 59c104289ece..44741e927d2b 100644
--- a/arch/mips/boot/dts/ingenic/ci20.dts
+++ b/arch/mips/boot/dts/ingenic/ci20.dts
@@ -4,6 +4,8 @@ 
 #include "jz4780.dtsi"
 #include <dt-bindings/clock/ingenic,tcu.h>
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/regulator/active-semi,8865-regulator.h>
 
 / {
 	compatible = "img,ci20", "ingenic,jz4780";
@@ -166,65 +168,81 @@ 
 		reg = <0x5a>;
 		status = "okay";
 
+/*
+Optional input supply properties:
+- for act8600:
+  - vp1-supply: The input supply for DCDC_REG1
+  - vp2-supply: The input supply for DCDC_REG2
+  - vp3-supply: The input supply for DCDC_REG3
+  - inl-supply: The input supply for LDO_REG5, LDO_REG6, LDO_REG7 and LDO_REG8
+  SUDCDC_REG4, LDO_REG9 and LDO_REG10 do not have separate supplies.
+*/
+
 		regulators {
 			vddcore: SUDCDC1 {
-				regulator-name = "VDDCORE";
+				regulator-name = "DCDC_REG1";
 				regulator-min-microvolt = <1100000>;
 				regulator-max-microvolt = <1100000>;
 				regulator-always-on;
 			};
 			vddmem: SUDCDC2 {
-				regulator-name = "VDDMEM";
+				regulator-name = "DCDC_REG2";
 				regulator-min-microvolt = <1500000>;
 				regulator-max-microvolt = <1500000>;
 				regulator-always-on;
 			};
 			vcc_33: SUDCDC3 {
-				regulator-name = "VCC33";
+				regulator-name = "DCDC_REG3";
 				regulator-min-microvolt = <3300000>;
 				regulator-max-microvolt = <3300000>;
 				regulator-always-on;
 			};
 			vcc_50: SUDCDC4 {
-				regulator-name = "VCC50";
+				regulator-name = "SUDCDC_REG4";
 				regulator-min-microvolt = <5000000>;
 				regulator-max-microvolt = <5000000>;
 				regulator-always-on;
 			};
 			vcc_25: LDO_REG5 {
-				regulator-name = "VCC25";
+				regulator-name = "LDO_REG5";
 				regulator-min-microvolt = <2500000>;
 				regulator-max-microvolt = <2500000>;
 				regulator-always-on;
 			};
 			wifi_io: LDO_REG6 {
-				regulator-name = "WIFIIO";
+				regulator-name = "LDO_REG6";
 				regulator-min-microvolt = <2500000>;
 				regulator-max-microvolt = <2500000>;
 				regulator-always-on;
 			};
 			vcc_28: LDO_REG7 {
-				regulator-name = "VCC28";
+				regulator-name = "LDO_REG7";
 				regulator-min-microvolt = <2800000>;
 				regulator-max-microvolt = <2800000>;
 				regulator-always-on;
 			};
 			vcc_15: LDO_REG8 {
-				regulator-name = "VCC15";
+				regulator-name = "LDO_REG8";
 				regulator-min-microvolt = <1500000>;
 				regulator-max-microvolt = <1500000>;
 				regulator-always-on;
 			};
-			vcc_18: LDO_REG9 {
-				regulator-name = "VCC18";
-				regulator-min-microvolt = <1800000>;
-				regulator-max-microvolt = <1800000>;
+			vrtc_18: LDO_REG9 {
+				regulator-name = "LDO_REG9";
+				/* Despite the datasheet stating 3.3V for REG9 and
+				   driver expecting that, REG9 outputs 1.8V.
+				   Likely the CI20 uses a chip variant.
+				   Since it is a simple on/off LDO the exact values
+				   do not matter.
+				*/
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
 				regulator-always-on;
 			};
 			vcc_11: LDO_REG10 {
-				regulator-name = "VCC11";
-				regulator-min-microvolt = <1100000>;
-				regulator-max-microvolt = <1100000>;
+				regulator-name = "LDO_REG10";
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <1200000>;
 				regulator-always-on;
 			};
 		};