| Message ID | 20230822152553.190858-2-martin@kaiser.cx (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hwrng: imx-rngc - use polling instead of interrupt | expand |
Am Dienstag, 22. August 2023, 17:25:51 CEST schrieb Martin Kaiser: > Use polling to detect the end of the rngc self test. This is much simpler > than using an interrupt and a completion. > > Active waiting is no disadvantage here. The self test is run during > probe, there's nothing we could do in parallel at this time. If this driver is built-in you are stalling the boot process while polling, no? Unless probe_type = PROBE_PREFER_ASYNCHRONOUS is set of course. Best regards, Alexander > Signed-off-by: Martin Kaiser <martin@kaiser.cx> > --- > drivers/char/hw_random/imx-rngc.c | 15 +++++++-------- > 1 file changed, 7 insertions(+), 8 deletions(-) > > diff --git a/drivers/char/hw_random/imx-rngc.c > b/drivers/char/hw_random/imx-rngc.c index e4b385b01b11..85207535fd12 100644 > --- a/drivers/char/hw_random/imx-rngc.c > +++ b/drivers/char/hw_random/imx-rngc.c > @@ -17,6 +17,7 @@ > #include <linux/hw_random.h> > #include <linux/completion.h> > #include <linux/io.h> > +#include <linux/iopoll.h> > #include <linux/bitfield.h> > > #define RNGC_VER_ID 0x0000 > @@ -101,21 +102,19 @@ static inline void imx_rngc_irq_unmask(struct imx_rngc > *rngc) > > static int imx_rngc_self_test(struct imx_rngc *rngc) > { > - u32 cmd; > + u32 cmd, status; > int ret; > > - imx_rngc_irq_unmask(rngc); > - > /* run self test */ > cmd = readl(rngc->base + RNGC_COMMAND); > writel(cmd | RNGC_CMD_SELF_TEST, rngc->base + RNGC_COMMAND); > > - ret = wait_for_completion_timeout(&rngc->rng_op_done, > msecs_to_jiffies(RNGC_TIMEOUT)); - imx_rngc_irq_mask_clear(rngc); > - if (!ret) > - return -ETIMEDOUT; > + ret = readl_poll_timeout(rngc->base + RNGC_STATUS, status, > + status & RNGC_STATUS_ST_DONE, 1000, RNGC_TIMEOUT * 1000); > + if (ret < 0) > + return ret; > > - return rngc->err_reg ? -EIO : 0; > + return readl(rngc->base + RNGC_ERROR) ? -EIO : 0; > } > > static int imx_rngc_read(struct hwrng *rng, void *data, size_t max, bool > wait)
Hi Alexander, thanks for reviewing the patches. Alexander Stein (alexander.stein@ew.tq-group.com) wrote: > Am Dienstag, 22. August 2023, 17:25:51 CEST schrieb Martin Kaiser: > > Use polling to detect the end of the rngc self test. This is much simpler > > than using an interrupt and a completion. > > Active waiting is no disadvantage here. The self test is run during > > probe, there's nothing we could do in parallel at this time. > If this driver is built-in you are stalling the boot process while > polling, no? According to the manual, "The self test takes approximately 29,000 cycles to complete." On my system, the rngb peripheral clock runs at 66.5MHz, i.e. the self test takes 436us. A poll interval of 480us and a timeout of 1.5ms are more appropriate than the current values (1ms poll interval, 3sec timeout). I'll fix this in v2. If it is unacceptable for a probe function to run for 1.5ms, we could probably move the self test to the init function. It's called when the rng is selected as the active rng. > Unless probe_type = PROBE_PREFER_ASYNCHRONOUS is set of > course. This does not work for drivers that use module_platform_driver_probe. We could switch to module_platform_driver. I'd prefer fixing the timing or moving the self test to init, though. Best regards, Martin
diff --git a/drivers/char/hw_random/imx-rngc.c b/drivers/char/hw_random/imx-rngc.c index e4b385b01b11..85207535fd12 100644 --- a/drivers/char/hw_random/imx-rngc.c +++ b/drivers/char/hw_random/imx-rngc.c @@ -17,6 +17,7 @@ #include <linux/hw_random.h> #include <linux/completion.h> #include <linux/io.h> +#include <linux/iopoll.h> #include <linux/bitfield.h> #define RNGC_VER_ID 0x0000 @@ -101,21 +102,19 @@ static inline void imx_rngc_irq_unmask(struct imx_rngc *rngc) static int imx_rngc_self_test(struct imx_rngc *rngc) { - u32 cmd; + u32 cmd, status; int ret; - imx_rngc_irq_unmask(rngc); - /* run self test */ cmd = readl(rngc->base + RNGC_COMMAND); writel(cmd | RNGC_CMD_SELF_TEST, rngc->base + RNGC_COMMAND); - ret = wait_for_completion_timeout(&rngc->rng_op_done, msecs_to_jiffies(RNGC_TIMEOUT)); - imx_rngc_irq_mask_clear(rngc); - if (!ret) - return -ETIMEDOUT; + ret = readl_poll_timeout(rngc->base + RNGC_STATUS, status, + status & RNGC_STATUS_ST_DONE, 1000, RNGC_TIMEOUT * 1000); + if (ret < 0) + return ret; - return rngc->err_reg ? -EIO : 0; + return readl(rngc->base + RNGC_ERROR) ? -EIO : 0; } static int imx_rngc_read(struct hwrng *rng, void *data, size_t max, bool wait)
Use polling to detect the end of the rngc self test. This is much simpler than using an interrupt and a completion. Active waiting is no disadvantage here. The self test is run during probe, there's nothing we could do in parallel at this time. Signed-off-by: Martin Kaiser <martin@kaiser.cx> --- drivers/char/hw_random/imx-rngc.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)