回复: [PATCH v5 2/2] hwrng: starfive: rework clk/reset teardown order for JHB100

[Lianfeng Ouyang]

> -----邮件原件-----
> 发件人: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
> 发送时间: 2026年7月13日 10:17
> 收件人: Lianfeng Ouyang <lianfeng.ouyang@xxxxxxxxxxxxxxxx>
> 抄送: Olivia Mackall <olivia@xxxxxxxxxxx>; Rob Herring <robh@xxxxxxxxxx>;
> Krzysztof Kozlowski <krzk+dt@xxxxxxxxxx>; Conor Dooley
> <conor+dt@xxxxxxxxxx>; Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>;
> linux-crypto@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx
> 主题: Re: [PATCH v5 2/2] hwrng: starfive: rework clk/reset teardown order for
> JHB100
> 
> On Mon, Jun 29, 2026 at 04:36:58PM +0800, lianfeng.ouyang wrote:
> > From: Lianfeng Ouyang <lianfeng.ouyang@xxxxxxxxxxxxxxxx>
> >
> > Rework the StarFive TRNG driver to address hardware-specific requirements
> >   for JHB100 SoC. To avoid reset-domain crossing glitches, the driver now
> >   ensures clocks are gated before asserting reset during teardown for
> >   JHB100, while JH7110 retains the original reset-first sequence.
> >
> > Add per-compatible match data (struct starfive_trng_data) describing the
> >   clock/reset teardown order, a new "starfive,jhb100-trng" compatible, and
> >   select the ordering from it.
> >
> > Fix the runtime-PM get/put balancing across the init/read/reseed/cleanup
> >   paths, manage PM and the clk/reset teardown via devm so all error paths
> >   unwind correctly, run the SEU-triggered reseed from a workqueue instead
> >   of hard IRQ, and serialise the command sequences with a mutex.
> >
> > Signed-off-by: Lianfeng Ouyang <lianfeng.ouyang@xxxxxxxxxxxxxxxx>
> > ---
> >  MAINTAINERS                          |   2 +-
> >  drivers/char/hw_random/jh7110-trng.c | 312
> +++++++++++++++++++++------
> >  2 files changed, 245 insertions(+), 69 deletions(-)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index d3a6b3f6b6a0..729b20ecc697 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -25280,7 +25280,7 @@ F:
> 	Documentation/devicetree/bindings/perf/starfive,jh8100-starlink-pmu.yam
> l
> >  F:	drivers/perf/starfive_starlink_pmu.c
> >
> >  STARFIVE TRNG DRIVER
> > -M:	Jia Jie Ho <jiajie.ho@xxxxxxxxxxxxxxxx>
> > +M:	Lianfeng Ouyang <lianfeng.ouyang@xxxxxxxxxxxxxxxx>
> >  S:	Supported
> >  F:	Documentation/devicetree/bindings/rng/starfive*
> >  F:	drivers/char/hw_random/jh7110-trng.c
> > diff --git a/drivers/char/hw_random/jh7110-trng.c
> b/drivers/char/hw_random/jh7110-trng.c
> > index 9776f4daa044..1434dcb6efed 100644
> > --- a/drivers/char/hw_random/jh7110-trng.c
> > +++ b/drivers/char/hw_random/jh7110-trng.c
> > @@ -92,22 +92,44 @@ enum mode {
> >  	PRNG_256BIT,
> >  };
> >
> > +/*
> > + * For JHB100, assert reset after disabling clocks to avoid
> > + * reset-domain crossing (RDC) induced glitches that can affect
> > + * downstream IPs.
> > + */
> > +enum seq_rst_clk {
> > +	SEQ_RST_FIRST,
> > +	SEQ_CLK_FIRST,
> > +};
> > +
> > +struct starfive_trng_data {
> > +	enum seq_rst_clk	seq_rst_clk;
> > +};
> > +
> >  struct starfive_trng {
> > -	struct device		*dev;
> > -	void __iomem		*base;
> > -	struct clk		*hclk;
> > -	struct clk		*ahb;
> > -	struct reset_control	*rst;
> > -	struct hwrng		rng;
> > -	struct completion	random_done;
> > -	struct completion	reseed_done;
> > -	u32			mode;
> > -	u32			mission;
> > -	u32			reseed;
> > -	/* protects against concurrent write to ctrl register */
> > -	spinlock_t		write_lock;
> > +	struct device			*dev;
> > +	void __iomem			*base;
> > +	int				irq;
> > +	struct clk			*hclk;
> > +	struct clk			*ahb;
> > +	struct reset_control		*rst;
> > +	struct hwrng			rng;
> > +	struct completion		random_done;
> > +	struct completion		reseed_done;
> > +	struct work_struct		work;
> > +	const struct starfive_trng_data *data;
> > +	u32				mode;
> > +	u32				mission;
> > +	u32				reseed;
> > +	u32				cleanup;
> > +	struct mutex			lock; /* protect trng cmd seq */
> 
> Doing a white-space change at the same time as a substantial change
> makes things hard to review.  Please split this up or just drop the
> white-space change until later.
> 

Okay, I will restore the spaces in the subsequent versions

> >  static int starfive_trng_read(struct hwrng *rng, void *buf, size_t max, bool
> wait)
> > @@ -247,7 +358,13 @@ static int starfive_trng_read(struct hwrng *rng, void
> *buf, size_t max, bool wai
> >  	struct starfive_trng *trng = to_trng(rng);
> >  	int ret;
> >
> > -	pm_runtime_get_sync(trng->dev);
> > +	ret = pm_runtime_resume_and_get(trng->dev);
> > +	if (ret < 0) {
> > +		dev_warn(trng->dev, "Failed to wake device for read: %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	mutex_lock(&trng->lock);
> 
> What happens when a non-waiting read call ends up spinning here
> waiting for a wait read call?
> 
> Thanks,
> --
> Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Too sensitive and sharp, I didn't notice this issue before. 
I will check the lock first through mutex_trylock when wait is 0

Best Regards,
Lianfeng Ouyang