mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH] ARM: dts: i.MX8MP: remove memory node
@ 2023-03-12 16:29 Marco Felsch
  2023-03-13 13:03 ` Lucas Stach
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Marco Felsch @ 2023-03-12 16:29 UTC (permalink / raw)
  To: barebox

since commit f083fffe52 ("ARM: dts: i.MX8MP: add DDRC compatibles") we
make use of the esdctl driver. This cause the below error since barebox
try to add the memory twice.

| imx-esdctl 3d400000.memory-controller@3d400000.of: probe failed: Device or resource busy
| initcall imx_esdctl_driver_init+0x0/0x2c failed: No such device

Remove the memory node to fix this.

This behaviour was seen on a i.mx8mp-evk but the
imx8mp-tqma8mpql-mba8mpxl also has a memory node and includes the
barebox.dtsi.

Fixes: f083fffe52 ("ARM: dts: i.MX8MP: add DDRC compatibles")
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 arch/arm/dts/imx8mp-evk.dts                | 2 ++
 arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/arch/arm/dts/imx8mp-evk.dts b/arch/arm/dts/imx8mp-evk.dts
index 0acc3731d5..c7e1f35d2d 100644
--- a/arch/arm/dts/imx8mp-evk.dts
+++ b/arch/arm/dts/imx8mp-evk.dts
@@ -30,6 +30,8 @@
 	};
 };
 
+/delete-node/ &{/memory@40000000};
+
 &ethphy1 {
 	reset-assert-us = <15000>;
 	reset-deassert-us = <100000>;
diff --git a/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts b/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts
index 6e81f58e27..bf23e40489 100644
--- a/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts
+++ b/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts
@@ -24,6 +24,8 @@
 	};
 };
 
+/delete-node/ &{/memory@40000000};
+
 &usdhc2 {
 	#address-cells = <1>;
 	#size-cells = <1>;
-- 
2.30.2




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] ARM: dts: i.MX8MP: remove memory node
  2023-03-12 16:29 [PATCH] ARM: dts: i.MX8MP: remove memory node Marco Felsch
@ 2023-03-13 13:03 ` Lucas Stach
  2023-03-13 13:46   ` Marco Felsch
  2023-03-13 13:16 ` Ahmad Fatoum
  2023-03-14 14:08 ` Sascha Hauer
  2 siblings, 1 reply; 9+ messages in thread
From: Lucas Stach @ 2023-03-13 13:03 UTC (permalink / raw)
  To: Marco Felsch, barebox

Am Sonntag, dem 12.03.2023 um 17:29 +0100 schrieb Marco Felsch:
> since commit f083fffe52 ("ARM: dts: i.MX8MP: add DDRC compatibles") we
> make use of the esdctl driver. This cause the below error since barebox
> try to add the memory twice.
> 
> > imx-esdctl 3d400000.memory-controller@3d400000.of: probe failed: Device or resource busy
> > initcall imx_esdctl_driver_init+0x0/0x2c failed: No such device
> 
I wonder why I didn't hit this. Probably some probe order thing.

> Remove the memory node to fix this.
> 
> This behaviour was seen on a i.mx8mp-evk but the
> imx8mp-tqma8mpql-mba8mpxl also has a memory node and includes the
> barebox.dtsi.
> 
> Fixes: f083fffe52 ("ARM: dts: i.MX8MP: add DDRC compatibles")
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  arch/arm/dts/imx8mp-evk.dts                | 2 ++
>  arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/arch/arm/dts/imx8mp-evk.dts b/arch/arm/dts/imx8mp-evk.dts
> index 0acc3731d5..c7e1f35d2d 100644
> --- a/arch/arm/dts/imx8mp-evk.dts
> +++ b/arch/arm/dts/imx8mp-evk.dts
> @@ -30,6 +30,8 @@
>  	};
>  };
>  
> +/delete-node/ &{/memory@40000000};
> +
Why not add this to arch/arm/dts/imx8mp.dtsi?

Regards,
Lucas

>  &ethphy1 {
>  	reset-assert-us = <15000>;
>  	reset-deassert-us = <100000>;
> diff --git a/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts b/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts
> index 6e81f58e27..bf23e40489 100644
> --- a/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts
> +++ b/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts
> @@ -24,6 +24,8 @@
>  	};
>  };
>  
> +/delete-node/ &{/memory@40000000};
> +
>  &usdhc2 {
>  	#address-cells = <1>;
>  	#size-cells = <1>;




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] ARM: dts: i.MX8MP: remove memory node
  2023-03-12 16:29 [PATCH] ARM: dts: i.MX8MP: remove memory node Marco Felsch
  2023-03-13 13:03 ` Lucas Stach
@ 2023-03-13 13:16 ` Ahmad Fatoum
  2023-03-13 13:49   ` Marco Felsch
  2023-03-14 14:08 ` Sascha Hauer
  2 siblings, 1 reply; 9+ messages in thread
From: Ahmad Fatoum @ 2023-03-13 13:16 UTC (permalink / raw)
  To: Marco Felsch, barebox

On 12.03.23 17:29, Marco Felsch wrote:
> since commit f083fffe52 ("ARM: dts: i.MX8MP: add DDRC compatibles") we
> make use of the esdctl driver. This cause the below error since barebox
> try to add the memory twice.
> 
> | imx-esdctl 3d400000.memory-controller@3d400000.of: probe failed: Device or resource busy
> | initcall imx_esdctl_driver_init+0x0/0x2c failed: No such device

Starting with commit 0a78ac84e9fe ("memory: fuse overlapping memory banks"),
this should not be leading to errors. Can you increase debugging in
common/resource.c and see why you run into this error?


> 
> Remove the memory node to fix this.
> 
> This behaviour was seen on a i.mx8mp-evk but the
> imx8mp-tqma8mpql-mba8mpxl also has a memory node and includes the
> barebox.dtsi.
> 
> Fixes: f083fffe52 ("ARM: dts: i.MX8MP: add DDRC compatibles")
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  arch/arm/dts/imx8mp-evk.dts                | 2 ++
>  arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/arch/arm/dts/imx8mp-evk.dts b/arch/arm/dts/imx8mp-evk.dts
> index 0acc3731d5..c7e1f35d2d 100644
> --- a/arch/arm/dts/imx8mp-evk.dts
> +++ b/arch/arm/dts/imx8mp-evk.dts
> @@ -30,6 +30,8 @@
>  	};
>  };
>  
> +/delete-node/ &{/memory@40000000};
> +
>  &ethphy1 {
>  	reset-assert-us = <15000>;
>  	reset-deassert-us = <100000>;
> diff --git a/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts b/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts
> index 6e81f58e27..bf23e40489 100644
> --- a/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts
> +++ b/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts
> @@ -24,6 +24,8 @@
>  	};
>  };
>  
> +/delete-node/ &{/memory@40000000};
> +
>  &usdhc2 {
>  	#address-cells = <1>;
>  	#size-cells = <1>;

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] ARM: dts: i.MX8MP: remove memory node
  2023-03-13 13:03 ` Lucas Stach
@ 2023-03-13 13:46   ` Marco Felsch
  0 siblings, 0 replies; 9+ messages in thread
From: Marco Felsch @ 2023-03-13 13:46 UTC (permalink / raw)
  To: Lucas Stach; +Cc: barebox

On 23-03-13, Lucas Stach wrote:
> Am Sonntag, dem 12.03.2023 um 17:29 +0100 schrieb Marco Felsch:
> > since commit f083fffe52 ("ARM: dts: i.MX8MP: add DDRC compatibles") we
> > make use of the esdctl driver. This cause the below error since barebox
> > try to add the memory twice.
> > 
> > > imx-esdctl 3d400000.memory-controller@3d400000.of: probe failed: Device or resource busy
> > > initcall imx_esdctl_driver_init+0x0/0x2c failed: No such device
> > 
> I wonder why I didn't hit this. Probably some probe order thing.

I think this is because of the fact that the boards do add the memory
node not the 8mp.dtsi and the debix don't have such a node.

> > Remove the memory node to fix this.
> > 
> > This behaviour was seen on a i.mx8mp-evk but the
> > imx8mp-tqma8mpql-mba8mpxl also has a memory node and includes the
> > barebox.dtsi.
> > 
> > Fixes: f083fffe52 ("ARM: dts: i.MX8MP: add DDRC compatibles")
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> >  arch/arm/dts/imx8mp-evk.dts                | 2 ++
> >  arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts | 2 ++
> >  2 files changed, 4 insertions(+)
> > 
> > diff --git a/arch/arm/dts/imx8mp-evk.dts b/arch/arm/dts/imx8mp-evk.dts
> > index 0acc3731d5..c7e1f35d2d 100644
> > --- a/arch/arm/dts/imx8mp-evk.dts
> > +++ b/arch/arm/dts/imx8mp-evk.dts
> > @@ -30,6 +30,8 @@
> >  	};
> >  };
> >  
> > +/delete-node/ &{/memory@40000000};
> > +
> Why not add this to arch/arm/dts/imx8mp.dtsi?

What does the compiler if ther is no such node? If the compiler just
ignore deleting not exisiting nodes I'm fine with your proposal. Thanks
for the review.

Regards,
  Marco

> 
> Regards,
> Lucas
> 
> >  &ethphy1 {
> >  	reset-assert-us = <15000>;
> >  	reset-deassert-us = <100000>;
> > diff --git a/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts b/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts
> > index 6e81f58e27..bf23e40489 100644
> > --- a/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts
> > +++ b/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts
> > @@ -24,6 +24,8 @@
> >  	};
> >  };
> >  
> > +/delete-node/ &{/memory@40000000};
> > +
> >  &usdhc2 {
> >  	#address-cells = <1>;
> >  	#size-cells = <1>;
> 
> 



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] ARM: dts: i.MX8MP: remove memory node
  2023-03-13 13:16 ` Ahmad Fatoum
@ 2023-03-13 13:49   ` Marco Felsch
  2023-03-13 13:58     ` Ahmad Fatoum
  0 siblings, 1 reply; 9+ messages in thread
From: Marco Felsch @ 2023-03-13 13:49 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

On 23-03-13, Ahmad Fatoum wrote:
> On 12.03.23 17:29, Marco Felsch wrote:
> > since commit f083fffe52 ("ARM: dts: i.MX8MP: add DDRC compatibles") we
> > make use of the esdctl driver. This cause the below error since barebox
> > try to add the memory twice.
> > 
> > | imx-esdctl 3d400000.memory-controller@3d400000.of: probe failed: Device or resource busy
> > | initcall imx_esdctl_driver_init+0x0/0x2c failed: No such device
> 
> Starting with commit 0a78ac84e9fe ("memory: fuse overlapping memory banks"),
> this should not be leading to errors. Can you increase debugging in
> common/resource.c and see why you run into this error?

I did this and saw that memory bank ram0 is added added twice:
 - first by generic of
 - 2nd by esdctl

Regards,
   Marco


> 
> 
> > 
> > Remove the memory node to fix this.
> > 
> > This behaviour was seen on a i.mx8mp-evk but the
> > imx8mp-tqma8mpql-mba8mpxl also has a memory node and includes the
> > barebox.dtsi.
> > 
> > Fixes: f083fffe52 ("ARM: dts: i.MX8MP: add DDRC compatibles")
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> >  arch/arm/dts/imx8mp-evk.dts                | 2 ++
> >  arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts | 2 ++
> >  2 files changed, 4 insertions(+)
> > 
> > diff --git a/arch/arm/dts/imx8mp-evk.dts b/arch/arm/dts/imx8mp-evk.dts
> > index 0acc3731d5..c7e1f35d2d 100644
> > --- a/arch/arm/dts/imx8mp-evk.dts
> > +++ b/arch/arm/dts/imx8mp-evk.dts
> > @@ -30,6 +30,8 @@
> >  	};
> >  };
> >  
> > +/delete-node/ &{/memory@40000000};
> > +
> >  &ethphy1 {
> >  	reset-assert-us = <15000>;
> >  	reset-deassert-us = <100000>;
> > diff --git a/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts b/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts
> > index 6e81f58e27..bf23e40489 100644
> > --- a/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts
> > +++ b/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts
> > @@ -24,6 +24,8 @@
> >  	};
> >  };
> >  
> > +/delete-node/ &{/memory@40000000};
> > +
> >  &usdhc2 {
> >  	#address-cells = <1>;
> >  	#size-cells = <1>;
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
> 



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] ARM: dts: i.MX8MP: remove memory node
  2023-03-13 13:49   ` Marco Felsch
@ 2023-03-13 13:58     ` Ahmad Fatoum
  2023-03-13 14:02       ` Ahmad Fatoum
  0 siblings, 1 reply; 9+ messages in thread
From: Ahmad Fatoum @ 2023-03-13 13:58 UTC (permalink / raw)
  To: Marco Felsch; +Cc: barebox

On 13.03.23 14:49, Marco Felsch wrote:
> On 23-03-13, Ahmad Fatoum wrote:
>> On 12.03.23 17:29, Marco Felsch wrote:
>>> since commit f083fffe52 ("ARM: dts: i.MX8MP: add DDRC compatibles") we
>>> make use of the esdctl driver. This cause the below error since barebox
>>> try to add the memory twice.
>>>
>>> | imx-esdctl 3d400000.memory-controller@3d400000.of: probe failed: Device or resource busy
>>> | initcall imx_esdctl_driver_init+0x0/0x2c failed: No such device
>>
>> Starting with commit 0a78ac84e9fe ("memory: fuse overlapping memory banks"),
>> this should not be leading to errors. Can you increase debugging in
>> common/resource.c and see why you run into this error?
> 
> I did this and saw that memory bank ram0 is added added twice:
>  - first by generic of
>  - 2nd by esdctl

See the first loop in barebox_add_memory_bank(). If you add
two overlapping IORESOURCE_MEM banks, the function will not
return an error. I think there might be a problem if you run
into an error. 

> 
> Regards,
>    Marco
> 
> 
>>
>>
>>>
>>> Remove the memory node to fix this.
>>>
>>> This behaviour was seen on a i.mx8mp-evk but the
>>> imx8mp-tqma8mpql-mba8mpxl also has a memory node and includes the
>>> barebox.dtsi.
>>>
>>> Fixes: f083fffe52 ("ARM: dts: i.MX8MP: add DDRC compatibles")
>>> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
>>> ---
>>>  arch/arm/dts/imx8mp-evk.dts                | 2 ++
>>>  arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts | 2 ++
>>>  2 files changed, 4 insertions(+)
>>>
>>> diff --git a/arch/arm/dts/imx8mp-evk.dts b/arch/arm/dts/imx8mp-evk.dts
>>> index 0acc3731d5..c7e1f35d2d 100644
>>> --- a/arch/arm/dts/imx8mp-evk.dts
>>> +++ b/arch/arm/dts/imx8mp-evk.dts
>>> @@ -30,6 +30,8 @@
>>>  	};
>>>  };
>>>  
>>> +/delete-node/ &{/memory@40000000};
>>> +
>>>  &ethphy1 {
>>>  	reset-assert-us = <15000>;
>>>  	reset-deassert-us = <100000>;
>>> diff --git a/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts b/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts
>>> index 6e81f58e27..bf23e40489 100644
>>> --- a/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts
>>> +++ b/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts
>>> @@ -24,6 +24,8 @@
>>>  	};
>>>  };
>>>  
>>> +/delete-node/ &{/memory@40000000};
>>> +
>>>  &usdhc2 {
>>>  	#address-cells = <1>;
>>>  	#size-cells = <1>;
>>
>> -- 
>> Pengutronix e.K.                           |                             |
>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>>
>>
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] ARM: dts: i.MX8MP: remove memory node
  2023-03-13 13:58     ` Ahmad Fatoum
@ 2023-03-13 14:02       ` Ahmad Fatoum
  2023-03-13 16:08         ` Marco Felsch
  0 siblings, 1 reply; 9+ messages in thread
From: Ahmad Fatoum @ 2023-03-13 14:02 UTC (permalink / raw)
  To: Marco Felsch; +Cc: barebox

On 13.03.23 14:58, Ahmad Fatoum wrote:
> On 13.03.23 14:49, Marco Felsch wrote:
>> On 23-03-13, Ahmad Fatoum wrote:
>>> On 12.03.23 17:29, Marco Felsch wrote:
>>>> since commit f083fffe52 ("ARM: dts: i.MX8MP: add DDRC compatibles") we
>>>> make use of the esdctl driver. This cause the below error since barebox
>>>> try to add the memory twice.
>>>>
>>>> | imx-esdctl 3d400000.memory-controller@3d400000.of: probe failed: Device or resource busy
>>>> | initcall imx_esdctl_driver_init+0x0/0x2c failed: No such device
>>>
>>> Starting with commit 0a78ac84e9fe ("memory: fuse overlapping memory banks"),
>>> this should not be leading to errors. Can you increase debugging in
>>> common/resource.c and see why you run into this error?
>>
>> I did this and saw that memory bank ram0 is added added twice:
>>  - first by generic of
>>  - 2nd by esdctl
> 
> See the first loop in barebox_add_memory_bank(). If you add
> two overlapping IORESOURCE_MEM banks, the function will not

s/overlapping/region fully within the other region/

> return an error. I think there might be a problem if you run
> into an error. 
> 
>>
>> Regards,
>>    Marco
>>
>>
>>>
>>>
>>>>
>>>> Remove the memory node to fix this.
>>>>
>>>> This behaviour was seen on a i.mx8mp-evk but the
>>>> imx8mp-tqma8mpql-mba8mpxl also has a memory node and includes the
>>>> barebox.dtsi.
>>>>
>>>> Fixes: f083fffe52 ("ARM: dts: i.MX8MP: add DDRC compatibles")
>>>> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
>>>> ---
>>>>  arch/arm/dts/imx8mp-evk.dts                | 2 ++
>>>>  arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts | 2 ++
>>>>  2 files changed, 4 insertions(+)
>>>>
>>>> diff --git a/arch/arm/dts/imx8mp-evk.dts b/arch/arm/dts/imx8mp-evk.dts
>>>> index 0acc3731d5..c7e1f35d2d 100644
>>>> --- a/arch/arm/dts/imx8mp-evk.dts
>>>> +++ b/arch/arm/dts/imx8mp-evk.dts
>>>> @@ -30,6 +30,8 @@
>>>>  	};
>>>>  };
>>>>  
>>>> +/delete-node/ &{/memory@40000000};
>>>> +
>>>>  &ethphy1 {
>>>>  	reset-assert-us = <15000>;
>>>>  	reset-deassert-us = <100000>;
>>>> diff --git a/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts b/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts
>>>> index 6e81f58e27..bf23e40489 100644
>>>> --- a/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts
>>>> +++ b/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts
>>>> @@ -24,6 +24,8 @@
>>>>  	};
>>>>  };
>>>>  
>>>> +/delete-node/ &{/memory@40000000};
>>>> +
>>>>  &usdhc2 {
>>>>  	#address-cells = <1>;
>>>>  	#size-cells = <1>;
>>>
>>> -- 
>>> Pengutronix e.K.                           |                             |
>>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
>>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>>>
>>>
>>
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] ARM: dts: i.MX8MP: remove memory node
  2023-03-13 14:02       ` Ahmad Fatoum
@ 2023-03-13 16:08         ` Marco Felsch
  0 siblings, 0 replies; 9+ messages in thread
From: Marco Felsch @ 2023-03-13 16:08 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

On 23-03-13, Ahmad Fatoum wrote:
> On 13.03.23 14:58, Ahmad Fatoum wrote:
> > On 13.03.23 14:49, Marco Felsch wrote:
> >> On 23-03-13, Ahmad Fatoum wrote:
> >>> On 12.03.23 17:29, Marco Felsch wrote:
> >>>> since commit f083fffe52 ("ARM: dts: i.MX8MP: add DDRC compatibles") we
> >>>> make use of the esdctl driver. This cause the below error since barebox
> >>>> try to add the memory twice.
> >>>>
> >>>> | imx-esdctl 3d400000.memory-controller@3d400000.of: probe failed: Device or resource busy
> >>>> | initcall imx_esdctl_driver_init+0x0/0x2c failed: No such device
> >>>
> >>> Starting with commit 0a78ac84e9fe ("memory: fuse overlapping memory banks"),
> >>> this should not be leading to errors. Can you increase debugging in
> >>> common/resource.c and see why you run into this error?
> >>
> >> I did this and saw that memory bank ram0 is added added twice:
> >>  - first by generic of
> >>  - 2nd by esdctl
> > 
> > See the first loop in barebox_add_memory_bank(). If you add
> > two overlapping IORESOURCE_MEM banks, the function will not
> 
> s/overlapping/region fully within the other region/

Thanks for the hint, I checked this and now see the issue:
 - 1st) of_add_memory_bank() add two banks for the 6G option:
    - ram0: 0x40000000  0xc0000000
    - ram1: 0x100000000 0xc0000000

 - 2nd) the esdctl dirver try to add only one bank for the 6G:
    - ram0: 0x40000000 0x180000000

The overlap mechanism is triggered and we enter the
barebox_grow_memory_bank() but since we already requested ram1 we can't
grow ram0. So the tqm8 board isn't effected and a 'lacy' approach is to
remvoe the memory node as suggested by Lucas.

Regards,
  Marco

> 
> > return an error. I think there might be a problem if you run
> > into an error. 
> > 
> >>
> >> Regards,
> >>    Marco
> >>
> >>
> >>>
> >>>
> >>>>
> >>>> Remove the memory node to fix this.
> >>>>
> >>>> This behaviour was seen on a i.mx8mp-evk but the
> >>>> imx8mp-tqma8mpql-mba8mpxl also has a memory node and includes the
> >>>> barebox.dtsi.
> >>>>
> >>>> Fixes: f083fffe52 ("ARM: dts: i.MX8MP: add DDRC compatibles")
> >>>> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> >>>> ---
> >>>>  arch/arm/dts/imx8mp-evk.dts                | 2 ++
> >>>>  arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts | 2 ++
> >>>>  2 files changed, 4 insertions(+)
> >>>>
> >>>> diff --git a/arch/arm/dts/imx8mp-evk.dts b/arch/arm/dts/imx8mp-evk.dts
> >>>> index 0acc3731d5..c7e1f35d2d 100644
> >>>> --- a/arch/arm/dts/imx8mp-evk.dts
> >>>> +++ b/arch/arm/dts/imx8mp-evk.dts
> >>>> @@ -30,6 +30,8 @@
> >>>>  	};
> >>>>  };
> >>>>  
> >>>> +/delete-node/ &{/memory@40000000};
> >>>> +
> >>>>  &ethphy1 {
> >>>>  	reset-assert-us = <15000>;
> >>>>  	reset-deassert-us = <100000>;
> >>>> diff --git a/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts b/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts
> >>>> index 6e81f58e27..bf23e40489 100644
> >>>> --- a/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts
> >>>> +++ b/arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts
> >>>> @@ -24,6 +24,8 @@
> >>>>  	};
> >>>>  };
> >>>>  
> >>>> +/delete-node/ &{/memory@40000000};
> >>>> +
> >>>>  &usdhc2 {
> >>>>  	#address-cells = <1>;
> >>>>  	#size-cells = <1>;
> >>>
> >>> -- 
> >>> Pengutronix e.K.                           |                             |
> >>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> >>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> >>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> >>>
> >>>
> >>
> > 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
> 



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] ARM: dts: i.MX8MP: remove memory node
  2023-03-12 16:29 [PATCH] ARM: dts: i.MX8MP: remove memory node Marco Felsch
  2023-03-13 13:03 ` Lucas Stach
  2023-03-13 13:16 ` Ahmad Fatoum
@ 2023-03-14 14:08 ` Sascha Hauer
  2 siblings, 0 replies; 9+ messages in thread
From: Sascha Hauer @ 2023-03-14 14:08 UTC (permalink / raw)
  To: Marco Felsch; +Cc: barebox

On Sun, Mar 12, 2023 at 05:29:28PM +0100, Marco Felsch wrote:
> since commit f083fffe52 ("ARM: dts: i.MX8MP: add DDRC compatibles") we
> make use of the esdctl driver. This cause the below error since barebox
> try to add the memory twice.
> 
> | imx-esdctl 3d400000.memory-controller@3d400000.of: probe failed: Device or resource busy
> | initcall imx_esdctl_driver_init+0x0/0x2c failed: No such device
> 
> Remove the memory node to fix this.
> 
> This behaviour was seen on a i.mx8mp-evk but the
> imx8mp-tqma8mpql-mba8mpxl also has a memory node and includes the
> barebox.dtsi.
> 
> Fixes: f083fffe52 ("ARM: dts: i.MX8MP: add DDRC compatibles")
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  arch/arm/dts/imx8mp-evk.dts                | 2 ++
>  arch/arm/dts/imx8mp-tqma8mpql-mba8mpxl.dts | 2 ++
>  2 files changed, 4 insertions(+)

Applied for now as it fixes a regression.

For the longer run I'd like to try another approach:

On i.MX as well as on other SoCs we read the DDR resources from the
memory controller. When we do this it should take precedence over the
device tree memory nodes. If reading from the memory controller returns
wrong DDR resources then this is a bug that should be fixed. Trying to
merge the memory banks read from the controller with the ones from the
device tree likely doesn't improve the situation.

Therefore the memory controller should register the memory banks along
with some we-know-it-better flag which disables reading the memory
nodes from the device tree. This leaves reading the memory nodes from
the device tree for the cases where no memory controller driver is
available.

There are some known cases where the memory controller is configured
with bigger SDRAM than is actually equipped on the board. We'll find
the memory mirrored than in the registered memory banks. For i.MX
we have imx_esdctl_disable() for this case, the same could be done
for other SoCs as well if needed.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-03-14 14:09 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-12 16:29 [PATCH] ARM: dts: i.MX8MP: remove memory node Marco Felsch
2023-03-13 13:03 ` Lucas Stach
2023-03-13 13:46   ` Marco Felsch
2023-03-13 13:16 ` Ahmad Fatoum
2023-03-13 13:49   ` Marco Felsch
2023-03-13 13:58     ` Ahmad Fatoum
2023-03-13 14:02       ` Ahmad Fatoum
2023-03-13 16:08         ` Marco Felsch
2023-03-14 14:08 ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox