Re: [PATCH] remoteproc: imx_rproc: merge ITCM and DTCM regions

[Mathieu Poirier <mathieu.poirier@linaro.org>]

On Fri, Jul 04, 2025 at 04:08:16PM -0300, Hiago De Franco wrote:
> Hi Mathieu,
> 
> On Fri, Jul 04, 2025 at 10:25:19AM -0600, Mathieu Poirier wrote:
> > Good morning,
> > 
> > On Thu, Jul 03, 2025 at 10:08:31AM -0300, Hiago De Franco wrote:
> > > From: Hiago De Franco <hiago.franco@toradex.com>
> > > 
> > > Merge the contiguous ITCM and DTCM regions into a single region to
> > > prevent failures when loading ELF files with large sections:
> > > 
> > > remoteproc remoteproc0: powering up imx-rproc
> > > remoteproc remoteproc0: Booting fw image rproc-imx-rproc-fw, size 151824
> > > imx-rproc imx8mp-cm7: Translation failed: da = 0x1f48 len = 0x1fcb0
> > > remoteproc remoteproc0: bad phdr da 0x1f48 mem 0x1fcb0
> > > remoteproc remoteproc0: Failed to load program segments: -22
> > > remoteproc remoteproc0: Boot failed: -22
> > > 
> > > This approach is the same as commit 8749919defb8 ("remoteproc:
> > > imx_rproc: Merge TCML/U").
> > > 
> > > Suggested-by: Ritesh Kumar <ritesh.kumar@toradex.com>
> > > Signed-off-by: Hiago De Franco <hiago.franco@toradex.com>
> > > ---
> > > Hi,
> > > 
> > > The ELF I tested had the following data section:
> > > 
> > > Memory region         Used Size  Region Size  %age Used
> > >     m_interrupts:         680 B         1 KB     66.41%
> > >           m_text:        6984 B       127 KB      5.37%
> > >           m_data:      130224 B       128 KB     99.35%
> > >          m_data2:          0 GB        16 MB      0.00%
> > > [100%] Built target hello_world_cm7.elf
> > > 
> > > Which triggered the error. After this patch, remoteproc was able to boot
> > > and work fine. Thanks!
> > > ---
> > >  drivers/remoteproc/imx_rproc.c | 6 ++----
> > >  1 file changed, 2 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> > > index 74299af1d7f1..bbf089ef48ee 100644
> > > --- a/drivers/remoteproc/imx_rproc.c
> > > +++ b/drivers/remoteproc/imx_rproc.c
> > > @@ -166,8 +166,8 @@ static const struct imx_rproc_att imx_rproc_att_imx8qxp[] = {
> > >  
> > >  static const struct imx_rproc_att imx_rproc_att_imx8mn[] = {
> > >  	/* dev addr , sys addr  , size	    , flags */
> > > -	/* ITCM   */
> > > -	{ 0x00000000, 0x007E0000, 0x00020000, ATT_OWN | ATT_IOMEM },
> > > +	/* D/ITCM */
> > > +	{ 0x00000000, 0x007E0000, 0x00040000, ATT_OWN | ATT_IOMEM },
> > >  	/* OCRAM_S */
> > >  	{ 0x00180000, 0x00180000, 0x00009000, 0 },
> > >  	/* OCRAM */
> > > @@ -180,8 +180,6 @@ static const struct imx_rproc_att imx_rproc_att_imx8mn[] = {
> > >  	{ 0x08000000, 0x08000000, 0x08000000, 0 },
> > >  	/* DDR (Code) - alias */
> > >  	{ 0x10000000, 0x40000000, 0x0FFE0000, 0 },
> > > -	/* DTCM */
> > > -	{ 0x20000000, 0x00800000, 0x00020000, ATT_OWN | ATT_IOMEM },
> > 
> > In commit 8749919defb8 "dev addr" and "sys addr" were both contiguous, but in
> > this patch "dev addr" is not.  How will this work with new kernel that use old
> > FW images?  Am I missing something?
> 
> No, you are correct, I think the use case I tested was not good enough.
> 
> If I understand correctly, this will break older firmware expecting
> .data at 0x20000000 because dev_addr is no longer mapped for DTCM entry.
> 

Correct.  Older firmware would still expect DTCM at 0x20000000.


> Do you think it is possible (or reccomend) another approach to fix this
> issue? In this case to keep using the TCM, instead of going to OCRAM or
> DDR.
>

To me the best way to proceed is understand why using the current mapping is a
problem.  The changelog describes a failure condition when dealing with large
sections but does not indicate _why_ that is happening.  I think that's what
needs to be fixed rather than trying to move mappings around.
 
> Thanks,
> Hiago.
> 
> > 
> > Thanks,
> > Mathieu
> > 
> > >  	/* OCRAM_S - alias */
> > >  	{ 0x20180000, 0x00180000, 0x00008000, ATT_OWN },
> > >  	/* OCRAM */
> > > -- 
> > > 2.39.5
> > >