[PATCH 0/3] Cleanup Spreadtrum/Unisoc ARM64 DT (part 2)

[PATCH 0/3] Cleanup Spreadtrum/Unisoc ARM64 DT (part 2)

[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

 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

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

[Stanislav Jakubek]

This property has hyphens/dashes, not underscores.

Signed-off-by: Stanislav Jakubek <stano.jakubek@gmail.com>
---
 arch/arm64/boot/dts/sprd/sp9860g-1h10.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/sprd/sp9860g-1h10.dts b/arch/arm64/boot/dts/sprd/sp9860g-1h10.dts
index 095b24a31313..cd8b1069e387 100644
--- a/arch/arm64/boot/dts/sprd/sp9860g-1h10.dts
+++ b/arch/arm64/boot/dts/sprd/sp9860g-1h10.dts
@@ -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>,
-- 
2.34.1

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

[Baolin Wang]

On 2024/8/15 18:12, Stanislav Jakubek wrote:
> This property has hyphens/dashes, not underscores.
> 
> Signed-off-by: Stanislav Jakubek <stano.jakubek@gmail.com>

Thanks for the fixing.
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>

> ---
>   arch/arm64/boot/dts/sprd/sp9860g-1h10.dts | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/sprd/sp9860g-1h10.dts b/arch/arm64/boot/dts/sprd/sp9860g-1h10.dts
> index 095b24a31313..cd8b1069e387 100644
> --- a/arch/arm64/boot/dts/sprd/sp9860g-1h10.dts
> +++ b/arch/arm64/boot/dts/sprd/sp9860g-1h10.dts
> @@ -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>,

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

[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 <stano.jakubek@gmail.com>
---
 arch/arm64/boot/dts/sprd/sc2731.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/sprd/sc2731.dtsi b/arch/arm64/boot/dts/sprd/sc2731.dtsi
index 12136e68dada..c8b30c0479fd 100644
--- a/arch/arm64/boot/dts/sprd/sc2731.dtsi
+++ b/arch/arm64/boot/dts/sprd/sc2731.dtsi
@@ -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>;
-- 
2.34.1

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

[Baolin Wang]

On 2024/8/15 18:13, Stanislav Jakubek wrote:
> 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 <stano.jakubek@gmail.com>

Thanks.
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>

> ---
>   arch/arm64/boot/dts/sprd/sc2731.dtsi | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/sprd/sc2731.dtsi b/arch/arm64/boot/dts/sprd/sc2731.dtsi
> index 12136e68dada..c8b30c0479fd 100644
> --- a/arch/arm64/boot/dts/sprd/sc2731.dtsi
> +++ b/arch/arm64/boot/dts/sprd/sc2731.dtsi
> @@ -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>;

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

[Stanislav Jakubek]

On Thu, Aug 15, 2024 at 12:13:18PM +0200, Stanislav Jakubek wrote:
> 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 <stano.jakubek@gmail.com>
> ---
>  arch/arm64/boot/dts/sprd/sc2731.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/sprd/sc2731.dtsi b/arch/arm64/boot/dts/sprd/sc2731.dtsi
> index 12136e68dada..c8b30c0479fd 100644
> --- a/arch/arm64/boot/dts/sprd/sc2731.dtsi
> +++ b/arch/arm64/boot/dts/sprd/sc2731.dtsi
> @@ -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>;
> -- 
> 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

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

[Chunyan Zhang]

On Thu, 31 Oct 2024 at 15:30, Stanislav Jakubek <stano.jakubek@gmail.com> wrote:
>
> On Thu, Aug 15, 2024 at 12:13:18PM +0200, Stanislav Jakubek wrote:
> > 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 <stano.jakubek@gmail.com>
> > ---
> >  arch/arm64/boot/dts/sprd/sc2731.dtsi | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/boot/dts/sprd/sc2731.dtsi b/arch/arm64/boot/dts/sprd/sc2731.dtsi
> > index 12136e68dada..c8b30c0479fd 100644
> > --- a/arch/arm64/boot/dts/sprd/sc2731.dtsi
> > +++ b/arch/arm64/boot/dts/sprd/sc2731.dtsi
> > @@ -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>;
> > --
> > 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"?

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

Thanks,
Chunyan

>
> [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

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

[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 <stano.jakubek@gmail.com>
---
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(-)

diff --git a/arch/arm64/boot/dts/sprd/sc2731.dtsi b/arch/arm64/boot/dts/sprd/sc2731.dtsi
index c8b30c0479fd..0eb4aca0a139 100644
--- a/arch/arm64/boot/dts/sprd/sc2731.dtsi
+++ b/arch/arm64/boot/dts/sprd/sc2731.dtsi
@@ -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 {
diff --git a/arch/arm64/boot/dts/sprd/sp9860g-1h10.dts b/arch/arm64/boot/dts/sprd/sp9860g-1h10.dts
index cd8b1069e387..a5f6acafee3a 100644
--- a/arch/arm64/boot/dts/sprd/sp9860g-1h10.dts
+++ b/arch/arm64/boot/dts/sprd/sp9860g-1h10.dts
@@ -84,6 +84,11 @@ bat: battery {
 	};
 };
 
+&pmic_fgu {
+	monitored-battery = <&bat>;
+	status = "okay";
+};
+
 &uart0 {
 	status = "okay";
 };
-- 
2.34.1

Re: [PATCH 0/3] Cleanup Spreadtrum/Unisoc ARM64 DT (part 2)

[Rob Herring (Arm)]

On Thu, 15 Aug 2024 12:12:21 +0200, Stanislav Jakubek wrote:
> 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(-)
> 
> --
> 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 cover.1723716331.git.stano.jakubek@gmail.com:

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#

Re: [PATCH 0/3] Cleanup Spreadtrum/Unisoc ARM64 DT (part 2)

[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

Re: [PATCH 0/3] Cleanup Spreadtrum/Unisoc ARM64 DT (part 2)

[Chunyan Zhang]

Hi Stanislav,

On Thu, 31 Oct 2024 at 02:06, Stanislav Jakubek <stano.jakubek@gmail.com> wrote:
>
> 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 soc@kernel.org directly.

Thanks,
Chunyan

Re: [PATCH 0/3] Cleanup Spreadtrum/Unisoc ARM64 DT (part 2)

[Stanislav Jakubek]

On Thu, Oct 31, 2024 at 10:08:27AM +0800, Chunyan Zhang wrote:
> Hi Stanislav,
> 
> On Thu, 31 Oct 2024 at 02:06, Stanislav Jakubek <stano.jakubek@gmail.com> wrote:
> >
> > 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

> Apart from that I suggest you send a pull request to soc@kernel.org directly.
> 
> Thanks,
> Chunyan