Good day, On Wed, Mar 05, 2025 at 12:00:36PM +0200, Daniel Baluta wrote: > DSP on i.MX8MP doesn't have a direct reset line so according to hardware > design team in order to handle assert/deassert/reset functionality we > need to use a combination of control bits from two modules. Audio block > control module for Run/Stall control of the DSP and DAP module in order > to do software reset. > > Initial design accessed RunStall bits from audio block control using the > syscon interface but this got NACKed by community because of abusing the > syscon interface [1]. > > So instead we use the reset controller API.
I am good with this patchset. The only thing I'd like so see modified is the above changelog. The second paragraph doesn't belong there, i.e it is not useful to remark on a design that was not accepted. Instead please concentrate on _why_ we are moving from the current design to using the reset controller API. This can go in the 6.15 merge window if you send me a V5 fast enough. Thanks, Mathieu > > [1] > https://patchwork.kernel.org/project/imx/patch/20250212085222.107102-6-daniel.bal...@nxp.com/ > > Reviewed-by: Peng Fan <peng....@nxp.com> > Reviewed-by: Frank Li <frank...@nxp.com> > Signed-off-by: Daniel Baluta <daniel.bal...@nxp.com> > --- > drivers/remoteproc/imx_dsp_rproc.c | 25 +++++++++++++++++-------- > drivers/remoteproc/imx_rproc.h | 2 ++ > 2 files changed, 19 insertions(+), 8 deletions(-) > > diff --git a/drivers/remoteproc/imx_dsp_rproc.c > b/drivers/remoteproc/imx_dsp_rproc.c > index ea5024919c2f..bc60edcdd661 100644 > --- a/drivers/remoteproc/imx_dsp_rproc.c > +++ b/drivers/remoteproc/imx_dsp_rproc.c > @@ -19,6 +19,7 @@ > #include <linux/pm_runtime.h> > #include <linux/regmap.h> > #include <linux/remoteproc.h> > +#include <linux/reset.h> > #include <linux/slab.h> > > #include "imx_rproc.h" > @@ -111,6 +112,7 @@ enum imx_dsp_rp_mbox_messages { > */ > struct imx_dsp_rproc { > struct regmap *regmap; > + struct reset_control *run_stall; > struct rproc *rproc; > const struct imx_dsp_rproc_dcfg *dsp_dcfg; > struct clk_bulk_data clks[DSP_RPROC_CLK_MAX]; > @@ -192,9 +194,7 @@ static int imx8mp_dsp_reset(struct imx_dsp_rproc *priv) > /* Keep reset asserted for 10 cycles */ > usleep_range(1, 2); > > - regmap_update_bits(priv->regmap, IMX8M_AudioDSP_REG2, > - IMX8M_AudioDSP_REG2_RUNSTALL, > - IMX8M_AudioDSP_REG2_RUNSTALL); > + reset_control_assert(priv->run_stall); > > /* Take the DSP out of reset and keep stalled for FW loading */ > pwrctl = readl(dap + IMX8M_DAP_PWRCTL); > @@ -231,13 +231,9 @@ static int imx8ulp_dsp_reset(struct imx_dsp_rproc *priv) > > /* Specific configuration for i.MX8MP */ > static const struct imx_rproc_dcfg dsp_rproc_cfg_imx8mp = { > - .src_reg = IMX8M_AudioDSP_REG2, > - .src_mask = IMX8M_AudioDSP_REG2_RUNSTALL, > - .src_start = 0, > - .src_stop = IMX8M_AudioDSP_REG2_RUNSTALL, > .att = imx_dsp_rproc_att_imx8mp, > .att_size = ARRAY_SIZE(imx_dsp_rproc_att_imx8mp), > - .method = IMX_RPROC_MMIO, > + .method = IMX_RPROC_RESET_CONTROLLER, > }; > > static const struct imx_dsp_rproc_dcfg imx_dsp_rproc_cfg_imx8mp = { > @@ -329,6 +325,9 @@ static int imx_dsp_rproc_start(struct rproc *rproc) > true, > rproc->bootaddr); > break; > + case IMX_RPROC_RESET_CONTROLLER: > + ret = reset_control_deassert(priv->run_stall); > + break; > default: > return -EOPNOTSUPP; > } > @@ -369,6 +368,9 @@ static int imx_dsp_rproc_stop(struct rproc *rproc) > false, > rproc->bootaddr); > break; > + case IMX_RPROC_RESET_CONTROLLER: > + ret = reset_control_assert(priv->run_stall); > + break; > default: > return -EOPNOTSUPP; > } > @@ -995,6 +997,13 @@ static int imx_dsp_rproc_detect_mode(struct > imx_dsp_rproc *priv) > > priv->regmap = regmap; > break; > + case IMX_RPROC_RESET_CONTROLLER: > + priv->run_stall = devm_reset_control_get_exclusive(dev, > "runstall"); > + if (IS_ERR(priv->run_stall)) { > + dev_err(dev, "Failed to get DSP runstall reset > control\n"); > + return PTR_ERR(priv->run_stall); > + } > + break; > default: > ret = -EOPNOTSUPP; > break; > diff --git a/drivers/remoteproc/imx_rproc.h b/drivers/remoteproc/imx_rproc.h > index 17a7d051c531..cfd38d37e146 100644 > --- a/drivers/remoteproc/imx_rproc.h > +++ b/drivers/remoteproc/imx_rproc.h > @@ -24,6 +24,8 @@ enum imx_rproc_method { > IMX_RPROC_SMC, > /* Through System Control Unit API */ > IMX_RPROC_SCU_API, > + /* Through Reset Controller API */ > + IMX_RPROC_RESET_CONTROLLER, > }; > > /* dcfg flags */ > -- > 2.43.0 >
