mbox series

[V2,0/6] remoteproc: imx_rproc: support firmware in DDR

Message ID 20230127092246.1470865-1-peng.fan@oss.nxp.com
Headers
Series remoteproc: imx_rproc: support firmware in DDR |

Message

Peng Fan (OSS) Jan. 27, 2023, 9:22 a.m. UTC 
  From: Peng Fan <peng.fan@nxp.com>

V2:
 patch 4 is introduced for sparse check warning fix
 
This pachset is to support i.MX8M and i.MX93 Cortex-M core firmware could
be in DDR, not just the default TCM.

i.MX8M needs stack/pc value be stored in TCML entry address[0,4], the
initial value could be got from firmware first section ".interrupts".
i.MX93 is a bit different, it just needs the address of .interrupts
section. NXP SDK always has .interrupts section.

So first we need find the .interrupts section from firmware, so patch 1
is to reuse the code of find_table to introduce a new API
rproc_elf_find_shdr to find shdr, the it could reused by i.MX driver.

Patch 2 is introduce devtype for i.MX8M/93

Although patch 3 is correct the mapping, but this area was never used
by NXP SW team, we directly use the DDR region, not the alias region.
Since this patchset is first to support firmware in DDR, mark this patch
as a fix does not make much sense.

patch 4 and 5 is support i.MX8M/93 firmware in DDR with parsing .interrupts
section. Detailed information in each patch commit message.

Patches were tested on i.MX8MQ-EVK i.MX8MP-EVK i.MX93-11x11-EVK



Peng Fan (6):
  remoteproc: elf_loader: introduce rproc_elf_find_shdr
  remoteproc: imx_rproc: add devtype
  remoteproc: imx_rproc: correct i.MX8MQ DDR Code alias mapping
  remoteproc: imx_rproc: force pointer type
  remoteproc: imx_rproc: set Cortex-M stack/pc to TCML
  remoteproc: imx_rproc: set address of .interrupts section as bootaddr

 drivers/remoteproc/imx_rproc.c             | 58 ++++++++++++--
 drivers/remoteproc/imx_rproc.h             |  6 ++
 drivers/remoteproc/remoteproc_elf_loader.c | 93 +++++++++++++---------
 drivers/remoteproc/remoteproc_internal.h   |  2 +
 4 files changed, 114 insertions(+), 45 deletions(-)

Comments

Daniel Baluta Feb. 3, 2023, 12:49 p.m. UTC | #1 
On Fri, Jan 27, 2023 at 11:26 AM Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote:
>
> From: Peng Fan <peng.fan@nxp.com>
>
> V2:
>  patch 4 is introduced for sparse check warning fix
>
> This pachset is to support i.MX8M and i.MX93 Cortex-M core firmware could
> be in DDR, not just the default TCM.
>
> i.MX8M needs stack/pc value be stored in TCML entry address[0,4], the
> initial value could be got from firmware first section ".interrupts".
> i.MX93 is a bit different, it just needs the address of .interrupts
> section. NXP SDK always has .interrupts section.
>
> So first we need find the .interrupts section from firmware, so patch 1
> is to reuse the code of find_table to introduce a new API
> rproc_elf_find_shdr to find shdr, the it could reused by i.MX driver.
>
> Patch 2 is introduce devtype for i.MX8M/93
>
> Although patch 3 is correct the mapping, but this area was never used
> by NXP SW team, we directly use the DDR region, not the alias region.
> Since this patchset is first to support firmware in DDR, mark this patch
> as a fix does not make much sense.
>
> patch 4 and 5 is support i.MX8M/93 firmware in DDR with parsing .interrupts
> section. Detailed information in each patch commit message.
>
> Patches were tested on i.MX8MQ-EVK i.MX8MP-EVK i.MX93-11x11-EVK

Hi Peng,

Few observations:

- bugfixes should come first in the series.
- in case we want to patches to be pushed back into stable releases
please add "Fixes: " tag.
Peng Fan Feb. 3, 2023, 12:55 p.m. UTC | #2 
Hi Daniel
> Subject: Re: [PATCH V2 0/6] remoteproc: imx_rproc: support firmware in
> DDR
> 
> On Fri, Jan 27, 2023 at 11:26 AM Peng Fan (OSS) <peng.fan@oss.nxp.com>
> wrote:
> >
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > V2:
> >  patch 4 is introduced for sparse check warning fix
> >
> > This pachset is to support i.MX8M and i.MX93 Cortex-M core firmware
> > could be in DDR, not just the default TCM.
> >
> > i.MX8M needs stack/pc value be stored in TCML entry address[0,4], the
> > initial value could be got from firmware first section ".interrupts".
> > i.MX93 is a bit different, it just needs the address of .interrupts
> > section. NXP SDK always has .interrupts section.
> >
> > So first we need find the .interrupts section from firmware, so patch
> > 1 is to reuse the code of find_table to introduce a new API
> > rproc_elf_find_shdr to find shdr, the it could reused by i.MX driver.
> >
> > Patch 2 is introduce devtype for i.MX8M/93
> >
> > Although patch 3 is correct the mapping, but this area was never used
> > by NXP SW team, we directly use the DDR region, not the alias region.
> > Since this patchset is first to support firmware in DDR, mark this
> > patch as a fix does not make much sense.
> >
> > patch 4 and 5 is support i.MX8M/93 firmware in DDR with parsing
> > .interrupts section. Detailed information in each patch commit message.
> >
> > Patches were tested on i.MX8MQ-EVK i.MX8MP-EVK i.MX93-11x11-EVK
> 
> Hi Peng,
> 
> Few observations:
> 
> - bugfixes should come first in the series.
> - in case we want to patches to be pushed back into stable releases please
> add "Fixes: " tag.

You mean patch 4: sparse warning fix?
Or patch 3 is correct the mapping? Or both? For patch 3, I would not take
it as fix, I just think there is no people using this ddr alias address. If you
prefer, I could add a fix tag for patch 3.

Thanks,
Peng.
Daniel Baluta Feb. 3, 2023, 1:03 p.m. UTC | #3 
On Fri, Feb 3, 2023 at 2:55 PM Peng Fan <peng.fan@nxp.com> wrote:
>
> Hi Daniel
> > Subject: Re: [PATCH V2 0/6] remoteproc: imx_rproc: support firmware in
> > DDR
> >
> > On Fri, Jan 27, 2023 at 11:26 AM Peng Fan (OSS) <peng.fan@oss.nxp.com>
> > wrote:
> > >
> > > From: Peng Fan <peng.fan@nxp.com>
> > >
> > > V2:
> > >  patch 4 is introduced for sparse check warning fix
> > >
> > > This pachset is to support i.MX8M and i.MX93 Cortex-M core firmware
> > > could be in DDR, not just the default TCM.
> > >
> > > i.MX8M needs stack/pc value be stored in TCML entry address[0,4], the
> > > initial value could be got from firmware first section ".interrupts".
> > > i.MX93 is a bit different, it just needs the address of .interrupts
> > > section. NXP SDK always has .interrupts section.
> > >
> > > So first we need find the .interrupts section from firmware, so patch
> > > 1 is to reuse the code of find_table to introduce a new API
> > > rproc_elf_find_shdr to find shdr, the it could reused by i.MX driver.
> > >
> > > Patch 2 is introduce devtype for i.MX8M/93
> > >
> > > Although patch 3 is correct the mapping, but this area was never used
> > > by NXP SW team, we directly use the DDR region, not the alias region.
> > > Since this patchset is first to support firmware in DDR, mark this
> > > patch as a fix does not make much sense.
> > >
> > > patch 4 and 5 is support i.MX8M/93 firmware in DDR with parsing
> > > .interrupts section. Detailed information in each patch commit message.
> > >
> > > Patches were tested on i.MX8MQ-EVK i.MX8MP-EVK i.MX93-11x11-EVK
> >
> > Hi Peng,
> >
> > Few observations:
> >
> > - bugfixes should come first in the series.
> > - in case we want to patches to be pushed back into stable releases please
> > add "Fixes: " tag.
>
> You mean patch 4: sparse warning fix?
> Or patch 3 is correct the mapping? Or both? For patch 3, I would not take
> it as fix, I just think there is no people using this ddr alias address. If you
> prefer, I could add a fix tag for patch 3.

Yes, I mean patch 3) which is definitely a thing that stable versions
could benefit.