diff mbox series

[3/3] ARM: dts: exynos: Fix charging regulator voltage and current for i9100

Message ID 20201202203516.43053-3-timon.baetz@protonmail.com
State Superseded
Headers show
Series [1/3] extcon: max8997: Add CHGINS and CHGRM interrupt handling | expand

Commit Message

Timon Baetz Dec. 2, 2020, 9:07 p.m. UTC
Set CHARGER current and CHARGER_CV voltage according to Galaxy S2 kernel
fork.

Signed-off-by: Timon Baetz <timon.baetz@protonmail.com>
---
 arch/arm/boot/dts/exynos4210-i9100.dts | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Timon Baetz Dec. 3, 2020, 5:46 a.m. UTC | #1
On Wednesday, December 2, 2020 11:04 PM, Krzysztof Kozlowski <krzk@kernel.org> wrote:

> On Wed, Dec 02, 2020 at 09:07:28PM +0000, Timon Baetz wrote:
>
> > Set CHARGER current and CHARGER_CV voltage according to Galaxy S2 kernel
> > fork.
> >
> > Signed-off-by: Timon Baetz timon.baetz@protonmail.com
> >
> > ------------------------------------------------------
> >
> > arch/arm/boot/dts/exynos4210-i9100.dts | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> > diff --git a/arch/arm/boot/dts/exynos4210-i9100.dts b/arch/arm/boot/dts/exynos4210-i9100.dts
> > index 9f8d927e0d21..2700d53ea01b 100644
> > --- a/arch/arm/boot/dts/exynos4210-i9100.dts
> > +++ b/arch/arm/boot/dts/exynos4210-i9100.dts
> > @@ -558,14 +558,14 @@ safe2_sreg: ESAFEOUT2 {
> >
> >       	charger_reg: CHARGER {
> >       		regulator-name = "CHARGER";
> >
> >
> > -       		regulator-min-microamp = <60000>;
> >
> >
> > -       		regulator-max-microamp = <2580000>;
> >
> >
> >
> > -       		regulator-min-microamp = <200000>;
> >
> >
> > -       		regulator-max-microamp = <950000>;
> >         	};
> >
> >         	chargercv_reg: CHARGER_CV {
> >         		regulator-name = "CHARGER_CV";
> >
> >
> >
> > -       		regulator-min-microvolt = <3800000>;
> >
> >
> > -       		regulator-max-microvolt = <4100000>;
> >
> >
> >
> > -       		regulator-min-microvolt = <4200000>;
> >
> >
> > -       		regulator-max-microvolt = <4200000>;
> >
> >
>
> I am looking at my sources of Android 3.0 for GT-I9100 but I cannot find
> charger voltages for it. Where did you find it?
>
> Best regards,
> Krzysztof

Thanks all the feedback Krzysztof,

Voltage is set in the charger probe function of the downstream kernel fork: https://github.com/LineageOS/android_kernel_samsung_smdk4412/blob/lineage-17.0/drivers/power/max8997_charger_u1.c#L390-L391

Mainline uses the regulator: https://github.com/torvalds/linux/blob/master/drivers/regulator/max8997-regulator.c#L418-L419
Krzysztof Kozlowski Dec. 3, 2020, 8:23 a.m. UTC | #2
On Thu, Dec 03, 2020 at 05:46:03AM +0000, Timon Bätz wrote:
> On Wednesday, December 2, 2020 11:04 PM, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> 
> > On Wed, Dec 02, 2020 at 09:07:28PM +0000, Timon Baetz wrote:
> >
> > > Set CHARGER current and CHARGER_CV voltage according to Galaxy S2 kernel
> > > fork.
> > >
> > > Signed-off-by: Timon Baetz timon.baetz@protonmail.com
> > >
> > > ------------------------------------------------------
> > >
> > > arch/arm/boot/dts/exynos4210-i9100.dts | 8 ++++----
> > > 1 file changed, 4 insertions(+), 4 deletions(-)
> > > diff --git a/arch/arm/boot/dts/exynos4210-i9100.dts b/arch/arm/boot/dts/exynos4210-i9100.dts
> > > index 9f8d927e0d21..2700d53ea01b 100644
> > > --- a/arch/arm/boot/dts/exynos4210-i9100.dts
> > > +++ b/arch/arm/boot/dts/exynos4210-i9100.dts
> > > @@ -558,14 +558,14 @@ safe2_sreg: ESAFEOUT2 {
> > >
> > >       	charger_reg: CHARGER {
> > >       		regulator-name = "CHARGER";
> > >
> > >
> > > -       		regulator-min-microamp = <60000>;
> > >
> > >
> > > -       		regulator-max-microamp = <2580000>;
> > >
> > >
> > >
> > > -       		regulator-min-microamp = <200000>;
> > >
> > >
> > > -       		regulator-max-microamp = <950000>;
> > >         	};
> > >
> > >         	chargercv_reg: CHARGER_CV {
> > >         		regulator-name = "CHARGER_CV";
> > >
> > >
> > >
> > > -       		regulator-min-microvolt = <3800000>;
> > >
> > >
> > > -       		regulator-max-microvolt = <4100000>;
> > >
> > >
> > >
> > > -       		regulator-min-microvolt = <4200000>;
> > >
> > >
> > > -       		regulator-max-microvolt = <4200000>;
> > >
> > >
> >
> > I am looking at my sources of Android 3.0 for GT-I9100 but I cannot find
> > charger voltages for it. Where did you find it?
> >
> > Best regards,
> > Krzysztof
> 
> Thanks all the feedback Krzysztof,
> 
> Voltage is set in the charger probe function of the downstream kernel fork: https://github.com/LineageOS/android_kernel_samsung_smdk4412/blob/lineage-17.0/drivers/power/max8997_charger_u1.c#L390-L391

You need to fix your email client to wrap lines.

The fork cannot be used as a reference because of poor quality of
explanations for origins of the code.

The commit which added 4.2 V is described as "samsung update 1" which
basically means nothing. If at least it was "drop sources of
GT-I9105"... but in this form it is useless.

For the things we are not sure how they should be implemented, we
sometimes accept the reason "vendor sources do like this". However Lineage
or any other fork are not vendor sources.

Therefore you need to provide a valid explanation for this voltage
change.

Best regards,
Krzysztof
Krzysztof Kozlowski Dec. 3, 2020, 9:50 a.m. UTC | #3
On Thu, Dec 03, 2020 at 10:23:01AM +0200, Krzysztof Kozlowski wrote:
> On Thu, Dec 03, 2020 at 05:46:03AM +0000, Timon Bätz wrote:
> > On Wednesday, December 2, 2020 11:04 PM, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > 
> > > On Wed, Dec 02, 2020 at 09:07:28PM +0000, Timon Baetz wrote:
> > >
> > > > Set CHARGER current and CHARGER_CV voltage according to Galaxy S2 kernel
> > > > fork.
> > > >
> > > > Signed-off-by: Timon Baetz timon.baetz@protonmail.com
> > > >
> > > > ------------------------------------------------------
> > > >
> > > > arch/arm/boot/dts/exynos4210-i9100.dts | 8 ++++----
> > > > 1 file changed, 4 insertions(+), 4 deletions(-)
> > > > diff --git a/arch/arm/boot/dts/exynos4210-i9100.dts b/arch/arm/boot/dts/exynos4210-i9100.dts
> > > > index 9f8d927e0d21..2700d53ea01b 100644
> > > > --- a/arch/arm/boot/dts/exynos4210-i9100.dts
> > > > +++ b/arch/arm/boot/dts/exynos4210-i9100.dts
> > > > @@ -558,14 +558,14 @@ safe2_sreg: ESAFEOUT2 {
> > > >
> > > >       	charger_reg: CHARGER {
> > > >       		regulator-name = "CHARGER";
> > > >
> > > >
> > > > -       		regulator-min-microamp = <60000>;
> > > >
> > > >
> > > > -       		regulator-max-microamp = <2580000>;
> > > >
> > > >
> > > >
> > > > -       		regulator-min-microamp = <200000>;
> > > >
> > > >
> > > > -       		regulator-max-microamp = <950000>;
> > > >         	};
> > > >
> > > >         	chargercv_reg: CHARGER_CV {
> > > >         		regulator-name = "CHARGER_CV";
> > > >
> > > >
> > > >
> > > > -       		regulator-min-microvolt = <3800000>;
> > > >
> > > >
> > > > -       		regulator-max-microvolt = <4100000>;
> > > >
> > > >
> > > >
> > > > -       		regulator-min-microvolt = <4200000>;
> > > >
> > > >
> > > > -       		regulator-max-microvolt = <4200000>;
> > > >
> > > >
> > >
> > > I am looking at my sources of Android 3.0 for GT-I9100 but I cannot find
> > > charger voltages for it. Where did you find it?
> > >
> > > Best regards,
> > > Krzysztof
> > 
> > Thanks all the feedback Krzysztof,
> > 
> > Voltage is set in the charger probe function of the downstream kernel fork: https://github.com/LineageOS/android_kernel_samsung_smdk4412/blob/lineage-17.0/drivers/power/max8997_charger_u1.c#L390-L391
> 
> You need to fix your email client to wrap lines.
> 
> The fork cannot be used as a reference because of poor quality of
> explanations for origins of the code.
> 
> The commit which added 4.2 V is described as "samsung update 1" which
> basically means nothing. If at least it was "drop sources of
> GT-I9105"... but in this form it is useless.
> 
> For the things we are not sure how they should be implemented, we
> sometimes accept the reason "vendor sources do like this". However Lineage
> or any other fork are not vendor sources.
> 
> Therefore you need to provide a valid explanation for this voltage
> change.

I checked vendor sources for Samsung Galaxy S2 Epic 4G Touch (SPH-D710)
and indeed it uses the max8997 charger U1 which sets v4.2 volts.

You can use it to fix up the commit msg.

Unfortunately it seems Samsung started to remove most of older
kernel source code from their OS compliance page. S1, S2 and S3 are
mostly gone. I was able to find just few remaining sources and I am now
updating my vendor-dump with them. I'll upload them later to
https://github.com/krzk/linux-vendor-backup .

Best regards,
Krzysztof
Stephan Gerhold Dec. 3, 2020, 1:08 p.m. UTC | #4
On Thu, Dec 03, 2020 at 11:50:41AM +0200, Krzysztof Kozlowski wrote:
> 
> Unfortunately it seems Samsung started to remove most of older
> kernel source code from their OS compliance page. S1, S2 and S3 are
> mostly gone. I was able to find just few remaining sources and I am now
> updating my vendor-dump with them. I'll upload them later to
> https://github.com/krzk/linux-vendor-backup .
> 

I don't know why they keep removing older kernel sources (it's pretty
stupid), but so far they added all of them back within a couple of days
after I made an inquiry on https://opensource.samsung.com/requestInquiry
(Note: They remove them again after a while so backups are always
 necessary...)

Might be worth a try if you need some of them :)

Stephan
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/exynos4210-i9100.dts b/arch/arm/boot/dts/exynos4210-i9100.dts
index 9f8d927e0d21..2700d53ea01b 100644
--- a/arch/arm/boot/dts/exynos4210-i9100.dts
+++ b/arch/arm/boot/dts/exynos4210-i9100.dts
@@ -558,14 +558,14 @@  safe2_sreg: ESAFEOUT2 {
 
 			charger_reg: CHARGER {
 				regulator-name = "CHARGER";
-				regulator-min-microamp = <60000>;
-				regulator-max-microamp = <2580000>;
+				regulator-min-microamp = <200000>;
+				regulator-max-microamp = <950000>;
 			};
 
 			chargercv_reg: CHARGER_CV {
 				regulator-name = "CHARGER_CV";
-				regulator-min-microvolt = <3800000>;
-				regulator-max-microvolt = <4100000>;
+				regulator-min-microvolt = <4200000>;
+				regulator-max-microvolt = <4200000>;
 				regulator-always-on;
 			};
 		};