[PATCH v2 0/2] Add trng driver to JHB100

[PATCH v2 0/2] Add trng driver to JHB100

[lianfeng.ouyang]

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

for jhb100, While IP assert async reset, it may generate glitch
and propagate to downstream IP. In order to solve RDC issue,
conduct clock gating before asserting reset to prevent generating glitch.

Optimization-driven PM logic. The original driver did not declare PM as
active in the probe, resulting in the count not being able to drop to 0

Lianfeng Ouyang (2):
  dt-bindings: Add bindings for StarFive JHB100 SoC trng controller
  hwrng: starfive: Update clk and reset sequence

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

--
2.43.0

[PATCH v2 1/2] dt-bindings: Add bindings for StarFive JHB100 SoC trng controller

[lianfeng.ouyang]

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

jh8100 is no longer supported
Jia Jie Ho 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 v2 2/2] hwrng: starfive: Update clk and reset sequence

[lianfeng.ouyang]

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

for jhb100, While IP assert async reset, it may generate glitch
and propagate to downstream IP. In order to solve RDC issue,
conduct clock gating before asserting reset to prevent generating glitch.

Jia Jie Ho has resigned

Signed-off-by: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
---
 MAINTAINERS                          |  2 +-
 drivers/char/hw_random/jh7110-trng.c | 83 ++++++++++++++++++++--------
 2 files changed, 61 insertions(+), 24 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..2265d856d593 100644
--- a/drivers/char/hw_random/jh7110-trng.c
+++ b/drivers/char/hw_random/jh7110-trng.c
@@ -78,6 +78,9 @@
 #define STARFIVE_ISTAT_SEED_DONE	BIT(1)
 #define STARFIVE_ISTAT_LFSR_LOCKUP	BIT(4)

+#define HW_SEQ_RESET_FIRST_THEN_CLK	0
+#define HW_SEQ_CLK_FIRST_THEN_RESET	1
+
 #define STARFIVE_RAND_LEN		sizeof(u32)

 #define to_trng(p)			container_of(p, struct starfive_trng, rng)
@@ -95,12 +98,14 @@ enum mode {
 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;
+	u32			hw_seq;
 	u32			mode;
 	u32			mission;
 	u32			reseed;
@@ -130,7 +135,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 +143,23 @@ 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;
+
+	pm_runtime_disable(trng->dev);
+	pm_runtime_dont_use_autosuspend(trng->dev);
+
+	if (trng->hw_seq == HW_SEQ_RESET_FIRST_THEN_CLK)
+		reset_control_assert(trng->rst);
+
+	clk_disable_unprepare(trng->ahb);
+	clk_disable_unprepare(trng->hclk);
+
+	if (trng->hw_seq == HW_SEQ_CLK_FIRST_THEN_RESET)
+		reset_control_assert(trng->rst);
+}
+
 static int starfive_trng_cmd(struct starfive_trng *trng, u32 cmd, bool wait)
 {
 	int wait_time = 1000;
@@ -174,13 +196,16 @@ static int starfive_trng_init(struct hwrng *rng)
 {
 	struct starfive_trng *trng = to_trng(rng);
 	u32 mode, intr = 0;
+	int ret;
+
+	pm_runtime_get_sync(trng->dev);

 	/* 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,7 +226,11 @@ 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);
+
+	pm_runtime_put_sync_autosuspend(trng->dev);
+
+	return ret;
 }

 static irqreturn_t starfive_trng_irq(int irq, void *priv)
@@ -235,11 +264,17 @@ static void starfive_trng_cleanup(struct hwrng *rng)
 {
 	struct starfive_trng *trng = to_trng(rng);

+	pm_runtime_get_sync(trng->dev);
+
+	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)
@@ -257,24 +292,26 @@ 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);

+	ret = max;
+
+end:
 	pm_runtime_put_sync_autosuspend(trng->dev);

-	return max;
+	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);
@@ -283,21 +320,22 @@ static int starfive_trng_probe(struct platform_device *pdev)

 	platform_set_drvdata(pdev, trng);
 	trng->dev = &pdev->dev;
+	trng->hw_seq = (kernel_ulong_t)of_device_get_match_data(&pdev->dev);

 	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);
 	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,
@@ -333,18 +371,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;
 }
@@ -377,7 +413,8 @@ static const struct dev_pm_ops starfive_trng_pm_ops = {
 };

 static const struct of_device_id trng_dt_ids[] __maybe_unused = {
-	{ .compatible = "starfive,jh7110-trng" },
+	{ .compatible = "starfive,jh7110-trng", .data = (void *)HW_SEQ_RESET_FIRST_THEN_CLK },
+	{ .compatible = "starfive,jhb100-trng", .data = (void *)HW_SEQ_CLK_FIRST_THEN_RESET },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, trng_dt_ids);
--
2.43.0

Re: [PATCH v2 1/2] dt-bindings: Add bindings for StarFive JHB100 SoC trng controller

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Removing the existing 'starfive,jh8100-trng' compatible string breaks backwards compatibility.
--

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

dt-bindings: Add bindings for StarFive JHB100 SoC trng controller

This commit adds device tree bindings for the StarFive JHB100 SoC TRNG
controller, updates the maintainer, and removes the JH8100 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

Does removing the existing "starfive,jh8100-trng" compatible string break
backwards compatibility?

Device tree bindings represent a stable ABI, and removing the previously
merged "starfive,jh8100-trng" string breaks dt-schema validation for any
existing or downstream device trees that used it. Even if the hardware is
no longer supported, the standard practice is to retain such strings,
perhaps optionally adding "deprecated: true", rather than deleting them
outright.

>  
>    reg:
>      maxItems: 1

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

Re: [PATCH v2 2/2] hwrng: starfive: Update clk and reset sequence

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] Unhandled error from `pm_runtime_get_sync()` leads to unsafe register access.
- [High] `starfive_trng_release()` causes a clock double-disable and hardware sequence violation if the device is suspended.
- [High] Devres ordering creates a race condition where the IRQ handler remains active after clocks are disabled.
- [High] The IRQ handler accesses hardware registers without verifying the device's PM state.
- [High] The IRQ handler is registered before hardware clocks are enabled, creating a window for a fatal crash.
- [High] A stale IRQ completion race allows `starfive_trng_read()` to return non-random bytes prematurely.
--

commit 9fe8263576b446ca8c08d308b365ff7601cef468
Author: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
Subject: hwrng: starfive: Update clk and reset sequence

This commit updates the clock and reset sequencing for the StarFive TRNG
driver to prevent glitches on the jhb100 platform. It introduces a specific
release function and integrates runtime PM across initialization, reading,
and cleanup.

> diff --git a/drivers/char/hw_random/jh7110-trng.c b/drivers/char/hw_random/jh7110-trng.c
> --- a/drivers/char/hw_random/jh7110-trng.c
> +++ b/drivers/char/hw_random/jh7110-trng.c
> @@ -138,6 +143,23 @@ 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;
> +
> +	pm_runtime_disable(trng->dev);
> +	pm_runtime_dont_use_autosuspend(trng->dev);
> +
> +	if (trng->hw_seq == HW_SEQ_RESET_FIRST_THEN_CLK)
> +		reset_control_assert(trng->rst);
> +
> +	clk_disable_unprepare(trng->ahb);
> +	clk_disable_unprepare(trng->hclk);

If the device is already runtime-suspended (e.g., autosuspend timer expired),
starfive_trng_suspend() would have already disabled these clocks. Since
pm_runtime_disable() does not wake up a suspended device, will calling
clk_disable_unprepare() here result in a clock enable count underflow?

Also, for the HW_SEQ_RESET_FIRST_THEN_CLK variant, could this assert the
hardware reset on a clock-gated IP block? Should the release function wake
the device (via pm_runtime_get_sync) before performing hardware teardown?

[ ... ]
> @@ -174,13 +196,16 @@ static int starfive_trng_init(struct hwrng *rng)
>  {
>  	struct starfive_trng *trng = to_trng(rng);
>  	u32 mode, intr = 0;
> +	int ret;
> +
> +	pm_runtime_get_sync(trng->dev);

Is it safe to ignore the return value of pm_runtime_get_sync() here? If the
device fails to resume, it returns a negative error code. Would the
subsequent register accesses (like writel/readl) occur on a device with
disabled clocks, potentially causing a synchronous external abort?

Does this same unhandled return value issue apply to the pm_runtime_get_sync()
calls added in starfive_trng_read() and starfive_trng_cleanup() as well?

[ ... ]
> @@ -257,24 +292,26 @@ 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);

Even though starfive_trng_wait_idle() polls until the hardware is idle,
could there still be a pending IRQ from a previous operation that hasn't
been processed by the CPU yet?

If that pending IRQ is handled right after reinit_completion() is called
inside starfive_trng_cmd(), would it complete the random_done completion
early, causing this function to read from STARFIVE_RAND0 while the hardware
is still actively generating the new random number?

[ ... ]
> @@ -283,21 +320,22 @@ static int starfive_trng_probe(struct platform_device *pdev)
[ ... ]
>  	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,
> +	ret = devm_request_irq(&pdev->dev, trng->irq, starfive_trng_irq, 0, pdev->name,
>  			       (void *)trng);

This registers the IRQ before the hardware clocks are enabled below. If an
interrupt is pending from a prior boot stage, will starfive_trng_irq()
fire immediately and attempt to read STARFIVE_ISTAT while clocks are still
gated off, potentially triggering a bus fault?

Additionally, does starfive_trng_irq() need to check the PM state using
pm_runtime_get_if_active() before accessing registers? If a spurious or
delayed interrupt fires while the device is runtime suspended, the handler
currently reads registers unconditionally.

[ ... ]
>  	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);

Devres actions execute in reverse order of creation. Since
devm_request_irq() is called before devm_add_action_or_reset(), won't
starfive_trng_release() run first during device unbind and disable the
clocks, leaving the IRQ active?

If an interrupt fires during this window, wouldn't it cause a bus fault
when the handler reads from a clock-gated IP?

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

Re: [PATCH v2 1/2] dt-bindings: Add bindings for StarFive JHB100 SoC trng controller

[Conor Dooley]

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

Sashiko devs,

On Mon, May 18, 2026 at 07:06:21AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Removing the existing 'starfive,jh8100-trng' compatible string breaks backwards compatibility.
> --
> 
> commit 847e83dc6a2024f6cb7a70fd0453035b2808bedb
> Author: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
> 
> dt-bindings: Add bindings for StarFive JHB100 SoC trng controller
> 
> This commit adds device tree bindings for the StarFive JHB100 SoC TRNG
> controller, updates the maintainer, and removes the JH8100 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
> 
> Does removing the existing "starfive,jh8100-trng" compatible string break
> backwards compatibility?

The commit message is perfunctory but does attempt to explains this.
Since the bot reply fails to retain the commit message as quoted text,
it's annoying to go figure that out.

It's really annoying that this happens, especially when the LLM text
references the commit message. Can you please change the email response
settings to retain the commit messages?

Cheers,
Conor.

> 
> Device tree bindings represent a stable ABI, and removing the previously
> merged "starfive,jh8100-trng" string breaks dt-schema validation for any
> existing or downstream device trees that used it. Even if the hardware is
> no longer supported, the standard practice is to retain such strings,
> perhaps optionally adding "deprecated: true", rather than deleting them
> outright.
> 
> >  
> >    reg:
> >      maxItems: 1
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260518065243.20865-1-lianfeng.ouyang@starfivetech.com?part=1

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

Re: [PATCH v2 1/2] dt-bindings: Add bindings for StarFive JHB100 SoC trng controller

[Conor Dooley]

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

On Mon, May 18, 2026 at 02:52:42PM +0800, lianfeng.ouyang wrote:
> From: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
> 
> jh8100 is no longer supported
> Jia Jie Ho has resigned

Please put some effort into your commit messages. Look around on LKML,
where do you ever seen commit messages as perfunctory as this?
Please speak to the other developers at Starfive about what the commit
messages should look like.

The first "sentence" here isn't even really accurate, is it?
The jh8100 was never even released to customers, right?

pw-bot: changes-requested

Thanks,
Conor.

> 
> 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
> 
> 

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

回复: [PATCH v2 1/2] dt-bindings: Add bindings for StarFive JHB100 SoC trng controller

[Lianfeng Ouyang]

> -----邮件原件-----
> 发件人: Conor Dooley <conor@kernel.org>
> 发送时间: 2026年5月19日 0:42
> 收件人: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
> 抄送: Olivia Mackall <olivia@selenic.com>; Herbert Xu
> <herbert@gondor.apana.org.au>; Rob Herring <robh@kernel.org>; Krzysztof
> Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Philipp
> Zabel <p.zabel@pengutronix.de>; linux-crypto@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> 主题: Re: [PATCH v2 1/2] dt-bindings: Add bindings for StarFive JHB100 SoC trng
> controller
> 
> On Mon, May 18, 2026 at 02:52:42PM +0800, lianfeng.ouyang wrote:
> > From: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
> >
> > jh8100 is no longer supported
> > Jia Jie Ho has resigned
> 
> Please put some effort into your commit messages. Look around on LKML,
> where do you ever seen commit messages as perfunctory as this?
> Please speak to the other developers at Starfive about what the commit
> messages should look like.
> 
> The first "sentence" here isn't even really accurate, is it?
> The jh8100 was never even released to customers, right?
> 
> pw-bot: changes-requested
> 
> Thanks,
> Conor.

I'm very sorry, I will provide more information for commit messages
Thanks,
Lianfeng.

> 
> >
> > 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 v2 2/2] hwrng: starfive: Update clk and reset sequence

[Lianfeng Ouyang]

Hi, Conor

Thanks for the review.

> -----邮件原件-----
> 发件人: sashiko-bot@kernel.org <sashiko-bot@kernel.org>
> 发送时间: 2026年5月18日 15:40
> 收件人: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
> 抄送: robh@kernel.org; devicetree@vger.kernel.org; conor+dt@kernel.org
> 主题: Re: [PATCH v2 2/2] hwrng: starfive: Update clk and reset sequence
> 
> Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to
> consider:
> - [High] Unhandled error from `pm_runtime_get_sync()` leads to unsafe register
> access.
> - [High] `starfive_trng_release()` causes a clock double-disable and hardware
> sequence violation if the device is suspended.
> - [High] Devres ordering creates a race condition where the IRQ handler
> remains active after clocks are disabled.
> - [High] The IRQ handler accesses hardware registers without verifying the
> device's PM state.
> - [High] The IRQ handler is registered before hardware clocks are enabled,
> creating a window for a fatal crash.
> - [High] A stale IRQ completion race allows `starfive_trng_read()` to return
> non-random bytes prematurely.
> --
> 
> commit 9fe8263576b446ca8c08d308b365ff7601cef468
> Author: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
> Subject: hwrng: starfive: Update clk and reset sequence
> 
> This commit updates the clock and reset sequencing for the StarFive TRNG
> driver to prevent glitches on the jhb100 platform. It introduces a specific release
> function and integrates runtime PM across initialization, reading, and cleanup.
> 
> > diff --git a/drivers/char/hw_random/jh7110-trng.c
> > b/drivers/char/hw_random/jh7110-trng.c
> > --- a/drivers/char/hw_random/jh7110-trng.c
> > +++ b/drivers/char/hw_random/jh7110-trng.c
> > @@ -138,6 +143,23 @@ 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;
> > +
> > +	pm_runtime_disable(trng->dev);
> > +	pm_runtime_dont_use_autosuspend(trng->dev);
> > +
> > +	if (trng->hw_seq == HW_SEQ_RESET_FIRST_THEN_CLK)
> > +		reset_control_assert(trng->rst);
> > +
> > +	clk_disable_unprepare(trng->ahb);
> > +	clk_disable_unprepare(trng->hclk);
> 
> If the device is already runtime-suspended (e.g., autosuspend timer expired),
> starfive_trng_suspend() would have already disabled these clocks. Since
> pm_runtime_disable() does not wake up a suspended device, will calling
> clk_disable_unprepare() here result in a clock enable count underflow?
> 
> Also, for the HW_SEQ_RESET_FIRST_THEN_CLK variant, could this assert the
> hardware reset on a clock-gated IP block? Should the release function wake the
> device (via pm_runtime_get_sync) before performing hardware teardown?

Thank you for pointing it out. There is indeed a hidden danger here, and I have fixed it in the code using pm_runtime_get-sync and pm_runtime_put_noide

> 
> [ ... ]
> > @@ -174,13 +196,16 @@ static int starfive_trng_init(struct hwrng *rng)
> > {
> >  	struct starfive_trng *trng = to_trng(rng);
> >  	u32 mode, intr = 0;
> > +	int ret;
> > +
> > +	pm_runtime_get_sync(trng->dev);
> 
> Is it safe to ignore the return value of pm_runtime_get_sync() here? If the
> device fails to resume, it returns a negative error code. Would the subsequent
> register accesses (like writel/readl) occur on a device with disabled clocks,
> potentially causing a synchronous external abort?
> 
> Does this same unhandled return value issue apply to the
> pm_runtime_get_sync() calls added in starfive_trng_read() and
> starfive_trng_cleanup() as well?

Yes, return value verification is required for all calls to pm_runtime_get-sync in the entire driver. I will fix this in the code. Thank you

> 
> [ ... ]
> > @@ -257,24 +292,26 @@ 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);
> 
> Even though starfive_trng_wait_idle() polls until the hardware is idle, could
> there still be a pending IRQ from a previous operation that hasn't been
> processed by the CPU yet?
> 
> If that pending IRQ is handled right after reinit_completion() is called inside
> starfive_trng_cmd(), would it complete the random_done completion early,
> causing this function to read from STARFIVE_RAND0 while the hardware is still
> actively generating the new random number?

Sorry, I didn't understand the problem here because every time the corresponding 
value is written to the STARFIVE-CTRL register, the corresponding interrupt is triggered,
 and then the corresponding completion amount is completed. That is to say, 
even if there is an interrupt after calling reinit_completion(), it is not related to the
 current completion amount being processed because the corresponding completion 
amount has already been processed in each reinit_completion and wait_for_completion_timeout

> 
> [ ... ]
> > @@ -283,21 +320,22 @@ static int starfive_trng_probe(struct
> > platform_device *pdev)
> [ ... ]
> >  	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,
> > +	ret = devm_request_irq(&pdev->dev, trng->irq, starfive_trng_irq, 0,
> > +pdev->name,
> >  			       (void *)trng);
> 
> This registers the IRQ before the hardware clocks are enabled below. If an
> interrupt is pending from a prior boot stage, will starfive_trng_irq() fire
> immediately and attempt to read STARFIVE_ISTAT while clocks are still gated off,
> potentially triggering a bus fault?

There should be no problem here. After devm_request_irq and before the clock is enabled,
the driver did not initialize the TRNG hardware, so it will not trigger an interrupt. The TRNG hardware is initialized in rng.init

> 
> Additionally, does starfive_trng_irq() need to check the PM state using
> pm_runtime_get_if_active() before accessing registers? If a spurious or delayed
> interrupt fires while the device is runtime suspended, the handler currently
> reads registers unconditionally.

This is a great suggestion. I have already added it to the code. Thank you for the reminder

> 
> [ ... ]
> >  	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);
> 
> Devres actions execute in reverse order of creation. Since
> devm_request_irq() is called before devm_add_action_or_reset(), won't
> starfive_trng_release() run first during device unbind and disable the clocks,
> leaving the IRQ active?
> 
> If an interrupt fires during this window, wouldn't it cause a bus fault when the
> handler reads from a clock-gated IP?

This issue should exist, and in future versions, I will turn off interrupts like Cleanup and then turn off the clock

> --
> Sashiko AI
> review · https://sashiko.dev/#/patchset/20260518065243.20865-1-lianfeng.ou
> yang@starfivetech.com?part=2