Messages in this thread | | | From | JiaJie Ho <> | Subject | RE: [PATCH v3 2/3] hwrng: starfive - Add TRNG driver for StarFive SoC | Date | Tue, 10 Jan 2023 07:45:17 +0000 |
| |
> -----Original Message----- > From: Herbert Xu <herbert@gondor.apana.org.au> > Sent: 10 January, 2023 3:41 PM > To: JiaJie Ho <jiajie.ho@starfivetech.com> > Cc: Olivia Mackall <olivia@selenic.com>; Rob Herring <robh+dt@kernel.org>; > Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Emil Renner > Berthing <kernel@esmil.dk>; Conor Dooley <conor.dooley@microchip.com>; > linux-crypto@vger.kernel.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; linux-riscv@lists.infradead.org > Subject: Re: [PATCH v3 2/3] hwrng: starfive - Add TRNG driver for StarFive > SoC > > On Tue, Jan 10, 2023 at 12:52:48AM +0800, Jia Jie Ho wrote: > > > > +static irqreturn_t starfive_trng_irq(int irq, void *priv) { > > + u32 status; > > + struct starfive_trng *trng = (struct starfive_trng *)priv; > > + > > + status = readl(trng->base + STARFIVE_ISTAT); > > + 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) { > > + 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*/ > > + writel(STARFIVE_CTRL_EXEC_RANDRESEED, trng->base + > STARFIVE_CTRL); > > This could occur at the same time as a GENE_RANDNUM write so perhaps > you should add some locking? >
I'll add those in the next version.
> > + ret = devm_request_irq(&pdev->dev, irq, starfive_trng_irq, 0, pdev- > >name, > > + (void *)trng); > > + if (ret) > > + return dev_err_probe(&pdev->dev, irq, > > + "Failed to register interrupt handler\n"); > > ... > > > + init_completion(&trng->random_done); > > + init_completion(&trng->reseed_done); > > These completion initialisations should be moved above the IRQ registration > because you should always be prepared to get spurious interrupts. >
Will update these too.
> Otherwise the patch looks good. >
Thanks again for reviewing the patch and providing the useful feedbacks.
Best regards, Jia Jie
| |