From: Peng Fan <[email protected]>
ENABLE_M4 should be set to 1 when loading code to TCM, otherwise
you will not able to replace the firmware after you stop m4.
Besides ENABLE_M4, we still need set SW_M4C_RST, because this bit
will be automatically set with SW_M4C_NON_SCLR_RST set.
Signed-off-by: Peng Fan <[email protected]>
---
V1:
Although this is an fix, but it is not critical, patch is based on
https://patchwork.kernel.org/project/linux-remoteproc/cover/[email protected]/
drivers/remoteproc/imx_rproc.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index cd2ca96a30e5..ce2ce42bee91 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -33,7 +33,8 @@
#define IMX7D_M4_START (IMX7D_ENABLE_M4 | IMX7D_SW_M4P_RST \
| IMX7D_SW_M4C_RST)
-#define IMX7D_M4_STOP (IMX7D_ENABLE_M4 | IMX7D_SW_M4C_NON_SCLR_RST)
+#define IMX7D_M4_STOP (IMX7D_ENABLE_M4 | IMX7D_SW_M4C_RST | \
+ IMX7D_SW_M4C_NON_SCLR_RST)
/* Address: 0x020D8000 */
#define IMX6SX_SRC_SCR 0x00
@@ -44,7 +45,8 @@
#define IMX6SX_M4_START (IMX6SX_ENABLE_M4 | IMX6SX_SW_M4P_RST \
| IMX6SX_SW_M4C_RST)
-#define IMX6SX_M4_STOP (IMX6SX_ENABLE_M4 | IMX6SX_SW_M4C_NON_SCLR_RST)
+#define IMX6SX_M4_STOP (IMX6SX_ENABLE_M4 | IMX6SX_SW_M4C_RST | \
+ IMX6SX_SW_M4C_NON_SCLR_RST)
#define IMX6SX_M4_RST_MASK (IMX6SX_ENABLE_M4 | IMX6SX_SW_M4P_RST \
| IMX6SX_SW_M4C_NON_SCLR_RST \
| IMX6SX_SW_M4C_RST)
@@ -691,7 +693,7 @@ static int imx_rproc_detect_mode(struct imx_rproc *priv)
return ret;
}
- if (!(val & dcfg->src_stop))
+ if ((val & dcfg->src_mask) != dcfg->src_stop)
priv->rproc->state = RPROC_DETACHED;
return 0;
--
2.30.0
On Tue, Jun 01, 2021 at 06:29:39PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <[email protected]>
>
> ENABLE_M4 should be set to 1 when loading code to TCM, otherwise
> you will not able to replace the firmware after you stop m4.
>
> Besides ENABLE_M4, we still need set SW_M4C_RST, because this bit
> will be automatically set with SW_M4C_NON_SCLR_RST set.
>
> Signed-off-by: Peng Fan <[email protected]>
> ---
>
> V1:
> Although this is an fix, but it is not critical, patch is based on
> https://patchwork.kernel.org/project/linux-remoteproc/cover/[email protected]/
>
> drivers/remoteproc/imx_rproc.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index cd2ca96a30e5..ce2ce42bee91 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -33,7 +33,8 @@
>
> #define IMX7D_M4_START (IMX7D_ENABLE_M4 | IMX7D_SW_M4P_RST \
> | IMX7D_SW_M4C_RST)
> -#define IMX7D_M4_STOP (IMX7D_ENABLE_M4 | IMX7D_SW_M4C_NON_SCLR_RST)
> +#define IMX7D_M4_STOP (IMX7D_ENABLE_M4 | IMX7D_SW_M4C_RST | \
> + IMX7D_SW_M4C_NON_SCLR_RST)
>
> /* Address: 0x020D8000 */
> #define IMX6SX_SRC_SCR 0x00
> @@ -44,7 +45,8 @@
>
> #define IMX6SX_M4_START (IMX6SX_ENABLE_M4 | IMX6SX_SW_M4P_RST \
> | IMX6SX_SW_M4C_RST)
> -#define IMX6SX_M4_STOP (IMX6SX_ENABLE_M4 | IMX6SX_SW_M4C_NON_SCLR_RST)
> +#define IMX6SX_M4_STOP (IMX6SX_ENABLE_M4 | IMX6SX_SW_M4C_RST | \
> + IMX6SX_SW_M4C_NON_SCLR_RST)
> #define IMX6SX_M4_RST_MASK (IMX6SX_ENABLE_M4 | IMX6SX_SW_M4P_RST \
> | IMX6SX_SW_M4C_NON_SCLR_RST \
> | IMX6SX_SW_M4C_RST)
> @@ -691,7 +693,7 @@ static int imx_rproc_detect_mode(struct imx_rproc *priv)
> return ret;
> }
>
> - if (!(val & dcfg->src_stop))
> + if ((val & dcfg->src_mask) != dcfg->src_stop)
> priv->rproc->state = RPROC_DETACHED;
Reviewed-by: Mathieu Poirier <[email protected]>
Bjorn hasn't picked up this set yet so it is best to send a v7 of [1] with this
change merged in it.
Thanks,
Mathieu
[1]. https://patchwork.kernel.org/project/linux-remoteproc/cover/[email protected]/
>
> return 0;
> --
> 2.30.0
>
On Tue 01 Jun 10:31 CDT 2021, Mathieu Poirier wrote:
> On Tue, Jun 01, 2021 at 06:29:39PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <[email protected]>
> >
> > ENABLE_M4 should be set to 1 when loading code to TCM, otherwise
> > you will not able to replace the firmware after you stop m4.
> >
> > Besides ENABLE_M4, we still need set SW_M4C_RST, because this bit
> > will be automatically set with SW_M4C_NON_SCLR_RST set.
> >
> > Signed-off-by: Peng Fan <[email protected]>
> > ---
> >
> > V1:
> > Although this is an fix, but it is not critical, patch is based on
> > https://patchwork.kernel.org/project/linux-remoteproc/cover/[email protected]/
> >
> > drivers/remoteproc/imx_rproc.c | 8 +++++---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> > index cd2ca96a30e5..ce2ce42bee91 100644
> > --- a/drivers/remoteproc/imx_rproc.c
> > +++ b/drivers/remoteproc/imx_rproc.c
> > @@ -33,7 +33,8 @@
> >
> > #define IMX7D_M4_START (IMX7D_ENABLE_M4 | IMX7D_SW_M4P_RST \
> > | IMX7D_SW_M4C_RST)
> > -#define IMX7D_M4_STOP (IMX7D_ENABLE_M4 | IMX7D_SW_M4C_NON_SCLR_RST)
> > +#define IMX7D_M4_STOP (IMX7D_ENABLE_M4 | IMX7D_SW_M4C_RST | \
> > + IMX7D_SW_M4C_NON_SCLR_RST)
> >
> > /* Address: 0x020D8000 */
> > #define IMX6SX_SRC_SCR 0x00
> > @@ -44,7 +45,8 @@
> >
> > #define IMX6SX_M4_START (IMX6SX_ENABLE_M4 | IMX6SX_SW_M4P_RST \
> > | IMX6SX_SW_M4C_RST)
> > -#define IMX6SX_M4_STOP (IMX6SX_ENABLE_M4 | IMX6SX_SW_M4C_NON_SCLR_RST)
> > +#define IMX6SX_M4_STOP (IMX6SX_ENABLE_M4 | IMX6SX_SW_M4C_RST | \
> > + IMX6SX_SW_M4C_NON_SCLR_RST)
> > #define IMX6SX_M4_RST_MASK (IMX6SX_ENABLE_M4 | IMX6SX_SW_M4P_RST \
> > | IMX6SX_SW_M4C_NON_SCLR_RST \
> > | IMX6SX_SW_M4C_RST)
> > @@ -691,7 +693,7 @@ static int imx_rproc_detect_mode(struct imx_rproc *priv)
> > return ret;
> > }
> >
> > - if (!(val & dcfg->src_stop))
> > + if ((val & dcfg->src_mask) != dcfg->src_stop)
> > priv->rproc->state = RPROC_DETACHED;
>
> Reviewed-by: Mathieu Poirier <[email protected]>
>
> Bjorn hasn't picked up this set yet so it is best to send a v7 of [1] with this
> change merged in it.
>
Sorry I had missed that [1] was fully reviewed, so I figured I'll just
apply the series and squash this in. But afaict this patch isn't a fix
to any of the patches in [1].
More so, this patch doesn't apply on top of either v5.13-rc1, linux-next
or [1] - because neither IMX7D_M4_STOP nor IMX6SX_M4_STOP has
IMX6SX_ENABLE_M4 included.
Peng, I've applied [1] now, please let me know what to do about this
patch.
Regards,
Bjorn
> Thanks,
> Mathieu
>
> [1]. https://patchwork.kernel.org/project/linux-remoteproc/cover/[email protected]/
>
> >
> > return 0;
> > --
> > 2.30.0
> >
Bjorn, Mathieu
> Subject: Re: [PATCH] remoteproc: imx_rproc: correct firmware reload
>
> On Tue 01 Jun 10:31 CDT 2021, Mathieu Poirier wrote:
>
> > On Tue, Jun 01, 2021 at 06:29:39PM +0800, Peng Fan (OSS) wrote:
> > > From: Peng Fan <[email protected]>
> > >
> > > ENABLE_M4 should be set to 1 when loading code to TCM, otherwise you
> > > will not able to replace the firmware after you stop m4.
> > >
> > > Besides ENABLE_M4, we still need set SW_M4C_RST, because this bit
> > > will be automatically set with SW_M4C_NON_SCLR_RST set.
> > >
> > > Signed-off-by: Peng Fan <[email protected]>
> > > ---
> > >
> > > V1:
> > > Although this is an fix, but it is not critical, patch is based on
> > >
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpa
> > >
> tchwork.kernel.org%2Fproject%2Flinux-remoteproc%2Fcover%2F162027412
> 3
> > >
> -1461-1-git-send-email-peng.fan%40oss.nxp.com%2F&data=04%7C01%
> 7C
> > >
> peng.fan%40nxp.com%7C471a7b81f5d44fc6f85b08d925165839%7C686ea1d
> 3bc2b
> > >
> 4c6fa92cd99c5c301635%7C0%7C0%7C637581600173592073%7CUnknown
> %7CTWFpbG
> > >
> Zsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6
> Mn
> > >
> 0%3D%7C1000&sdata=5JerdzsSmjZ%2Flowd3AXo3gmr6bdHM9DCg55kv
> UmTWQw%
> > > 3D&reserved=0
> > >
> > > drivers/remoteproc/imx_rproc.c | 8 +++++---
> > > 1 file changed, 5 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/remoteproc/imx_rproc.c
> > > b/drivers/remoteproc/imx_rproc.c index cd2ca96a30e5..ce2ce42bee91
> > > 100644
> > > --- a/drivers/remoteproc/imx_rproc.c
> > > +++ b/drivers/remoteproc/imx_rproc.c
> > > @@ -33,7 +33,8 @@
> > >
> > > #define IMX7D_M4_START (IMX7D_ENABLE_M4 |
> IMX7D_SW_M4P_RST \
> > > | IMX7D_SW_M4C_RST)
> > > -#define IMX7D_M4_STOP (IMX7D_ENABLE_M4 |
> IMX7D_SW_M4C_NON_SCLR_RST)
> > > +#define IMX7D_M4_STOP (IMX7D_ENABLE_M4 |
> IMX7D_SW_M4C_RST | \
> > > + IMX7D_SW_M4C_NON_SCLR_RST)
> > >
> > > /* Address: 0x020D8000 */
> > > #define IMX6SX_SRC_SCR 0x00
> > > @@ -44,7 +45,8 @@
> > >
> > > #define IMX6SX_M4_START (IMX6SX_ENABLE_M4 |
> IMX6SX_SW_M4P_RST \
> > > | IMX6SX_SW_M4C_RST)
> > > -#define IMX6SX_M4_STOP (IMX6SX_ENABLE_M4 |
> IMX6SX_SW_M4C_NON_SCLR_RST)
> > > +#define IMX6SX_M4_STOP (IMX6SX_ENABLE_M4 |
> IMX6SX_SW_M4C_RST | \
> > > + IMX6SX_SW_M4C_NON_SCLR_RST)
> > > #define IMX6SX_M4_RST_MASK (IMX6SX_ENABLE_M4 |
> IMX6SX_SW_M4P_RST \
> > > | IMX6SX_SW_M4C_NON_SCLR_RST \
> > > | IMX6SX_SW_M4C_RST)
> > > @@ -691,7 +693,7 @@ static int imx_rproc_detect_mode(struct
> imx_rproc *priv)
> > > return ret;
> > > }
> > >
> > > - if (!(val & dcfg->src_stop))
> > > + if ((val & dcfg->src_mask) != dcfg->src_stop)
> > > priv->rproc->state = RPROC_DETACHED;
> >
> > Reviewed-by: Mathieu Poirier <[email protected]>
> >
> > Bjorn hasn't picked up this set yet so it is best to send a v7 of [1]
> > with this change merged in it.
> >
>
> Sorry I had missed that [1] was fully reviewed, so I figured I'll just apply the
> series and squash this in. But afaict this patch isn't a fix to any of the patches
> in [1].
>
> More so, this patch doesn't apply on top of either v5.13-rc1, linux-next or [1] -
> because neither IMX7D_M4_STOP nor IMX6SX_M4_STOP has
> IMX6SX_ENABLE_M4 included.
I'll send v2 to avoid the conflict soon.
>
>
> Peng, I've applied [1] now, please let me know what to do about this patch.
Thanks,
Peng.
>
> Regards,
> Bjorn
>
> > Thanks,
> > Mathieu
> >
> > [1].
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
> >
> hwork.kernel.org%2Fproject%2Flinux-remoteproc%2Fcover%2F1620274123-
> 146
> >
> 1-1-git-send-email-peng.fan%40oss.nxp.com%2F&data=04%7C01%7Cpe
> ng.f
> >
> an%40nxp.com%7C471a7b81f5d44fc6f85b08d925165839%7C686ea1d3bc2b
> 4c6fa92c
> >
> d99c5c301635%7C0%7C0%7C637581600173592073%7CUnknown%7CTWFp
> bGZsb3d8eyJW
> >
> IjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C
> 1000&
> >
> amp;sdata=5JerdzsSmjZ%2Flowd3AXo3gmr6bdHM9DCg55kvUmTWQw%3D&
> amp;reserve
> > d=0
> >
> > >
> > > return 0;
> > > --
> > > 2.30.0
> > >