[PATCH 0/3] hwrng: imx-rngc - simplify register definitions

Martin Kaiser posted 3 patches 2 years, 9 months ago
There is a newer version of this series
drivers/char/hw_random/imx-rngc.c | 32 +++++++++++++++----------------
1 file changed, 15 insertions(+), 17 deletions(-)
[PATCH 0/3] hwrng: imx-rngc - simplify register definitions
Posted by Martin Kaiser 2 years, 9 months ago
This series simplifies some definitions for register bits and fields. We can
then use the FIELD_GET macro to read register fields instead of masking and
shifting manually.

Tested on an imx258 board (rngb).

Martin Kaiser (3):
  hwrng: imx-rngc - use bitfield macros to read fifo level
  hwrng: imx-rngc - use bitfield macros to read rng type
  hwrng: imx-rngc - use BIT(x) for register bit defines

 drivers/char/hw_random/imx-rngc.c | 32 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 17 deletions(-)

-- 
2.30.2
[PATCH v2 0/3] hwrng: imx-rngc - simplify register definitions
Posted by Martin Kaiser 2 years, 8 months ago
This series simplifies some definitions for register bits and fields. We can
then use the FIELD_GET macro to read register fields instead of masking and
shifting manually.

Tested on an imx258 board (rngb).

v2:
 - simpler check if random bytes are available

Martin Kaiser (3):
  hwrng: imx-rngc - simpler check for available random bytes
  hwrng: imx-rngc - use bitfield macros to read rng type
  hwrng: imx-rngc - use BIT(x) for register bit defines

 drivers/char/hw_random/imx-rngc.c | 35 +++++++++++++------------------
 1 file changed, 15 insertions(+), 20 deletions(-)

-- 
2.30.2
Re: [PATCH v2 0/3] hwrng: imx-rngc - simplify register definitions
Posted by Herbert Xu 2 years, 8 months ago
On Fri, May 19, 2023 at 06:04:30PM +0200, Martin Kaiser wrote:
> This series simplifies some definitions for register bits and fields. We can
> then use the FIELD_GET macro to read register fields instead of masking and
> shifting manually.
> 
> Tested on an imx258 board (rngb).
> 
> v2:
>  - simpler check if random bytes are available
> 
> Martin Kaiser (3):
>   hwrng: imx-rngc - simpler check for available random bytes
>   hwrng: imx-rngc - use bitfield macros to read rng type
>   hwrng: imx-rngc - use BIT(x) for register bit defines
> 
>  drivers/char/hw_random/imx-rngc.c | 35 +++++++++++++------------------
>  1 file changed, 15 insertions(+), 20 deletions(-)
> 
> -- 
> 2.30.2

All applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
[PATCH v2 1/3] hwrng: imx-rngc - simpler check for available random bytes
Posted by Martin Kaiser 2 years, 8 months ago
The "level" field in the status register contains the number of random
bytes that are available in the FIFO.  Use GENMASK to extract this field.
We only want to check if level is 0 or if we can read another byte.
There's no need for the shift or the level variable.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
v2:
 - remove the shift when checking the fifo level field

 drivers/char/hw_random/imx-rngc.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/char/hw_random/imx-rngc.c b/drivers/char/hw_random/imx-rngc.c
index a1c24148ed31..cf29c323453a 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/bitfield.h>
 
 #define RNGC_VER_ID			0x0000
 #define RNGC_COMMAND			0x0004
@@ -44,8 +45,7 @@
 #define RNGC_CTRL_AUTO_SEED		0x00000010
 
 #define RNGC_STATUS_ERROR		0x00010000
-#define RNGC_STATUS_FIFO_LEVEL_MASK	0x00000f00
-#define RNGC_STATUS_FIFO_LEVEL_SHIFT	8
+#define RNGC_STATUS_FIFO_LEVEL_MASK	GENMASK(11, 8)
 #define RNGC_STATUS_SEED_DONE		0x00000020
 #define RNGC_STATUS_ST_DONE		0x00000010
 
@@ -122,7 +122,6 @@ static int imx_rngc_read(struct hwrng *rng, void *data, size_t max, bool wait)
 {
 	struct imx_rngc *rngc = container_of(rng, struct imx_rngc, rng);
 	unsigned int status;
-	unsigned int level;
 	int retval = 0;
 
 	while (max >= sizeof(u32)) {
@@ -132,11 +131,7 @@ static int imx_rngc_read(struct hwrng *rng, void *data, size_t max, bool wait)
 		if (status & RNGC_STATUS_ERROR)
 			break;
 
-		/* how many random numbers are in FIFO? [0-16] */
-		level = (status & RNGC_STATUS_FIFO_LEVEL_MASK) >>
-			RNGC_STATUS_FIFO_LEVEL_SHIFT;
-
-		if (level) {
+		if (status & RNGC_STATUS_FIFO_LEVEL_MASK) {
 			/* retrieve a random number from FIFO */
 			*(u32 *)data = readl(rngc->base + RNGC_FIFO);
 
-- 
2.30.2
[PATCH v2 2/3] hwrng: imx-rngc - use bitfield macros to read rng type
Posted by Martin Kaiser 2 years, 8 months ago
Use the mechanism from bitfield.h to read the rng type field in the
version_id register. This makes the code a tiny bit simpler.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/char/hw_random/imx-rngc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/char/hw_random/imx-rngc.c b/drivers/char/hw_random/imx-rngc.c
index cf29c323453a..b5f7b91bd13e 100644
--- a/drivers/char/hw_random/imx-rngc.c
+++ b/drivers/char/hw_random/imx-rngc.c
@@ -27,7 +27,7 @@
 #define RNGC_FIFO			0x0014
 
 /* the fields in the ver id register */
-#define RNGC_TYPE_SHIFT		28
+#define RNG_TYPE			GENMASK(31, 28)
 #define RNGC_VER_MAJ_SHIFT		8
 
 /* the rng_type field */
@@ -251,7 +251,7 @@ static int imx_rngc_probe(struct platform_device *pdev)
 		return irq;
 
 	ver_id = readl(rngc->base + RNGC_VER_ID);
-	rng_type = ver_id >> RNGC_TYPE_SHIFT;
+	rng_type = FIELD_GET(RNG_TYPE, ver_id);
 	/*
 	 * This driver supports only RNGC and RNGB. (There's a different
 	 * driver for RNGA.)
-- 
2.30.2
[PATCH v2 3/3] hwrng: imx-rngc - use BIT(x) for register bit defines
Posted by Martin Kaiser 2 years, 8 months ago
Rewrite the defines for register bits to use BIT(x) instead of writing
out the 32-bit number. This makes it easier to compare the code with the
register descriptions in the reference manual.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/char/hw_random/imx-rngc.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/char/hw_random/imx-rngc.c b/drivers/char/hw_random/imx-rngc.c
index b5f7b91bd13e..9c6988c658e2 100644
--- a/drivers/char/hw_random/imx-rngc.c
+++ b/drivers/char/hw_random/imx-rngc.c
@@ -35,19 +35,19 @@
 #define RNGC_TYPE_RNGC			0x2
 
 
-#define RNGC_CMD_CLR_ERR		0x00000020
-#define RNGC_CMD_CLR_INT		0x00000010
-#define RNGC_CMD_SEED			0x00000002
-#define RNGC_CMD_SELF_TEST		0x00000001
+#define RNGC_CMD_CLR_ERR		BIT(5)
+#define RNGC_CMD_CLR_INT		BIT(4)
+#define RNGC_CMD_SEED			BIT(1)
+#define RNGC_CMD_SELF_TEST		BIT(0)
 
-#define RNGC_CTRL_MASK_ERROR		0x00000040
-#define RNGC_CTRL_MASK_DONE		0x00000020
-#define RNGC_CTRL_AUTO_SEED		0x00000010
+#define RNGC_CTRL_MASK_ERROR		BIT(6)
+#define RNGC_CTRL_MASK_DONE		BIT(5)
+#define RNGC_CTRL_AUTO_SEED		BIT(4)
 
-#define RNGC_STATUS_ERROR		0x00010000
+#define RNGC_STATUS_ERROR		BIT(16)
 #define RNGC_STATUS_FIFO_LEVEL_MASK	GENMASK(11, 8)
-#define RNGC_STATUS_SEED_DONE		0x00000020
-#define RNGC_STATUS_ST_DONE		0x00000010
+#define RNGC_STATUS_SEED_DONE		BIT(5)
+#define RNGC_STATUS_ST_DONE		BIT(4)
 
 #define RNGC_ERROR_STATUS_STAT_ERR	0x00000008
 
-- 
2.30.2