On Tue, May 06, 2025 at 09:36:19AM -0300, Hiago De Franco wrote: > Hi Peng, > > On Tue, May 06, 2025 at 12:38:35PM +0800, Peng Fan wrote: > > On Mon, May 05, 2025 at 12:48:47PM -0300, Hiago De Franco wrote: > > >From: Hiago De Franco <hiago.fra...@toradex.com> > > > > > >For the i.MX8X and i.MX8 family SoCs, when the M-core is powered up > > >before Linux starts (e.g., by the bootloader) and it is being managed by > > >the SCU, the SCFW will not allow the kernel to enable the clock again. > > >This currently causes an SCU fault reset when the M-core is up and > > >running and the kernel boots, resetting the system. > > > > > >Therefore, add a check in the clock enable function to not execute it if > > >the M-core is being managed by the SCU. > > > > > >This change affects only the i.MX8X and i.MX8 family SoCs, as this is > > >under the IMX_RPROC_SCU_API method. > > > > I would rewrite as below: " > > > > For the i.MX8X and i.MX8 family SoCs, when the M-core is powered up > > by the bootloader, M-core and Linux are in same SCFW(System Controller > > Firmware) partition, so linux has permission to control M-core. > > > > But when M-core is started, the SCFW will automatically enable the clock > > and configure the rate, and any users that wanna to enable the clock > > will get error 'LOCKED' from SCFW. So current imx_rproc.c probe function > > gets failure because clk_prepare_enable returns failure. Then > > the power domain of M-core is powered off when M-core is still running, > > SCU(System Controller Unit) will get a fault reset, and system restarts. > > > > To address the issue, ignore handling the clk for i.MX8X and i.MX8 M-core, > > because SCFW automatically enables and configures the clock. > > " > > > > You may update if you wanna. > > > > > > > >Signed-off-by: Hiago De Franco <hiago.fra...@toradex.com> > > >Suggested-by: Peng Fan <peng....@oss.nxp.com> > > > > -> peng....@nxp.com > > Thanks for the review, I will update the suggestions on a v2. Meanwhile, > I will wait a little bit for other feedbacks. >
I suggest you go ahead with a v2 - I have a fair amount of patches to review and my time to do so is currently very limited. > > > > Thanks, > > Peng > > > > >--- > > > drivers/remoteproc/imx_rproc.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > >diff --git a/drivers/remoteproc/imx_rproc.c > > >b/drivers/remoteproc/imx_rproc.c > > >index 74299af1d7f1..627e57a88db2 100644 > > >--- a/drivers/remoteproc/imx_rproc.c > > >+++ b/drivers/remoteproc/imx_rproc.c > > >@@ -1029,8 +1029,8 @@ static int imx_rproc_clk_enable(struct imx_rproc > > >*priv) > > > struct device *dev = priv->dev; > > > int ret; > > > > > >- /* Remote core is not under control of Linux */ > > >- if (dcfg->method == IMX_RPROC_NONE) > > >+ /* Remote core is not under control of Linux or it is managed by SCU > > >API */ > > >+ if (dcfg->method == IMX_RPROC_NONE || dcfg->method == IMX_RPROC_SCU_API) > > > return 0; > > > > > > priv->clk = devm_clk_get(dev, NULL); > > >-- > > >2.39.5 > > > > > Cheers, > Hiago.
