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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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
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 <[email protected]>
---
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
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 <[email protected]>
>> ---
>> 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
>
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 <[email protected]>
>>> ---
>>> 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
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 <[email protected]>
> ---
> 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/[email protected]/
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
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 <[email protected]>
>>>> ---
>>>> 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
>
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 <[email protected]>
>>>>> ---
>>>>> 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