linux-remoteproc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] remoteproc: imx_rproc: correct firmware reload
@ 2021-06-01 10:29 Peng Fan (OSS)
  2021-06-01 15:31 ` Mathieu Poirier
  0 siblings, 1 reply; 4+ messages in thread
From: Peng Fan (OSS) @ 2021-06-01 10:29 UTC (permalink / raw)
  To: ohad, bjorn.andersson, mathieu.poirier, o.rempel
  Cc: shawnguo, s.hauer, kernel, festevam, linux-imx, linux-remoteproc,
	linux-arm-kernel, linux-kernel, Peng Fan

From: Peng Fan <peng.fan@nxp.com>

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 <peng.fan@nxp.com>
---

V1:
 Although this is an fix, but it is not critical, patch is based on
 https://patchwork.kernel.org/project/linux-remoteproc/cover/1620274123-1461-1-git-send-email-peng.fan@oss.nxp.com/

 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


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH] remoteproc: imx_rproc: correct firmware reload
  2021-06-01 10:29 [PATCH] remoteproc: imx_rproc: correct firmware reload Peng Fan (OSS)
@ 2021-06-01 15:31 ` Mathieu Poirier
  2021-06-01 16:00   ` Bjorn Andersson
  0 siblings, 1 reply; 4+ messages in thread
From: Mathieu Poirier @ 2021-06-01 15:31 UTC (permalink / raw)
  To: Peng Fan (OSS)
  Cc: ohad, bjorn.andersson, o.rempel, shawnguo, s.hauer, kernel,
	festevam, linux-imx, linux-remoteproc, linux-arm-kernel,
	linux-kernel, Peng Fan

On Tue, Jun 01, 2021 at 06:29:39PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> 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 <peng.fan@nxp.com>
> ---
> 
> V1:
>  Although this is an fix, but it is not critical, patch is based on
>  https://patchwork.kernel.org/project/linux-remoteproc/cover/1620274123-1461-1-git-send-email-peng.fan@oss.nxp.com/
> 
>  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 <mathieu.poirier@linaro.org>

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/1620274123-1461-1-git-send-email-peng.fan@oss.nxp.com/

>  
>  	return 0;
> -- 
> 2.30.0
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] remoteproc: imx_rproc: correct firmware reload
  2021-06-01 15:31 ` Mathieu Poirier
@ 2021-06-01 16:00   ` Bjorn Andersson
  2021-06-02  6:05     ` Peng Fan
  0 siblings, 1 reply; 4+ messages in thread
From: Bjorn Andersson @ 2021-06-01 16:00 UTC (permalink / raw)
  To: Mathieu Poirier
  Cc: Peng Fan (OSS),
	ohad, o.rempel, shawnguo, s.hauer, kernel, festevam, linux-imx,
	linux-remoteproc, linux-arm-kernel, linux-kernel, Peng Fan

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 <peng.fan@nxp.com>
> > 
> > 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 <peng.fan@nxp.com>
> > ---
> > 
> > V1:
> >  Although this is an fix, but it is not critical, patch is based on
> >  https://patchwork.kernel.org/project/linux-remoteproc/cover/1620274123-1461-1-git-send-email-peng.fan@oss.nxp.com/
> > 
> >  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 <mathieu.poirier@linaro.org>
> 
> 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/1620274123-1461-1-git-send-email-peng.fan@oss.nxp.com/
> 
> >  
> >  	return 0;
> > -- 
> > 2.30.0
> > 

^ permalink raw reply	[flat|nested] 4+ messages in thread
* RE: [PATCH] remoteproc: imx_rproc: correct firmware reload
  2021-06-01 16:00   ` Bjorn Andersson
@ 2021-06-02  6:05     ` Peng Fan
  0 siblings, 0 replies; 4+ messages in thread
From: Peng Fan @ 2021-06-02  6:05 UTC (permalink / raw)
  To: Bjorn Andersson, Mathieu Poirier
  Cc: Peng Fan (OSS),
	ohad, o.rempel, shawnguo, s.hauer, kernel, festevam,
	dl-linux-imx, linux-remoteproc, linux-arm-kernel, linux-kernel

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 <peng.fan@nxp.com>
> > >
> > > 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 <peng.fan@nxp.com>
> > > ---
> > >
> > > 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&amp;data=04%7C01%
> 7C
> > >
> peng.fan%40nxp.com%7C471a7b81f5d44fc6f85b08d925165839%7C686ea1d
> 3bc2b
> > >
> 4c6fa92cd99c5c301635%7C0%7C0%7C637581600173592073%7CUnknown
> %7CTWFpbG
> > >
> Zsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6
> Mn
> > >
> 0%3D%7C1000&amp;sdata=5JerdzsSmjZ%2Flowd3AXo3gmr6bdHM9DCg55kv
> UmTWQw%
> > > 3D&amp;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 <mathieu.poirier@linaro.org>
> >
> > 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&amp;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
> > >

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-06-02  6:05 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-01 10:29 [PATCH] remoteproc: imx_rproc: correct firmware reload Peng Fan (OSS)
2021-06-01 15:31 ` Mathieu Poirier
2021-06-01 16:00   ` Bjorn Andersson
2021-06-02  6:05     ` Peng Fan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).