* [PATCH v1 0/2] Add trng driver to JHB100 @ 2026-05-12 6:24 lianfeng.ouyang 2026-05-12 6:24 ` [PATCH v1 1/2] dt-bindings: Add bindings for StarFive JHB100 SoC trng controller lianfeng.ouyang 2026-05-12 6:24 ` [PATCH v1 2/2] hwrng: starfive: Update clk and reset sequence lianfeng.ouyang 0 siblings, 2 replies; 7+ messages in thread From: lianfeng.ouyang @ 2026-05-12 6:24 UTC (permalink / raw) To: Olivia Mackall, Herbert Xu, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel Cc: linux-crypto, devicetree, linux-kernel 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. 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 | 2 +- MAINTAINERS | 2 +- drivers/char/hw_random/jh7110-trng.c | 18 ++++++++++++++++-- 3 files changed, 18 insertions(+), 4 deletions(-) -- 2.43.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v1 1/2] dt-bindings: Add bindings for StarFive JHB100 SoC trng controller. 2026-05-12 6:24 [PATCH v1 0/2] Add trng driver to JHB100 lianfeng.ouyang @ 2026-05-12 6:24 ` lianfeng.ouyang 2026-05-12 17:15 ` Conor Dooley 2026-05-13 5:17 ` sashiko-bot 2026-05-12 6:24 ` [PATCH v1 2/2] hwrng: starfive: Update clk and reset sequence lianfeng.ouyang 1 sibling, 2 replies; 7+ messages in thread From: lianfeng.ouyang @ 2026-05-12 6:24 UTC (permalink / raw) To: Olivia Mackall, Herbert Xu, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel Cc: linux-crypto, devicetree, linux-kernel From: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com> Signed-off-by: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com> --- Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml b/Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml index 4639247e9e51..11346d77b2f6 100644 --- a/Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml +++ b/Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml @@ -13,8 +13,8 @@ properties: compatible: oneOf: - items: - - const: starfive,jh8100-trng - const: starfive,jh7110-trng + - const: starfive,jhb100-trng - const: starfive,jh7110-trng reg: -- 2.43.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v1 1/2] dt-bindings: Add bindings for StarFive JHB100 SoC trng controller. 2026-05-12 6:24 ` [PATCH v1 1/2] dt-bindings: Add bindings for StarFive JHB100 SoC trng controller lianfeng.ouyang @ 2026-05-12 17:15 ` Conor Dooley 2026-05-12 19:35 ` Conor Dooley 2026-05-13 5:17 ` sashiko-bot 1 sibling, 1 reply; 7+ messages in thread From: Conor Dooley @ 2026-05-12 17:15 UTC (permalink / raw) To: lianfeng.ouyang Cc: Olivia Mackall, Herbert Xu, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, linux-crypto, devicetree, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1083 bytes --] On Tue, May 12, 2026 at 02:24:03PM +0800, lianfeng.ouyang wrote: > From: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com> > > Signed-off-by: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com> > --- > Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml b/Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml > index 4639247e9e51..11346d77b2f6 100644 > --- a/Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml > +++ b/Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml > @@ -13,8 +13,8 @@ properties: > compatible: > oneOf: > - items: > - - const: starfive,jh8100-trng > - const: starfive,jh7110-trng > + - const: starfive,jhb100-trng You need to add a commit message here explaining why removing the jh8100 is okay. pw-bot: changes-requested > - const: starfive,jh7110-trng > > reg: > -- > 2.43.0 > > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 1/2] dt-bindings: Add bindings for StarFive JHB100 SoC trng controller. 2026-05-12 17:15 ` Conor Dooley @ 2026-05-12 19:35 ` Conor Dooley 0 siblings, 0 replies; 7+ messages in thread From: Conor Dooley @ 2026-05-12 19:35 UTC (permalink / raw) To: lianfeng.ouyang Cc: Olivia Mackall, Herbert Xu, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, linux-crypto, devicetree, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1315 bytes --] On Tue, May 12, 2026 at 06:15:07PM +0100, Conor Dooley wrote: > On Tue, May 12, 2026 at 02:24:03PM +0800, lianfeng.ouyang wrote: > > From: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com> > > > > Signed-off-by: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com> > > --- > > Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml b/Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml > > index 4639247e9e51..11346d77b2f6 100644 > > --- a/Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml > > +++ b/Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml > > @@ -13,8 +13,8 @@ properties: > > compatible: > > oneOf: > > - items: > > - - const: starfive,jh8100-trng > > - const: starfive,jh7110-trng > > + - const: starfive,jhb100-trng > > You need to add a commit message here explaining why removing the jh8100 > is okay. > pw-bot: changes-requested Additionally, given the driver changes, it looks like using a jh7110 fallback is invalid anyway. > > > - const: starfive,jh7110-trng > > > > reg: > > -- > > 2.43.0 > > > > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 1/2] dt-bindings: Add bindings for StarFive JHB100 SoC trng controller. 2026-05-12 6:24 ` [PATCH v1 1/2] dt-bindings: Add bindings for StarFive JHB100 SoC trng controller lianfeng.ouyang 2026-05-12 17:15 ` Conor Dooley @ 2026-05-13 5:17 ` sashiko-bot 1 sibling, 0 replies; 7+ messages in thread From: sashiko-bot @ 2026-05-13 5:17 UTC (permalink / raw) To: lianfeng.ouyang; +Cc: conor+dt, devicetree, robh, krzk+dt Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Critical] The commit silently removes the `starfive,jh8100-trng` compatible string, breaking backward ABI compatibility. - [High] The schema enforces an invalid device tree compatible array order (generic fallback before specific device) for the new JHB100 SoC. -- commit eafedaf1268d7bf5cd0934352607dcf457417047 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 TRNG controller on the StarFive JHB100 SoC. > diff --git a/Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml b/Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml > index 4639247e9e510..11346d77b2f6d 100644 > --- a/Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml > +++ b/Documentation/devicetree/bindings/rng/starfive,jh7110-trng.yaml > @@ -13,8 +13,8 @@ properties: > compatible: > oneOf: > - items: > - - const: starfive,jh8100-trng Does this break backward ABI compatibility? Device tree bindings are considered kernel ABI, and removing this line drops support for any existing device trees that use the starfive,jh8100-trng compatible string. The commit message doesn't document or justify the removal of the JH8100 support. > - const: starfive,jh7110-trng > + - const: starfive,jhb100-trng Does this enforce an invalid device tree compatible array order? In YAML binding schemas, the items list enforces the exact sequence required in the device tree source. By placing starfive,jh7110-trng before starfive,jhb100-trng, this sequence puts the generic fallback before the specific hardware variant. This violates the device tree specification which mandates that the most specific hardware match must come first. To support both variants falling back to jh7110, would it be better to use an enum for the first item containing both specific strings, followed by the fallback constant? > - const: starfive,jh7110-trng -- Sashiko AI review · https://sashiko.dev/#/patchset/20260512062404.4540-1-lianfeng.ouyang@starfivetech.com?part=1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v1 2/2] hwrng: starfive: Update clk and reset sequence 2026-05-12 6:24 [PATCH v1 0/2] Add trng driver to JHB100 lianfeng.ouyang 2026-05-12 6:24 ` [PATCH v1 1/2] dt-bindings: Add bindings for StarFive JHB100 SoC trng controller lianfeng.ouyang @ 2026-05-12 6:24 ` lianfeng.ouyang 2026-05-13 5:56 ` sashiko-bot 1 sibling, 1 reply; 7+ messages in thread From: lianfeng.ouyang @ 2026-05-12 6:24 UTC (permalink / raw) To: Olivia Mackall, Herbert Xu, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel Cc: linux-crypto, devicetree, linux-kernel 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 | 18 ++++++++++++++++-- 2 files changed, 17 insertions(+), 3 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..f5eb434c94f5 100644 --- a/drivers/char/hw_random/jh7110-trng.c +++ b/drivers/char/hw_random/jh7110-trng.c @@ -234,12 +234,18 @@ static irqreturn_t starfive_trng_irq(int irq, void *priv) static void starfive_trng_cleanup(struct hwrng *rng) { struct starfive_trng *trng = to_trng(rng); + bool is_jhb100 = device_is_compatible(trng->dev, "starfive,jhb100-trng"); writel(0, trng->base + STARFIVE_CTRL); - reset_control_assert(trng->rst); + if (!is_jhb100) + reset_control_assert(trng->rst); + clk_disable_unprepare(trng->hclk); clk_disable_unprepare(trng->ahb); + + if (is_jhb100) + reset_control_assert(trng->rst); } static int starfive_trng_read(struct hwrng *rng, void *buf, size_t max, bool wait) @@ -337,12 +343,19 @@ static int starfive_trng_probe(struct platform_device *pdev) ret = devm_hwrng_register(&pdev->dev, &trng->rng); if (ret) { + bool is_jhb100 = device_is_compatible(trng->dev, "starfive,jhb100-trng"); + pm_runtime_disable(&pdev->dev); - reset_control_assert(trng->rst); + if (!is_jhb100) + reset_control_assert(trng->rst); + clk_disable_unprepare(trng->ahb); clk_disable_unprepare(trng->hclk); + if (is_jhb100) + reset_control_assert(trng->rst); + return dev_err_probe(&pdev->dev, ret, "Failed to register hwrng\n"); } @@ -378,6 +391,7 @@ 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,jhb100-trng" }, { } }; MODULE_DEVICE_TABLE(of, trng_dt_ids); -- 2.43.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v1 2/2] hwrng: starfive: Update clk and reset sequence 2026-05-12 6:24 ` [PATCH v1 2/2] hwrng: starfive: Update clk and reset sequence lianfeng.ouyang @ 2026-05-13 5:56 ` sashiko-bot 0 siblings, 0 replies; 7+ messages in thread From: sashiko-bot @ 2026-05-13 5:56 UTC (permalink / raw) To: lianfeng.ouyang; +Cc: robh, krzk+dt, devicetree, conor+dt Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [High] The patch fails to guarantee that clocks are physically gated before asserting reset in cleanup(), rendering the glitch prevention ineffective when the device is active. - [High] Misuse of hwrng cleanup and init callbacks leads to system crashes on RNG toggle. - [High] Unsafe IRQ handler execution after clocks are gated in cleanup() causes bus faults. - [High] Broken Runtime PM state initialization causes permanent clock leakage. -- commit 097b8dfdf62a7320bf5e3edd8a668729f1364217 Author: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com> hwrng: starfive: Update clk and reset sequence For jhb100, this conducts clock gating before asserting reset to prevent generating glitches that propagate to downstream IP. > 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 > @@ -234,12 +234,18 @@ static irqreturn_t starfive_trng_irq(int irq, void *priv) > static void starfive_trng_cleanup(struct hwrng *rng) > { > struct starfive_trng *trng = to_trng(rng); > + bool is_jhb100 = device_is_compatible(trng->dev, "starfive,jhb100-trng"); > > writel(0, trng->base + STARFIVE_CTRL); > > - reset_control_assert(trng->rst); > + if (!is_jhb100) > + reset_control_assert(trng->rst); > + > clk_disable_unprepare(trng->hclk); > clk_disable_unprepare(trng->ahb); > + > + if (is_jhb100) > + reset_control_assert(trng->rst); > } Does this sequence guarantee the clocks are physically gated before asserting the reset? Because the probe function manually enables the clocks without calling pm_runtime_set_active(), the physical clock count starts at 1. If the RNG is deselected while the device is still active (for example, within the 100ms autosuspend window after a read), the clk_disable_unprepare() calls here will only decrement the clock count to 1. Could the physical clocks remain running when reset_control_assert() is executed, generating the exact downstream glitch this patch intends to prevent? The driver may need to synchronize with Runtime PM first. This isn't a bug introduced by this patch, but does managing hardware states this way in cleanup() lead to a crash if the RNG is re-selected? The hwrng core invokes cleanup() when the RNG is deselected via sysfs, which drops the clock count. If the user subsequently re-selects the RNG, the core invokes starfive_trng_init(). Since init() does not re-enable the clocks or call pm_runtime_get_sync(), wouldn't its immediate writel() to the STARFIVE_AUTO_AGE register trigger a synchronous external abort? Also pre-existing, but does gating the clocks here leave the device vulnerable to a bus fault? The interrupt handler remains fully active because cleanup() can be executed at runtime via sysfs without unbinding the driver. If a spurious interrupt or delayed lockup interrupt fires while the RNG is deselected, wouldn't starfive_trng_irq() attempt to read registers with the clocks gated? Should the driver clear the interrupt enable register and synchronize IRQs before gating the clocks? [ ... ] > @@ -337,12 +343,19 @@ static int starfive_trng_probe(struct platform_device *pdev) > > ret = devm_hwrng_register(&pdev->dev, &trng->rng); > if (ret) { > + bool is_jhb100 = device_is_compatible(trng->dev, "starfive,jhb100-trng"); > + > pm_runtime_disable(&pdev->dev); > > - reset_control_assert(trng->rst); > + if (!is_jhb100) > + reset_control_assert(trng->rst); > + > clk_disable_unprepare(trng->ahb); > clk_disable_unprepare(trng->hclk); > > + if (is_jhb100) > + reset_control_assert(trng->rst); > + > return dev_err_probe(&pdev->dev, ret, "Failed to register hwrng\n"); > } This isn't a bug in the new error handling, but earlier in the probe sequence, the driver manually invokes clk_prepare_enable() for both clocks, initializing their enable counts to 1, but fails to call pm_runtime_set_active(). Doesn't the PM core therefore default the runtime status to RPM_SUSPENDED? Every time starfive_trng_read() executes, pm_runtime_get_sync() transitions the device to RPM_ACTIVE, calling starfive_trng_resume() which increments the clock counts to 2. When the autosuspend timer invokes starfive_trng_suspend(), it decrements the counts back to 1. Does this mean the clocks are never physically turned off during normal operation, completely preventing the hardware from powering down? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260512062404.4540-1-lianfeng.ouyang@starfivetech.com?part=2 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-05-13 5:56 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-05-12 6:24 [PATCH v1 0/2] Add trng driver to JHB100 lianfeng.ouyang 2026-05-12 6:24 ` [PATCH v1 1/2] dt-bindings: Add bindings for StarFive JHB100 SoC trng controller lianfeng.ouyang 2026-05-12 17:15 ` Conor Dooley 2026-05-12 19:35 ` Conor Dooley 2026-05-13 5:17 ` sashiko-bot 2026-05-12 6:24 ` [PATCH v1 2/2] hwrng: starfive: Update clk and reset sequence lianfeng.ouyang 2026-05-13 5:56 ` sashiko-bot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox