[PATCH v4 0/2] hwrng: starfive: updates for jh7110-trng DT binding and rework clk/reset teardown

[PATCH v4 0/2] hwrng: starfive: updates for jh7110-trng DT binding and rework 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.

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          | 223 ++++++++++++++----
 3 files changed, 180 insertions(+), 55 deletions(-)

--
2.43.0

[PATCH v4 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>
---
 .../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 v4 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.

Fixes RPM handling by marking the device as RPM_ACTIVE after clocks and
  reset are deasserted but before pm_runtime_enable(), allowing the usage
  count to drop to zero and enabling autosuspend.

Balances pm_runtime_get/put calls in init, read, and cleanup paths,
  and moves low-level disable/reset operations into a devm
  action (starfive_trng_release()) for correct error-path unwind
  ordering.

Improvements include proper mutex protection for TRNG command sequences,
  enhanced clock enable error handling.

Signed-off-by: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
---
 MAINTAINERS                          |   2 +-
 drivers/char/hw_random/jh7110-trng.c | 223 +++++++++++++++++++++------
 2 files changed, 176 insertions(+), 49 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.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 9776f4daa044..cafc873b9ebf 100644
--- a/drivers/char/hw_random/jh7110-trng.c
+++ b/drivers/char/hw_random/jh7110-trng.c
@@ -92,20 +92,36 @@ 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;
+	const struct starfive_trng_data *data;
+	u32				mode;
+	u32				mission;
+	u32				reseed;
+	struct mutex			lock; /* protect trng cmd seq */
+	spinlock_t			write_lock; /* protects register access seq */
 };
 
 static u16 autoreq;
@@ -130,7 +146,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 +154,31 @@ 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);
+	}
+
+	pm_runtime_dont_use_autosuspend(trng->dev);
+	pm_runtime_disable(trng->dev);
+}
+
 static int starfive_trng_cmd(struct starfive_trng *trng, u32 cmd, bool wait)
 {
 	int wait_time = 1000;
@@ -174,13 +215,22 @@ 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);
 
 	/* 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,24 +251,33 @@ 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 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);
@@ -228,18 +287,37 @@ static irqreturn_t starfive_trng_irq(int irq, void *priv)
 		spin_unlock(&trng->write_lock);
 	}
 
+	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);
+		return;
+	}
+
+	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);
+	pm_runtime_put_sync(trng->dev);
 }
 
 static int starfive_trng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
@@ -247,7 +325,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);
 
 	if (trng->mode == PRNG_256BIT)
 		max = min_t(size_t, max, (STARFIVE_RAND_LEN * 8));
@@ -257,24 +341,28 @@ 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)
-			return -ETIMEDOUT;
+			goto end;
 	}
 
 	ret = starfive_trng_cmd(trng, STARFIVE_CTRL_GENE_RANDNUM, wait);
 	if (ret)
-		return ret;
+		goto end;
 
 	memcpy_fromio(buf, trng->base + STARFIVE_RAND0, max);
 
-	pm_runtime_put_sync_autosuspend(trng->dev);
+	ret = max;
+
+end:
+	mutex_unlock(&trng->lock);
 
-	return max;
+	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);
@@ -282,22 +370,32 @@ 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;
+	trng->irq = platform_get_irq(pdev, 0);
+	if (trng->irq < 0)
+		return trng->irq;
 
 	init_completion(&trng->random_done);
 	init_completion(&trng->reseed_done);
+	mutex_init(&trng->lock);
 	spin_lock_init(&trng->write_lock);
 
-	ret = devm_request_irq(&pdev->dev, irq, starfive_trng_irq, 0, pdev->name,
+	ret = devm_request_irq(&pdev->dev, trng->irq, starfive_trng_irq, 0, pdev->name,
 			       (void *)trng);
 	if (ret)
 		return dev_err_probe(&pdev->dev, ret,
@@ -318,8 +416,19 @@ 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);
+	ret = clk_prepare_enable(trng->hclk);
+	if (ret) {
+		dev_err(&pdev->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(&pdev->dev, "ahb clk_enable failed: %d\n", ret);
+		return ret;
+	}
+
 	reset_control_deassert(trng->rst);
 
 	trng->rng.name = dev_driver_string(&pdev->dev);
@@ -333,18 +442,16 @@ static int starfive_trng_probe(struct platform_device *pdev)
 
 	pm_runtime_use_autosuspend(&pdev->dev);
 	pm_runtime_set_autosuspend_delay(&pdev->dev, 100);
+	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
 
-	ret = devm_hwrng_register(&pdev->dev, &trng->rng);
-	if (ret) {
-		pm_runtime_disable(&pdev->dev);
-
-		reset_control_assert(trng->rst);
-		clk_disable_unprepare(trng->ahb);
-		clk_disable_unprepare(trng->hclk);
+	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;
 }
@@ -361,10 +468,21 @@ static int __maybe_unused starfive_trng_suspend(struct device *dev)
 
 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;
+	}
 
 	return 0;
 }
@@ -376,8 +494,17 @@ static const struct dev_pm_ops starfive_trng_pm_ops = {
 			   starfive_trng_resume, NULL)
 };
 
+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[] __maybe_unused = {
-	{ .compatible = "starfive,jh7110-trng" },
+	{ .compatible = "starfive,jh7110-trng", .data = &jh7110_data },
+	{ .compatible = "starfive,jhb100-trng", .data = &jhb100_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, trng_dt_ids);
-- 
2.43.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Removal of 'starfive,jh8100-trng' compatible string breaks device tree ABI.
--

commit 55269300e907ca4d974f2fe40b38e11260ec7ecb
Author: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>

dt-bindings: rng: starfive,jh7110-trng: add jhb100, drop jh8100

This commit updates the StarFive TRNG device tree bindings to reflect
current SoC support. It adds the starfive,jhb100-trng compatible string
and drops the obsolete starfive,jh8100-trng compatible string.

> diff --git a/Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml b/Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml
> index 4639247e9e510..d21769b7d54e6 100644
> --- a/Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml
> +++ b/Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml
[ ... ]
>  properties:
>    compatible:
> -    oneOf:
> -      - items:
> -          - const: starfive,jh8100-trng
> -          - const: starfive,jh7110-trng
> -      - const: starfive,jh7110-trng
> +    enum:
> +      - starfive,jh7110-trng
> +      - starfive,jhb100-trng

[Severity: Medium]
Does removing the starfive,jh8100-trng compatible string break device tree
ABI compatibility?

Device tree bindings are generally considered a stable ABI, so existing or
downstream device trees using this compatible string might fail schema
validation during dtbs_check.

Should this compatible string be retained and perhaps marked as deprecated
instead of being completely removed from the properties section?

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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [High] The devm release action skips hardware reset assertion due to a synchronous suspend in the cleanup path.
- [High] Failing to mask interrupts during runtime suspend leads to an unhandled interrupt storm during device resume.

Pre-existing issues:
- [High] The device IRQ is requested before hardware and PM are initialized, risking an immediate IRQ storm during boot.
- [High] Non-blocking reads invariably fail with -ETIMEDOUT due to a 0-jiffy timeout calculation.
--

commit d88dd84c5abe42f5a9e074b3a3e4819cbff1fcc8
Author: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>

hwrng: starfive: rework clk/reset teardown order for JHB100

Reworks the StarFive TRNG driver to handle JHB100 hardware-specific
requirements for clk/reset teardown. Adds Runtime PM integration to manage
device power state and includes a devm action for cleanup.

> diff --git a/drivers/char/hw_random/jh7110-trng.c b/drivers/char/hw_random/jh7110-trng.c
> index 9776f4daa0445..cafc873b9ebf8 100644
> --- a/drivers/char/hw_random/jh7110-trng.c
> +++ b/drivers/char/hw_random/jh7110-trng.c
> @@ -138,6 +154,31 @@ 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)) {

[Severity: High]
Does this PM state check accidentally skip hardware reset assertion during
driver unbind?

The HWRNG unregisters first during teardown, which calls
starfive_trng_cleanup() where pm_runtime_put_sync() synchronously suspends
the device. When this devm action executes immediately after, the device is
already suspended, making this condition false and completely bypassing the
reset_control_assert(trng->rst) logic.

Could checking pm_runtime_status_suspended() outside a lock also leave a
race window where autosuspend might trigger and cause double clock disables?

> +		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);
> +	}
> +
> +	pm_runtime_dont_use_autosuspend(trng->dev);
> +	pm_runtime_disable(trng->dev);
> +}
> +

[ ... ]

> @@ -201,24 +251,33 @@ static int starfive_trng_init(struct hwrng *rng)
>  
>  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;
> +	}

[Severity: High]
Will this return cause an infinite interrupt storm if an interrupt fires
during device resume?

If starfive_trng_suspend() doesn't mask device interrupts via STARFIVE_IE,
an interrupt could trigger while suspended or when clocks are re-enabled in
starfive_trng_resume().

Because the PM core transitions the device through RPM_RESUMING,
pm_runtime_get_if_active() returns 0. Returning IRQ_NONE without clearing
the interrupt leaves the level-triggered IRQ asserted, locking up the CPU.

>  
>  	status = readl(trng->base + STARFIVE_ISTAT);
> -	if (status & STARFIVE_ISTAT_RAND_RDY) {

[ ... ]

> @@ -247,7 +325,13 @@ 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)
> -			return -ETIMEDOUT;
> +			goto end;
>  	}
>  
>  	ret = starfive_trng_cmd(trng, STARFIVE_CTRL_GENE_RANDNUM, wait);
>  	if (ret)
> -		return ret;
> +		goto end;

[Severity: High]
This isn't a bug introduced by this patch, but will non-blocking reads
invariably fail with -ETIMEDOUT due to a 0-jiffy timeout calculation?

In starfive_trng_cmd(), when wait is false, wait_time is 40. This evaluates
as usecs_to_jiffies(40). On systems with standard HZ=1000 or lower, this
results in 0 jiffies. 

Calling wait_for_completion_timeout() with 0 causes an immediate failure if
the completion is not already signaled, guaranteeing that non-blocking reads
always time out.

>  
>  	memcpy_fromio(buf, trng->base + STARFIVE_RAND0, max);
>  
> -	pm_runtime_put_sync_autosuspend(trng->dev);
> +	ret = max;
> +
> +end:
> +	mutex_unlock(&trng->lock);

[ ... ]

> @@ -282,22 +370,32 @@ static int starfive_trng_probe(struct platform_device *pdev)
>  	trng->irq = platform_get_irq(pdev, 0);
>  	if (trng->irq < 0)
>  		return trng->irq;
>  
>  	init_completion(&trng->random_done);
>  	init_completion(&trng->reseed_done);
> +	mutex_init(&trng->lock);
>  	spin_lock_init(&trng->write_lock);
>  
> -	ret = devm_request_irq(&pdev->dev, irq, starfive_trng_irq, 0, pdev->name,
> +	ret = devm_request_irq(&pdev->dev, trng->irq, starfive_trng_irq, 0, pdev->name,
>  			       (void *)trng);

[Severity: High]
This is a pre-existing issue, but does requesting the IRQ here before
hardware and Runtime PM are initialized risk an immediate IRQ storm?

If the bootloader leaves the TRNG with a pending interrupt, the IRQ can fire
immediately. The newly added PM checks change this from a potential crash
into an interrupt storm, since starfive_trng_irq() will see
pm_runtime_get_if_active() return 0 and bail with IRQ_NONE before
acknowledging the hardware interrupt.

>  	if (ret)
>  		return dev_err_probe(&pdev->dev, ret,
>  				     "Failed to register interrupt handler\n");
>

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

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 75 bytes --]

Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]