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

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

[Martin Kaiser]

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 1/3] hwrng: imx-rngc - use bitfield macros to read fifo level

[Martin Kaiser]

Use the mechanism from bitfield.h to read the fifo level field in the
status register. This makes the code a tiny bit simpler.

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

diff --git a/drivers/char/hw_random/imx-rngc.c b/drivers/char/hw_random/imx-rngc.c
index a1c24148ed31..861a615ac7df 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		GENMASK(11, 8)
 #define RNGC_STATUS_SEED_DONE		0x00000020
 #define RNGC_STATUS_ST_DONE		0x00000010
 
@@ -133,9 +133,7 @@ static int imx_rngc_read(struct hwrng *rng, void *data, size_t max, bool wait)
 			break;
 
 		/* how many random numbers are in FIFO? [0-16] */
-		level = (status & RNGC_STATUS_FIFO_LEVEL_MASK) >>
-			RNGC_STATUS_FIFO_LEVEL_SHIFT;
-
+		level = FIELD_GET(RNGC_STATUS_FIFO_LEVEL, status);
 		if (level) {
 			/* retrieve a random number from FIFO */
 			*(u32 *)data = readl(rngc->base + RNGC_FIFO);
-- 
2.30.2

[PATCH 2/3] hwrng: imx-rngc - use bitfield macros to read rng type

[Martin Kaiser]

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 861a615ac7df..546dc3655e5b 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 */
@@ -254,7 +254,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 3/3] hwrng: imx-rngc - use BIT(x) for register bit defines

[Martin Kaiser]

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 546dc3655e5b..6872d1c0060b 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		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

Re: [PATCH 1/3] hwrng: imx-rngc - use bitfield macros to read fifo level

[Herbert Xu]

On Thu, Apr 27, 2023 at 08:53:55PM +0200, Martin Kaiser wrote:
>
> @@ -133,9 +133,7 @@ static int imx_rngc_read(struct hwrng *rng, void *data, size_t max, bool wait)
>  			break;
>  
>  		/* how many random numbers are in FIFO? [0-16] */
> -		level = (status & RNGC_STATUS_FIFO_LEVEL_MASK) >>
> -			RNGC_STATUS_FIFO_LEVEL_SHIFT;
> -
> +		level = FIELD_GET(RNGC_STATUS_FIFO_LEVEL, status);

Wouldn't it be simpler to just get rid of the shift?

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

Re: [PATCH 1/3] hwrng: imx-rngc - use bitfield macros to read fifo level

[Martin Kaiser]

Thus wrote Herbert Xu (herbert@gondor.apana.org.au):

> On Thu, Apr 27, 2023 at 08:53:55PM +0200, Martin Kaiser wrote:

> > @@ -133,9 +133,7 @@ static int imx_rngc_read(struct hwrng *rng, void *data, size_t max, bool wait)
> >  			break;

> >  		/* how many random numbers are in FIFO? [0-16] */
> > -		level = (status & RNGC_STATUS_FIFO_LEVEL_MASK) >>
> > -			RNGC_STATUS_FIFO_LEVEL_SHIFT;
> > -
> > +		level = FIELD_GET(RNGC_STATUS_FIFO_LEVEL, status);

> Wouldn't it be simpler to just get rid of the shift?

Yes, we're only checking that level > 0, there's no need for the shift.
I'll submit a new version of the patch.

Thanks,
Martin

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

[Martin Kaiser]

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

[PATCH v2 1/3] hwrng: imx-rngc - simpler check for available random bytes

[Martin Kaiser]

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

[Martin Kaiser]

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

[Martin Kaiser]

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

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

[Herbert Xu]

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