> Subject: Re: [PATCH 4/5] remoteproc: stm32: Allow hold boot management > by the SCMI reset controller > > > > On 4/4/23 06:55, Peng Fan wrote: > >> Subject: [PATCH 4/5] remoteproc: stm32: Allow hold boot management > by > >> the SCMI reset controller > >> > >> The hold boot can be managed by the SCMI controller as a reset. > >> If the "hold_boot" reset is defined in the device tree, use it. > >> Else use the syscon controller directly to access to the register. > >> > >> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@xxxxxxxxxxx> > >> --- > >> drivers/remoteproc/stm32_rproc.c | 34 > >> ++++++++++++++++++++++++++----- > >> - > >> 1 file changed, 28 insertions(+), 6 deletions(-) > >> > >> diff --git a/drivers/remoteproc/stm32_rproc.c > >> b/drivers/remoteproc/stm32_rproc.c > >> index 4be651e734ee..6b0d8f30a5c7 100644 > >> --- a/drivers/remoteproc/stm32_rproc.c > >> +++ b/drivers/remoteproc/stm32_rproc.c > >> @@ -78,6 +78,7 @@ struct stm32_mbox { > >> > >> struct stm32_rproc { > >> struct reset_control *rst; > >> + struct reset_control *hold_boot_rst; > >> struct stm32_syscon hold_boot; > >> struct stm32_syscon pdds; > >> struct stm32_syscon m4_state; > >> @@ -398,6 +399,14 @@ static int stm32_rproc_set_hold_boot(struct > >> rproc *rproc, bool hold) > >> struct stm32_syscon hold_boot = ddata->hold_boot; > >> int val, err; > >> > >> + if (ddata->hold_boot_rst) { > >> + /* Use the SCMI reset controller */ > >> + if (!hold) > >> + return reset_control_deassert(ddata- > >>> hold_boot_rst); > >> + else > >> + return reset_control_assert(ddata->hold_boot_rst); > >> + } > >> + > >> val = hold ? HOLD_BOOT : RELEASE_BOOT; > >> > >> err = regmap_update_bits(hold_boot.map, hold_boot.reg, @@ - > >> 693,16 +702,29 @@ static int stm32_rproc_parse_dt(struct > >> platform_device *pdev, > >> dev_info(dev, "wdg irq registered\n"); > >> } > >> > >> - ddata->rst = devm_reset_control_get_by_index(dev, 0); > >> + ddata->rst = devm_reset_control_get(dev, "mcu_rst"); > > [Peng Fan] > > This may break legacy device tree. > > That partially true by the fact that I impose the "reset-names" property (but > also suppress the "st,syscfg-tz" property) > > But this should not be the case as the arch/arm/boot/dts/stm32mp151.dtsi > is updated in patch 2/5. The DTS files associated to this chip include it. DT maintainers may comment, from my understanding is updating driver should not break legacy dts, saying 5.15, 5.10 dts. Regards, Peng. > > Thanks, > Arnaud > > > > > > Regards, > > Peng. > > > >> if (IS_ERR(ddata->rst)) > >> return dev_err_probe(dev, PTR_ERR(ddata->rst), > >> "failed to get mcu_reset\n"); > >> > >> - err = stm32_rproc_get_syscon(np, "st,syscfg-holdboot", > >> - &ddata->hold_boot); > >> - if (err) { > >> - dev_err(dev, "failed to get hold boot\n"); > >> - return err; > >> + ddata->hold_boot_rst = devm_reset_control_get(dev, "hold_boot"); > >> + if (IS_ERR(ddata->hold_boot_rst)) { > >> + if (PTR_ERR(ddata->hold_boot_rst) == -EPROBE_DEFER) > >> + return PTR_ERR(ddata->hold_boot_rst); > >> + ddata->hold_boot_rst = NULL; > >> + } > >> + > >> + if (!ddata->hold_boot_rst) { > >> + /* > >> + * If the hold boot is not managed by the SCMI reset > >> controller, > >> + * manage it through the syscon controller > >> + */ > >> + err = stm32_rproc_get_syscon(np, "st,syscfg-holdboot", > >> + &ddata->hold_boot); > >> + if (err) { > >> + dev_err(dev, "failed to get hold boot\n"); > >> + return err; > >> + } > >> } > >> > >> err = stm32_rproc_get_syscon(np, "st,syscfg-pdds", &ddata->pdds); > >> -- > >> 2.25.1 > >