> > +STARFIVE TRNG DRIVER > > +M: Jia Jie Ho <jiajie.ho@xxxxxxxxxxxxxxxx> > > +S: Maintained > > +F: Documentation/devicetree/bindings/rng/starfive* > > +F: drivers/char/hw_random/starfive-trng.c > > minor nit (so don't submit another version just to fix this): > This should be Supported, no? > Hi Conor, I'll update this in next version together with other comments below. > > diff --git a/drivers/char/hw_random/Makefile > > b/drivers/char/hw_random/Makefile index 3e948cf04476..f68ac370847f > > 100644 > > --- a/drivers/char/hw_random/Makefile > > +++ b/drivers/char/hw_random/Makefile > > @@ -47,3 +47,4 @@ obj-$(CONFIG_HW_RANDOM_XIPHERA) += xiphera- > trng.o > > obj-$(CONFIG_HW_RANDOM_ARM_SMCCC_TRNG) += arm_smccc_trng.o > > obj-$(CONFIG_HW_RANDOM_CN10K) += cn10k-rng.o > > obj-$(CONFIG_HW_RANDOM_POLARFIRE_SOC) += mpfs-rng.o > > +obj-$(CONFIG_HW_RANDOM_STARFIVE) += starfive-trng.o > > Is "STARFIVE" a bit too general of a name here and in the Kconfig entry? > I don't have a TRM for the JH7100, but this name (and the Kconfig text) > would give me the impression that I can use it there too. > Does this driver support both? > 7100 uses a different trng module but this same generator might be used in future products, so I left it generic. Would it be better to specify the product? > > +static int starfive_trng_probe(struct platform_device *pdev) [...] > > + ret = devm_hwrng_register(&pdev->dev, &trng->rng); > > + if (ret) { > > + dev_err_probe(&pdev->dev, ret, "Failed to register > hwrng\n"); > > + goto err_fail_register; > > + } > > + > > + pm_runtime_use_autosuspend(&pdev->dev); > > + pm_runtime_set_autosuspend_delay(&pdev->dev, 100); > > > + pm_runtime_enable(&pdev->dev); > > > + > > + return 0; > > + > > +err_fail_register: > > > + pm_runtime_disable(&pdev->dev); > > This was only enabled after the only goto for this label, does it serve a > purpose? > I know little about runtime PM, it just caught my eye. > I looked at the other rng drivers that had calls to pm_runtime_enable(), but > they all seem to do their pm enablement _before_ calling hwrng_register(). > Again, I am not familiar with runtime PM, but curious why you are doing > things differently, that's all. It does make more sense to move pm_runtime_enable before registering the generator to align with codes in the goto label. I'll fix this part. Thanks again for reviewing the patches. Best regards, Jia Jie