Cleanup Spreadtrum/Unisoc ARM64 DT (part 2)

PATCH Superseded Stanislav Jakubek 4 participants 12 messages Aug 15, 2024 – Nov 1, 2024

Subsystems (2)

Device tree bindings Arm/spreadtrum SoC support

lore

Share & tools

lore

Stanislav Jakubek

author

2024-08-15 10:12 UTC #0 lore raw

cover Cleanup Spreadtrum/Unisoc ARM64 DT (part 2)

This series continues cleaning up various issues with the Spreadtrum/Unisoc ARM64 DeviceTrees.

Stanislav Jakubek (3):
arm64: dts: sprd: sp9860g-1h10: fix
constant-charge-voltage-max-microvolt property
arm64: dts: sprd: sc2731: fix bat-detect-gpios property
arm64: dts: sprd: sc2731: move fuel-gauge monitored-battery to device
DTS

 arch/arm64/boot/dts/sprd/sc2731.dtsi      | 6 +++---
 arch/arm64/boot/dts/sprd/sp9860g-1h10.dts | 7 ++++++-
 2 files changed, 9 insertions(+), 4 deletions(-)

Signature · 1 line

2.34.1

Stanislav Jakubek

author

2024-08-15 10:12 UTC #1 lore raw

PATCH 1/3 arm64: dts: sprd: sp9860g-1h10: fix constant-charge-voltage-max-microvolt property

This property has hyphens/dashes, not underscores.

Signed-off-by Stanislav Jakubek <[email protected]>

---
 arch/arm64/boot/dts/sprd/sp9860g-1h10.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm64/boot/dts/sprd/sp9860g-1h10.dts +1 -1

index 095b24a31313..cd8b1069e387 100644
@@ -71,7 +71,7 @@ bat: battery {
 		compatible = "simple-battery";
 		charge-full-design-microamp-hours = <1900000>;
 		charge-term-current-microamp = <120000>;
-		constant_charge_voltage_max_microvolt = <4350000>;
+		constant-charge-voltage-max-microvolt = <4350000>;
 		internal-resistance-micro-ohms = <250000>;
 		ocv-capacity-celsius = <20>;
 		ocv-capacity-table-0 = <4185000 100>, <4113000 95>, <4066000 90>,

Signature · 1 line

2.34.1

Baolin Wang

Reviewed-by on v1 1/3

2024-08-16 01:37 UTC #2 lore raw

Re: [PATCH 1/3] arm64: dts: sprd: sp9860g-1h10: fix constant-charge-voltage-max-microvolt property

Quoted 3 lines · Stanislav Jakubek

This property has hyphens/dashes, not underscores.
Signed-off-by Stanislav Jakubek <[email protected]>

Thanks for the fixing.

Reviewed-by Baolin Wang <[email protected]>

Quoted 17 lines

---
  arch/arm64/boot/dts/sprd/sp9860g-1h10.dts | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm64/boot/dts/sprd/sp9860g-1h10.dts +1 -1

index 095b24a31313..cd8b1069e387 100644
@@ -71,7 +71,7 @@ bat: battery {
  		compatible = "simple-battery";
  		charge-full-design-microamp-hours = <1900000>;
  		charge-term-current-microamp = <120000>;
-		constant_charge_voltage_max_microvolt = <4350000>;
+		constant-charge-voltage-max-microvolt = <4350000>;
  		internal-resistance-micro-ohms = <250000>;
  		ocv-capacity-celsius = <20>;
  		ocv-capacity-table-0 = <4185000 100>, <4113000 95>, <4066000 90>,

Stanislav Jakubek

author

2024-08-15 10:13 UTC #3 lore raw

PATCH 2/3 arm64: dts: sprd: sc2731: fix bat-detect-gpios property

In reply to Stanislav Jakubek

According to DT bindings, the property is called bat-detect-gpios, not bat-detect-gpio. Update the property as such.

Signed-off-by Stanislav Jakubek <[email protected]>

---
 arch/arm64/boot/dts/sprd/sc2731.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm64/boot/dts/sprd/sc2731.dtsi +1 -1

index 12136e68dada..c8b30c0479fd 100644
@@ -97,7 +97,7 @@ pmic_adc: adc@480 {
 		fuel-gauge@a00 {
 			compatible = "sprd,sc2731-fgu";
 			reg = <0xa00>;
-			bat-detect-gpio = <&pmic_eic 9 GPIO_ACTIVE_HIGH>;
+			bat-detect-gpios = <&pmic_eic 9 GPIO_ACTIVE_HIGH>;
 			io-channels = <&pmic_adc 3>, <&pmic_adc 6>;
 			io-channel-names = "bat-temp", "charge-vol";
 			monitored-battery = <&bat>;

Signature · 1 line

2.34.1

Baolin Wang

Reviewed-by on v1 2/3

2024-08-16 01:46 UTC #4 lore raw

Re: [PATCH 2/3] arm64: dts: sprd: sc2731: fix bat-detect-gpios property

Quoted 4 lines · Stanislav Jakubek

According to DT bindings, the property is called bat-detect-gpios, not bat-detect-gpio. Update the property as such.
Signed-off-by Stanislav Jakubek <[email protected]>

Thanks.

Reviewed-by Baolin Wang <[email protected]>

Quoted 17 lines

---
  arch/arm64/boot/dts/sprd/sc2731.dtsi | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm64/boot/dts/sprd/sc2731.dtsi +1 -1

index 12136e68dada..c8b30c0479fd 100644
@@ -97,7 +97,7 @@ pmic_adc: adc@480 {
  		fuel-gauge@a00 {
  			compatible = "sprd,sc2731-fgu";
  			reg = <0xa00>;
-			bat-detect-gpio = <&pmic_eic 9 GPIO_ACTIVE_HIGH>;
+			bat-detect-gpios = <&pmic_eic 9 GPIO_ACTIVE_HIGH>;
  			io-channels = <&pmic_adc 3>, <&pmic_adc 6>;
  			io-channel-names = "bat-temp", "charge-vol";
  			monitored-battery = <&bat>;

Stanislav Jakubek

author

2024-10-31 07:30 UTC #5 lore raw

Re: [PATCH 2/3] arm64: dts: sprd: sc2731: fix bat-detect-gpios property

In reply to Stanislav Jakubek

Quoted 24 lines · Stanislav Jakubek

According to DT bindings, the property is called bat-detect-gpios, not bat-detect-gpio. Update the property as such.

Signed-off-by Stanislav Jakubek <[email protected]>

---
 arch/arm64/boot/dts/sprd/sc2731.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm64/boot/dts/sprd/sc2731.dtsi +1 -1

index 12136e68dada..c8b30c0479fd 100644
@@ -97,7 +97,7 @@ pmic_adc: adc@480 {
 		fuel-gauge@a00 {
 			compatible = "sprd,sc2731-fgu";
 			reg = <0xa00>;
-			bat-detect-gpio = <&pmic_eic 9 GPIO_ACTIVE_HIGH>;
+			bat-detect-gpios = <&pmic_eic 9 GPIO_ACTIVE_HIGH>;
 			io-channels = <&pmic_adc 3>, <&pmic_adc 6>;
 			io-channel-names = "bat-temp", "charge-vol";
 			monitored-battery = <&bat>;

Signature · 1 line

2.34.1

Hi all,

looking at this patch again, it seems there's some confusion going on. The bindings specify the property as "battery-detect-gpios", but the driver checks for "bat-detect" [1]. I assume the DTS was added with "bat-detect" to make it work, even though it's not conformant to bindings. It has been this way since the bindings/driver/DTS were introduced back in 2018/2019.

What would be the correct way to fix this? Change the bindings to "bat-detect-gpios" or change the driver/DTS to "battery-detect-gpios"?

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/power/supply/sc27xx_fuel_gauge.c?h=v6.11#n1186

Regards,
Stanislav

Chunyan Zhang

reviewer

2024-11-01 01:53 UTC #6 lore raw

Re: [PATCH 2/3] arm64: dts: sprd: sc2731: fix bat-detect-gpios property

Quoted 36 lines · Stanislav Jakubek

Quoted 22 lines · Stanislav Jakubek
According to DT bindings, the property is called bat-detect-gpios, not bat-detect-gpio. Update the property as such.
Signed-off-by Stanislav Jakubek <[email protected]>
---
 arch/arm64/boot/dts/sprd/sc2731.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/boot/dts/sprd/sc2731.dtsi +1 -2
index 12136e68dada..c8b30c0479fd 100644
@@ -97,7 +97,7 @@ pmic_adc: adc@480 {
              fuel-gauge@a00 {
                      compatible = "sprd,sc2731-fgu";
                      reg = <0xa00>;
-                     bat-detect-gpio = <&pmic_eic 9 GPIO_ACTIVE_HIGH>;
+                     bat-detect-gpios = <&pmic_eic 9 GPIO_ACTIVE_HIGH>;
                      io-channels = <&pmic_adc 3>, <&pmic_adc 6>;
                      io-channel-names = "bat-temp", "charge-vol";
                      monitored-battery = <&bat>;
--
Quoted 2 lines
2.34.1
Hi all,
looking at this patch again, it seems there's some confusion going on. The bindings specify the property as "battery-detect-gpios", but the driver checks for "bat-detect" [1]. I assume the DTS was added with "bat-detect" to make it work, even though it's not conformant to bindings. It has been this way since the bindings/driver/DTS were introduced back in 2018/2019.

Quoted 2 lines

What would be the correct way to fix this? Change the bindings to "bat-detect-gpios" or change the driver/DTS to "battery-detect-gpios"?

I would prefer the second, i.e. change to "battery-detect-gpios" in the driver and DTS.

Thanks,
Chunyan

Quoted 5 lines

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/power/supply/sc27xx_fuel_gauge.c?h=v6.11#n1186
Regards,
Stanislav

Stanislav Jakubek

author

2024-08-15 10:13 UTC #7 lore raw

PATCH 3/3 arm64: dts: sprd: sc2731: move fuel-gauge monitored-battery to device DTS

In reply to Stanislav Jakubek

The monitored-battery property is a property of the board, not the PMIC. Move this property to the DTS of its only user, sp9860g-1h10. While at it, disable the fuel-gauge node by default and enable it only for its users, as it requires board-specific properties to work correctly.

Signed-off-by Stanislav Jakubek <[email protected]>

---
Note that the 'sprd,calib-resistance-micro-ohms' property isn't specified as I do not have the board or any other source of information about it.

If the SPRD folks could chime in here, that'd be great.

 arch/arm64/boot/dts/sprd/sc2731.dtsi      | 4 ++--
 arch/arm64/boot/dts/sprd/sp9860g-1h10.dts | 5 +++++
 2 files changed, 7 insertions(+), 2 deletions(-)

arch/arm64/boot/dts/sprd/sc2731.dtsi +2 -2

index c8b30c0479fd..0eb4aca0a139 100644
@@ -94,17 +94,17 @@ pmic_adc: adc@480 {
 			nvmem-cells = <&adc_big_scale>, <&adc_small_scale>;
 		};
 
-		fuel-gauge@a00 {
+		pmic_fgu: fuel-gauge@a00 {
 			compatible = "sprd,sc2731-fgu";
 			reg = <0xa00>;
 			bat-detect-gpios = <&pmic_eic 9 GPIO_ACTIVE_HIGH>;
 			io-channels = <&pmic_adc 3>, <&pmic_adc 6>;
 			io-channel-names = "bat-temp", "charge-vol";
-			monitored-battery = <&bat>;
 			nvmem-cell-names = "fgu_calib";
 			nvmem-cells = <&fgu_calib>;
 			interrupt-parent = <&sc2731_pmic>;
 			interrupts = <4>;
+			status = "disabled";
 		};
 
 		vibrator@ec8 {

arch/arm64/boot/dts/sprd/sp9860g-1h10.dts +5 -0

index cd8b1069e387..a5f6acafee3a 100644
@@ -84,6 +84,11 @@ bat: battery {
 	};
 };
 
+&pmic_fgu {
+	monitored-battery = <&bat>;
+	status = "okay";
+};
+
 &uart0 {
 	status = "okay";
 };

Signature · 1 line

2.34.1

Rob Herring (Arm)

maintainer

2024-08-15 18:28 UTC #8 lore raw

In reply to Stanislav Jakubek

Quoted 10 lines · Stanislav Jakubek

This series continues cleaning up various issues with the Spreadtrum/Unisoc ARM64 DeviceTrees.
Stanislav Jakubek (3):
arm64: dts: sprd: sp9860g-1h10: fix
constant-charge-voltage-max-microvolt property
arm64: dts: sprd: sc2731: fix bat-detect-gpios property
arm64: dts: sprd: sc2731: move fuel-gauge monitored-battery to device
DTS

Quoted 9 lines

 arch/arm64/boot/dts/sprd/sc2731.dtsi      | 6 +++---
 arch/arm64/boot/dts/sprd/sp9860g-1h10.dts | 7 ++++++-
 2 files changed, 9 insertions(+), 4 deletions(-)

--
2.34.1

My bot found new DTB warnings on the .dts files added or changed in this series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings are fixed by another series. Ultimately, it is up to the platform maintainer whether these warnings are acceptable or not. No need to reply unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then make sure dt-schema is up to date:

  pip3 install dtschema --upgrade

New warnings running 'make CHECK_DTBS=y sprd/sp9860g-1h10.dtb' for [email protected]:

arch/arm64/boot/dts/sprd/sp9860g-1h10.dtb: fuel-gauge@a00: 'bat-detect-gpios', 'interrupt-parent', 'interrupts' do not match any of the regexes: 'pinctrl-[0-9]+'
from schema $id: http://devicetree.org/schemas/power/supply/sc27xx-fg.yaml#
arch/arm64/boot/dts/sprd/sp9860g-1h10.dtb: battery: 'internal-resistance-micro-ohms' does not match any of the regexes: '^ocv-capacity-table-[0-9]+$', 'pinctrl-[0-9]+'
from schema $id: http://devicetree.org/schemas/power/supply/battery.yaml#

Stanislav Jakubek

author

2024-10-30 18:06 UTC #9 lore raw

In reply to Stanislav Jakubek

Hi all,

not sure which tree this should go through, but could someone pick this up? It's been sitting for ~2.5 months.

Thanks,
Stanislav

Chunyan Zhang

reviewer

2024-10-31 02:09 UTC #10 lore raw

Hi Stanislav,

Quoted 5 lines · Stanislav Jakubek

Hi all,
not sure which tree this should go through, but could someone pick this up? It's been sitting for ~2.5 months.

One question is what do we do about those warnings reported by Rob's bot? Apart from that I suggest you send a pull request to [email protected] directly.

Thanks,
Chunyan

Stanislav Jakubek

author

2024-10-31 07:22 UTC #11 lore raw

Quoted 10 lines · Chunyan Zhang

Hi Stanislav,
Quoted 5 lines · Stanislav Jakubek
Hi all,
not sure which tree this should go through, but could someone pick this up? It's been sitting for ~2.5 months.
One question is what do we do about those warnings reported by Rob's bot?

Hi, you're correct, we should do something about those, I'll be sending a V2 soon. Also found an issue with the 2nd patch, will write it up there.

Regards,
Stanislav

Quoted 4 lines

Apart from that I suggest you send a pull request to [email protected] directly.
Thanks,
Chunyan