[PATCH 1/7] ARM: dts: s5pv210: Split memory nodes to match spec

[PATCH 1/7] ARM: dts: s5pv210: Split memory nodes to match spec

[Jonathan Bakker]

Memory nodes should only have a singular reg property in them, so
split the memory nodes such that there is only per node.

Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
---
 arch/arm/boot/dts/s5pv210-aquila.dts |  8 ++++++--
 arch/arm/boot/dts/s5pv210-aries.dtsi | 14 +++++++++++---
 arch/arm/boot/dts/s5pv210-goni.dts   | 14 +++++++++++---
 3 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/s5pv210-aquila.dts b/arch/arm/boot/dts/s5pv210-aquila.dts
index 6423348034b6..6984479ddba3 100644
--- a/arch/arm/boot/dts/s5pv210-aquila.dts
+++ b/arch/arm/boot/dts/s5pv210-aquila.dts
@@ -29,8 +29,12 @@
 
 	memory@30000000 {
 		device_type = "memory";
-		reg = <0x30000000 0x05000000
-			0x40000000 0x18000000>;
+		reg = <0x30000000 0x05000000>;
+	};
+
+	memory@40000000 {
+		device_type = "memory";
+		reg = <0x40000000 0x18000000>;
 	};
 
 	pmic_ap_clk: clock-0 {
diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
index 160f8cd9a68d..70ff56daf4cb 100644
--- a/arch/arm/boot/dts/s5pv210-aries.dtsi
+++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
@@ -24,9 +24,17 @@
 
 	memory@30000000 {
 		device_type = "memory";
-		reg = <0x30000000 0x05000000
-			0x40000000 0x10000000
-			0x50000000 0x08000000>;
+		reg = <0x30000000 0x05000000>;
+	};
+
+	memory@40000000 {
+		device_type = "memory";
+		reg = <0x40000000 0x10000000>;
+	};
+
+	memory@50000000 {
+		device_type = "memory";
+		reg = <0x50000000 0x08000000>;
 	};
 
 	reserved-memory {
diff --git a/arch/arm/boot/dts/s5pv210-goni.dts b/arch/arm/boot/dts/s5pv210-goni.dts
index c6f39147cb96..2c66ec5cbfbb 100644
--- a/arch/arm/boot/dts/s5pv210-goni.dts
+++ b/arch/arm/boot/dts/s5pv210-goni.dts
@@ -30,9 +30,17 @@
 
 	memory@30000000 {
 		device_type = "memory";
-		reg = <0x30000000 0x05000000
-			0x40000000 0x10000000
-			0x50000000 0x08000000>;
+		reg = <0x30000000 0x05000000>;
+	};
+
+	memory@40000000 {
+		device_type = "memory";
+		reg = <0x40000000 0x10000000>;
+	};
+
+	memory@50000000 {
+		device_type = "memory";
+		reg = <0x50000000 0x08000000>;
 	};
 
 	pmic_ap_clk: clock-0 {
-- 
2.20.1

[PATCH 1/7] ARM: dts: s5pv210: Split memory nodes to match spec

[Jonathan Bakker]

Memory nodes should only have a singular reg property in them, so
split the memory nodes such that there is only per node.

Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
---
 arch/arm/boot/dts/s5pv210-aquila.dts |  8 ++++++--
 arch/arm/boot/dts/s5pv210-aries.dtsi | 14 +++++++++++---
 arch/arm/boot/dts/s5pv210-goni.dts   | 14 +++++++++++---
 3 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/s5pv210-aquila.dts b/arch/arm/boot/dts/s5pv210-aquila.dts
index 6423348034b6..6984479ddba3 100644
--- a/arch/arm/boot/dts/s5pv210-aquila.dts
+++ b/arch/arm/boot/dts/s5pv210-aquila.dts
@@ -29,8 +29,12 @@
 
 	memory@30000000 {
 		device_type = "memory";
-		reg = <0x30000000 0x05000000
-			0x40000000 0x18000000>;
+		reg = <0x30000000 0x05000000>;
+	};
+
+	memory@40000000 {
+		device_type = "memory";
+		reg = <0x40000000 0x18000000>;
 	};
 
 	pmic_ap_clk: clock-0 {
diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
index 160f8cd9a68d..70ff56daf4cb 100644
--- a/arch/arm/boot/dts/s5pv210-aries.dtsi
+++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
@@ -24,9 +24,17 @@
 
 	memory@30000000 {
 		device_type = "memory";
-		reg = <0x30000000 0x05000000
-			0x40000000 0x10000000
-			0x50000000 0x08000000>;
+		reg = <0x30000000 0x05000000>;
+	};
+
+	memory@40000000 {
+		device_type = "memory";
+		reg = <0x40000000 0x10000000>;
+	};
+
+	memory@50000000 {
+		device_type = "memory";
+		reg = <0x50000000 0x08000000>;
 	};
 
 	reserved-memory {
diff --git a/arch/arm/boot/dts/s5pv210-goni.dts b/arch/arm/boot/dts/s5pv210-goni.dts
index c6f39147cb96..2c66ec5cbfbb 100644
--- a/arch/arm/boot/dts/s5pv210-goni.dts
+++ b/arch/arm/boot/dts/s5pv210-goni.dts
@@ -30,9 +30,17 @@
 
 	memory@30000000 {
 		device_type = "memory";
-		reg = <0x30000000 0x05000000
-			0x40000000 0x10000000
-			0x50000000 0x08000000>;
+		reg = <0x30000000 0x05000000>;
+	};
+
+	memory@40000000 {
+		device_type = "memory";
+		reg = <0x40000000 0x10000000>;
+	};
+
+	memory@50000000 {
+		device_type = "memory";
+		reg = <0x50000000 0x08000000>;
 	};
 
 	pmic_ap_clk: clock-0 {
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/7] ARM: dts: s5pv210: Split memory nodes to match spec

[Krzysztof Kozlowski]

On 22/03/2022 21:11, Jonathan Bakker wrote:
> Memory nodes should only have a singular reg property in them, so
> split the memory nodes such that there is only per node.
> 
> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
> ---
>  arch/arm/boot/dts/s5pv210-aquila.dts |  8 ++++++--
>  arch/arm/boot/dts/s5pv210-aries.dtsi | 14 +++++++++++---
>  arch/arm/boot/dts/s5pv210-goni.dts   | 14 +++++++++++---
>  3 files changed, 28 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/s5pv210-aquila.dts b/arch/arm/boot/dts/s5pv210-aquila.dts
> index 6423348034b6..6984479ddba3 100644
> --- a/arch/arm/boot/dts/s5pv210-aquila.dts
> +++ b/arch/arm/boot/dts/s5pv210-aquila.dts
> @@ -29,8 +29,12 @@
>  
>  	memory@30000000 {
>  		device_type = "memory";
> -		reg = <0x30000000 0x05000000
> -			0x40000000 0x18000000>;
> +		reg = <0x30000000 0x05000000>;
> +	};
> +
> +	memory@40000000 {
> +		device_type = "memory";
> +		reg = <0x40000000 0x18000000>;
>  	};
>  
>  	pmic_ap_clk: clock-0 {
> diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
> index 160f8cd9a68d..70ff56daf4cb 100644
> --- a/arch/arm/boot/dts/s5pv210-aries.dtsi
> +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
> @@ -24,9 +24,17 @@
>  
>  	memory@30000000 {
>  		device_type = "memory";
> -		reg = <0x30000000 0x05000000
> -			0x40000000 0x10000000
> -			0x50000000 0x08000000>;

0x40000000 to 0x58000000 is continues, so I wonder why it is split? Look
at Aquila DTS.



Best regards,
Krzysztof

Re: [PATCH 1/7] ARM: dts: s5pv210: Split memory nodes to match spec

[Krzysztof Kozlowski]

On 22/03/2022 21:11, Jonathan Bakker wrote:
> Memory nodes should only have a singular reg property in them, so
> split the memory nodes such that there is only per node.
> 
> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
> ---
>  arch/arm/boot/dts/s5pv210-aquila.dts |  8 ++++++--
>  arch/arm/boot/dts/s5pv210-aries.dtsi | 14 +++++++++++---
>  arch/arm/boot/dts/s5pv210-goni.dts   | 14 +++++++++++---
>  3 files changed, 28 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/s5pv210-aquila.dts b/arch/arm/boot/dts/s5pv210-aquila.dts
> index 6423348034b6..6984479ddba3 100644
> --- a/arch/arm/boot/dts/s5pv210-aquila.dts
> +++ b/arch/arm/boot/dts/s5pv210-aquila.dts
> @@ -29,8 +29,12 @@
>  
>  	memory@30000000 {
>  		device_type = "memory";
> -		reg = <0x30000000 0x05000000
> -			0x40000000 0x18000000>;
> +		reg = <0x30000000 0x05000000>;
> +	};
> +
> +	memory@40000000 {
> +		device_type = "memory";
> +		reg = <0x40000000 0x18000000>;
>  	};
>  
>  	pmic_ap_clk: clock-0 {
> diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
> index 160f8cd9a68d..70ff56daf4cb 100644
> --- a/arch/arm/boot/dts/s5pv210-aries.dtsi
> +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
> @@ -24,9 +24,17 @@
>  
>  	memory@30000000 {
>  		device_type = "memory";
> -		reg = <0x30000000 0x05000000
> -			0x40000000 0x10000000
> -			0x50000000 0x08000000>;

0x40000000 to 0x58000000 is continues, so I wonder why it is split? Look
at Aquila DTS.



Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/7] ARM: dts: s5pv210: Split memory nodes to match spec

[Jonathan Bakker]

Hi Krzysztof,

On 2022-03-23 3:54 a.m., Krzysztof Kozlowski wrote:
> On 22/03/2022 21:11, Jonathan Bakker wrote:
>> Memory nodes should only have a singular reg property in them, so
>> split the memory nodes such that there is only per node.
>>
>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>> ---
>>  arch/arm/boot/dts/s5pv210-aquila.dts |  8 ++++++--
>>  arch/arm/boot/dts/s5pv210-aries.dtsi | 14 +++++++++++---
>>  arch/arm/boot/dts/s5pv210-goni.dts   | 14 +++++++++++---
>>  3 files changed, 28 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/s5pv210-aquila.dts b/arch/arm/boot/dts/s5pv210-aquila.dts
>> index 6423348034b6..6984479ddba3 100644
>> --- a/arch/arm/boot/dts/s5pv210-aquila.dts
>> +++ b/arch/arm/boot/dts/s5pv210-aquila.dts
>> @@ -29,8 +29,12 @@
>>  
>>  	memory@30000000 {
>>  		device_type = "memory";
>> -		reg = <0x30000000 0x05000000
>> -			0x40000000 0x18000000>;
>> +		reg = <0x30000000 0x05000000>;
>> +	};
>> +
>> +	memory@40000000 {
>> +		device_type = "memory";
>> +		reg = <0x40000000 0x18000000>;
>>  	};
>>  
>>  	pmic_ap_clk: clock-0 {
>> diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
>> index 160f8cd9a68d..70ff56daf4cb 100644
>> --- a/arch/arm/boot/dts/s5pv210-aries.dtsi
>> +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
>> @@ -24,9 +24,17 @@
>>  
>>  	memory@30000000 {
>>  		device_type = "memory";
>> -		reg = <0x30000000 0x05000000
>> -			0x40000000 0x10000000
>> -			0x50000000 0x08000000>;
> 
> 0x40000000 to 0x58000000 is continues, so I wonder why it is split? Look
> at Aquila DTS.
> 
> 

Yes, it was split in the vendor kernel as well [1], and that's been continued along
here.  I personally don't see a reason to keep it split, but there might be something
I'm not aware of.

Thanks,
Jonathan

[1] https://github.com/xc-racer99/blastoff_kernel_samsung_galaxys4g/blob/gingerbread/arch/arm/mach-s5pv210/mach-herring.c#L4116

> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 1/7] ARM: dts: s5pv210: Split memory nodes to match spec

[Jonathan Bakker]

Hi Krzysztof,

On 2022-03-23 3:54 a.m., Krzysztof Kozlowski wrote:
> On 22/03/2022 21:11, Jonathan Bakker wrote:
>> Memory nodes should only have a singular reg property in them, so
>> split the memory nodes such that there is only per node.
>>
>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>> ---
>>  arch/arm/boot/dts/s5pv210-aquila.dts |  8 ++++++--
>>  arch/arm/boot/dts/s5pv210-aries.dtsi | 14 +++++++++++---
>>  arch/arm/boot/dts/s5pv210-goni.dts   | 14 +++++++++++---
>>  3 files changed, 28 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/s5pv210-aquila.dts b/arch/arm/boot/dts/s5pv210-aquila.dts
>> index 6423348034b6..6984479ddba3 100644
>> --- a/arch/arm/boot/dts/s5pv210-aquila.dts
>> +++ b/arch/arm/boot/dts/s5pv210-aquila.dts
>> @@ -29,8 +29,12 @@
>>  
>>  	memory@30000000 {
>>  		device_type = "memory";
>> -		reg = <0x30000000 0x05000000
>> -			0x40000000 0x18000000>;
>> +		reg = <0x30000000 0x05000000>;
>> +	};
>> +
>> +	memory@40000000 {
>> +		device_type = "memory";
>> +		reg = <0x40000000 0x18000000>;
>>  	};
>>  
>>  	pmic_ap_clk: clock-0 {
>> diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
>> index 160f8cd9a68d..70ff56daf4cb 100644
>> --- a/arch/arm/boot/dts/s5pv210-aries.dtsi
>> +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
>> @@ -24,9 +24,17 @@
>>  
>>  	memory@30000000 {
>>  		device_type = "memory";
>> -		reg = <0x30000000 0x05000000
>> -			0x40000000 0x10000000
>> -			0x50000000 0x08000000>;
> 
> 0x40000000 to 0x58000000 is continues, so I wonder why it is split? Look
> at Aquila DTS.
> 
> 

Yes, it was split in the vendor kernel as well [1], and that's been continued along
here.  I personally don't see a reason to keep it split, but there might be something
I'm not aware of.

Thanks,
Jonathan

[1] https://github.com/xc-racer99/blastoff_kernel_samsung_galaxys4g/blob/gingerbread/arch/arm/mach-s5pv210/mach-herring.c#L4116

> 
> Best regards,
> Krzysztof
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/7] ARM: dts: s5pv210: Split memory nodes to match spec

[Krzysztof Kozlowski]

On 23/03/2022 15:59, Jonathan Bakker wrote:
> Hi Krzysztof,
> 
> On 2022-03-23 3:54 a.m., Krzysztof Kozlowski wrote:
>> On 22/03/2022 21:11, Jonathan Bakker wrote:
>>> Memory nodes should only have a singular reg property in them, so
>>> split the memory nodes such that there is only per node.
>>>
>>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>>> ---
>>>  arch/arm/boot/dts/s5pv210-aquila.dts |  8 ++++++--
>>>  arch/arm/boot/dts/s5pv210-aries.dtsi | 14 +++++++++++---
>>>  arch/arm/boot/dts/s5pv210-goni.dts   | 14 +++++++++++---
>>>  3 files changed, 28 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/s5pv210-aquila.dts b/arch/arm/boot/dts/s5pv210-aquila.dts
>>> index 6423348034b6..6984479ddba3 100644
>>> --- a/arch/arm/boot/dts/s5pv210-aquila.dts
>>> +++ b/arch/arm/boot/dts/s5pv210-aquila.dts
>>> @@ -29,8 +29,12 @@
>>>  
>>>  	memory@30000000 {
>>>  		device_type = "memory";
>>> -		reg = <0x30000000 0x05000000
>>> -			0x40000000 0x18000000>;
>>> +		reg = <0x30000000 0x05000000>;
>>> +	};
>>> +
>>> +	memory@40000000 {
>>> +		device_type = "memory";
>>> +		reg = <0x40000000 0x18000000>;
>>>  	};
>>>  
>>>  	pmic_ap_clk: clock-0 {
>>> diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
>>> index 160f8cd9a68d..70ff56daf4cb 100644
>>> --- a/arch/arm/boot/dts/s5pv210-aries.dtsi
>>> +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
>>> @@ -24,9 +24,17 @@
>>>  
>>>  	memory@30000000 {
>>>  		device_type = "memory";
>>> -		reg = <0x30000000 0x05000000
>>> -			0x40000000 0x10000000
>>> -			0x50000000 0x08000000>;
>>
>> 0x40000000 to 0x58000000 is continues, so I wonder why it is split? Look
>> at Aquila DTS.
>>
>>
> 
> Yes, it was split in the vendor kernel as well [1], and that's been continued along
> here.  I personally don't see a reason to keep it split, but there might be something
> I'm not aware of.
> 

I guess they wanted maybe to express the physical banks. Fine with me.
Just your explanation is not entirely correct:
> Memory nodes should only have a singular reg property in them,
one reg but it can have multiple items. Why do think multiple reg items
is not allowed?


Best regards,
Krzysztof

Re: [PATCH 1/7] ARM: dts: s5pv210: Split memory nodes to match spec

[Krzysztof Kozlowski]

On 23/03/2022 15:59, Jonathan Bakker wrote:
> Hi Krzysztof,
> 
> On 2022-03-23 3:54 a.m., Krzysztof Kozlowski wrote:
>> On 22/03/2022 21:11, Jonathan Bakker wrote:
>>> Memory nodes should only have a singular reg property in them, so
>>> split the memory nodes such that there is only per node.
>>>
>>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>>> ---
>>>  arch/arm/boot/dts/s5pv210-aquila.dts |  8 ++++++--
>>>  arch/arm/boot/dts/s5pv210-aries.dtsi | 14 +++++++++++---
>>>  arch/arm/boot/dts/s5pv210-goni.dts   | 14 +++++++++++---
>>>  3 files changed, 28 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/s5pv210-aquila.dts b/arch/arm/boot/dts/s5pv210-aquila.dts
>>> index 6423348034b6..6984479ddba3 100644
>>> --- a/arch/arm/boot/dts/s5pv210-aquila.dts
>>> +++ b/arch/arm/boot/dts/s5pv210-aquila.dts
>>> @@ -29,8 +29,12 @@
>>>  
>>>  	memory@30000000 {
>>>  		device_type = "memory";
>>> -		reg = <0x30000000 0x05000000
>>> -			0x40000000 0x18000000>;
>>> +		reg = <0x30000000 0x05000000>;
>>> +	};
>>> +
>>> +	memory@40000000 {
>>> +		device_type = "memory";
>>> +		reg = <0x40000000 0x18000000>;
>>>  	};
>>>  
>>>  	pmic_ap_clk: clock-0 {
>>> diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
>>> index 160f8cd9a68d..70ff56daf4cb 100644
>>> --- a/arch/arm/boot/dts/s5pv210-aries.dtsi
>>> +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
>>> @@ -24,9 +24,17 @@
>>>  
>>>  	memory@30000000 {
>>>  		device_type = "memory";
>>> -		reg = <0x30000000 0x05000000
>>> -			0x40000000 0x10000000
>>> -			0x50000000 0x08000000>;
>>
>> 0x40000000 to 0x58000000 is continues, so I wonder why it is split? Look
>> at Aquila DTS.
>>
>>
> 
> Yes, it was split in the vendor kernel as well [1], and that's been continued along
> here.  I personally don't see a reason to keep it split, but there might be something
> I'm not aware of.
> 

I guess they wanted maybe to express the physical banks. Fine with me.
Just your explanation is not entirely correct:
> Memory nodes should only have a singular reg property in them,
one reg but it can have multiple items. Why do think multiple reg items
is not allowed?


Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/7] ARM: dts: s5pv210: Split memory nodes to match spec

[Jonathan Bakker]

On 2022-03-23 8:06 a.m., Krzysztof Kozlowski wrote:
> On 23/03/2022 15:59, Jonathan Bakker wrote:
>> Hi Krzysztof,
>>
>> On 2022-03-23 3:54 a.m., Krzysztof Kozlowski wrote:
>>> On 22/03/2022 21:11, Jonathan Bakker wrote:
>>>> Memory nodes should only have a singular reg property in them, so
>>>> split the memory nodes such that there is only per node.
>>>>
>>>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>>>> ---
>>>>  arch/arm/boot/dts/s5pv210-aquila.dts |  8 ++++++--
>>>>  arch/arm/boot/dts/s5pv210-aries.dtsi | 14 +++++++++++---
>>>>  arch/arm/boot/dts/s5pv210-goni.dts   | 14 +++++++++++---
>>>>  3 files changed, 28 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/arch/arm/boot/dts/s5pv210-aquila.dts b/arch/arm/boot/dts/s5pv210-aquila.dts
>>>> index 6423348034b6..6984479ddba3 100644
>>>> --- a/arch/arm/boot/dts/s5pv210-aquila.dts
>>>> +++ b/arch/arm/boot/dts/s5pv210-aquila.dts
>>>> @@ -29,8 +29,12 @@
>>>>  
>>>>  	memory@30000000 {
>>>>  		device_type = "memory";
>>>> -		reg = <0x30000000 0x05000000
>>>> -			0x40000000 0x18000000>;
>>>> +		reg = <0x30000000 0x05000000>;
>>>> +	};
>>>> +
>>>> +	memory@40000000 {
>>>> +		device_type = "memory";
>>>> +		reg = <0x40000000 0x18000000>;
>>>>  	};
>>>>  
>>>>  	pmic_ap_clk: clock-0 {
>>>> diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
>>>> index 160f8cd9a68d..70ff56daf4cb 100644
>>>> --- a/arch/arm/boot/dts/s5pv210-aries.dtsi
>>>> +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
>>>> @@ -24,9 +24,17 @@
>>>>  
>>>>  	memory@30000000 {
>>>>  		device_type = "memory";
>>>> -		reg = <0x30000000 0x05000000
>>>> -			0x40000000 0x10000000
>>>> -			0x50000000 0x08000000>;
>>>
>>> 0x40000000 to 0x58000000 is continues, so I wonder why it is split? Look
>>> at Aquila DTS.
>>>
>>>
>>
>> Yes, it was split in the vendor kernel as well [1], and that's been continued along
>> here.  I personally don't see a reason to keep it split, but there might be something
>> I'm not aware of.
>>
> 
> I guess they wanted maybe to express the physical banks. Fine with me.
> Just your explanation is not entirely correct:
>> Memory nodes should only have a singular reg property in them,
> one reg but it can have multiple items. Why do think multiple reg items
> is not allowed?
> 

I was basing it off of this warning when running make dtbs_check

rch/arm/boot/dts/s5pv210-aquila.dt.yaml: /: memory@30000000:reg:0: [805306368, 83886080, 1073741824, 402653184] is too long
	From schema: /home/jon/.local/lib/python3.7/site-packages/dtschema/schemas/reg.yaml

and this solved the warning, booted, and I still had the correct
amount of memory.

Would

	memory@30000000 {
		device_type = "memory";
		reg = <0x30000000 0x05000000>,
			<0x40000000 0x18000000>;
	};

be equally correct?  (untested).

Thanks,
Jonathan

> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 1/7] ARM: dts: s5pv210: Split memory nodes to match spec

[Jonathan Bakker]

On 2022-03-23 8:06 a.m., Krzysztof Kozlowski wrote:
> On 23/03/2022 15:59, Jonathan Bakker wrote:
>> Hi Krzysztof,
>>
>> On 2022-03-23 3:54 a.m., Krzysztof Kozlowski wrote:
>>> On 22/03/2022 21:11, Jonathan Bakker wrote:
>>>> Memory nodes should only have a singular reg property in them, so
>>>> split the memory nodes such that there is only per node.
>>>>
>>>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>>>> ---
>>>>  arch/arm/boot/dts/s5pv210-aquila.dts |  8 ++++++--
>>>>  arch/arm/boot/dts/s5pv210-aries.dtsi | 14 +++++++++++---
>>>>  arch/arm/boot/dts/s5pv210-goni.dts   | 14 +++++++++++---
>>>>  3 files changed, 28 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/arch/arm/boot/dts/s5pv210-aquila.dts b/arch/arm/boot/dts/s5pv210-aquila.dts
>>>> index 6423348034b6..6984479ddba3 100644
>>>> --- a/arch/arm/boot/dts/s5pv210-aquila.dts
>>>> +++ b/arch/arm/boot/dts/s5pv210-aquila.dts
>>>> @@ -29,8 +29,12 @@
>>>>  
>>>>  	memory@30000000 {
>>>>  		device_type = "memory";
>>>> -		reg = <0x30000000 0x05000000
>>>> -			0x40000000 0x18000000>;
>>>> +		reg = <0x30000000 0x05000000>;
>>>> +	};
>>>> +
>>>> +	memory@40000000 {
>>>> +		device_type = "memory";
>>>> +		reg = <0x40000000 0x18000000>;
>>>>  	};
>>>>  
>>>>  	pmic_ap_clk: clock-0 {
>>>> diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
>>>> index 160f8cd9a68d..70ff56daf4cb 100644
>>>> --- a/arch/arm/boot/dts/s5pv210-aries.dtsi
>>>> +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
>>>> @@ -24,9 +24,17 @@
>>>>  
>>>>  	memory@30000000 {
>>>>  		device_type = "memory";
>>>> -		reg = <0x30000000 0x05000000
>>>> -			0x40000000 0x10000000
>>>> -			0x50000000 0x08000000>;
>>>
>>> 0x40000000 to 0x58000000 is continues, so I wonder why it is split? Look
>>> at Aquila DTS.
>>>
>>>
>>
>> Yes, it was split in the vendor kernel as well [1], and that's been continued along
>> here.  I personally don't see a reason to keep it split, but there might be something
>> I'm not aware of.
>>
> 
> I guess they wanted maybe to express the physical banks. Fine with me.
> Just your explanation is not entirely correct:
>> Memory nodes should only have a singular reg property in them,
> one reg but it can have multiple items. Why do think multiple reg items
> is not allowed?
> 

I was basing it off of this warning when running make dtbs_check

rch/arm/boot/dts/s5pv210-aquila.dt.yaml: /: memory@30000000:reg:0: [805306368, 83886080, 1073741824, 402653184] is too long
	From schema: /home/jon/.local/lib/python3.7/site-packages/dtschema/schemas/reg.yaml

and this solved the warning, booted, and I still had the correct
amount of memory.

Would

	memory@30000000 {
		device_type = "memory";
		reg = <0x30000000 0x05000000>,
			<0x40000000 0x18000000>;
	};

be equally correct?  (untested).

Thanks,
Jonathan

> 
> Best regards,
> Krzysztof
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/7] ARM: dts: s5pv210: Split memory nodes to match spec

[Krzysztof Kozlowski]

On 23/03/2022 18:05, Jonathan Bakker wrote:
> 
> 
> On 2022-03-23 8:06 a.m., Krzysztof Kozlowski wrote:
>> On 23/03/2022 15:59, Jonathan Bakker wrote:
>>> Hi Krzysztof,
>>>
>>> On 2022-03-23 3:54 a.m., Krzysztof Kozlowski wrote:
>>>> On 22/03/2022 21:11, Jonathan Bakker wrote:
>>>>> Memory nodes should only have a singular reg property in them, so
>>>>> split the memory nodes such that there is only per node.
>>>>>
>>>>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>>>>> ---
>>>>>  arch/arm/boot/dts/s5pv210-aquila.dts |  8 ++++++--
>>>>>  arch/arm/boot/dts/s5pv210-aries.dtsi | 14 +++++++++++---
>>>>>  arch/arm/boot/dts/s5pv210-goni.dts   | 14 +++++++++++---
>>>>>  3 files changed, 28 insertions(+), 8 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/s5pv210-aquila.dts b/arch/arm/boot/dts/s5pv210-aquila.dts
>>>>> index 6423348034b6..6984479ddba3 100644
>>>>> --- a/arch/arm/boot/dts/s5pv210-aquila.dts
>>>>> +++ b/arch/arm/boot/dts/s5pv210-aquila.dts
>>>>> @@ -29,8 +29,12 @@
>>>>>  
>>>>>  	memory@30000000 {
>>>>>  		device_type = "memory";
>>>>> -		reg = <0x30000000 0x05000000
>>>>> -			0x40000000 0x18000000>;
>>>>> +		reg = <0x30000000 0x05000000>;
>>>>> +	};
>>>>> +
>>>>> +	memory@40000000 {
>>>>> +		device_type = "memory";
>>>>> +		reg = <0x40000000 0x18000000>;
>>>>>  	};
>>>>>  
>>>>>  	pmic_ap_clk: clock-0 {
>>>>> diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
>>>>> index 160f8cd9a68d..70ff56daf4cb 100644
>>>>> --- a/arch/arm/boot/dts/s5pv210-aries.dtsi
>>>>> +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
>>>>> @@ -24,9 +24,17 @@
>>>>>  
>>>>>  	memory@30000000 {
>>>>>  		device_type = "memory";
>>>>> -		reg = <0x30000000 0x05000000
>>>>> -			0x40000000 0x10000000
>>>>> -			0x50000000 0x08000000>;
>>>>
>>>> 0x40000000 to 0x58000000 is continues, so I wonder why it is split? Look
>>>> at Aquila DTS.
>>>>
>>>>
>>>
>>> Yes, it was split in the vendor kernel as well [1], and that's been continued along
>>> here.  I personally don't see a reason to keep it split, but there might be something
>>> I'm not aware of.
>>>
>>
>> I guess they wanted maybe to express the physical banks. Fine with me.
>> Just your explanation is not entirely correct:
>>> Memory nodes should only have a singular reg property in them,
>> one reg but it can have multiple items. Why do think multiple reg items
>> is not allowed?
>>
> 
> I was basing it off of this warning when running make dtbs_check
> 
> rch/arm/boot/dts/s5pv210-aquila.dt.yaml: /: memory@30000000:reg:0: [805306368, 83886080, 1073741824, 402653184] is too long
> 	From schema: /home/jon/.local/lib/python3.7/site-packages/dtschema/schemas/reg.yaml
> 
> and this solved the warning, booted, and I still had the correct
> amount of memory.
> 
> Would
> 
> 	memory@30000000 {
> 		device_type = "memory";
> 		reg = <0x30000000 0x05000000>,
> 			<0x40000000 0x18000000>;
> 	};
> 
> be equally correct?  (untested).

Yes, this one should be correct.


Best regards,
Krzysztof

Re: [PATCH 1/7] ARM: dts: s5pv210: Split memory nodes to match spec

[Krzysztof Kozlowski]

On 23/03/2022 18:05, Jonathan Bakker wrote:
> 
> 
> On 2022-03-23 8:06 a.m., Krzysztof Kozlowski wrote:
>> On 23/03/2022 15:59, Jonathan Bakker wrote:
>>> Hi Krzysztof,
>>>
>>> On 2022-03-23 3:54 a.m., Krzysztof Kozlowski wrote:
>>>> On 22/03/2022 21:11, Jonathan Bakker wrote:
>>>>> Memory nodes should only have a singular reg property in them, so
>>>>> split the memory nodes such that there is only per node.
>>>>>
>>>>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>>>>> ---
>>>>>  arch/arm/boot/dts/s5pv210-aquila.dts |  8 ++++++--
>>>>>  arch/arm/boot/dts/s5pv210-aries.dtsi | 14 +++++++++++---
>>>>>  arch/arm/boot/dts/s5pv210-goni.dts   | 14 +++++++++++---
>>>>>  3 files changed, 28 insertions(+), 8 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/s5pv210-aquila.dts b/arch/arm/boot/dts/s5pv210-aquila.dts
>>>>> index 6423348034b6..6984479ddba3 100644
>>>>> --- a/arch/arm/boot/dts/s5pv210-aquila.dts
>>>>> +++ b/arch/arm/boot/dts/s5pv210-aquila.dts
>>>>> @@ -29,8 +29,12 @@
>>>>>  
>>>>>  	memory@30000000 {
>>>>>  		device_type = "memory";
>>>>> -		reg = <0x30000000 0x05000000
>>>>> -			0x40000000 0x18000000>;
>>>>> +		reg = <0x30000000 0x05000000>;
>>>>> +	};
>>>>> +
>>>>> +	memory@40000000 {
>>>>> +		device_type = "memory";
>>>>> +		reg = <0x40000000 0x18000000>;
>>>>>  	};
>>>>>  
>>>>>  	pmic_ap_clk: clock-0 {
>>>>> diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
>>>>> index 160f8cd9a68d..70ff56daf4cb 100644
>>>>> --- a/arch/arm/boot/dts/s5pv210-aries.dtsi
>>>>> +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
>>>>> @@ -24,9 +24,17 @@
>>>>>  
>>>>>  	memory@30000000 {
>>>>>  		device_type = "memory";
>>>>> -		reg = <0x30000000 0x05000000
>>>>> -			0x40000000 0x10000000
>>>>> -			0x50000000 0x08000000>;
>>>>
>>>> 0x40000000 to 0x58000000 is continues, so I wonder why it is split? Look
>>>> at Aquila DTS.
>>>>
>>>>
>>>
>>> Yes, it was split in the vendor kernel as well [1], and that's been continued along
>>> here.  I personally don't see a reason to keep it split, but there might be something
>>> I'm not aware of.
>>>
>>
>> I guess they wanted maybe to express the physical banks. Fine with me.
>> Just your explanation is not entirely correct:
>>> Memory nodes should only have a singular reg property in them,
>> one reg but it can have multiple items. Why do think multiple reg items
>> is not allowed?
>>
> 
> I was basing it off of this warning when running make dtbs_check
> 
> rch/arm/boot/dts/s5pv210-aquila.dt.yaml: /: memory@30000000:reg:0: [805306368, 83886080, 1073741824, 402653184] is too long
> 	From schema: /home/jon/.local/lib/python3.7/site-packages/dtschema/schemas/reg.yaml
> 
> and this solved the warning, booted, and I still had the correct
> amount of memory.
> 
> Would
> 
> 	memory@30000000 {
> 		device_type = "memory";
> 		reg = <0x30000000 0x05000000>,
> 			<0x40000000 0x18000000>;
> 	};
> 
> be equally correct?  (untested).

Yes, this one should be correct.


Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/7] ARM: dts: s5pv210: Adjust I2S entries to match spec

[Jonathan Bakker]

Based on the device tree spec, clocks should be ordered tx/rx.
Re-order from rx/tx to avoid warnings when running make dtbs_check

Additionally, the number of #sound-dai-cells should be 1, not 0

Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
---
 arch/arm/boot/dts/s5pv210-aries.dtsi |  2 +-
 arch/arm/boot/dts/s5pv210.dtsi       | 18 +++++++++---------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
index 70ff56daf4cb..503b5a50ef1a 100644
--- a/arch/arm/boot/dts/s5pv210-aries.dtsi
+++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
@@ -644,7 +644,7 @@
 };
 
 &i2s0 {
-	dmas = <&pdma0 9>, <&pdma0 10>, <&pdma0 11>;
+	dmas = <&pdma0 10>, <&pdma0 9>, <&pdma0 11>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/s5pv210.dtsi b/arch/arm/boot/dts/s5pv210.dtsi
index 353ba7b09a0c..56c1d9a19570 100644
--- a/arch/arm/boot/dts/s5pv210.dtsi
+++ b/arch/arm/boot/dts/s5pv210.dtsi
@@ -239,8 +239,8 @@
 			reg = <0xeee30000 0x1000>;
 			interrupt-parent = <&vic2>;
 			interrupts = <16>;
-			dma-names = "rx", "tx", "tx-sec";
-			dmas = <&pdma1 9>, <&pdma1 10>, <&pdma1 11>;
+			dma-names = "tx", "rx", "tx-sec";
+			dmas = <&pdma1 10>, <&pdma1 9>, <&pdma1 11>;
 			clock-names = "iis",
 				      "i2s_opclk0",
 				      "i2s_opclk1";
@@ -250,7 +250,7 @@
 			samsung,idma-addr = <0xc0010000>;
 			pinctrl-names = "default";
 			pinctrl-0 = <&i2s0_bus>;
-			#sound-dai-cells = <0>;
+			#sound-dai-cells = <1>;
 			status = "disabled";
 		};
 
@@ -259,13 +259,13 @@
 			reg = <0xe2100000 0x1000>;
 			interrupt-parent = <&vic2>;
 			interrupts = <17>;
-			dma-names = "rx", "tx";
-			dmas = <&pdma1 12>, <&pdma1 13>;
+			dma-names = "tx", "rx";
+			dmas = <&pdma1 13>, <&pdma1 12>;
 			clock-names = "iis", "i2s_opclk0";
 			clocks = <&clocks CLK_I2S1>, <&clocks SCLK_AUDIO1>;
 			pinctrl-names = "default";
 			pinctrl-0 = <&i2s1_bus>;
-			#sound-dai-cells = <0>;
+			#sound-dai-cells = <1>;
 			status = "disabled";
 		};
 
@@ -274,13 +274,13 @@
 			reg = <0xe2a00000 0x1000>;
 			interrupt-parent = <&vic2>;
 			interrupts = <18>;
-			dma-names = "rx", "tx";
-			dmas = <&pdma1 14>, <&pdma1 15>;
+			dma-names = "tx", "rx";
+			dmas = <&pdma1 15>, <&pdma1 14>;
 			clock-names = "iis", "i2s_opclk0";
 			clocks = <&clocks CLK_I2S2>, <&clocks SCLK_AUDIO2>;
 			pinctrl-names = "default";
 			pinctrl-0 = <&i2s2_bus>;
-			#sound-dai-cells = <0>;
+			#sound-dai-cells = <1>;
 			status = "disabled";
 		};
 
-- 
2.20.1

[PATCH 2/7] ARM: dts: s5pv210: Adjust I2S entries to match spec

[Jonathan Bakker]

Based on the device tree spec, clocks should be ordered tx/rx.
Re-order from rx/tx to avoid warnings when running make dtbs_check

Additionally, the number of #sound-dai-cells should be 1, not 0

Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
---
 arch/arm/boot/dts/s5pv210-aries.dtsi |  2 +-
 arch/arm/boot/dts/s5pv210.dtsi       | 18 +++++++++---------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
index 70ff56daf4cb..503b5a50ef1a 100644
--- a/arch/arm/boot/dts/s5pv210-aries.dtsi
+++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
@@ -644,7 +644,7 @@
 };
 
 &i2s0 {
-	dmas = <&pdma0 9>, <&pdma0 10>, <&pdma0 11>;
+	dmas = <&pdma0 10>, <&pdma0 9>, <&pdma0 11>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/s5pv210.dtsi b/arch/arm/boot/dts/s5pv210.dtsi
index 353ba7b09a0c..56c1d9a19570 100644
--- a/arch/arm/boot/dts/s5pv210.dtsi
+++ b/arch/arm/boot/dts/s5pv210.dtsi
@@ -239,8 +239,8 @@
 			reg = <0xeee30000 0x1000>;
 			interrupt-parent = <&vic2>;
 			interrupts = <16>;
-			dma-names = "rx", "tx", "tx-sec";
-			dmas = <&pdma1 9>, <&pdma1 10>, <&pdma1 11>;
+			dma-names = "tx", "rx", "tx-sec";
+			dmas = <&pdma1 10>, <&pdma1 9>, <&pdma1 11>;
 			clock-names = "iis",
 				      "i2s_opclk0",
 				      "i2s_opclk1";
@@ -250,7 +250,7 @@
 			samsung,idma-addr = <0xc0010000>;
 			pinctrl-names = "default";
 			pinctrl-0 = <&i2s0_bus>;
-			#sound-dai-cells = <0>;
+			#sound-dai-cells = <1>;
 			status = "disabled";
 		};
 
@@ -259,13 +259,13 @@
 			reg = <0xe2100000 0x1000>;
 			interrupt-parent = <&vic2>;
 			interrupts = <17>;
-			dma-names = "rx", "tx";
-			dmas = <&pdma1 12>, <&pdma1 13>;
+			dma-names = "tx", "rx";
+			dmas = <&pdma1 13>, <&pdma1 12>;
 			clock-names = "iis", "i2s_opclk0";
 			clocks = <&clocks CLK_I2S1>, <&clocks SCLK_AUDIO1>;
 			pinctrl-names = "default";
 			pinctrl-0 = <&i2s1_bus>;
-			#sound-dai-cells = <0>;
+			#sound-dai-cells = <1>;
 			status = "disabled";
 		};
 
@@ -274,13 +274,13 @@
 			reg = <0xe2a00000 0x1000>;
 			interrupt-parent = <&vic2>;
 			interrupts = <18>;
-			dma-names = "rx", "tx";
-			dmas = <&pdma1 14>, <&pdma1 15>;
+			dma-names = "tx", "rx";
+			dmas = <&pdma1 15>, <&pdma1 14>;
 			clock-names = "iis", "i2s_opclk0";
 			clocks = <&clocks CLK_I2S2>, <&clocks SCLK_AUDIO2>;
 			pinctrl-names = "default";
 			pinctrl-0 = <&i2s2_bus>;
-			#sound-dai-cells = <0>;
+			#sound-dai-cells = <1>;
 			status = "disabled";
 		};
 
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/7] ARM: dts: s5pv210: Adjust I2S entries to match spec

[Krzysztof Kozlowski]

On 23/03/2022 16:03, Jonathan Bakker wrote:
> Based on the device tree spec, clocks should be ordered tx/rx.
> Re-order from rx/tx to avoid warnings when running make dtbs_check
> 
> Additionally, the number of #sound-dai-cells should be 1, not 0
> 
> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
> ---
>  arch/arm/boot/dts/s5pv210-aries.dtsi |  2 +-
>  arch/arm/boot/dts/s5pv210.dtsi       | 18 +++++++++---------
>  2 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
> index 70ff56daf4cb..503b5a50ef1a 100644
> --- a/arch/arm/boot/dts/s5pv210-aries.dtsi
> +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
> @@ -644,7 +644,7 @@
>  };
>  
>  &i2s0 {
> -	dmas = <&pdma0 9>, <&pdma0 10>, <&pdma0 11>;
> +	dmas = <&pdma0 10>, <&pdma0 9>, <&pdma0 11>;
>  	status = "okay";

Except that fix that's the same commit as here:
https://lore.kernel.org/all/20200907161141.31034-26-krzk@kernel.org/
so just extend it.

sound-dai-cells should go to a separate commit. But are you sure they
are correct? The Fascinate 4G seems to be using them as cells=0.


Best regards,
Krzysztof

Re: [PATCH 2/7] ARM: dts: s5pv210: Adjust I2S entries to match spec

[Krzysztof Kozlowski]

On 23/03/2022 16:03, Jonathan Bakker wrote:
> Based on the device tree spec, clocks should be ordered tx/rx.
> Re-order from rx/tx to avoid warnings when running make dtbs_check
> 
> Additionally, the number of #sound-dai-cells should be 1, not 0
> 
> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
> ---
>  arch/arm/boot/dts/s5pv210-aries.dtsi |  2 +-
>  arch/arm/boot/dts/s5pv210.dtsi       | 18 +++++++++---------
>  2 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
> index 70ff56daf4cb..503b5a50ef1a 100644
> --- a/arch/arm/boot/dts/s5pv210-aries.dtsi
> +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
> @@ -644,7 +644,7 @@
>  };
>  
>  &i2s0 {
> -	dmas = <&pdma0 9>, <&pdma0 10>, <&pdma0 11>;
> +	dmas = <&pdma0 10>, <&pdma0 9>, <&pdma0 11>;
>  	status = "okay";

Except that fix that's the same commit as here:
https://lore.kernel.org/all/20200907161141.31034-26-krzk@kernel.org/
so just extend it.

sound-dai-cells should go to a separate commit. But are you sure they
are correct? The Fascinate 4G seems to be using them as cells=0.


Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/7] ARM: dts: s5pv210: Adjust I2S entries to match spec

[Krzysztof Kozlowski]

On 23/03/2022 16:14, Krzysztof Kozlowski wrote:
> On 23/03/2022 16:03, Jonathan Bakker wrote:
>> Based on the device tree spec, clocks should be ordered tx/rx.
>> Re-order from rx/tx to avoid warnings when running make dtbs_check
>>
>> Additionally, the number of #sound-dai-cells should be 1, not 0
>>
>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>> ---
>>  arch/arm/boot/dts/s5pv210-aries.dtsi |  2 +-
>>  arch/arm/boot/dts/s5pv210.dtsi       | 18 +++++++++---------
>>  2 files changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
>> index 70ff56daf4cb..503b5a50ef1a 100644
>> --- a/arch/arm/boot/dts/s5pv210-aries.dtsi
>> +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
>> @@ -644,7 +644,7 @@
>>  };
>>  
>>  &i2s0 {
>> -	dmas = <&pdma0 9>, <&pdma0 10>, <&pdma0 11>;
>> +	dmas = <&pdma0 10>, <&pdma0 9>, <&pdma0 11>;
>>  	status = "okay";
> 
> Except that fix that's the same commit as here:
> https://lore.kernel.org/all/20200907161141.31034-26-krzk@kernel.org/
> so just extend it.
> 
> sound-dai-cells should go to a separate commit. But are you sure they
> are correct? The Fascinate 4G seems to be using them as cells=0.

See my previous patch and discussion:
https://lore.kernel.org/all/20200907161141.31034-10-krzk@kernel.org/


Best regards,
Krzysztof

Re: [PATCH 2/7] ARM: dts: s5pv210: Adjust I2S entries to match spec

[Krzysztof Kozlowski]

On 23/03/2022 16:14, Krzysztof Kozlowski wrote:
> On 23/03/2022 16:03, Jonathan Bakker wrote:
>> Based on the device tree spec, clocks should be ordered tx/rx.
>> Re-order from rx/tx to avoid warnings when running make dtbs_check
>>
>> Additionally, the number of #sound-dai-cells should be 1, not 0
>>
>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>> ---
>>  arch/arm/boot/dts/s5pv210-aries.dtsi |  2 +-
>>  arch/arm/boot/dts/s5pv210.dtsi       | 18 +++++++++---------
>>  2 files changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
>> index 70ff56daf4cb..503b5a50ef1a 100644
>> --- a/arch/arm/boot/dts/s5pv210-aries.dtsi
>> +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
>> @@ -644,7 +644,7 @@
>>  };
>>  
>>  &i2s0 {
>> -	dmas = <&pdma0 9>, <&pdma0 10>, <&pdma0 11>;
>> +	dmas = <&pdma0 10>, <&pdma0 9>, <&pdma0 11>;
>>  	status = "okay";
> 
> Except that fix that's the same commit as here:
> https://lore.kernel.org/all/20200907161141.31034-26-krzk@kernel.org/
> so just extend it.
> 
> sound-dai-cells should go to a separate commit. But are you sure they
> are correct? The Fascinate 4G seems to be using them as cells=0.

See my previous patch and discussion:
https://lore.kernel.org/all/20200907161141.31034-10-krzk@kernel.org/


Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/7] ARM: dts: s5pv210: Adjust I2S entries to match spec

[Jonathan Bakker]

Hi Krzysztof,

On 2022-03-23 8:15 a.m., Krzysztof Kozlowski wrote:
> On 23/03/2022 16:14, Krzysztof Kozlowski wrote:
>> On 23/03/2022 16:03, Jonathan Bakker wrote:
>>> Based on the device tree spec, clocks should be ordered tx/rx.
>>> Re-order from rx/tx to avoid warnings when running make dtbs_check
>>>
>>> Additionally, the number of #sound-dai-cells should be 1, not 0
>>>
>>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>>> ---
>>>  arch/arm/boot/dts/s5pv210-aries.dtsi |  2 +-
>>>  arch/arm/boot/dts/s5pv210.dtsi       | 18 +++++++++---------
>>>  2 files changed, 10 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
>>> index 70ff56daf4cb..503b5a50ef1a 100644
>>> --- a/arch/arm/boot/dts/s5pv210-aries.dtsi
>>> +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
>>> @@ -644,7 +644,7 @@
>>>  };
>>>  
>>>  &i2s0 {
>>> -	dmas = <&pdma0 9>, <&pdma0 10>, <&pdma0 11>;
>>> +	dmas = <&pdma0 10>, <&pdma0 9>, <&pdma0 11>;
>>>  	status = "okay";
>>
>> Except that fix that's the same commit as here:
>> https://lore.kernel.org/all/20200907161141.31034-26-krzk@kernel.org/
>> so just extend it.
>>
>> sound-dai-cells should go to a separate commit. But are you sure they
>> are correct? The Fascinate 4G seems to be using them as cells=0.
> 
> See my previous patch and discussion:
> https://lore.kernel.org/all/20200907161141.31034-10-krzk@kernel.org/
> 

Thanks, I'd totally forgotten about this series from the past.  I'll re-test
those commits and submit your copies of them in v2 if that's OK with you and
that they're confirmed functional?

Jonathan

> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 2/7] ARM: dts: s5pv210: Adjust I2S entries to match spec

[Jonathan Bakker]

Hi Krzysztof,

On 2022-03-23 8:15 a.m., Krzysztof Kozlowski wrote:
> On 23/03/2022 16:14, Krzysztof Kozlowski wrote:
>> On 23/03/2022 16:03, Jonathan Bakker wrote:
>>> Based on the device tree spec, clocks should be ordered tx/rx.
>>> Re-order from rx/tx to avoid warnings when running make dtbs_check
>>>
>>> Additionally, the number of #sound-dai-cells should be 1, not 0
>>>
>>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>>> ---
>>>  arch/arm/boot/dts/s5pv210-aries.dtsi |  2 +-
>>>  arch/arm/boot/dts/s5pv210.dtsi       | 18 +++++++++---------
>>>  2 files changed, 10 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
>>> index 70ff56daf4cb..503b5a50ef1a 100644
>>> --- a/arch/arm/boot/dts/s5pv210-aries.dtsi
>>> +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
>>> @@ -644,7 +644,7 @@
>>>  };
>>>  
>>>  &i2s0 {
>>> -	dmas = <&pdma0 9>, <&pdma0 10>, <&pdma0 11>;
>>> +	dmas = <&pdma0 10>, <&pdma0 9>, <&pdma0 11>;
>>>  	status = "okay";
>>
>> Except that fix that's the same commit as here:
>> https://lore.kernel.org/all/20200907161141.31034-26-krzk@kernel.org/
>> so just extend it.
>>
>> sound-dai-cells should go to a separate commit. But are you sure they
>> are correct? The Fascinate 4G seems to be using them as cells=0.
> 
> See my previous patch and discussion:
> https://lore.kernel.org/all/20200907161141.31034-10-krzk@kernel.org/
> 

Thanks, I'd totally forgotten about this series from the past.  I'll re-test
those commits and submit your copies of them in v2 if that's OK with you and
that they're confirmed functional?

Jonathan

> 
> Best regards,
> Krzysztof
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/7] ARM: dts: s5pv210: Adjust I2S entries to match spec

[Krzysztof Kozlowski]

On 23/03/2022 18:24, Jonathan Bakker wrote:
> Hi Krzysztof,
> 
> On 2022-03-23 8:15 a.m., Krzysztof Kozlowski wrote:
>> On 23/03/2022 16:14, Krzysztof Kozlowski wrote:
>>> On 23/03/2022 16:03, Jonathan Bakker wrote:
>>>> Based on the device tree spec, clocks should be ordered tx/rx.
>>>> Re-order from rx/tx to avoid warnings when running make dtbs_check
>>>>
>>>> Additionally, the number of #sound-dai-cells should be 1, not 0
>>>>
>>>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>>>> ---
>>>>  arch/arm/boot/dts/s5pv210-aries.dtsi |  2 +-
>>>>  arch/arm/boot/dts/s5pv210.dtsi       | 18 +++++++++---------
>>>>  2 files changed, 10 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
>>>> index 70ff56daf4cb..503b5a50ef1a 100644
>>>> --- a/arch/arm/boot/dts/s5pv210-aries.dtsi
>>>> +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
>>>> @@ -644,7 +644,7 @@
>>>>  };
>>>>  
>>>>  &i2s0 {
>>>> -	dmas = <&pdma0 9>, <&pdma0 10>, <&pdma0 11>;
>>>> +	dmas = <&pdma0 10>, <&pdma0 9>, <&pdma0 11>;
>>>>  	status = "okay";
>>>
>>> Except that fix that's the same commit as here:
>>> https://lore.kernel.org/all/20200907161141.31034-26-krzk@kernel.org/
>>> so just extend it.
>>>
>>> sound-dai-cells should go to a separate commit. But are you sure they
>>> are correct? The Fascinate 4G seems to be using them as cells=0.
>>
>> See my previous patch and discussion:
>> https://lore.kernel.org/all/20200907161141.31034-10-krzk@kernel.org/
>>
> 
> Thanks, I'd totally forgotten about this series from the past.  I'll re-test
> those commits and submit your copies of them in v2 if that's OK with you and
> that they're confirmed functional?

My dma fixes change lacked this aries fix. The sound-dai-cells needed
more rethinking.


Best regards,
Krzysztof

Re: [PATCH 2/7] ARM: dts: s5pv210: Adjust I2S entries to match spec

[Krzysztof Kozlowski]

On 23/03/2022 18:24, Jonathan Bakker wrote:
> Hi Krzysztof,
> 
> On 2022-03-23 8:15 a.m., Krzysztof Kozlowski wrote:
>> On 23/03/2022 16:14, Krzysztof Kozlowski wrote:
>>> On 23/03/2022 16:03, Jonathan Bakker wrote:
>>>> Based on the device tree spec, clocks should be ordered tx/rx.
>>>> Re-order from rx/tx to avoid warnings when running make dtbs_check
>>>>
>>>> Additionally, the number of #sound-dai-cells should be 1, not 0
>>>>
>>>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>>>> ---
>>>>  arch/arm/boot/dts/s5pv210-aries.dtsi |  2 +-
>>>>  arch/arm/boot/dts/s5pv210.dtsi       | 18 +++++++++---------
>>>>  2 files changed, 10 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
>>>> index 70ff56daf4cb..503b5a50ef1a 100644
>>>> --- a/arch/arm/boot/dts/s5pv210-aries.dtsi
>>>> +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
>>>> @@ -644,7 +644,7 @@
>>>>  };
>>>>  
>>>>  &i2s0 {
>>>> -	dmas = <&pdma0 9>, <&pdma0 10>, <&pdma0 11>;
>>>> +	dmas = <&pdma0 10>, <&pdma0 9>, <&pdma0 11>;
>>>>  	status = "okay";
>>>
>>> Except that fix that's the same commit as here:
>>> https://lore.kernel.org/all/20200907161141.31034-26-krzk@kernel.org/
>>> so just extend it.
>>>
>>> sound-dai-cells should go to a separate commit. But are you sure they
>>> are correct? The Fascinate 4G seems to be using them as cells=0.
>>
>> See my previous patch and discussion:
>> https://lore.kernel.org/all/20200907161141.31034-10-krzk@kernel.org/
>>
> 
> Thanks, I'd totally forgotten about this series from the past.  I'll re-test
> those commits and submit your copies of them in v2 if that's OK with you and
> that they're confirmed functional?

My dma fixes change lacked this aries fix. The sound-dai-cells needed
more rethinking.


Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/7] ARM: dts: s5pv210: Adjust I2S entries to match spec

[Krzysztof Kozlowski]

On 24/03/2022 12:49, Krzysztof Kozlowski wrote:
> On 23/03/2022 18:24, Jonathan Bakker wrote:
>> Hi Krzysztof,
>>
>> On 2022-03-23 8:15 a.m., Krzysztof Kozlowski wrote:
>>> On 23/03/2022 16:14, Krzysztof Kozlowski wrote:
>>>> On 23/03/2022 16:03, Jonathan Bakker wrote:
>>>>> Based on the device tree spec, clocks should be ordered tx/rx.
>>>>> Re-order from rx/tx to avoid warnings when running make dtbs_check
>>>>>
>>>>> Additionally, the number of #sound-dai-cells should be 1, not 0
>>>>>
>>>>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>>>>> ---
>>>>>  arch/arm/boot/dts/s5pv210-aries.dtsi |  2 +-
>>>>>  arch/arm/boot/dts/s5pv210.dtsi       | 18 +++++++++---------
>>>>>  2 files changed, 10 insertions(+), 10 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
>>>>> index 70ff56daf4cb..503b5a50ef1a 100644
>>>>> --- a/arch/arm/boot/dts/s5pv210-aries.dtsi
>>>>> +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
>>>>> @@ -644,7 +644,7 @@
>>>>>  };
>>>>>  
>>>>>  &i2s0 {
>>>>> -	dmas = <&pdma0 9>, <&pdma0 10>, <&pdma0 11>;
>>>>> +	dmas = <&pdma0 10>, <&pdma0 9>, <&pdma0 11>;
>>>>>  	status = "okay";
>>>>
>>>> Except that fix that's the same commit as here:
>>>> https://lore.kernel.org/all/20200907161141.31034-26-krzk@kernel.org/
>>>> so just extend it.
>>>>
>>>> sound-dai-cells should go to a separate commit. But are you sure they
>>>> are correct? The Fascinate 4G seems to be using them as cells=0.
>>>
>>> See my previous patch and discussion:
>>> https://lore.kernel.org/all/20200907161141.31034-10-krzk@kernel.org/
>>>
>>
>> Thanks, I'd totally forgotten about this series from the past.  I'll re-test
>> those commits and submit your copies of them in v2 if that's OK with you and
>> that they're confirmed functional?
> 
> My dma fixes change lacked this aries fix. The sound-dai-cells needed
> more rethinking.
> 

Hi Jonathan,

Any plans for checking/fixing/testing and resending the sound-dai-cells
fixes?

Best regards,
Krzysztof

Re: [PATCH 2/7] ARM: dts: s5pv210: Adjust I2S entries to match spec

[Krzysztof Kozlowski]

On 24/03/2022 12:49, Krzysztof Kozlowski wrote:
> On 23/03/2022 18:24, Jonathan Bakker wrote:
>> Hi Krzysztof,
>>
>> On 2022-03-23 8:15 a.m., Krzysztof Kozlowski wrote:
>>> On 23/03/2022 16:14, Krzysztof Kozlowski wrote:
>>>> On 23/03/2022 16:03, Jonathan Bakker wrote:
>>>>> Based on the device tree spec, clocks should be ordered tx/rx.
>>>>> Re-order from rx/tx to avoid warnings when running make dtbs_check
>>>>>
>>>>> Additionally, the number of #sound-dai-cells should be 1, not 0
>>>>>
>>>>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>>>>> ---
>>>>>  arch/arm/boot/dts/s5pv210-aries.dtsi |  2 +-
>>>>>  arch/arm/boot/dts/s5pv210.dtsi       | 18 +++++++++---------
>>>>>  2 files changed, 10 insertions(+), 10 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
>>>>> index 70ff56daf4cb..503b5a50ef1a 100644
>>>>> --- a/arch/arm/boot/dts/s5pv210-aries.dtsi
>>>>> +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
>>>>> @@ -644,7 +644,7 @@
>>>>>  };
>>>>>  
>>>>>  &i2s0 {
>>>>> -	dmas = <&pdma0 9>, <&pdma0 10>, <&pdma0 11>;
>>>>> +	dmas = <&pdma0 10>, <&pdma0 9>, <&pdma0 11>;
>>>>>  	status = "okay";
>>>>
>>>> Except that fix that's the same commit as here:
>>>> https://lore.kernel.org/all/20200907161141.31034-26-krzk@kernel.org/
>>>> so just extend it.
>>>>
>>>> sound-dai-cells should go to a separate commit. But are you sure they
>>>> are correct? The Fascinate 4G seems to be using them as cells=0.
>>>
>>> See my previous patch and discussion:
>>> https://lore.kernel.org/all/20200907161141.31034-10-krzk@kernel.org/
>>>
>>
>> Thanks, I'd totally forgotten about this series from the past.  I'll re-test
>> those commits and submit your copies of them in v2 if that's OK with you and
>> that they're confirmed functional?
> 
> My dma fixes change lacked this aries fix. The sound-dai-cells needed
> more rethinking.
> 

Hi Jonathan,

Any plans for checking/fixing/testing and resending the sound-dai-cells
fixes?

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/7] ARM: dts: s5pv210: Adjust I2S entries to match spec

[Jonathan Bakker]

On 2023-04-21 02:44, Krzysztof Kozlowski wrote:
> On 24/03/2022 12:49, Krzysztof Kozlowski wrote:
>> On 23/03/2022 18:24, Jonathan Bakker wrote:
>>> Hi Krzysztof,
>>>
>>> On 2022-03-23 8:15 a.m., Krzysztof Kozlowski wrote:
>>>> On 23/03/2022 16:14, Krzysztof Kozlowski wrote:
>>>>> On 23/03/2022 16:03, Jonathan Bakker wrote:
>>>>>> Based on the device tree spec, clocks should be ordered tx/rx.
>>>>>> Re-order from rx/tx to avoid warnings when running make dtbs_check
>>>>>>
>>>>>> Additionally, the number of #sound-dai-cells should be 1, not 0
>>>>>>
>>>>>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>>>>>> ---
>>>>>>  arch/arm/boot/dts/s5pv210-aries.dtsi |  2 +-
>>>>>>  arch/arm/boot/dts/s5pv210.dtsi       | 18 +++++++++---------
>>>>>>  2 files changed, 10 insertions(+), 10 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
>>>>>> index 70ff56daf4cb..503b5a50ef1a 100644
>>>>>> --- a/arch/arm/boot/dts/s5pv210-aries.dtsi
>>>>>> +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
>>>>>> @@ -644,7 +644,7 @@
>>>>>>  };
>>>>>>  
>>>>>>  &i2s0 {
>>>>>> -	dmas = <&pdma0 9>, <&pdma0 10>, <&pdma0 11>;
>>>>>> +	dmas = <&pdma0 10>, <&pdma0 9>, <&pdma0 11>;
>>>>>>  	status = "okay";
>>>>>
>>>>> Except that fix that's the same commit as here:
>>>>> https://lore.kernel.org/all/20200907161141.31034-26-krzk@kernel.org/
>>>>> so just extend it.
>>>>>
>>>>> sound-dai-cells should go to a separate commit. But are you sure they
>>>>> are correct? The Fascinate 4G seems to be using them as cells=0.
>>>>
>>>> See my previous patch and discussion:
>>>> https://lore.kernel.org/all/20200907161141.31034-10-krzk@kernel.org/
>>>>
>>>
>>> Thanks, I'd totally forgotten about this series from the past.  I'll re-test
>>> those commits and submit your copies of them in v2 if that's OK with you and
>>> that they're confirmed functional?
>>
>> My dma fixes change lacked this aries fix. The sound-dai-cells needed
>> more rethinking.
>>
> 
> Hi Jonathan,
> 
> Any plans for checking/fixing/testing and resending the sound-dai-cells
> fixes?
> 
> Best regards,
> Krzysztof
> 

Oops, I forgot about this again...  Unfortunately, I don't really have the time
right now to do so, and don't know when or if I will.

Thanks,
Jonathan

Re: [PATCH 2/7] ARM: dts: s5pv210: Adjust I2S entries to match spec

[Jonathan Bakker]

On 2023-04-21 02:44, Krzysztof Kozlowski wrote:
> On 24/03/2022 12:49, Krzysztof Kozlowski wrote:
>> On 23/03/2022 18:24, Jonathan Bakker wrote:
>>> Hi Krzysztof,
>>>
>>> On 2022-03-23 8:15 a.m., Krzysztof Kozlowski wrote:
>>>> On 23/03/2022 16:14, Krzysztof Kozlowski wrote:
>>>>> On 23/03/2022 16:03, Jonathan Bakker wrote:
>>>>>> Based on the device tree spec, clocks should be ordered tx/rx.
>>>>>> Re-order from rx/tx to avoid warnings when running make dtbs_check
>>>>>>
>>>>>> Additionally, the number of #sound-dai-cells should be 1, not 0
>>>>>>
>>>>>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>>>>>> ---
>>>>>>  arch/arm/boot/dts/s5pv210-aries.dtsi |  2 +-
>>>>>>  arch/arm/boot/dts/s5pv210.dtsi       | 18 +++++++++---------
>>>>>>  2 files changed, 10 insertions(+), 10 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
>>>>>> index 70ff56daf4cb..503b5a50ef1a 100644
>>>>>> --- a/arch/arm/boot/dts/s5pv210-aries.dtsi
>>>>>> +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
>>>>>> @@ -644,7 +644,7 @@
>>>>>>  };
>>>>>>  
>>>>>>  &i2s0 {
>>>>>> -	dmas = <&pdma0 9>, <&pdma0 10>, <&pdma0 11>;
>>>>>> +	dmas = <&pdma0 10>, <&pdma0 9>, <&pdma0 11>;
>>>>>>  	status = "okay";
>>>>>
>>>>> Except that fix that's the same commit as here:
>>>>> https://lore.kernel.org/all/20200907161141.31034-26-krzk@kernel.org/
>>>>> so just extend it.
>>>>>
>>>>> sound-dai-cells should go to a separate commit. But are you sure they
>>>>> are correct? The Fascinate 4G seems to be using them as cells=0.
>>>>
>>>> See my previous patch and discussion:
>>>> https://lore.kernel.org/all/20200907161141.31034-10-krzk@kernel.org/
>>>>
>>>
>>> Thanks, I'd totally forgotten about this series from the past.  I'll re-test
>>> those commits and submit your copies of them in v2 if that's OK with you and
>>> that they're confirmed functional?
>>
>> My dma fixes change lacked this aries fix. The sound-dai-cells needed
>> more rethinking.
>>
> 
> Hi Jonathan,
> 
> Any plans for checking/fixing/testing and resending the sound-dai-cells
> fixes?
> 
> Best regards,
> Krzysztof
> 

Oops, I forgot about this again...  Unfortunately, I don't really have the time
right now to do so, and don't know when or if I will.

Thanks,
Jonathan

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/7] ARM: dts: s5pv210: Adjust DMA node names to match spec

[Jonathan Bakker]

DMA node names should be dma-controller according to the DT spec,
so rename them from pdma/mdma.  Prevents warnings when running
make dtbs_check

Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
---
 arch/arm/boot/dts/s5pv210.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/s5pv210.dtsi b/arch/arm/boot/dts/s5pv210.dtsi
index 56c1d9a19570..14e6a270363f 100644
--- a/arch/arm/boot/dts/s5pv210.dtsi
+++ b/arch/arm/boot/dts/s5pv210.dtsi
@@ -117,7 +117,7 @@
 			};
 		};
 
-		pdma0: dma@e0900000 {
+		pdma0: dma-controller@e0900000 {
 			compatible = "arm,pl330", "arm,primecell";
 			reg = <0xe0900000 0x1000>;
 			interrupt-parent = <&vic0>;
@@ -129,7 +129,7 @@
 			#dma-requests = <32>;
 		};
 
-		pdma1: dma@e0a00000 {
+		pdma1: dma-controller@e0a00000 {
 			compatible = "arm,pl330", "arm,primecell";
 			reg = <0xe0a00000 0x1000>;
 			interrupt-parent = <&vic0>;
@@ -528,7 +528,7 @@
 			clock-names = "sclk_fimg2d", "fimg2d";
 		};
 
-		mdma1: mdma@fa200000 {
+		mdma1: dma-controller@fa200000 {
 			compatible = "arm,pl330", "arm,primecell";
 			reg = <0xfa200000 0x1000>;
 			interrupt-parent = <&vic0>;
-- 
2.20.1

[PATCH 3/7] ARM: dts: s5pv210: Adjust DMA node names to match spec

[Jonathan Bakker]

DMA node names should be dma-controller according to the DT spec,
so rename them from pdma/mdma.  Prevents warnings when running
make dtbs_check

Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
---
 arch/arm/boot/dts/s5pv210.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/s5pv210.dtsi b/arch/arm/boot/dts/s5pv210.dtsi
index 56c1d9a19570..14e6a270363f 100644
--- a/arch/arm/boot/dts/s5pv210.dtsi
+++ b/arch/arm/boot/dts/s5pv210.dtsi
@@ -117,7 +117,7 @@
 			};
 		};
 
-		pdma0: dma@e0900000 {
+		pdma0: dma-controller@e0900000 {
 			compatible = "arm,pl330", "arm,primecell";
 			reg = <0xe0900000 0x1000>;
 			interrupt-parent = <&vic0>;
@@ -129,7 +129,7 @@
 			#dma-requests = <32>;
 		};
 
-		pdma1: dma@e0a00000 {
+		pdma1: dma-controller@e0a00000 {
 			compatible = "arm,pl330", "arm,primecell";
 			reg = <0xe0a00000 0x1000>;
 			interrupt-parent = <&vic0>;
@@ -528,7 +528,7 @@
 			clock-names = "sclk_fimg2d", "fimg2d";
 		};
 
-		mdma1: mdma@fa200000 {
+		mdma1: dma-controller@fa200000 {
 			compatible = "arm,pl330", "arm,primecell";
 			reg = <0xfa200000 0x1000>;
 			interrupt-parent = <&vic0>;
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/7] ARM: dts: s5pv210: Remove spi-cs-high on panel in Aries

Since commit 766c6b63aa04 ("spi: fix client driver breakages when using
GPIO descriptors"), the panel has been blank due to an inverted CS GPIO.
In order to correct this, drop the spi-cs-high from the panel SPI device.

Fixes: 766c6b63aa04 ("spi: fix client driver breakages when using GPIO descriptors")
Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
---
 arch/arm/boot/dts/s5pv210-aries.dtsi | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
index 503b5a50ef1a..b5d625f12132 100644
--- a/arch/arm/boot/dts/s5pv210-aries.dtsi
+++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
@@ -572,7 +572,6 @@
 			reset-gpios = <&mp05 5 GPIO_ACTIVE_LOW>;
 			vdd3-supply = <&ldo7_reg>;
 			vci-supply = <&ldo17_reg>;
-			spi-cs-high;
 			spi-max-frequency = <1200000>;
 
 			pinctrl-names = "default";
-- 
2.20.1

[PATCH 4/7] ARM: dts: s5pv210: Remove spi-cs-high on panel in Aries

[Jonathan Bakker]

Since commit 766c6b63aa04 ("spi: fix client driver breakages when using
GPIO descriptors"), the panel has been blank due to an inverted CS GPIO.
In order to correct this, drop the spi-cs-high from the panel SPI device.

Fixes: 766c6b63aa04 ("spi: fix client driver breakages when using GPIO descriptors")
Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
---
 arch/arm/boot/dts/s5pv210-aries.dtsi | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
index 503b5a50ef1a..b5d625f12132 100644
--- a/arch/arm/boot/dts/s5pv210-aries.dtsi
+++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
@@ -572,7 +572,6 @@
 			reset-gpios = <&mp05 5 GPIO_ACTIVE_LOW>;
 			vdd3-supply = <&ldo7_reg>;
 			vci-supply = <&ldo17_reg>;
-			spi-cs-high;
 			spi-max-frequency = <1200000>;
 
 			pinctrl-names = "default";
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/7] ARM: dts: s5pv210: Remove spi-cs-high on panel in Aries

[Krzysztof Kozlowski]

On 23/03/2022 16:03, Jonathan Bakker wrote:
> Since commit 766c6b63aa04 ("spi: fix client driver breakages when using
> GPIO descriptors"), the panel has been blank due to an inverted CS GPIO.
> In order to correct this, drop the spi-cs-high from the panel SPI device.
> 
> Fixes: 766c6b63aa04 ("spi: fix client driver breakages when using GPIO descriptors")
> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
> ---
>  arch/arm/boot/dts/s5pv210-aries.dtsi | 1 -
>  1 file changed, 1 deletion(-)
> 

+CC stable please.

Such fixes should be either a separate patchset or put them first in the
series.

Best regards,
Krzysztof

Re: [PATCH 4/7] ARM: dts: s5pv210: Remove spi-cs-high on panel in Aries

[Krzysztof Kozlowski]

On 23/03/2022 16:03, Jonathan Bakker wrote:
> Since commit 766c6b63aa04 ("spi: fix client driver breakages when using
> GPIO descriptors"), the panel has been blank due to an inverted CS GPIO.
> In order to correct this, drop the spi-cs-high from the panel SPI device.
> 
> Fixes: 766c6b63aa04 ("spi: fix client driver breakages when using GPIO descriptors")
> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
> ---
>  arch/arm/boot/dts/s5pv210-aries.dtsi | 1 -
>  1 file changed, 1 deletion(-)
> 

+CC stable please.

Such fixes should be either a separate patchset or put them first in the
series.

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 5/7] ARM: dts: s5pv210: Correct interrupt name for bluetooth in Aries

[Jonathan Bakker]

Correct the name of the bluetooth interrupt from host-wake to
host-wakeup.

Fixes: 1c65b6184441b
Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
---
 arch/arm/boot/dts/s5pv210-aries.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
index b5d625f12132..13a065f5df5e 100644
--- a/arch/arm/boot/dts/s5pv210-aries.dtsi
+++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
@@ -902,7 +902,7 @@
 		device-wakeup-gpios = <&gpg3 4 GPIO_ACTIVE_HIGH>;
 		interrupt-parent = <&gph2>;
 		interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
-		interrupt-names = "host-wake";
+		interrupt-names = "host-wakeup";
 	};
 };
 
-- 
2.20.1

[PATCH 5/7] ARM: dts: s5pv210: Correct interrupt name for bluetooth in Aries

[Jonathan Bakker]

Correct the name of the bluetooth interrupt from host-wake to
host-wakeup.

Fixes: 1c65b6184441b
Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
---
 arch/arm/boot/dts/s5pv210-aries.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
index b5d625f12132..13a065f5df5e 100644
--- a/arch/arm/boot/dts/s5pv210-aries.dtsi
+++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
@@ -902,7 +902,7 @@
 		device-wakeup-gpios = <&gpg3 4 GPIO_ACTIVE_HIGH>;
 		interrupt-parent = <&gph2>;
 		interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
-		interrupt-names = "host-wake";
+		interrupt-names = "host-wakeup";
 	};
 };
 
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/7] ARM: dts: s5pv210: Correct interrupt name for bluetooth in Aries

[krzk]

On 23/03/2022 16:03, Jonathan Bakker wrote:
> Correct the name of the bluetooth interrupt from host-wake to
> host-wakeup.
> 
> Fixes: 1c65b6184441b

Incorrect tag.

Best regards,
Krzysztof

Re: [PATCH 5/7] ARM: dts: s5pv210: Correct interrupt name for bluetooth in Aries

[krzk]

On 23/03/2022 16:03, Jonathan Bakker wrote:
> Correct the name of the bluetooth interrupt from host-wake to
> host-wakeup.
> 
> Fixes: 1c65b6184441b

Incorrect tag.

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 6/7] ARM: dts: s5pv210: Add charger regulator to max8998 in Aries

[Jonathan Bakker]

Enabling and disabling of the charger is done via a regulator on
them max8998 PMIC.  Add control of it in preparation for full
charging support.

Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
---
 arch/arm/boot/dts/s5pv210-aries.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
index 13a065f5df5e..781d843da89b 100644
--- a/arch/arm/boot/dts/s5pv210-aries.dtsi
+++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
@@ -424,6 +424,12 @@
 				safe2_sreg: ESAFEOUT2 {
 					regulator-name = "SAFEOUT2";
 				};
+
+				charger_reg: CHARGER {
+					regulator-name = "CHARGER";
+					regulator-min-microamp = <90000>;
+					regulator-max-microamp = <800000>;
+				};
 			};
 		};
 	};
-- 
2.20.1

[PATCH 6/7] ARM: dts: s5pv210: Add charger regulator to max8998 in Aries

[Jonathan Bakker]

Enabling and disabling of the charger is done via a regulator on
them max8998 PMIC.  Add control of it in preparation for full
charging support.

Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
---
 arch/arm/boot/dts/s5pv210-aries.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
index 13a065f5df5e..781d843da89b 100644
--- a/arch/arm/boot/dts/s5pv210-aries.dtsi
+++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
@@ -424,6 +424,12 @@
 				safe2_sreg: ESAFEOUT2 {
 					regulator-name = "SAFEOUT2";
 				};
+
+				charger_reg: CHARGER {
+					regulator-name = "CHARGER";
+					regulator-min-microamp = <90000>;
+					regulator-max-microamp = <800000>;
+				};
 			};
 		};
 	};
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 7/7] ARM: dts: s5pv210: Add charger support in Aries

[Jonathan Bakker]

Add charger-manager support to Aries boards to allow safe
charging of the battery without the need for userspace control.

Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
---
 arch/arm/boot/dts/s5pv210-fascinate4g.dts | 162 ++++++++++++++++++++++
 arch/arm/boot/dts/s5pv210-galaxys.dts     | 144 +++++++++++++++++++
 2 files changed, 306 insertions(+)

diff --git a/arch/arm/boot/dts/s5pv210-fascinate4g.dts b/arch/arm/boot/dts/s5pv210-fascinate4g.dts
index 7427c84f1126..9530231b7a70 100644
--- a/arch/arm/boot/dts/s5pv210-fascinate4g.dts
+++ b/arch/arm/boot/dts/s5pv210-fascinate4g.dts
@@ -57,6 +57,168 @@
 		pinctrl-0 = <&main_micbias_ena>;
 	};
 
+	thermal-zones {
+		batt_thermal: batt-thermal {
+			polling-delay-passive = <60000>; /* 60 seconds */
+			polling-delay = <600000>; /* 600 seconds */
+
+			thermal-sensors = <&batt_thermistor>;
+		};
+	};
+
+	batt_thermistor: thermal-sensor-0 {
+		compatible = "generic-adc-thermal";
+		#thermal-sensor-cells = <0>;
+		io-channels = <&adc 6>;
+		io-channel-names = "sensor-channel";
+
+		temperature-lookup-table = <
+			(-20000) 1859
+			(-19000) 1846
+			(-18000) 1832
+			(-17000) 1818
+			(-16000) 1804
+			(-15000) 1790
+			(-14000) 1773
+			(-13000) 1756
+			(-12000) 1739
+			(-11000) 1722
+			(-10000) 1705
+			(-9000) 1691
+			(-8000) 1677
+			(-7000) 1663
+			(-6000) 1649
+			(-5000) 1635
+			(-4000) 1550
+			(-3000) 1510
+			(-2000) 1500
+			(-1000) 1490
+			0 1480
+			1000 1470
+			2000 1460
+			3000 1450
+			4000 1430
+			5000 1420
+			6000 1406
+			7000 1386
+			8000 1366
+			9000 1346
+			10000 1326
+			11000 1302
+			12000 1278
+			13000 1254
+			14000 1230
+			15000 1206
+			16000 1182
+			17000 1158
+			18000 1134
+			19000 1110
+			20000 1086
+			21000 1059
+			22000 1035
+			23000 1011
+			24000 987
+			25000 963
+			26000 937
+			27000 913
+			28000 889
+			29000 865
+			30000 841
+			31000 816
+			32000 794
+			33000 772
+			34000 750
+			35000 728
+			36000 708
+			37000 690
+			38000 672
+			39000 654
+			40000 636
+			41000 616
+			42000 599
+			43000 580
+			44000 565
+			45000 548
+			46000 529
+			47000 512
+			48000 495
+			49000 478
+			50000 461
+			51000 440
+			52000 431
+			53000 416
+			54000 405
+			55000 396
+			56000 375
+			57000 360
+			58000 347
+			59000 334
+			60000 325
+			61000 311
+			62000 303
+			63000 296
+			64000 290
+			65000 279
+			66000 265
+			67000 254
+			68000 240
+			69000 220
+			70000 206>;
+	};
+
+	charger_manager: charger-manager-0 {
+		compatible = "charger-manager";
+
+		cm-name = "cm-battery";
+
+		chg-reg-supply = <&charger_reg>;
+
+		/* poll when externally connected */
+		cm-poll-mode = <2>;
+		cm-poll-interval = <30>;
+
+		/* read supply status for status */
+		cm-battery-stat = <0>;
+
+		cm-battery-temp-diff = <5>;
+		cm-battery-hot = <5000>;
+		cm-battery-cold = <400>;
+		cm-battery-cold-in-minus;
+
+		/* 6hr max charging time */
+		cm-charging-max = <21600000>;
+		/* 1.5hr max discharging time */
+		cm-discharging-max = <5400000>;
+
+		cm-fuel-gauge = "battery";
+
+		cm-fullbatt-voltage = <4176000>;
+		cm-fullbatt-vchkdrop-ms = <30000>;
+		cm-fullbatt-vchkdrop-volt = <176000>;
+		cm-fullbatt-soc = <92>;
+
+		cm-num-chargers = <1>;
+		cm-chargers = "max8998_pmic";
+
+		cm-thermal-zone = "batt-thermal";
+
+		regulator-0 {
+			cm-regulator-name = "chg-reg";
+			cable-0 {
+				cm-cable-name = "USB";
+				cm-cable-extcon = "7-0025";
+				cm-cable-min = <475000>;
+				cm-cable-max = <500000>;
+			};
+			cable-1 {
+				cm-cable-name = "SDP";
+				cm-cable-extcon = "7-0025";
+				cm-cable-min = <380000>;
+				cm-cable-max = <600000>;
+			};
+		};
+	};
+
 	sound {
 		compatible = "samsung,fascinate4g-wm8994";
 
diff --git a/arch/arm/boot/dts/s5pv210-galaxys.dts b/arch/arm/boot/dts/s5pv210-galaxys.dts
index eeec2bdece11..240cd0a5c9cb 100644
--- a/arch/arm/boot/dts/s5pv210-galaxys.dts
+++ b/arch/arm/boot/dts/s5pv210-galaxys.dts
@@ -74,6 +74,150 @@
 		};
 	};
 
+	thermal-zones {
+		batt_thermal: batt-thermal {
+			polling-delay-passive = <60000>; /* 60 seconds */
+			polling-delay = <600000>; /* 600 seconds */
+
+			thermal-sensors = <&batt_thermistor>;
+		};
+	};
+
+	batt_thermistor: thermal-sensor-0 {
+		compatible = "generic-adc-thermal";
+		#thermal-sensor-cells = <0>;
+		io-channels = <&adc 6>;
+		io-channel-names = "sensor-channel";
+
+		temperature-lookup-table = <
+			(-7000) 1667
+			(-6000) 1658
+			(-5000) 1632
+			(-4000) 1619
+			(-3000) 1614
+			(-2000) 1596
+			(-1000) 1577
+			0 1559
+			1000 1536
+			2000 1513
+			3000 1491
+			4000 1468
+			5000 1445
+			6000 1421
+			7000 1396
+			8000 1372
+			9000 1348
+			10000 1324
+			11000 1299
+			12000 1275
+			13000 1251
+			14000 1226
+			15000 1202
+			16000 1178
+			17000 1155
+			18000 1131
+			19000 1108
+			20000 1084
+			21000 1060
+			22000 1037
+			23000 1013
+			24000 990
+			25000 966
+			26000 943
+			27000 920
+			28000 898
+			29000 875
+			30000 852
+			31000 829
+			32000 806
+			33000 784
+			34000 761
+			35000 738
+			36000 718
+			37000 697
+			38000 677
+			39000 656
+			40000 636
+			41000 615
+			42000 595
+			43000 574
+			44000 554
+			45000 533
+			46000 518
+			47000 503
+			48000 487
+			49000 472
+			50000 457
+			51000 442
+			52000 427
+			53000 411
+			54000 396
+			55000 381
+			56000 368
+			57000 354
+			58000 341
+			59000 324
+			60000 306
+			61000 299
+			62000 293
+			63000 286
+			64000 275
+			65000 264>;
+	};
+
+	charger_manager: charger-manager-0 {
+		compatible = "charger-manager";
+
+		cm-name = "cm-battery";
+
+		chg-reg-supply = <&charger_reg>;
+
+		/* poll when externally connected */
+		cm-poll-mode = <2>;
+		cm-poll-interval = <30>;
+
+		/* read supply status for status */
+		cm-battery-stat = <0>;
+
+		cm-battery-temp-diff = <5>;
+		cm-battery-hot = <6300>;
+		cm-battery-cold = <400>;
+		cm-battery-cold-in-minus;
+
+		/* 6hr max charging time */
+		cm-charging-max = <21600000>;
+		/* 1.5hr max discharging time */
+		cm-discharging-max = <5400000>;
+
+		cm-fuel-gauge = "battery";
+
+		cm-fullbatt-voltage = <4176000>;
+		cm-fullbatt-vchkdrop-ms = <30000>;
+		cm-fullbatt-vchkdrop-volt = <176000>;
+		cm-fullbatt-soc = <92>;
+
+		cm-num-chargers = <1>;
+		cm-chargers = "max8998_pmic";
+
+		cm-thermal-zone = "batt-thermal";
+
+		regulator-0 {
+			cm-regulator-name = "chg-reg";
+			cable-0 {
+				cm-cable-name = "USB";
+				cm-cable-extcon = "7-0025";
+				cm-cable-min = <475000>;
+				cm-cable-max = <500000>;
+			};
+			cable-1 {
+				cm-cable-name = "SDP";
+				cm-cable-extcon = "7-0025";
+				cm-cable-min = <380000>;
+				cm-cable-max = <600000>;
+			};
+		};
+	};
+
 	micbias_reg: regulator-fixed-3 {
 		compatible = "regulator-fixed";
 		regulator-name = "MICBIAS";
-- 
2.20.1

[PATCH 7/7] ARM: dts: s5pv210: Add charger support in Aries

[Jonathan Bakker]

Add charger-manager support to Aries boards to allow safe
charging of the battery without the need for userspace control.

Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
---
 arch/arm/boot/dts/s5pv210-fascinate4g.dts | 162 ++++++++++++++++++++++
 arch/arm/boot/dts/s5pv210-galaxys.dts     | 144 +++++++++++++++++++
 2 files changed, 306 insertions(+)

diff --git a/arch/arm/boot/dts/s5pv210-fascinate4g.dts b/arch/arm/boot/dts/s5pv210-fascinate4g.dts
index 7427c84f1126..9530231b7a70 100644
--- a/arch/arm/boot/dts/s5pv210-fascinate4g.dts
+++ b/arch/arm/boot/dts/s5pv210-fascinate4g.dts
@@ -57,6 +57,168 @@
 		pinctrl-0 = <&main_micbias_ena>;
 	};
 
+	thermal-zones {
+		batt_thermal: batt-thermal {
+			polling-delay-passive = <60000>; /* 60 seconds */
+			polling-delay = <600000>; /* 600 seconds */
+
+			thermal-sensors = <&batt_thermistor>;
+		};
+	};
+
+	batt_thermistor: thermal-sensor-0 {
+		compatible = "generic-adc-thermal";
+		#thermal-sensor-cells = <0>;
+		io-channels = <&adc 6>;
+		io-channel-names = "sensor-channel";
+
+		temperature-lookup-table = <
+			(-20000) 1859
+			(-19000) 1846
+			(-18000) 1832
+			(-17000) 1818
+			(-16000) 1804
+			(-15000) 1790
+			(-14000) 1773
+			(-13000) 1756
+			(-12000) 1739
+			(-11000) 1722
+			(-10000) 1705
+			(-9000) 1691
+			(-8000) 1677
+			(-7000) 1663
+			(-6000) 1649
+			(-5000) 1635
+			(-4000) 1550
+			(-3000) 1510
+			(-2000) 1500
+			(-1000) 1490
+			0 1480
+			1000 1470
+			2000 1460
+			3000 1450
+			4000 1430
+			5000 1420
+			6000 1406
+			7000 1386
+			8000 1366
+			9000 1346
+			10000 1326
+			11000 1302
+			12000 1278
+			13000 1254
+			14000 1230
+			15000 1206
+			16000 1182
+			17000 1158
+			18000 1134
+			19000 1110
+			20000 1086
+			21000 1059
+			22000 1035
+			23000 1011
+			24000 987
+			25000 963
+			26000 937
+			27000 913
+			28000 889
+			29000 865
+			30000 841
+			31000 816
+			32000 794
+			33000 772
+			34000 750
+			35000 728
+			36000 708
+			37000 690
+			38000 672
+			39000 654
+			40000 636
+			41000 616
+			42000 599
+			43000 580
+			44000 565
+			45000 548
+			46000 529
+			47000 512
+			48000 495
+			49000 478
+			50000 461
+			51000 440
+			52000 431
+			53000 416
+			54000 405
+			55000 396
+			56000 375
+			57000 360
+			58000 347
+			59000 334
+			60000 325
+			61000 311
+			62000 303
+			63000 296
+			64000 290
+			65000 279
+			66000 265
+			67000 254
+			68000 240
+			69000 220
+			70000 206>;
+	};
+
+	charger_manager: charger-manager-0 {
+		compatible = "charger-manager";
+
+		cm-name = "cm-battery";
+
+		chg-reg-supply = <&charger_reg>;
+
+		/* poll when externally connected */
+		cm-poll-mode = <2>;
+		cm-poll-interval = <30>;
+
+		/* read supply status for status */
+		cm-battery-stat = <0>;
+
+		cm-battery-temp-diff = <5>;
+		cm-battery-hot = <5000>;
+		cm-battery-cold = <400>;
+		cm-battery-cold-in-minus;
+
+		/* 6hr max charging time */
+		cm-charging-max = <21600000>;
+		/* 1.5hr max discharging time */
+		cm-discharging-max = <5400000>;
+
+		cm-fuel-gauge = "battery";
+
+		cm-fullbatt-voltage = <4176000>;
+		cm-fullbatt-vchkdrop-ms = <30000>;
+		cm-fullbatt-vchkdrop-volt = <176000>;
+		cm-fullbatt-soc = <92>;
+
+		cm-num-chargers = <1>;
+		cm-chargers = "max8998_pmic";
+
+		cm-thermal-zone = "batt-thermal";
+
+		regulator-0 {
+			cm-regulator-name = "chg-reg";
+			cable-0 {
+				cm-cable-name = "USB";
+				cm-cable-extcon = "7-0025";
+				cm-cable-min = <475000>;
+				cm-cable-max = <500000>;
+			};
+			cable-1 {
+				cm-cable-name = "SDP";
+				cm-cable-extcon = "7-0025";
+				cm-cable-min = <380000>;
+				cm-cable-max = <600000>;
+			};
+		};
+	};
+
 	sound {
 		compatible = "samsung,fascinate4g-wm8994";
 
diff --git a/arch/arm/boot/dts/s5pv210-galaxys.dts b/arch/arm/boot/dts/s5pv210-galaxys.dts
index eeec2bdece11..240cd0a5c9cb 100644
--- a/arch/arm/boot/dts/s5pv210-galaxys.dts
+++ b/arch/arm/boot/dts/s5pv210-galaxys.dts
@@ -74,6 +74,150 @@
 		};
 	};
 
+	thermal-zones {
+		batt_thermal: batt-thermal {
+			polling-delay-passive = <60000>; /* 60 seconds */
+			polling-delay = <600000>; /* 600 seconds */
+
+			thermal-sensors = <&batt_thermistor>;
+		};
+	};
+
+	batt_thermistor: thermal-sensor-0 {
+		compatible = "generic-adc-thermal";
+		#thermal-sensor-cells = <0>;
+		io-channels = <&adc 6>;
+		io-channel-names = "sensor-channel";
+
+		temperature-lookup-table = <
+			(-7000) 1667
+			(-6000) 1658
+			(-5000) 1632
+			(-4000) 1619
+			(-3000) 1614
+			(-2000) 1596
+			(-1000) 1577
+			0 1559
+			1000 1536
+			2000 1513
+			3000 1491
+			4000 1468
+			5000 1445
+			6000 1421
+			7000 1396
+			8000 1372
+			9000 1348
+			10000 1324
+			11000 1299
+			12000 1275
+			13000 1251
+			14000 1226
+			15000 1202
+			16000 1178
+			17000 1155
+			18000 1131
+			19000 1108
+			20000 1084
+			21000 1060
+			22000 1037
+			23000 1013
+			24000 990
+			25000 966
+			26000 943
+			27000 920
+			28000 898
+			29000 875
+			30000 852
+			31000 829
+			32000 806
+			33000 784
+			34000 761
+			35000 738
+			36000 718
+			37000 697
+			38000 677
+			39000 656
+			40000 636
+			41000 615
+			42000 595
+			43000 574
+			44000 554
+			45000 533
+			46000 518
+			47000 503
+			48000 487
+			49000 472
+			50000 457
+			51000 442
+			52000 427
+			53000 411
+			54000 396
+			55000 381
+			56000 368
+			57000 354
+			58000 341
+			59000 324
+			60000 306
+			61000 299
+			62000 293
+			63000 286
+			64000 275
+			65000 264>;
+	};
+
+	charger_manager: charger-manager-0 {
+		compatible = "charger-manager";
+
+		cm-name = "cm-battery";
+
+		chg-reg-supply = <&charger_reg>;
+
+		/* poll when externally connected */
+		cm-poll-mode = <2>;
+		cm-poll-interval = <30>;
+
+		/* read supply status for status */
+		cm-battery-stat = <0>;
+
+		cm-battery-temp-diff = <5>;
+		cm-battery-hot = <6300>;
+		cm-battery-cold = <400>;
+		cm-battery-cold-in-minus;
+
+		/* 6hr max charging time */
+		cm-charging-max = <21600000>;
+		/* 1.5hr max discharging time */
+		cm-discharging-max = <5400000>;
+
+		cm-fuel-gauge = "battery";
+
+		cm-fullbatt-voltage = <4176000>;
+		cm-fullbatt-vchkdrop-ms = <30000>;
+		cm-fullbatt-vchkdrop-volt = <176000>;
+		cm-fullbatt-soc = <92>;
+
+		cm-num-chargers = <1>;
+		cm-chargers = "max8998_pmic";
+
+		cm-thermal-zone = "batt-thermal";
+
+		regulator-0 {
+			cm-regulator-name = "chg-reg";
+			cable-0 {
+				cm-cable-name = "USB";
+				cm-cable-extcon = "7-0025";
+				cm-cable-min = <475000>;
+				cm-cable-max = <500000>;
+			};
+			cable-1 {
+				cm-cable-name = "SDP";
+				cm-cable-extcon = "7-0025";
+				cm-cable-min = <380000>;
+				cm-cable-max = <600000>;
+			};
+		};
+	};
+
 	micbias_reg: regulator-fixed-3 {
 		compatible = "regulator-fixed";
 		regulator-name = "MICBIAS";
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 7/7] ARM: dts: s5pv210: Add charger support in Aries

[krzk]

On 23/03/2022 16:03, Jonathan Bakker wrote:
> Add charger-manager support to Aries boards to allow safe
> charging of the battery without the need for userspace control.
> 
> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
> ---
>  arch/arm/boot/dts/s5pv210-fascinate4g.dts | 162 ++++++++++++++++++++++
>  arch/arm/boot/dts/s5pv210-galaxys.dts     | 144 +++++++++++++++++++
>  2 files changed, 306 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/s5pv210-fascinate4g.dts b/arch/arm/boot/dts/s5pv210-fascinate4g.dts
> index 7427c84f1126..9530231b7a70 100644
> --- a/arch/arm/boot/dts/s5pv210-fascinate4g.dts
> +++ b/arch/arm/boot/dts/s5pv210-fascinate4g.dts
> @@ -57,6 +57,168 @@
>  		pinctrl-0 = <&main_micbias_ena>;
>  	};
>  
> +	thermal-zones {
> +		batt_thermal: batt-thermal {
> +			polling-delay-passive = <60000>; /* 60 seconds */

There is no passive cooling device, so why do you need it?

> +			polling-delay = <600000>; /* 600 seconds */
> +
> +			thermal-sensors = <&batt_thermistor>;
> +		};
> +	};
> +
> +	batt_thermistor: thermal-sensor-0 {
> +		compatible = "generic-adc-thermal";
> +		#thermal-sensor-cells = <0>;
> +		io-channels = <&adc 6>;
> +		io-channel-names = "sensor-channel";
> +
> +		temperature-lookup-table = <
> +			(-20000) 1859
> +			(-19000) 1846
> +			(-18000) 1832
> +			(-17000) 1818
> +			(-16000) 1804
> +			(-15000) 1790
> +			(-14000) 1773
> +			(-13000) 1756
> +			(-12000) 1739
> +			(-11000) 1722
> +			(-10000) 1705
> +			(-9000) 1691
> +			(-8000) 1677
> +			(-7000) 1663
> +			(-6000) 1649
> +			(-5000) 1635
> +			(-4000) 1550
> +			(-3000) 1510
> +			(-2000) 1500
> +			(-1000) 1490
> +			0 1480
> +			1000 1470
> +			2000 1460
> +			3000 1450
> +			4000 1430
> +			5000 1420
> +			6000 1406
> +			7000 1386
> +			8000 1366
> +			9000 1346
> +			10000 1326
> +			11000 1302
> +			12000 1278
> +			13000 1254
> +			14000 1230
> +			15000 1206
> +			16000 1182
> +			17000 1158
> +			18000 1134
> +			19000 1110
> +			20000 1086
> +			21000 1059
> +			22000 1035
> +			23000 1011
> +			24000 987
> +			25000 963
> +			26000 937
> +			27000 913
> +			28000 889
> +			29000 865
> +			30000 841
> +			31000 816
> +			32000 794
> +			33000 772
> +			34000 750
> +			35000 728
> +			36000 708
> +			37000 690
> +			38000 672
> +			39000 654
> +			40000 636
> +			41000 616
> +			42000 599
> +			43000 580
> +			44000 565
> +			45000 548
> +			46000 529
> +			47000 512
> +			48000 495
> +			49000 478
> +			50000 461
> +			51000 440
> +			52000 431
> +			53000 416
> +			54000 405
> +			55000 396
> +			56000 375
> +			57000 360
> +			58000 347
> +			59000 334
> +			60000 325
> +			61000 311
> +			62000 303
> +			63000 296
> +			64000 290
> +			65000 279
> +			66000 265
> +			67000 254
> +			68000 240
> +			69000 220
> +			70000 206>;
> +	};
> +
> +	charger_manager: charger-manager-0 {
> +		compatible = "charger-manager";

Sorry, this is not a hardware. It's a hack to configure kernel charging
driver via DT which was made deprecated.

Best regards,
Krzysztof

Re: [PATCH 7/7] ARM: dts: s5pv210: Add charger support in Aries

[krzk]

On 23/03/2022 16:03, Jonathan Bakker wrote:
> Add charger-manager support to Aries boards to allow safe
> charging of the battery without the need for userspace control.
> 
> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
> ---
>  arch/arm/boot/dts/s5pv210-fascinate4g.dts | 162 ++++++++++++++++++++++
>  arch/arm/boot/dts/s5pv210-galaxys.dts     | 144 +++++++++++++++++++
>  2 files changed, 306 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/s5pv210-fascinate4g.dts b/arch/arm/boot/dts/s5pv210-fascinate4g.dts
> index 7427c84f1126..9530231b7a70 100644
> --- a/arch/arm/boot/dts/s5pv210-fascinate4g.dts
> +++ b/arch/arm/boot/dts/s5pv210-fascinate4g.dts
> @@ -57,6 +57,168 @@
>  		pinctrl-0 = <&main_micbias_ena>;
>  	};
>  
> +	thermal-zones {
> +		batt_thermal: batt-thermal {
> +			polling-delay-passive = <60000>; /* 60 seconds */

There is no passive cooling device, so why do you need it?

> +			polling-delay = <600000>; /* 600 seconds */
> +
> +			thermal-sensors = <&batt_thermistor>;
> +		};
> +	};
> +
> +	batt_thermistor: thermal-sensor-0 {
> +		compatible = "generic-adc-thermal";
> +		#thermal-sensor-cells = <0>;
> +		io-channels = <&adc 6>;
> +		io-channel-names = "sensor-channel";
> +
> +		temperature-lookup-table = <
> +			(-20000) 1859
> +			(-19000) 1846
> +			(-18000) 1832
> +			(-17000) 1818
> +			(-16000) 1804
> +			(-15000) 1790
> +			(-14000) 1773
> +			(-13000) 1756
> +			(-12000) 1739
> +			(-11000) 1722
> +			(-10000) 1705
> +			(-9000) 1691
> +			(-8000) 1677
> +			(-7000) 1663
> +			(-6000) 1649
> +			(-5000) 1635
> +			(-4000) 1550
> +			(-3000) 1510
> +			(-2000) 1500
> +			(-1000) 1490
> +			0 1480
> +			1000 1470
> +			2000 1460
> +			3000 1450
> +			4000 1430
> +			5000 1420
> +			6000 1406
> +			7000 1386
> +			8000 1366
> +			9000 1346
> +			10000 1326
> +			11000 1302
> +			12000 1278
> +			13000 1254
> +			14000 1230
> +			15000 1206
> +			16000 1182
> +			17000 1158
> +			18000 1134
> +			19000 1110
> +			20000 1086
> +			21000 1059
> +			22000 1035
> +			23000 1011
> +			24000 987
> +			25000 963
> +			26000 937
> +			27000 913
> +			28000 889
> +			29000 865
> +			30000 841
> +			31000 816
> +			32000 794
> +			33000 772
> +			34000 750
> +			35000 728
> +			36000 708
> +			37000 690
> +			38000 672
> +			39000 654
> +			40000 636
> +			41000 616
> +			42000 599
> +			43000 580
> +			44000 565
> +			45000 548
> +			46000 529
> +			47000 512
> +			48000 495
> +			49000 478
> +			50000 461
> +			51000 440
> +			52000 431
> +			53000 416
> +			54000 405
> +			55000 396
> +			56000 375
> +			57000 360
> +			58000 347
> +			59000 334
> +			60000 325
> +			61000 311
> +			62000 303
> +			63000 296
> +			64000 290
> +			65000 279
> +			66000 265
> +			67000 254
> +			68000 240
> +			69000 220
> +			70000 206>;
> +	};
> +
> +	charger_manager: charger-manager-0 {
> +		compatible = "charger-manager";

Sorry, this is not a hardware. It's a hack to configure kernel charging
driver via DT which was made deprecated.

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 7/7] ARM: dts: s5pv210: Add charger support in Aries

[Jonathan Bakker]

On 2022-03-23 8:31 a.m., krzk@kernel.org wrote:
> On 23/03/2022 16:03, Jonathan Bakker wrote:
>> Add charger-manager support to Aries boards to allow safe
>> charging of the battery without the need for userspace control.
>>
>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>> ---
>>  arch/arm/boot/dts/s5pv210-fascinate4g.dts | 162 ++++++++++++++++++++++
>>  arch/arm/boot/dts/s5pv210-galaxys.dts     | 144 +++++++++++++++++++
>>  2 files changed, 306 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/s5pv210-fascinate4g.dts b/arch/arm/boot/dts/s5pv210-fascinate4g.dts
>> index 7427c84f1126..9530231b7a70 100644
>> --- a/arch/arm/boot/dts/s5pv210-fascinate4g.dts
>> +++ b/arch/arm/boot/dts/s5pv210-fascinate4g.dts
>> @@ -57,6 +57,168 @@
>>  		pinctrl-0 = <&main_micbias_ena>;
>>  	};
>>  
>> +	thermal-zones {
>> +		batt_thermal: batt-thermal {
>> +			polling-delay-passive = <60000>; /* 60 seconds */
> 
> There is no passive cooling device, so why do you need it?
> 

The charger manager code needs a passive cooling device, so that's
why this is present here.

>> +			polling-delay = <600000>; /* 600 seconds */
>> +
>> +			thermal-sensors = <&batt_thermistor>;
>> +		};
>> +	};
>> +
>> +	batt_thermistor: thermal-sensor-0 {
>> +		compatible = "generic-adc-thermal";
>> +		#thermal-sensor-cells = <0>;
>> +		io-channels = <&adc 6>;
>> +		io-channel-names = "sensor-channel";
>> +
>> +		temperature-lookup-table = <
>> +			(-20000) 1859
>> +			(-19000) 1846
>> +			(-18000) 1832
>> +			(-17000) 1818
>> +			(-16000) 1804
>> +			(-15000) 1790
>> +			(-14000) 1773
>> +			(-13000) 1756
>> +			(-12000) 1739
>> +			(-11000) 1722
>> +			(-10000) 1705
>> +			(-9000) 1691
>> +			(-8000) 1677
>> +			(-7000) 1663
>> +			(-6000) 1649
>> +			(-5000) 1635
>> +			(-4000) 1550
>> +			(-3000) 1510
>> +			(-2000) 1500
>> +			(-1000) 1490
>> +			0 1480
>> +			1000 1470
>> +			2000 1460
>> +			3000 1450
>> +			4000 1430
>> +			5000 1420
>> +			6000 1406
>> +			7000 1386
>> +			8000 1366
>> +			9000 1346
>> +			10000 1326
>> +			11000 1302
>> +			12000 1278
>> +			13000 1254
>> +			14000 1230
>> +			15000 1206
>> +			16000 1182
>> +			17000 1158
>> +			18000 1134
>> +			19000 1110
>> +			20000 1086
>> +			21000 1059
>> +			22000 1035
>> +			23000 1011
>> +			24000 987
>> +			25000 963
>> +			26000 937
>> +			27000 913
>> +			28000 889
>> +			29000 865
>> +			30000 841
>> +			31000 816
>> +			32000 794
>> +			33000 772
>> +			34000 750
>> +			35000 728
>> +			36000 708
>> +			37000 690
>> +			38000 672
>> +			39000 654
>> +			40000 636
>> +			41000 616
>> +			42000 599
>> +			43000 580
>> +			44000 565
>> +			45000 548
>> +			46000 529
>> +			47000 512
>> +			48000 495
>> +			49000 478
>> +			50000 461
>> +			51000 440
>> +			52000 431
>> +			53000 416
>> +			54000 405
>> +			55000 396
>> +			56000 375
>> +			57000 360
>> +			58000 347
>> +			59000 334
>> +			60000 325
>> +			61000 311
>> +			62000 303
>> +			63000 296
>> +			64000 290
>> +			65000 279
>> +			66000 265
>> +			67000 254
>> +			68000 240
>> +			69000 220
>> +			70000 206>;
>> +	};
>> +
>> +	charger_manager: charger-manager-0 {
>> +		compatible = "charger-manager";
> 
> Sorry, this is not a hardware. It's a hack to configure kernel charging
> driver via DT which was made deprecated.

Thanks, I missed the deprecation notice in the binding file.

What would be the better way of creating a functional charging system?
A new device-specific driver?  Userspace monitoring of temperature/connected
device and extensions to the max8998 driver for enabling/disabling/configuring
charging via the power supply subsystem instead of the regulator subsystem?
Something else?

The way I understand the charging system, there is

- The fuelgauge (max17042)
- The max8998 charger portion, including the ability to vary the current
- The thermistor for checking battery temperature
- The FSA9480 to determine what sort of cable is connected

Thanks,
Jonathan

> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 7/7] ARM: dts: s5pv210: Add charger support in Aries

[Jonathan Bakker]

On 2022-03-23 8:31 a.m., krzk@kernel.org wrote:
> On 23/03/2022 16:03, Jonathan Bakker wrote:
>> Add charger-manager support to Aries boards to allow safe
>> charging of the battery without the need for userspace control.
>>
>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>> ---
>>  arch/arm/boot/dts/s5pv210-fascinate4g.dts | 162 ++++++++++++++++++++++
>>  arch/arm/boot/dts/s5pv210-galaxys.dts     | 144 +++++++++++++++++++
>>  2 files changed, 306 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/s5pv210-fascinate4g.dts b/arch/arm/boot/dts/s5pv210-fascinate4g.dts
>> index 7427c84f1126..9530231b7a70 100644
>> --- a/arch/arm/boot/dts/s5pv210-fascinate4g.dts
>> +++ b/arch/arm/boot/dts/s5pv210-fascinate4g.dts
>> @@ -57,6 +57,168 @@
>>  		pinctrl-0 = <&main_micbias_ena>;
>>  	};
>>  
>> +	thermal-zones {
>> +		batt_thermal: batt-thermal {
>> +			polling-delay-passive = <60000>; /* 60 seconds */
> 
> There is no passive cooling device, so why do you need it?
> 

The charger manager code needs a passive cooling device, so that's
why this is present here.

>> +			polling-delay = <600000>; /* 600 seconds */
>> +
>> +			thermal-sensors = <&batt_thermistor>;
>> +		};
>> +	};
>> +
>> +	batt_thermistor: thermal-sensor-0 {
>> +		compatible = "generic-adc-thermal";
>> +		#thermal-sensor-cells = <0>;
>> +		io-channels = <&adc 6>;
>> +		io-channel-names = "sensor-channel";
>> +
>> +		temperature-lookup-table = <
>> +			(-20000) 1859
>> +			(-19000) 1846
>> +			(-18000) 1832
>> +			(-17000) 1818
>> +			(-16000) 1804
>> +			(-15000) 1790
>> +			(-14000) 1773
>> +			(-13000) 1756
>> +			(-12000) 1739
>> +			(-11000) 1722
>> +			(-10000) 1705
>> +			(-9000) 1691
>> +			(-8000) 1677
>> +			(-7000) 1663
>> +			(-6000) 1649
>> +			(-5000) 1635
>> +			(-4000) 1550
>> +			(-3000) 1510
>> +			(-2000) 1500
>> +			(-1000) 1490
>> +			0 1480
>> +			1000 1470
>> +			2000 1460
>> +			3000 1450
>> +			4000 1430
>> +			5000 1420
>> +			6000 1406
>> +			7000 1386
>> +			8000 1366
>> +			9000 1346
>> +			10000 1326
>> +			11000 1302
>> +			12000 1278
>> +			13000 1254
>> +			14000 1230
>> +			15000 1206
>> +			16000 1182
>> +			17000 1158
>> +			18000 1134
>> +			19000 1110
>> +			20000 1086
>> +			21000 1059
>> +			22000 1035
>> +			23000 1011
>> +			24000 987
>> +			25000 963
>> +			26000 937
>> +			27000 913
>> +			28000 889
>> +			29000 865
>> +			30000 841
>> +			31000 816
>> +			32000 794
>> +			33000 772
>> +			34000 750
>> +			35000 728
>> +			36000 708
>> +			37000 690
>> +			38000 672
>> +			39000 654
>> +			40000 636
>> +			41000 616
>> +			42000 599
>> +			43000 580
>> +			44000 565
>> +			45000 548
>> +			46000 529
>> +			47000 512
>> +			48000 495
>> +			49000 478
>> +			50000 461
>> +			51000 440
>> +			52000 431
>> +			53000 416
>> +			54000 405
>> +			55000 396
>> +			56000 375
>> +			57000 360
>> +			58000 347
>> +			59000 334
>> +			60000 325
>> +			61000 311
>> +			62000 303
>> +			63000 296
>> +			64000 290
>> +			65000 279
>> +			66000 265
>> +			67000 254
>> +			68000 240
>> +			69000 220
>> +			70000 206>;
>> +	};
>> +
>> +	charger_manager: charger-manager-0 {
>> +		compatible = "charger-manager";
> 
> Sorry, this is not a hardware. It's a hack to configure kernel charging
> driver via DT which was made deprecated.

Thanks, I missed the deprecation notice in the binding file.

What would be the better way of creating a functional charging system?
A new device-specific driver?  Userspace monitoring of temperature/connected
device and extensions to the max8998 driver for enabling/disabling/configuring
charging via the power supply subsystem instead of the regulator subsystem?
Something else?

The way I understand the charging system, there is

- The fuelgauge (max17042)
- The max8998 charger portion, including the ability to vary the current
- The thermistor for checking battery temperature
- The FSA9480 to determine what sort of cable is connected

Thanks,
Jonathan

> 
> Best regards,
> Krzysztof
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 7/7] ARM: dts: s5pv210: Add charger support in Aries

[Krzysztof Kozlowski]

On 23/03/2022 18:20, Jonathan Bakker wrote:
> 
> 
> On 2022-03-23 8:31 a.m., krzk@kernel.org wrote:
>> On 23/03/2022 16:03, Jonathan Bakker wrote:
>>> Add charger-manager support to Aries boards to allow safe
>>> charging of the battery without the need for userspace control.
>>>
>>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>>> ---
>>>  arch/arm/boot/dts/s5pv210-fascinate4g.dts | 162 ++++++++++++++++++++++
>>>  arch/arm/boot/dts/s5pv210-galaxys.dts     | 144 +++++++++++++++++++
>>>  2 files changed, 306 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/s5pv210-fascinate4g.dts b/arch/arm/boot/dts/s5pv210-fascinate4g.dts
>>> index 7427c84f1126..9530231b7a70 100644
>>> --- a/arch/arm/boot/dts/s5pv210-fascinate4g.dts
>>> +++ b/arch/arm/boot/dts/s5pv210-fascinate4g.dts
>>> @@ -57,6 +57,168 @@
>>>  		pinctrl-0 = <&main_micbias_ena>;
>>>  	};
>>>  
>>> +	thermal-zones {
>>> +		batt_thermal: batt-thermal {
>>> +			polling-delay-passive = <60000>; /* 60 seconds */
>>
>> There is no passive cooling device, so why do you need it?
>>
> 
> The charger manager code needs a passive cooling device, so that's
> why this is present here.
> 
>>> +			polling-delay = <600000>; /* 600 seconds */
>>> +
>>> +			thermal-sensors = <&batt_thermistor>;
>>> +		};
>>> +	};
>>> +
>>> +	batt_thermistor: thermal-sensor-0 {
>>> +		compatible = "generic-adc-thermal";
>>> +		#thermal-sensor-cells = <0>;
>>> +		io-channels = <&adc 6>;
>>> +		io-channel-names = "sensor-channel";
>>> +
>>> +		temperature-lookup-table = <
>>> +			(-20000) 1859
>>> +			(-19000) 1846
>>> +			(-18000) 1832
>>> +			(-17000) 1818
>>> +			(-16000) 1804
>>> +			(-15000) 1790
>>> +			(-14000) 1773
>>> +			(-13000) 1756
>>> +			(-12000) 1739
>>> +			(-11000) 1722
>>> +			(-10000) 1705
>>> +			(-9000) 1691
>>> +			(-8000) 1677
>>> +			(-7000) 1663
>>> +			(-6000) 1649
>>> +			(-5000) 1635
>>> +			(-4000) 1550
>>> +			(-3000) 1510
>>> +			(-2000) 1500
>>> +			(-1000) 1490
>>> +			0 1480
>>> +			1000 1470
>>> +			2000 1460
>>> +			3000 1450
>>> +			4000 1430
>>> +			5000 1420
>>> +			6000 1406
>>> +			7000 1386
>>> +			8000 1366
>>> +			9000 1346
>>> +			10000 1326
>>> +			11000 1302
>>> +			12000 1278
>>> +			13000 1254
>>> +			14000 1230
>>> +			15000 1206
>>> +			16000 1182
>>> +			17000 1158
>>> +			18000 1134
>>> +			19000 1110
>>> +			20000 1086
>>> +			21000 1059
>>> +			22000 1035
>>> +			23000 1011
>>> +			24000 987
>>> +			25000 963
>>> +			26000 937
>>> +			27000 913
>>> +			28000 889
>>> +			29000 865
>>> +			30000 841
>>> +			31000 816
>>> +			32000 794
>>> +			33000 772
>>> +			34000 750
>>> +			35000 728
>>> +			36000 708
>>> +			37000 690
>>> +			38000 672
>>> +			39000 654
>>> +			40000 636
>>> +			41000 616
>>> +			42000 599
>>> +			43000 580
>>> +			44000 565
>>> +			45000 548
>>> +			46000 529
>>> +			47000 512
>>> +			48000 495
>>> +			49000 478
>>> +			50000 461
>>> +			51000 440
>>> +			52000 431
>>> +			53000 416
>>> +			54000 405
>>> +			55000 396
>>> +			56000 375
>>> +			57000 360
>>> +			58000 347
>>> +			59000 334
>>> +			60000 325
>>> +			61000 311
>>> +			62000 303
>>> +			63000 296
>>> +			64000 290
>>> +			65000 279
>>> +			66000 265
>>> +			67000 254
>>> +			68000 240
>>> +			69000 220
>>> +			70000 206>;
>>> +	};
>>> +
>>> +	charger_manager: charger-manager-0 {
>>> +		compatible = "charger-manager";
>>
>> Sorry, this is not a hardware. It's a hack to configure kernel charging
>> driver via DT which was made deprecated.
> 
> Thanks, I missed the deprecation notice in the binding file.
> 
> What would be the better way of creating a functional charging system?
> A new device-specific driver?

I am not sure, but maybe you could use charger-manager, just configure
it from user-space (or add such features). Better ask power supply
maintainer. But anyway charger-manager is mostly abandoned. I don't
think anyone develops it.

>  Userspace monitoring of temperature/connected
> device and extensions to the max8998 driver for enabling/disabling/configuring
> charging via the power supply subsystem instead of the regulator subsystem?
> Something else?

Enabling charging via regulators was done only for some drivers, I think
for charger-manager. I don't think it is the recommended way now.

Everything should be controlled rather via power supply from user-space.

How postmarketos or lineageos are doing it?

> 
> The way I understand the charging system, there is
> 
> - The fuelgauge (max17042)
> - The max8998 charger portion, including the ability to vary the current
> - The thermistor for checking battery temperature
> - The FSA9480 to determine what sort of cable is connected
> 

Best regards,
Krzysztof

Re: [PATCH 7/7] ARM: dts: s5pv210: Add charger support in Aries

[Krzysztof Kozlowski]

On 23/03/2022 18:20, Jonathan Bakker wrote:
> 
> 
> On 2022-03-23 8:31 a.m., krzk@kernel.org wrote:
>> On 23/03/2022 16:03, Jonathan Bakker wrote:
>>> Add charger-manager support to Aries boards to allow safe
>>> charging of the battery without the need for userspace control.
>>>
>>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>>> ---
>>>  arch/arm/boot/dts/s5pv210-fascinate4g.dts | 162 ++++++++++++++++++++++
>>>  arch/arm/boot/dts/s5pv210-galaxys.dts     | 144 +++++++++++++++++++
>>>  2 files changed, 306 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/s5pv210-fascinate4g.dts b/arch/arm/boot/dts/s5pv210-fascinate4g.dts
>>> index 7427c84f1126..9530231b7a70 100644
>>> --- a/arch/arm/boot/dts/s5pv210-fascinate4g.dts
>>> +++ b/arch/arm/boot/dts/s5pv210-fascinate4g.dts
>>> @@ -57,6 +57,168 @@
>>>  		pinctrl-0 = <&main_micbias_ena>;
>>>  	};
>>>  
>>> +	thermal-zones {
>>> +		batt_thermal: batt-thermal {
>>> +			polling-delay-passive = <60000>; /* 60 seconds */
>>
>> There is no passive cooling device, so why do you need it?
>>
> 
> The charger manager code needs a passive cooling device, so that's
> why this is present here.
> 
>>> +			polling-delay = <600000>; /* 600 seconds */
>>> +
>>> +			thermal-sensors = <&batt_thermistor>;
>>> +		};
>>> +	};
>>> +
>>> +	batt_thermistor: thermal-sensor-0 {
>>> +		compatible = "generic-adc-thermal";
>>> +		#thermal-sensor-cells = <0>;
>>> +		io-channels = <&adc 6>;
>>> +		io-channel-names = "sensor-channel";
>>> +
>>> +		temperature-lookup-table = <
>>> +			(-20000) 1859
>>> +			(-19000) 1846
>>> +			(-18000) 1832
>>> +			(-17000) 1818
>>> +			(-16000) 1804
>>> +			(-15000) 1790
>>> +			(-14000) 1773
>>> +			(-13000) 1756
>>> +			(-12000) 1739
>>> +			(-11000) 1722
>>> +			(-10000) 1705
>>> +			(-9000) 1691
>>> +			(-8000) 1677
>>> +			(-7000) 1663
>>> +			(-6000) 1649
>>> +			(-5000) 1635
>>> +			(-4000) 1550
>>> +			(-3000) 1510
>>> +			(-2000) 1500
>>> +			(-1000) 1490
>>> +			0 1480
>>> +			1000 1470
>>> +			2000 1460
>>> +			3000 1450
>>> +			4000 1430
>>> +			5000 1420
>>> +			6000 1406
>>> +			7000 1386
>>> +			8000 1366
>>> +			9000 1346
>>> +			10000 1326
>>> +			11000 1302
>>> +			12000 1278
>>> +			13000 1254
>>> +			14000 1230
>>> +			15000 1206
>>> +			16000 1182
>>> +			17000 1158
>>> +			18000 1134
>>> +			19000 1110
>>> +			20000 1086
>>> +			21000 1059
>>> +			22000 1035
>>> +			23000 1011
>>> +			24000 987
>>> +			25000 963
>>> +			26000 937
>>> +			27000 913
>>> +			28000 889
>>> +			29000 865
>>> +			30000 841
>>> +			31000 816
>>> +			32000 794
>>> +			33000 772
>>> +			34000 750
>>> +			35000 728
>>> +			36000 708
>>> +			37000 690
>>> +			38000 672
>>> +			39000 654
>>> +			40000 636
>>> +			41000 616
>>> +			42000 599
>>> +			43000 580
>>> +			44000 565
>>> +			45000 548
>>> +			46000 529
>>> +			47000 512
>>> +			48000 495
>>> +			49000 478
>>> +			50000 461
>>> +			51000 440
>>> +			52000 431
>>> +			53000 416
>>> +			54000 405
>>> +			55000 396
>>> +			56000 375
>>> +			57000 360
>>> +			58000 347
>>> +			59000 334
>>> +			60000 325
>>> +			61000 311
>>> +			62000 303
>>> +			63000 296
>>> +			64000 290
>>> +			65000 279
>>> +			66000 265
>>> +			67000 254
>>> +			68000 240
>>> +			69000 220
>>> +			70000 206>;
>>> +	};
>>> +
>>> +	charger_manager: charger-manager-0 {
>>> +		compatible = "charger-manager";
>>
>> Sorry, this is not a hardware. It's a hack to configure kernel charging
>> driver via DT which was made deprecated.
> 
> Thanks, I missed the deprecation notice in the binding file.
> 
> What would be the better way of creating a functional charging system?
> A new device-specific driver?

I am not sure, but maybe you could use charger-manager, just configure
it from user-space (or add such features). Better ask power supply
maintainer. But anyway charger-manager is mostly abandoned. I don't
think anyone develops it.

>  Userspace monitoring of temperature/connected
> device and extensions to the max8998 driver for enabling/disabling/configuring
> charging via the power supply subsystem instead of the regulator subsystem?
> Something else?

Enabling charging via regulators was done only for some drivers, I think
for charger-manager. I don't think it is the recommended way now.

Everything should be controlled rather via power supply from user-space.

How postmarketos or lineageos are doing it?

> 
> The way I understand the charging system, there is
> 
> - The fuelgauge (max17042)
> - The max8998 charger portion, including the ability to vary the current
> - The thermistor for checking battery temperature
> - The FSA9480 to determine what sort of cable is connected
> 

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 7/7] ARM: dts: s5pv210: Add charger support in Aries

[Jonathan Bakker]

Hi Krzysztof,

On 2022-03-24 4:47 a.m., Krzysztof Kozlowski wrote:
> On 23/03/2022 18:20, Jonathan Bakker wrote:
>>
>>
>> On 2022-03-23 8:31 a.m., krzk@kernel.org wrote:
>>> On 23/03/2022 16:03, Jonathan Bakker wrote:
>>>> Add charger-manager support to Aries boards to allow safe
>>>> charging of the battery without the need for userspace control.
>>>>
>>>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>>>> ---
>>>>  arch/arm/boot/dts/s5pv210-fascinate4g.dts | 162 ++++++++++++++++++++++
>>>>  arch/arm/boot/dts/s5pv210-galaxys.dts     | 144 +++++++++++++++++++
>>>>  2 files changed, 306 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/s5pv210-fascinate4g.dts b/arch/arm/boot/dts/s5pv210-fascinate4g.dts
>>>> index 7427c84f1126..9530231b7a70 100644
>>>> --- a/arch/arm/boot/dts/s5pv210-fascinate4g.dts
>>>> +++ b/arch/arm/boot/dts/s5pv210-fascinate4g.dts
>>>> @@ -57,6 +57,168 @@
>>>>  		pinctrl-0 = <&main_micbias_ena>;
>>>>  	};
>>>>  
>>>> +	thermal-zones {
>>>> +		batt_thermal: batt-thermal {
>>>> +			polling-delay-passive = <60000>; /* 60 seconds */
>>>
>>> There is no passive cooling device, so why do you need it?
>>>
>>
>> The charger manager code needs a passive cooling device, so that's
>> why this is present here.
>>
>>>> +			polling-delay = <600000>; /* 600 seconds */
>>>> +
>>>> +			thermal-sensors = <&batt_thermistor>;
>>>> +		};
>>>> +	};
>>>> +
>>>> +	batt_thermistor: thermal-sensor-0 {
>>>> +		compatible = "generic-adc-thermal";
>>>> +		#thermal-sensor-cells = <0>;
>>>> +		io-channels = <&adc 6>;
>>>> +		io-channel-names = "sensor-channel";
>>>> +
>>>> +		temperature-lookup-table = <
>>>> +			(-20000) 1859
>>>> +			(-19000) 1846
>>>> +			(-18000) 1832
>>>> +			(-17000) 1818
>>>> +			(-16000) 1804
>>>> +			(-15000) 1790
>>>> +			(-14000) 1773
>>>> +			(-13000) 1756
>>>> +			(-12000) 1739
>>>> +			(-11000) 1722
>>>> +			(-10000) 1705
>>>> +			(-9000) 1691
>>>> +			(-8000) 1677
>>>> +			(-7000) 1663
>>>> +			(-6000) 1649
>>>> +			(-5000) 1635
>>>> +			(-4000) 1550
>>>> +			(-3000) 1510
>>>> +			(-2000) 1500
>>>> +			(-1000) 1490
>>>> +			0 1480
>>>> +			1000 1470
>>>> +			2000 1460
>>>> +			3000 1450
>>>> +			4000 1430
>>>> +			5000 1420
>>>> +			6000 1406
>>>> +			7000 1386
>>>> +			8000 1366
>>>> +			9000 1346
>>>> +			10000 1326
>>>> +			11000 1302
>>>> +			12000 1278
>>>> +			13000 1254
>>>> +			14000 1230
>>>> +			15000 1206
>>>> +			16000 1182
>>>> +			17000 1158
>>>> +			18000 1134
>>>> +			19000 1110
>>>> +			20000 1086
>>>> +			21000 1059
>>>> +			22000 1035
>>>> +			23000 1011
>>>> +			24000 987
>>>> +			25000 963
>>>> +			26000 937
>>>> +			27000 913
>>>> +			28000 889
>>>> +			29000 865
>>>> +			30000 841
>>>> +			31000 816
>>>> +			32000 794
>>>> +			33000 772
>>>> +			34000 750
>>>> +			35000 728
>>>> +			36000 708
>>>> +			37000 690
>>>> +			38000 672
>>>> +			39000 654
>>>> +			40000 636
>>>> +			41000 616
>>>> +			42000 599
>>>> +			43000 580
>>>> +			44000 565
>>>> +			45000 548
>>>> +			46000 529
>>>> +			47000 512
>>>> +			48000 495
>>>> +			49000 478
>>>> +			50000 461
>>>> +			51000 440
>>>> +			52000 431
>>>> +			53000 416
>>>> +			54000 405
>>>> +			55000 396
>>>> +			56000 375
>>>> +			57000 360
>>>> +			58000 347
>>>> +			59000 334
>>>> +			60000 325
>>>> +			61000 311
>>>> +			62000 303
>>>> +			63000 296
>>>> +			64000 290
>>>> +			65000 279
>>>> +			66000 265
>>>> +			67000 254
>>>> +			68000 240
>>>> +			69000 220
>>>> +			70000 206>;
>>>> +	};
>>>> +
>>>> +	charger_manager: charger-manager-0 {
>>>> +		compatible = "charger-manager";
>>>
>>> Sorry, this is not a hardware. It's a hack to configure kernel charging
>>> driver via DT which was made deprecated.
>>
>> Thanks, I missed the deprecation notice in the binding file.
>>
>> What would be the better way of creating a functional charging system?
>> A new device-specific driver?
> 
> I am not sure, but maybe you could use charger-manager, just configure
> it from user-space (or add such features). Better ask power supply
> maintainer. But anyway charger-manager is mostly abandoned. I don't
> think anyone develops it.
> 

Yeah, I made it so that it could be probed a year and a half ago, but
there's been very little use of it - there's no in-tree users of it that
I can tell.

>>  Userspace monitoring of temperature/connected
>> device and extensions to the max8998 driver for enabling/disabling/configuring
>> charging via the power supply subsystem instead of the regulator subsystem?
>> Something else?
> 
> Enabling charging via regulators was done only for some drivers, I think
> for charger-manager. I don't think it is the recommended way now.
> 
> Everything should be controlled rather via power supply from user-space.
> 
> How postmarketos or lineageos are doing it?
> 

It's quite varied, but I can't find any examples of a userspace
daemon watching temperature/connection type, although it should be
fairly simple to implement.  A few examples:

- Maemo Leste uses a plugin to their MCE daemon to shutdown on low battery
- postmarketOS uses whatever is in kernel and whatever the DE provides to shutdown
on low battery
 - It also has a power stats collecting software, IIRC, although this might be DE
specific
- LineageOS/Android has healthd, but this appears to be more for gathering stats

In terms of in-tree examples, there's quite the variety:

- The AB8500 driver (used in some STE phones) is fairly similar situation,
with separate fuelgauge, charger and temperature drivers bound together by
an "algorithm" driver that checks temperatures and what is connected (very
similar to charger-manager, but specific to that HW).
- Nokia N900 has temperature reading integrated, but no shutoff
- Pinephone/Pinetab appears to have separate USB/AC charger drivers and
a battery driver, but no temperature monitoring.

So it rather looks like there's no consensus.  I guess I'll do some more research
and drop this patch for now.

Thanks,
Jonathan

>>
>> The way I understand the charging system, there is
>>
>> - The fuelgauge (max17042)
>> - The max8998 charger portion, including the ability to vary the current
>> - The thermistor for checking battery temperature
>> - The FSA9480 to determine what sort of cable is connected
>>
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 7/7] ARM: dts: s5pv210: Add charger support in Aries

[Jonathan Bakker]

Hi Krzysztof,

On 2022-03-24 4:47 a.m., Krzysztof Kozlowski wrote:
> On 23/03/2022 18:20, Jonathan Bakker wrote:
>>
>>
>> On 2022-03-23 8:31 a.m., krzk@kernel.org wrote:
>>> On 23/03/2022 16:03, Jonathan Bakker wrote:
>>>> Add charger-manager support to Aries boards to allow safe
>>>> charging of the battery without the need for userspace control.
>>>>
>>>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>>>> ---
>>>>  arch/arm/boot/dts/s5pv210-fascinate4g.dts | 162 ++++++++++++++++++++++
>>>>  arch/arm/boot/dts/s5pv210-galaxys.dts     | 144 +++++++++++++++++++
>>>>  2 files changed, 306 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/s5pv210-fascinate4g.dts b/arch/arm/boot/dts/s5pv210-fascinate4g.dts
>>>> index 7427c84f1126..9530231b7a70 100644
>>>> --- a/arch/arm/boot/dts/s5pv210-fascinate4g.dts
>>>> +++ b/arch/arm/boot/dts/s5pv210-fascinate4g.dts
>>>> @@ -57,6 +57,168 @@
>>>>  		pinctrl-0 = <&main_micbias_ena>;
>>>>  	};
>>>>  
>>>> +	thermal-zones {
>>>> +		batt_thermal: batt-thermal {
>>>> +			polling-delay-passive = <60000>; /* 60 seconds */
>>>
>>> There is no passive cooling device, so why do you need it?
>>>
>>
>> The charger manager code needs a passive cooling device, so that's
>> why this is present here.
>>
>>>> +			polling-delay = <600000>; /* 600 seconds */
>>>> +
>>>> +			thermal-sensors = <&batt_thermistor>;
>>>> +		};
>>>> +	};
>>>> +
>>>> +	batt_thermistor: thermal-sensor-0 {
>>>> +		compatible = "generic-adc-thermal";
>>>> +		#thermal-sensor-cells = <0>;
>>>> +		io-channels = <&adc 6>;
>>>> +		io-channel-names = "sensor-channel";
>>>> +
>>>> +		temperature-lookup-table = <
>>>> +			(-20000) 1859
>>>> +			(-19000) 1846
>>>> +			(-18000) 1832
>>>> +			(-17000) 1818
>>>> +			(-16000) 1804
>>>> +			(-15000) 1790
>>>> +			(-14000) 1773
>>>> +			(-13000) 1756
>>>> +			(-12000) 1739
>>>> +			(-11000) 1722
>>>> +			(-10000) 1705
>>>> +			(-9000) 1691
>>>> +			(-8000) 1677
>>>> +			(-7000) 1663
>>>> +			(-6000) 1649
>>>> +			(-5000) 1635
>>>> +			(-4000) 1550
>>>> +			(-3000) 1510
>>>> +			(-2000) 1500
>>>> +			(-1000) 1490
>>>> +			0 1480
>>>> +			1000 1470
>>>> +			2000 1460
>>>> +			3000 1450
>>>> +			4000 1430
>>>> +			5000 1420
>>>> +			6000 1406
>>>> +			7000 1386
>>>> +			8000 1366
>>>> +			9000 1346
>>>> +			10000 1326
>>>> +			11000 1302
>>>> +			12000 1278
>>>> +			13000 1254
>>>> +			14000 1230
>>>> +			15000 1206
>>>> +			16000 1182
>>>> +			17000 1158
>>>> +			18000 1134
>>>> +			19000 1110
>>>> +			20000 1086
>>>> +			21000 1059
>>>> +			22000 1035
>>>> +			23000 1011
>>>> +			24000 987
>>>> +			25000 963
>>>> +			26000 937
>>>> +			27000 913
>>>> +			28000 889
>>>> +			29000 865
>>>> +			30000 841
>>>> +			31000 816
>>>> +			32000 794
>>>> +			33000 772
>>>> +			34000 750
>>>> +			35000 728
>>>> +			36000 708
>>>> +			37000 690
>>>> +			38000 672
>>>> +			39000 654
>>>> +			40000 636
>>>> +			41000 616
>>>> +			42000 599
>>>> +			43000 580
>>>> +			44000 565
>>>> +			45000 548
>>>> +			46000 529
>>>> +			47000 512
>>>> +			48000 495
>>>> +			49000 478
>>>> +			50000 461
>>>> +			51000 440
>>>> +			52000 431
>>>> +			53000 416
>>>> +			54000 405
>>>> +			55000 396
>>>> +			56000 375
>>>> +			57000 360
>>>> +			58000 347
>>>> +			59000 334
>>>> +			60000 325
>>>> +			61000 311
>>>> +			62000 303
>>>> +			63000 296
>>>> +			64000 290
>>>> +			65000 279
>>>> +			66000 265
>>>> +			67000 254
>>>> +			68000 240
>>>> +			69000 220
>>>> +			70000 206>;
>>>> +	};
>>>> +
>>>> +	charger_manager: charger-manager-0 {
>>>> +		compatible = "charger-manager";
>>>
>>> Sorry, this is not a hardware. It's a hack to configure kernel charging
>>> driver via DT which was made deprecated.
>>
>> Thanks, I missed the deprecation notice in the binding file.
>>
>> What would be the better way of creating a functional charging system?
>> A new device-specific driver?
> 
> I am not sure, but maybe you could use charger-manager, just configure
> it from user-space (or add such features). Better ask power supply
> maintainer. But anyway charger-manager is mostly abandoned. I don't
> think anyone develops it.
> 

Yeah, I made it so that it could be probed a year and a half ago, but
there's been very little use of it - there's no in-tree users of it that
I can tell.

>>  Userspace monitoring of temperature/connected
>> device and extensions to the max8998 driver for enabling/disabling/configuring
>> charging via the power supply subsystem instead of the regulator subsystem?
>> Something else?
> 
> Enabling charging via regulators was done only for some drivers, I think
> for charger-manager. I don't think it is the recommended way now.
> 
> Everything should be controlled rather via power supply from user-space.
> 
> How postmarketos or lineageos are doing it?
> 

It's quite varied, but I can't find any examples of a userspace
daemon watching temperature/connection type, although it should be
fairly simple to implement.  A few examples:

- Maemo Leste uses a plugin to their MCE daemon to shutdown on low battery
- postmarketOS uses whatever is in kernel and whatever the DE provides to shutdown
on low battery
 - It also has a power stats collecting software, IIRC, although this might be DE
specific
- LineageOS/Android has healthd, but this appears to be more for gathering stats

In terms of in-tree examples, there's quite the variety:

- The AB8500 driver (used in some STE phones) is fairly similar situation,
with separate fuelgauge, charger and temperature drivers bound together by
an "algorithm" driver that checks temperatures and what is connected (very
similar to charger-manager, but specific to that HW).
- Nokia N900 has temperature reading integrated, but no shutoff
- Pinephone/Pinetab appears to have separate USB/AC charger drivers and
a battery driver, but no temperature monitoring.

So it rather looks like there's no consensus.  I guess I'll do some more research
and drop this patch for now.

Thanks,
Jonathan

>>
>> The way I understand the charging system, there is
>>
>> - The fuelgauge (max17042)
>> - The max8998 charger portion, including the ability to vary the current
>> - The thermistor for checking battery temperature
>> - The FSA9480 to determine what sort of cable is connected
>>
> 
> Best regards,
> Krzysztof
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel