diff mbox series

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

Message ID CY4PR04MB0567E33A07D8761C2D485327CB179@CY4PR04MB0567.namprd04.prod.outlook.com (mailing list archive)
State New, archived
Headers show
Series ARM: dts: s5pv210: Bugfixes, features, and improvements | expand

Commit Message

Jonathan Bakker March 22, 2022, 8:11 p.m. UTC
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(-)

Comments

Krzysztof Kozlowski March 23, 2022, 10:54 a.m. UTC | #1
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
Jonathan Bakker March 23, 2022, 2:59 p.m. UTC | #2
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
>
Krzysztof Kozlowski March 23, 2022, 3:06 p.m. UTC | #3
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
Krzysztof Kozlowski March 23, 2022, 3:14 p.m. UTC | #4
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
Krzysztof Kozlowski March 23, 2022, 3:15 p.m. UTC | #5
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
Jonathan Bakker March 23, 2022, 5:05 p.m. UTC | #6
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
>
Jonathan Bakker March 23, 2022, 5:24 p.m. UTC | #7
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
>
Krzysztof Kozlowski March 24, 2022, 11:23 a.m. UTC | #8
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
Krzysztof Kozlowski March 24, 2022, 11:49 a.m. UTC | #9
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
Krzysztof Kozlowski April 21, 2023, 9:44 a.m. UTC | #10
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
Jonathan Bakker May 3, 2023, 2:33 a.m. UTC | #11
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
diff mbox series

Patch

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 {