Re: [PATCH v2 2/6] peci: controller: Add StarFive JHB100 PECI driver

[Changhuang Liang]

Hi, Phillipp

Thanks for the review.

> On Mo, 2026-09-28 at 00:59 -0700, Changhuang Liang wrote:
> > Add PECI controller driver for StarFive JHB100 SoC. The driver
> > supports PECI protocol communication for CPU thermal management.
> >
> > For this controller, the special clock and reset operation sequence is:
> >   probe: clk_prepare_enable() then reset_control_deassert()
> >   remove: clk_disable_unprepare() then reset_control_assert()
> >
> > Co-developed-by: Mason Huo <mason.huo@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Mason Huo <mason.huo@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Changhuang Liang <changhuang.liang@xxxxxxxxxxxxxxxx>
> > ---
> >  MAINTAINERS                             |   8 +
> >  drivers/peci/controller/Kconfig         |  17 ++
> >  drivers/peci/controller/Makefile        |   1 +
> >  drivers/peci/controller/peci-starfive.c | 376
> > ++++++++++++++++++++++++
> >  4 files changed, 402 insertions(+)
> >  create mode 100644 drivers/peci/controller/peci-starfive.c
> >
> [...]
> > diff --git a/drivers/peci/controller/peci-starfive.c
> > b/drivers/peci/controller/peci-starfive.c
> > new file mode 100644
> > index 000000000000..6004cb27a774
> > --- /dev/null
> > +++ b/drivers/peci/controller/peci-starfive.c
> > @@ -0,0 +1,376 @@
> [...]
> > +static int starfive_peci_init_clk_rst(struct starfive_peci *priv) {
> > +	int ret;
> > +
> > +	priv->clk = devm_clk_get(priv->dev, NULL);
> > +	if (IS_ERR(priv->clk))
> > +		return dev_err_probe(priv->dev, PTR_ERR(priv->clk),
> > +				     "Failed to get peci clock\n");
> > +
> > +	priv->rst = devm_reset_control_get(priv->dev, NULL);
> 
> Please use devm_reset_control_get_exclusive() directly.

Okay, I'll update this.

> > +	if (IS_ERR(priv->rst))
> > +		return dev_err_probe(priv->dev, PTR_ERR(priv->rst),
> > +				     "Failed to get reset control\n");
> > +
> > +	ret = clk_prepare_enable(priv->clk);
> > +	if (ret)
> > +		return dev_err_probe(priv->dev, ret,
> > +				     "Failed to enable peci clock\n");
> > +
> > +	ret = reset_control_deassert(priv->rst);
> > +	if (ret) {
> > +		clk_disable_unprepare(priv->clk);
> > +		return dev_err_probe(priv->dev, ret,
> > +				     "Failed to deassert reset control\n");
> > +	}
> > +
> > +	return devm_add_action_or_reset(priv->dev,
> starfive_peci_clk_reset_release,
> > +					priv);
> 
> This could be simplified with devm_clk_get_enabled() and
> devm_reset_control_get_exclusive_deasserted().

This controller has a special clock reset sequence, and using this 
simplified version doesn't seem to meet the requirements.

probe: clk_prepare_enable() then reset_control_deassert()
remove: clk_disable_unprepare() then reset_control_assert()

Best Regards,
Changhuang