| Message ID | CY4PR04MB0567E33A07D8761C2D485327CB179@CY4PR04MB0567.namprd04.prod.outlook.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/7] ARM: dts: s5pv210: Split memory nodes to match spec | expand |
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
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 >
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
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 >
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
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 {
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(-)