[PATCH v8 0/2] hwrng: starfive: add JHB100 support and fix clk/reset teardown

[PATCH v8 0/2] hwrng: starfive: add JHB100 support and fix clk/reset teardown

[lianfeng.ouyang]

From: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>

This patch series adds support for the JHB100 SoC TRNG and fixes
  clock/reset teardown ordering issues.

The first patch updates the device tree bindings by removing the
  obsolete JH8100 compatible string and adding JHB100 support
  while updating the maintainer.

The second patch reworks the driver to ensure proper clock gating
  before reset assertion for JHB100 to avoid reset-domain crossing
  glitches, fixes RPM usage count handling, and improves error
  path cleanup through devm actions.

Changes in v1:
 - Remove jh8100-trng and Fix the compatible description in .yaml
 - add JHB100 and fix clk/reset teardown logic code

Changes in v2:
 - Balance the PM via pm_runtime_set_active() in ->probe
 - Add devm_add_action_or_reset() to register a resource rollback function

Changes in v3:
 - Supplement .yaml commit message
 - Add pm_runtime_get_if_active() to check PM in irq
 - Balances pm_runtime_get/put calls in init, read, and cleanup paths
 - Add per-compatible match data for the teardown order

Changes in v4:
 - Modify .yaml commit information to non point description
 - serialise the command sequences with a mutex.

Changes in v5:
 - Run the reseed from a workqueue instead of hard IRQ context.
 - Balance the probe reset_control_deassert() via a ->cleanup flag in the
   PM suspend/resume callbacks (was skipped when already suspended).
 - Use devm_pm_runtime_set_active_enabled() instead of manual PM enable.
 - Register the IRQ after clk/reset/PM setup; reuse
    starfive_trng_release() on request_irq() failure.
 - Check reset_control_deassert();
 - drop of_match_ptr() from the match table.

Changes in v6:
 - check lock in starfive_trng_read() using mutex_trylock when wait is 0

Changes in v7:
 - using mutex_trylock when wait is 0, otherwise use mutex_lock()

Changes in v8:
 - rebase patches against the latest tree

Lianfeng Ouyang (2):
  dt-bindings: rng: starfive,jh7110-trng: add jhb100, drop jh8100
  hwrng: starfive: rework clk/reset teardown order for JHB100

 .../bindings/rng/starfive,jh7110-trng.yaml    |  10 +-
 MAINTAINERS                                   |   2 +-
 drivers/char/hw_random/jh7110-trng.c          | 297 ++++++++++++++----
 3 files changed, 243 insertions(+), 66 deletions(-)

--
2.43.0

[PATCH v8 1/2] dt-bindings: rng: starfive,jh7110-trng: add jhb100, drop jh8100

[lianfeng.ouyang]

From: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>

Update the StarFive TRNG DT bindings to reflect current SoC support.

The obsolete "starfive,jh8100-trng" compatible string is removed since
  JH8100 SoC is no longer supported. A new "starfive,jhb100-trng"
  compatible string is added for JHB100 SoC TRNG support.

The maintainer entry is also updated to reflect current ownership as the
  previous maintainer has resigned.

Signed-off-by: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
 .../devicetree/bindings/rng/starfive,jh7110-trng.yaml  | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml b/Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml
index 4639247e9e51..d21769b7d54e 100644
--- a/Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml
+++ b/Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml
@@ -7,15 +7,13 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
 title: StarFive SoC TRNG Module
 
 maintainers:
-  - Jia Jie Ho <jiajie.ho@starfivetech.com>
+  - Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
 
 properties:
   compatible:
-    oneOf:
-      - items:
-          - const: starfive,jh8100-trng
-          - const: starfive,jh7110-trng
-      - const: starfive,jh7110-trng
+    enum:
+      - starfive,jh7110-trng
+      - starfive,jhb100-trng
 
   reg:
     maxItems: 1
-- 
2.43.0

[PATCH v8 2/2] hwrng: starfive: rework clk/reset teardown order for JHB100

[lianfeng.ouyang]

From: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>

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@starfivetech.com>
---
 MAINTAINERS                          |   2 +-
 drivers/char/hw_random/jh7110-trng.c | 297 +++++++++++++++++++++------
 2 files changed, 239 insertions(+), 60 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 6d998ed051a8..5211aa88048f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -26239,7 +26239,7 @@ F:	Documentation/devicetree/bindings/perf/starfive,jh8100-starlink-pmu.yaml
 F:	drivers/perf/starfive_starlink_pmu.c
 
 STARFIVE TRNG DRIVER
-M:	Jia Jie Ho <jiajie.ho@starfivetech.com>
+M:	Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
 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 aee12caab578..ecc7d5bd7bf5 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;
+	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			cleanup;
 	u32			mission;
 	u32			reseed;
-	/* protects against concurrent write to ctrl register */
-	spinlock_t		write_lock;
+	struct mutex		lock; /* protect trng cmd seq */
 };
 
+static inline struct starfive_trng *work_to_trng(struct work_struct *work)
+{
+	return container_of(work, struct starfive_trng, work);
+}
+
 static u16 autoreq;
 module_param(autoreq, ushort, 0);
 MODULE_PARM_DESC(autoreq, "Auto-reseeding after random number requests by host reaches specified counter:\n"
@@ -130,7 +152,7 @@ static inline int starfive_trng_wait_idle(struct starfive_trng *trng)
 					  10, 100000);
 }
 
-static inline void starfive_trng_irq_mask_clear(struct starfive_trng *trng)
+static inline void starfive_trng_irq_clear(struct starfive_trng *trng)
 {
 	/* clear register: ISTAT */
 	u32 data = readl(trng->base + STARFIVE_ISTAT);
@@ -138,6 +160,28 @@ static inline void starfive_trng_irq_mask_clear(struct starfive_trng *trng)
 	writel(data, trng->base + STARFIVE_ISTAT);
 }
 
+static void starfive_trng_release(void *data)
+{
+	struct starfive_trng *trng = data;
+
+	if (!pm_runtime_status_suspended(trng->dev)) {
+		writel(0, trng->base + STARFIVE_IE);
+		starfive_trng_irq_clear(trng);
+
+		if (trng->irq >= 0)
+			synchronize_irq(trng->irq);
+
+		if (trng->data->seq_rst_clk == SEQ_RST_FIRST)
+			reset_control_assert(trng->rst);
+
+		clk_disable_unprepare(trng->ahb);
+		clk_disable_unprepare(trng->hclk);
+
+		if (trng->data->seq_rst_clk == SEQ_CLK_FIRST)
+			reset_control_assert(trng->rst);
+	}
+}
+
 static int starfive_trng_cmd(struct starfive_trng *trng, u32 cmd, bool wait)
 {
 	int wait_time = 1000;
@@ -149,17 +193,13 @@ static int starfive_trng_cmd(struct starfive_trng *trng, u32 cmd, bool wait)
 	switch (cmd) {
 	case STARFIVE_CTRL_GENE_RANDNUM:
 		reinit_completion(&trng->random_done);
-		spin_lock_irq(&trng->write_lock);
 		writel(cmd, trng->base + STARFIVE_CTRL);
-		spin_unlock_irq(&trng->write_lock);
 		if (!wait_for_completion_timeout(&trng->random_done, usecs_to_jiffies(wait_time)))
 			return -ETIMEDOUT;
 		break;
 	case STARFIVE_CTRL_EXEC_RANDRESEED:
 		reinit_completion(&trng->reseed_done);
-		spin_lock_irq(&trng->write_lock);
 		writel(cmd, trng->base + STARFIVE_CTRL);
-		spin_unlock_irq(&trng->write_lock);
 		if (!wait_for_completion_timeout(&trng->reseed_done, usecs_to_jiffies(wait_time)))
 			return -ETIMEDOUT;
 		break;
@@ -174,13 +214,24 @@ static int starfive_trng_init(struct hwrng *rng)
 {
 	struct starfive_trng *trng = to_trng(rng);
 	u32 mode, intr = 0;
+	int ret;
+
+	ret = pm_runtime_resume_and_get(trng->dev);
+	if (ret < 0) {
+		dev_warn(trng->dev, "Failed to wake device for init: %d\n", ret);
+		return ret;
+	}
+
+	mutex_lock(&trng->lock);
+
+	WRITE_ONCE(trng->cleanup, 0);
 
 	/* setup Auto Request/Age register */
 	writel(autoage, trng->base + STARFIVE_AUTO_AGE);
 	writel(autoreq, trng->base + STARFIVE_AUTO_RQSTS);
 
 	/* clear register: ISTAT */
-	starfive_trng_irq_mask_clear(trng);
+	starfive_trng_irq_clear(trng);
 
 	intr |= STARFIVE_IE_ALL;
 	writel(intr, trng->base + STARFIVE_IE);
@@ -201,45 +252,105 @@ static int starfive_trng_init(struct hwrng *rng)
 
 	writel(mode, trng->base + STARFIVE_MODE);
 
-	return starfive_trng_cmd(trng, STARFIVE_CTRL_EXEC_RANDRESEED, 1);
+	ret = starfive_trng_cmd(trng, STARFIVE_CTRL_EXEC_RANDRESEED, 1);
+
+	mutex_unlock(&trng->lock);
+
+	pm_runtime_put_autosuspend(trng->dev);
+
+	return ret;
+}
+
+static void starfive_trng_randreseed_work(struct work_struct *work)
+{
+	struct starfive_trng *trng = work_to_trng(work);
+	int ret;
+
+	ret = pm_runtime_resume_and_get(trng->dev);
+	if (ret < 0) {
+		dev_warn(trng->dev, "Failed to wake device for reseed: %d\n", ret);
+		return;
+	}
+
+	mutex_lock(&trng->lock);
+
+	if (READ_ONCE(trng->cleanup))
+		goto unlock;
+
+	reinit_completion(&trng->reseed_done);
+	writel(STARFIVE_CTRL_EXEC_RANDRESEED, trng->base + STARFIVE_CTRL);
+
+unlock:
+	mutex_unlock(&trng->lock);
+
+	pm_runtime_put_autosuspend(trng->dev);
 }
 
 static irqreturn_t starfive_trng_irq(int irq, void *priv)
 {
+	int ret;
 	u32 status;
 	struct starfive_trng *trng = (struct starfive_trng *)priv;
 
+	ret = pm_runtime_get_if_active(trng->dev);
+	if (ret <= 0) {
+		dev_err_ratelimited(trng->dev, "pm is inactive in irq\n");
+		return IRQ_NONE;
+	}
+
 	status = readl(trng->base + STARFIVE_ISTAT);
-	if (status & STARFIVE_ISTAT_RAND_RDY) {
+	if (status & STARFIVE_ISTAT_RAND_RDY)
 		writel(STARFIVE_ISTAT_RAND_RDY, trng->base + STARFIVE_ISTAT);
-		complete(&trng->random_done);
-	}
 
-	if (status & STARFIVE_ISTAT_SEED_DONE) {
+	if (status & STARFIVE_ISTAT_SEED_DONE)
 		writel(STARFIVE_ISTAT_SEED_DONE, trng->base + STARFIVE_ISTAT);
-		complete(&trng->reseed_done);
-	}
 
 	if (status & STARFIVE_ISTAT_LFSR_LOCKUP) {
 		writel(STARFIVE_ISTAT_LFSR_LOCKUP, trng->base + STARFIVE_ISTAT);
 		/* SEU occurred, reseeding required*/
-		spin_lock(&trng->write_lock);
-		writel(STARFIVE_CTRL_EXEC_RANDRESEED, trng->base + STARFIVE_CTRL);
-		spin_unlock(&trng->write_lock);
+		schedule_work(&trng->work);
 	}
 
+	if (status & STARFIVE_ISTAT_RAND_RDY)
+		complete(&trng->random_done);
+
+	if (status & STARFIVE_ISTAT_SEED_DONE)
+		complete(&trng->reseed_done);
+
+	pm_runtime_put_noidle(trng->dev);
+
 	return IRQ_HANDLED;
 }
 
 static void starfive_trng_cleanup(struct hwrng *rng)
 {
 	struct starfive_trng *trng = to_trng(rng);
+	int ret;
+
+	ret = pm_runtime_resume_and_get(trng->dev);
+	if (ret < 0) {
+		dev_warn(trng->dev, "Failed to wake device for cleanup: %d\n", ret);
+		goto end;
+	}
+
+	mutex_lock(&trng->lock);
+
+	writel(0, trng->base + STARFIVE_IE);
+	starfive_trng_irq_clear(trng);
+
+	if (trng->irq >= 0)
+		synchronize_irq(trng->irq);
 
 	writel(0, trng->base + STARFIVE_CTRL);
 
-	reset_control_assert(trng->rst);
-	clk_disable_unprepare(trng->hclk);
-	clk_disable_unprepare(trng->ahb);
+	WRITE_ONCE(trng->cleanup, 1);
+
+	mutex_unlock(&trng->lock);
+
+	pm_runtime_put_sync(trng->dev);
+
+end:
+	cancel_work_sync(&trng->work);
 }
 
 static int starfive_trng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
@@ -247,7 +358,18 @@ 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;
+	}
+
+	if (wait) {
+		mutex_lock(&trng->lock);
+	} else {
+		if (!mutex_trylock(&trng->lock))
+			return -EAGAIN;
+	}
 
 	if (trng->mode == PRNG_256BIT)
 		max = min_t(size_t, max, (STARFIVE_RAND_LEN * 8));
@@ -256,28 +378,29 @@ static int starfive_trng_read(struct hwrng *rng, void *buf, size_t max, bool wai
 
 	if (wait) {
 		ret = starfive_trng_wait_idle(trng);
-		if (ret) {
-			ret = -ETIMEDOUT;
-			goto out_put;
-		}
+		if (ret)
+			goto end;
 	}
 
 	ret = starfive_trng_cmd(trng, STARFIVE_CTRL_GENE_RANDNUM, wait);
 	if (ret)
-		goto out_put;
+		goto end;
 
 	memcpy_fromio(buf, trng->base + STARFIVE_RAND0, max);
+
 	ret = max;
 
-out_put:
-	pm_runtime_put_sync_autosuspend(trng->dev);
+end:
+	mutex_unlock(&trng->lock);
+
+	pm_runtime_put_autosuspend(trng->dev);
+
 	return ret;
 }
 
 static int starfive_trng_probe(struct platform_device *pdev)
 {
 	int ret;
-	int irq;
 	struct starfive_trng *trng;
 
 	trng = devm_kzalloc(&pdev->dev, sizeof(*trng), GFP_KERNEL);
@@ -285,26 +408,22 @@ static int starfive_trng_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	platform_set_drvdata(pdev, trng);
+
 	trng->dev = &pdev->dev;
+	trng->data = of_device_get_match_data(&pdev->dev);
+	if (!trng->data)
+		return -EINVAL;
+
+	if (trng->data->seq_rst_clk != SEQ_RST_FIRST && trng->data->seq_rst_clk != SEQ_CLK_FIRST) {
+		dev_err(&pdev->dev, "Unknown seq_rst_clk value\n");
+		return -EINVAL;
+	}
 
 	trng->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(trng->base))
 		return dev_err_probe(&pdev->dev, PTR_ERR(trng->base),
 				     "Error remapping memory for platform device.\n");
 
-	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
-		return irq;
-
-	init_completion(&trng->random_done);
-	init_completion(&trng->reseed_done);
-	spin_lock_init(&trng->write_lock);
-
-	ret = devm_request_irq(&pdev->dev, irq, starfive_trng_irq, 0, pdev->name,
-			       (void *)trng);
-	if (ret)
-		return ret;
-
 	trng->hclk = devm_clk_get(&pdev->dev, "hclk");
 	if (IS_ERR(trng->hclk))
 		return dev_err_probe(&pdev->dev, PTR_ERR(trng->hclk),
@@ -320,9 +439,14 @@ static int starfive_trng_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, PTR_ERR(trng->rst),
 				     "Error getting hardware reset line\n");
 
-	clk_prepare_enable(trng->hclk);
-	clk_prepare_enable(trng->ahb);
-	reset_control_deassert(trng->rst);
+	init_completion(&trng->random_done);
+	init_completion(&trng->reseed_done);
+	mutex_init(&trng->lock);
+	INIT_WORK(&trng->work, starfive_trng_randreseed_work);
+
+	trng->irq = platform_get_irq(pdev, 0);
+	if (trng->irq < 0)
+		return trng->irq;
 
 	trng->rng.name = dev_driver_string(&pdev->dev);
 	trng->rng.init = starfive_trng_init;
@@ -333,40 +457,86 @@ static int starfive_trng_probe(struct platform_device *pdev)
 	trng->mission = 1;
 	trng->reseed = RANDOM_RESEED;
 
-	pm_runtime_use_autosuspend(&pdev->dev);
-	pm_runtime_set_autosuspend_delay(&pdev->dev, 100);
-	pm_runtime_enable(&pdev->dev);
+	ret = clk_prepare_enable(trng->hclk);
+	if (ret) {
+		dev_err(&pdev->dev, "hclk clk_enable failed: %d\n", ret);
+		return ret;
+	}
 
-	ret = devm_hwrng_register(&pdev->dev, &trng->rng);
+	ret = clk_prepare_enable(trng->ahb);
 	if (ret) {
-		pm_runtime_disable(&pdev->dev);
+		clk_disable_unprepare(trng->hclk);
+		dev_err(&pdev->dev, "ahb clk_enable failed: %d\n", ret);
+		return ret;
+	}
 
-		reset_control_assert(trng->rst);
+	ret = reset_control_deassert(trng->rst);
+	if (ret) {
 		clk_disable_unprepare(trng->ahb);
 		clk_disable_unprepare(trng->hclk);
+		dev_err(&pdev->dev, "failed to deassert trng\n");
+		return ret;
+	}
 
-		return dev_err_probe(&pdev->dev, ret, "Failed to register hwrng\n");
+	pm_runtime_use_autosuspend(&pdev->dev);
+	pm_runtime_set_autosuspend_delay(&pdev->dev, 100);
+	devm_pm_runtime_set_active_enabled(&pdev->dev);
+
+	ret = devm_request_irq(&pdev->dev, trng->irq, starfive_trng_irq, 0, pdev->name,
+			       (void *)trng);
+	if (ret) {
+		starfive_trng_release(trng);
+		return dev_err_probe(&pdev->dev, ret, "Failed to register interrupt handler\n");
 	}
 
+	ret = devm_add_action_or_reset(&pdev->dev, starfive_trng_release, trng);
+	if (ret)
+		return ret;
+
+	ret = devm_hwrng_register(&pdev->dev, &trng->rng);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "Failed to register hwrng\n");
+
 	return 0;
 }
 
 static int __maybe_unused starfive_trng_suspend(struct device *dev)
 {
 	struct starfive_trng *trng = dev_get_drvdata(dev);
+	bool cleanup = READ_ONCE(trng->cleanup);
+
+	if (cleanup && trng->data->seq_rst_clk == SEQ_RST_FIRST)
+		reset_control_assert(trng->rst);
 
-	clk_disable_unprepare(trng->hclk);
 	clk_disable_unprepare(trng->ahb);
+	clk_disable_unprepare(trng->hclk);
+
+	if (cleanup && trng->data->seq_rst_clk == SEQ_CLK_FIRST)
+		reset_control_assert(trng->rst);
 
 	return 0;
 }
 
 static int __maybe_unused starfive_trng_resume(struct device *dev)
 {
+	int ret;
 	struct starfive_trng *trng = dev_get_drvdata(dev);
 
-	clk_prepare_enable(trng->hclk);
-	clk_prepare_enable(trng->ahb);
+	ret = clk_prepare_enable(trng->hclk);
+	if (ret) {
+		dev_err(trng->dev, "hclk clk_enable failed: %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(trng->ahb);
+	if (ret) {
+		clk_disable_unprepare(trng->hclk);
+		dev_err(trng->dev, "ahb clk_enable failed: %d\n", ret);
+		return ret;
+	}
+
+	if (READ_ONCE(trng->cleanup))
+		reset_control_deassert(trng->rst);
 
 	return 0;
 }
@@ -378,8 +548,17 @@ static const struct dev_pm_ops starfive_trng_pm_ops = {
 			   starfive_trng_resume, NULL)
 };
 
-static const struct of_device_id trng_dt_ids[] __maybe_unused = {
-	{ .compatible = "starfive,jh7110-trng" },
+static const struct starfive_trng_data jh7110_data = {
+	.seq_rst_clk = SEQ_RST_FIRST,
+};
+
+static const struct starfive_trng_data jhb100_data = {
+	.seq_rst_clk = SEQ_CLK_FIRST,
+};
+
+static const struct of_device_id trng_dt_ids[] = {
+	{ .compatible = "starfive,jh7110-trng", .data = &jh7110_data },
+	{ .compatible = "starfive,jhb100-trng", .data = &jhb100_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, trng_dt_ids);
@@ -389,7 +568,7 @@ static struct platform_driver starfive_trng_driver = {
 	.driver	= {
 		.name		= "jh7110-trng",
 		.pm		= &starfive_trng_pm_ops,
-		.of_match_table	= of_match_ptr(trng_dt_ids),
+		.of_match_table	= trng_dt_ids,
 	},
 };
 
-- 
2.43.0

Re: [PATCH v8 2/2] hwrng: starfive: rework clk/reset teardown order for JHB100

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] PM runtime reference leak in starfive_trng_read() on lock contention.
- [High] TOCTOU race in starfive_trng_release leading to double clock disable or bus error.
- [High] Prematurely dropping PM runtime reference during asynchronous hardware reseed.
- [High] Pending workqueue task is not cancelled on module teardown or probe error path, leading to Use-After-Free.
- [High] Shared reset control is left permanently deasserted if unbound while runtime-suspended and uninitialized.
- [High] Missing interrupt mask in starfive_trng_suspend causes an interrupt storm.
--

commit 7c6643a978a103f1c37f18a5471c5dac7b74eeb6
Author: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
Subject: hwrng: starfive: rework clk/reset teardown order for JHB100

This commit reworks the clock and reset teardown sequence to address
hardware-specific requirements for the JHB100 SoC. It introduces a per-compatible
match data structure to dictate reset ordering and migrates the SEU-triggered
hardware reseed path to a workqueue. The driver also switches to devm actions
for teardown management.

> diff --git a/drivers/char/hw_random/jh7110-trng.c b/drivers/char/hw_random/jh7110-trng.c
> index aee12caab5780..ecc7d5bd7bf51 100644
> --- a/drivers/char/hw_random/jh7110-trng.c
> +++ b/drivers/char/hw_random/jh7110-trng.c
[ ... ]
> @@ -138,6 +160,28 @@ static inline void starfive_trng_irq_mask_clear(struct starfive_trng *trng)
>  	writel(data, trng->base + STARFIVE_ISTAT);
>  }
>  
> +static void starfive_trng_release(void *data)
> +{
> +	struct starfive_trng *trng = data;
> +

[Severity: High]
Could this lead to a use-after-free if probe fails? 

The trng->work is not cancelled in this release action. If 
starfive_trng_probe() fails after registering the IRQ, an SEU interrupt could
schedule trng->work right before the trng structure is freed, leaving a
pending workqueue item pointing to freed memory.

> +	if (!pm_runtime_status_suspended(trng->dev)) {

[Severity: High]
Is there a race condition here with asynchronous autosuspend? 

The PM status is checked without ensuring PM transitions are halted. If the
device is active, this proceeds to manually disable clocks. 

If a pending autosuspend timer fires concurrently, starfive_trng_suspend()
might execute on another CPU and also attempt to disable clocks, potentially
leading to clock reference underflows or bus errors.

[Severity: High]
Does skipping this block when suspended cause the shared reset control to
remain permanently deasserted? 

If probe fails after deasserting the reset, and the device autosuspends before
starfive_trng_release() executes, the reset won't be re-asserted here.
starfive_trng_suspend() also skips asserting the reset if trng->cleanup is 0.

> +		writel(0, trng->base + STARFIVE_IE);
> +		starfive_trng_irq_clear(trng);
> +
> +		if (trng->irq >= 0)
> +			synchronize_irq(trng->irq);
> +
[ ... ]
> +static void starfive_trng_randreseed_work(struct work_struct *work)
> +{
> +	struct starfive_trng *trng = work_to_trng(work);
> +	int ret;
> +
> +	ret = pm_runtime_resume_and_get(trng->dev);
> +	if (ret < 0) {
> +		dev_warn(trng->dev, "Failed to wake device for reseed: %d\n", ret);
> +		return;
> +	}
> +
> +	mutex_lock(&trng->lock);
> +
> +	if (READ_ONCE(trng->cleanup))
> +		goto unlock;
> +
> +	reinit_completion(&trng->reseed_done);
> +	writel(STARFIVE_CTRL_EXEC_RANDRESEED, trng->base + STARFIVE_CTRL);
> +
> +unlock:
> +	mutex_unlock(&trng->lock);
> +
> +	pm_runtime_put_autosuspend(trng->dev);
> +}

[Severity: High]
Does this prematurely drop the PM runtime reference while the hardware is
still operating? 

Writing STARFIVE_CTRL_EXEC_RANDRESEED initiates a hardware reseed. Unlike
starfive_trng_cmd() which waits for completion, this workqueue function does
not wait for the reseed_done completion. 

Dropping the reference immediately could allow the device to suspend and
clocks to gate while the TRNG is actively reseeding, potentially corrupting
the IP state.

[ ... ]
> @@ -247,7 +358,18 @@ 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;
> +	}
> +
> +	if (wait) {
> +		mutex_lock(&trng->lock);
> +	} else {
> +		if (!mutex_trylock(&trng->lock))
> +			return -EAGAIN;
> +	}

[Severity: High]
Does this early return leak the PM usage counter? 

The counter is incremented via pm_runtime_resume_and_get() at the top of
starfive_trng_read(). 

Returning -EAGAIN here completely bypasses the pm_runtime_put_autosuspend()
cleanup call at the bottom of the function. If called repeatedly by the hwrng
core, could this overflow the usage counter?

[ ... ]
>  static int __maybe_unused starfive_trng_suspend(struct device *dev)
>  {
>  	struct starfive_trng *trng = dev_get_drvdata(dev);
> +	bool cleanup = READ_ONCE(trng->cleanup);
> +
> +	if (cleanup && trng->data->seq_rst_clk == SEQ_RST_FIRST)
> +		reset_control_assert(trng->rst);
>  
> -	clk_disable_unprepare(trng->hclk);
>  	clk_disable_unprepare(trng->ahb);
> +	clk_disable_unprepare(trng->hclk);

[Severity: High]
Could this cause an interrupt storm when background auto-reseeding is active?

The clocks are disabled here without writing 0 to STARFIVE_IE to mask the
interrupt source. If an interrupt like SEED_DONE fires right as the clocks are
gated, the IRQ line might remain latched high. 

When starfive_trng_irq() runs, it checks pm_runtime_get_if_active() and
returns IRQ_NONE without clearing the hardware interrupt, causing the IRQ to
fire repeatedly and hang the CPU.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907031603.188483-1-lianfeng.ouyang@starfivetech.com?part=2

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

[Lianfeng Ouyang]

> -----邮件原件-----
> 发件人: sashiko-bot@kernel.org <sashiko-bot@kernel.org>
> 发送时间: 2026年9月7日 13:04
> 收件人: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
> 抄送: robh@kernel.org; conor+dt@kernel.org; devicetree@vger.kernel.org
> 主题: Re: [PATCH v8 2/2] hwrng: starfive: rework clk/reset teardown order for
> JHB100
> 
> Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to
> consider:
> - [High] PM runtime reference leak in starfive_trng_read() on lock contention.
> - [High] TOCTOU race in starfive_trng_release leading to double clock disable or
> bus error.
> - [High] Prematurely dropping PM runtime reference during asynchronous
> hardware reseed.
> - [High] Pending workqueue task is not cancelled on module teardown or probe
> error path, leading to Use-After-Free.
> - [High] Shared reset control is left permanently deasserted if unbound while
> runtime-suspended and uninitialized.
> - [High] Missing interrupt mask in starfive_trng_suspend causes an interrupt
> storm.
> --
> 
> commit 7c6643a978a103f1c37f18a5471c5dac7b74eeb6
> Author: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
> Subject: hwrng: starfive: rework clk/reset teardown order for JHB100
> 
> This commit reworks the clock and reset teardown sequence to address
> hardware-specific requirements for the JHB100 SoC. It introduces a
> per-compatible
> match data structure to dictate reset ordering and migrates the SEU-triggered
> hardware reseed path to a workqueue. The driver also switches to devm actions
> for teardown management.
> 
> > diff --git a/drivers/char/hw_random/jh7110-trng.c
> b/drivers/char/hw_random/jh7110-trng.c
> > index aee12caab5780..ecc7d5bd7bf51 100644
> > --- a/drivers/char/hw_random/jh7110-trng.c
> > +++ b/drivers/char/hw_random/jh7110-trng.c
> [ ... ]
> > @@ -138,6 +160,28 @@ static inline void
> starfive_trng_irq_mask_clear(struct starfive_trng *trng)
> >  	writel(data, trng->base + STARFIVE_ISTAT);
> >  }
> >
> > +static void starfive_trng_release(void *data)
> > +{
> > +	struct starfive_trng *trng = data;
> > +
> 
> [Severity: High]
> Could this lead to a use-after-free if probe fails?
> 
> The trng->work is not cancelled in this release action. If
> starfive_trng_probe() fails after registering the IRQ, an SEU interrupt could
> schedule trng->work right before the trng structure is freed, leaving a
> pending workqueue item pointing to freed memory.
> 
> > +	if (!pm_runtime_status_suspended(trng->dev)) {
> 
> [Severity: High]
> Is there a race condition here with asynchronous autosuspend?
> 
> The PM status is checked without ensuring PM transitions are halted. If the
> device is active, this proceeds to manually disable clocks.
> 
> If a pending autosuspend timer fires concurrently, starfive_trng_suspend()
> might execute on another CPU and also attempt to disable clocks, potentially
> leading to clock reference underflows or bus errors.
> 
> [Severity: High]
> Does skipping this block when suspended cause the shared reset control to
> remain permanently deasserted?
> 
> If probe fails after deasserting the reset, and the device autosuspends before
> starfive_trng_release() executes, the reset won't be re-asserted here.
> starfive_trng_suspend() also skips asserting the reset if trng->cleanup is 0.
> 
> > +		writel(0, trng->base + STARFIVE_IE);
> > +		starfive_trng_irq_clear(trng);
> > +
> > +		if (trng->irq >= 0)
> > +			synchronize_irq(trng->irq);
> > +
> [ ... ]
> > +static void starfive_trng_randreseed_work(struct work_struct *work)
> > +{
> > +	struct starfive_trng *trng = work_to_trng(work);
> > +	int ret;
> > +
> > +	ret = pm_runtime_resume_and_get(trng->dev);
> > +	if (ret < 0) {
> > +		dev_warn(trng->dev, "Failed to wake device for reseed: %d\n", ret);
> > +		return;
> > +	}
> > +
> > +	mutex_lock(&trng->lock);
> > +
> > +	if (READ_ONCE(trng->cleanup))
> > +		goto unlock;
> > +
> > +	reinit_completion(&trng->reseed_done);
> > +	writel(STARFIVE_CTRL_EXEC_RANDRESEED, trng->base +
> STARFIVE_CTRL);
> > +
> > +unlock:
> > +	mutex_unlock(&trng->lock);
> > +
> > +	pm_runtime_put_autosuspend(trng->dev);
> > +}
> 
> [Severity: High]
> Does this prematurely drop the PM runtime reference while the hardware is
> still operating?
> 
> Writing STARFIVE_CTRL_EXEC_RANDRESEED initiates a hardware reseed.
> Unlike
> starfive_trng_cmd() which waits for completion, this workqueue function does
> not wait for the reseed_done completion.
> 
> Dropping the reference immediately could allow the device to suspend and
> clocks to gate while the TRNG is actively reseeding, potentially corrupting
> the IP state.
> 
> [ ... ]
> > @@ -247,7 +358,18 @@ 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;
> > +	}
> > +
> > +	if (wait) {
> > +		mutex_lock(&trng->lock);
> > +	} else {
> > +		if (!mutex_trylock(&trng->lock))
> > +			return -EAGAIN;
> > +	}
> 
> [Severity: High]
> Does this early return leak the PM usage counter?
> 
> The counter is incremented via pm_runtime_resume_and_get() at the top of
> starfive_trng_read().
> 
> Returning -EAGAIN here completely bypasses the
> pm_runtime_put_autosuspend()
> cleanup call at the bottom of the function. If called repeatedly by the hwrng
> core, could this overflow the usage counter?
> 
> [ ... ]
> >  static int __maybe_unused starfive_trng_suspend(struct device *dev)
> >  {
> >  	struct starfive_trng *trng = dev_get_drvdata(dev);
> > +	bool cleanup = READ_ONCE(trng->cleanup);
> > +
> > +	if (cleanup && trng->data->seq_rst_clk == SEQ_RST_FIRST)
> > +		reset_control_assert(trng->rst);
> >
> > -	clk_disable_unprepare(trng->hclk);
> >  	clk_disable_unprepare(trng->ahb);
> > +	clk_disable_unprepare(trng->hclk);
> 
> [Severity: High]
> Could this cause an interrupt storm when background auto-reseeding is active?
> 
> The clocks are disabled here without writing 0 to STARFIVE_IE to mask the
> interrupt source. If an interrupt like SEED_DONE fires right as the clocks are
> gated, the IRQ line might remain latched high.
> 
> When starfive_trng_irq() runs, it checks pm_runtime_get_if_active() and
> returns IRQ_NONE without clearing the hardware interrupt, causing the IRQ to
> fire repeatedly and hang the CPU.
> 
> --
> Sashiko AI
> review · https://sashiko.dev/#/patchset/20260907031603.188483-1-lianfeng.o
> uyang@starfivetech.com?part=2

Hi Sashiko

Thank you for the detailed AI review. The analysis is spot on.

I have confirmed the following issues and will address them in next version:
1. Fixed the PM runtime reference leak in starfive_trng_read() by adding
	pm_runtime_put_autosuspend() on the -EAGAIN path.
2. Resolved the TOCTOU race and double clock disable by adding cancel_work_sync() 
	and proper PM state handling in starfive_trng_release().
3. Fixed the premature runtime reference drop in starfive_trng_randreseed_work() 
	by waiting for the reseed_done completion before calling pm_runtime_put_autosuspend().
4. Ensured the workqueue is cancelled on both probe failure and module teardown 
	to prevent Use-After-Free.
5. Fixed the interrupt storm issue by masking and clearing interrupts in 
	starfive_trng_suspend() before disabling clocks.
6. Ensured the shared reset is properly asserted regardless of the autosuspend state during cleanup.

I will send out v9 shortly.

Best regards,
Lianfeng Ouyang