Re: [PATCH 4/5] remoteproc: stm32: Allow hold boot management by the SCMI reset controller

[Alexandre TORGUE]

Hi

On 4/6/23 07:16, Peng Fan wrote:
> > 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.

An old DT should continue to work with a new kernel.

Alex

> 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