[2/9] MIPS: DTS: CI20: Fix ACT8600 regulator node names

Message ID 20230604145642.200577-3-paul@crapouillou.net
State New
Headers
Series MIPS: CI20: Add WiFi / Bluetooth support |

Commit Message

Paul Cercueil June 4, 2023, 2:56 p.m. UTC
  The Device Tree was using invalid node names for the ACT8600 regulators.
To be fair, it is not the original committer's fault, as the
documentation did gives invalid names as well.

In theory, the fix should have been to modify the driver to accept the
alternative names. However, even though the act8865 driver spits
warnings, the kernel seemed to work fine with what is currently
supported upstream. For that reason, I think it is okay to just update
the DTS.

I removed the "regulator-name" too, since they really didn't bring any
information. The node names are enough.

Fixes: 73f2b940474d ("MIPS: CI20: DTS: Add I2C nodes")
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 arch/mips/boot/dts/ingenic/ci20.dts | 27 ++++++++-------------------
 1 file changed, 8 insertions(+), 19 deletions(-)
  

Comments

H. Nikolaus Schaller June 15, 2023, 7 a.m. UTC | #1
> Am 04.06.2023 um 16:56 schrieb Paul Cercueil <paul@crapouillou.net>:
> 
> The Device Tree was using invalid node names for the ACT8600 regulators.
> To be fair, it is not the original committer's fault, as the
> documentation did gives invalid names as well.

s/did gives /did give /

> 
> In theory, the fix should have been to modify the driver to accept the
> alternative names. However, even though the act8865 driver spits
> warnings, the kernel seemed to work fine with what is currently
> supported upstream. For that reason, I think it is okay to just update
> the DTS.
> 
> I removed the "regulator-name" too, since they really didn't bring any
> information. The node names are enough.

For me this patch breaks boot on my CI20 V2a - but I don't see why.
Maybe the driver or something else relies on regulator names indirectly?

Last and only signs of activity:

U-Boot 2013.10-rc3-00096-gef995a1-dirty (Apr 13 2019 - 19:15:18)

Board: ci20 (r2) (Ingenic XBurst JZ4780 SoC)
DRAM:  1 GiB
NAND:  8192 MiB
MMC:   jz_mmc msc1: 0
*** Warning - bad CRC, using default environment

In:    eserial4
Out:   eserial4
Err:   eserial4
Net:   dm9000
Hit any key to stop autoboot:  0 
4357173 bytes read in 724 ms (5.7 MiB/s)
## Booting kernel from Legacy Image at 88000000 ...
   Image Name:   Linux-6.4.0-rc6+
   Image Type:   MIPS Linux Kernel Image (gzip compressed)
   Data Size:    4357109 Bytes = 4.2 MiB
   Load Address: 80100000
   Entry Point:  80718080
   Verifying Checksum ... OK
   Uncompressing Kernel Image ... OK

Starting kernel ...

[    0.070854] jz4780-nemc 13410000.nemc: /nemc@13410000/efuse@d0 requests invalid bank 0
[    0.078858] jz4780-nemc 13410000.nemc: /nemc@13410000/efuse@d0 has no addresses
[    0.109013] jz4740-rtc 10003000.rtc: hctosys: unable to read the hardware clock
[    0.199104] dm9000 16000000.dm9000: read wrong id 0x00000a46

--- hangs ---

> 
> Fixes: 73f2b940474d ("MIPS: CI20: DTS: Add I2C nodes")
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
> arch/mips/boot/dts/ingenic/ci20.dts | 27 ++++++++-------------------
> 1 file changed, 8 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/mips/boot/dts/ingenic/ci20.dts b/arch/mips/boot/dts/ingenic/ci20.dts
> index e76953dce2e7..5361606c5e13 100644
> --- a/arch/mips/boot/dts/ingenic/ci20.dts
> +++ b/arch/mips/boot/dts/ingenic/ci20.dts
> @@ -237,59 +237,49 @@ &i2c0 {
> 	act8600: act8600@5a {
> 		compatible = "active-semi,act8600";
> 		reg = <0x5a>;
> -		status = "okay";
> 
> 		regulators {
> -			vddcore: SUDCDC1 {
> -				regulator-name = "DCDC_REG1";
> +			vddcore: DCDC1 {
> 				regulator-min-microvolt = <1100000>;
> 				regulator-max-microvolt = <1100000>;
> 				regulator-always-on;
> 			};
> -			vddmem: SUDCDC2 {
> -				regulator-name = "DCDC_REG2";
> +			vddmem: DCDC2 {
> 				regulator-min-microvolt = <1500000>;
> 				regulator-max-microvolt = <1500000>;
> 				regulator-always-on;
> 			};
> -			vcc_33: SUDCDC3 {
> -				regulator-name = "DCDC_REG3";
> +			vcc_33: DCDC3 {
> 				regulator-min-microvolt = <3300000>;
> 				regulator-max-microvolt = <3300000>;
> 				regulator-always-on;
> 			};
> -			vcc_50: SUDCDC4 {
> -				regulator-name = "SUDCDC_REG4";
> +			vcc_50: SUDCDC_REG4 {
> 				regulator-min-microvolt = <5000000>;
> 				regulator-max-microvolt = <5000000>;
> 				regulator-always-on;
> 			};
> -			vcc_25: LDO_REG5 {
> -				regulator-name = "LDO_REG5";
> +			vcc_25: LDO5 {
> 				regulator-min-microvolt = <2500000>;
> 				regulator-max-microvolt = <2500000>;
> 				regulator-always-on;
> 			};
> -			wifi_io: LDO_REG6 {
> -				regulator-name = "LDO_REG6";
> +			wifi_io: LDO6 {
> 				regulator-min-microvolt = <2500000>;
> 				regulator-max-microvolt = <2500000>;
> 				regulator-always-on;
> 			};
> -			vcc_28: LDO_REG7 {
> -				regulator-name = "LDO_REG7";
> +			cim_io_28: LDO7 {
> 				regulator-min-microvolt = <2800000>;
> 				regulator-max-microvolt = <2800000>;
> 				regulator-always-on;
> 			};
> -			vcc_15: LDO_REG8 {
> -				regulator-name = "LDO_REG8";
> +			cim_io_15: LDO8 {
> 				regulator-min-microvolt = <1500000>;
> 				regulator-max-microvolt = <1500000>;
> 				regulator-always-on;
> 			};
> 			vrtc_18: LDO_REG9 {
> -				regulator-name = "LDO_REG9";
> 				/* Despite the datasheet stating 3.3V
> 				 * for REG9 and the driver expecting that,
> 				 * REG9 outputs 1.8V.
> @@ -303,7 +293,6 @@ vrtc_18: LDO_REG9 {
> 				regulator-always-on;
> 			};
> 			vcc_11: LDO_REG10 {
> -				regulator-name = "LDO_REG10";
> 				regulator-min-microvolt = <1200000>;
> 				regulator-max-microvolt = <1200000>;
> 				regulator-always-on;
> -- 
> 2.39.2
>
  

Patch

diff --git a/arch/mips/boot/dts/ingenic/ci20.dts b/arch/mips/boot/dts/ingenic/ci20.dts
index e76953dce2e7..5361606c5e13 100644
--- a/arch/mips/boot/dts/ingenic/ci20.dts
+++ b/arch/mips/boot/dts/ingenic/ci20.dts
@@ -237,59 +237,49 @@  &i2c0 {
 	act8600: act8600@5a {
 		compatible = "active-semi,act8600";
 		reg = <0x5a>;
-		status = "okay";
 
 		regulators {
-			vddcore: SUDCDC1 {
-				regulator-name = "DCDC_REG1";
+			vddcore: DCDC1 {
 				regulator-min-microvolt = <1100000>;
 				regulator-max-microvolt = <1100000>;
 				regulator-always-on;
 			};
-			vddmem: SUDCDC2 {
-				regulator-name = "DCDC_REG2";
+			vddmem: DCDC2 {
 				regulator-min-microvolt = <1500000>;
 				regulator-max-microvolt = <1500000>;
 				regulator-always-on;
 			};
-			vcc_33: SUDCDC3 {
-				regulator-name = "DCDC_REG3";
+			vcc_33: DCDC3 {
 				regulator-min-microvolt = <3300000>;
 				regulator-max-microvolt = <3300000>;
 				regulator-always-on;
 			};
-			vcc_50: SUDCDC4 {
-				regulator-name = "SUDCDC_REG4";
+			vcc_50: SUDCDC_REG4 {
 				regulator-min-microvolt = <5000000>;
 				regulator-max-microvolt = <5000000>;
 				regulator-always-on;
 			};
-			vcc_25: LDO_REG5 {
-				regulator-name = "LDO_REG5";
+			vcc_25: LDO5 {
 				regulator-min-microvolt = <2500000>;
 				regulator-max-microvolt = <2500000>;
 				regulator-always-on;
 			};
-			wifi_io: LDO_REG6 {
-				regulator-name = "LDO_REG6";
+			wifi_io: LDO6 {
 				regulator-min-microvolt = <2500000>;
 				regulator-max-microvolt = <2500000>;
 				regulator-always-on;
 			};
-			vcc_28: LDO_REG7 {
-				regulator-name = "LDO_REG7";
+			cim_io_28: LDO7 {
 				regulator-min-microvolt = <2800000>;
 				regulator-max-microvolt = <2800000>;
 				regulator-always-on;
 			};
-			vcc_15: LDO_REG8 {
-				regulator-name = "LDO_REG8";
+			cim_io_15: LDO8 {
 				regulator-min-microvolt = <1500000>;
 				regulator-max-microvolt = <1500000>;
 				regulator-always-on;
 			};
 			vrtc_18: LDO_REG9 {
-				regulator-name = "LDO_REG9";
 				/* Despite the datasheet stating 3.3V
 				 * for REG9 and the driver expecting that,
 				 * REG9 outputs 1.8V.
@@ -303,7 +293,6 @@  vrtc_18: LDO_REG9 {
 				regulator-always-on;
 			};
 			vcc_11: LDO_REG10 {
-				regulator-name = "LDO_REG10";
 				regulator-min-microvolt = <1200000>;
 				regulator-max-microvolt = <1200000>;
 				regulator-always-on;