[PATCH v3 1/2] dt-bindings: rng: timeriomem_rng: add width and mask properties

[PATCH v3 1/2] dt-bindings: rng: timeriomem_rng: add width and mask properties

[Jad Keskes]

Add optional width (8, 16, 32) and mask properties to the binding.
The width selects the bus access size for reads. The mask is ANDed
with the raw register value to allow only the entropy-bearing bits
through.

Update the example to show a typical 8-bit configuration.
Update SPDX to dual license to match kernel convention.
Drop the misleading '32-bit aligned' constraint from the reg
description since alignment now depends on the configured width.

Signed-off-by: Jad Keskes <inasj268@gmail.com>
---
 .../bindings/rng/timeriomem_rng.yaml          | 48 +++++++++++++++----
 1 file changed, 40 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/rng/timeriomem_rng.yaml b/Documentation/devicetree/bindings/rng/timeriomem_rng.yaml
index 4754174e9849..636305f211c8 100644
--- a/Documentation/devicetree/bindings/rng/timeriomem_rng.yaml
+++ b/Documentation/devicetree/bindings/rng/timeriomem_rng.yaml
@@ -1,10 +1,16 @@
-# SPDX-License-Identifier: GPL-2.0-only
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
 %YAML 1.2
 ---
 $id: http://devicetree.org/schemas/rng/timeriomem_rng.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: TimerIO Random Number Generator
+title: Timer IOMEM Hardware Random Number Generator
+
+description: |
+  This binding covers platforms that have a single IO memory address which
+  provides periodic random data.  The driver reads from the address at a
+  fixed interval, returning a configurable-width value masked to the desired
+  bits.
 
 maintainers:
   - Krzysztof Kozlowski <krzk@kernel.org>
@@ -13,9 +19,17 @@ properties:
   compatible:
     const: timeriomem_rng
 
+  reg:
+    maxItems: 1
+    description:
+      Base address to sample from.  Must be aligned to the configured access
+      width (1, 2, or 4 bytes) and at least that wide.
+
   period:
     $ref: /schemas/types.yaml#/definitions/uint32
-    description: wait time in microseconds to use between samples
+    description:
+      Interval in microseconds between reads.  New random data is expected to
+      be available at this rate.
 
   quality:
     $ref: /schemas/types.yaml#/definitions/uint32
@@ -26,16 +40,26 @@ properties:
       instead.  Note that the default quality is usually zero which disables
       using this rng to automatically fill the kernel's entropy pool.
 
-  reg:
-    maxItems: 1
+  width:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    default: 32
+    enum: [8, 16, 32]
     description:
-      Base address to sample from. Currently 'reg' must be at least four bytes
-      wide and 32-bit aligned.
+      Access width in bits.  Determines whether the read is performed as
+      an 8-bit, 16-bit, or 32-bit bus access.
+
+  mask:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    default: 0xFFFFFFFF
+    description:
+      Mask applied to the value read from the register.  Bits set to 0 in
+      the mask are cleared in the output data.  Default (no mask) passes
+      all bits through.
 
 required:
   - compatible
-  - period
   - reg
+  - period
 
 additionalProperties: false
 
@@ -46,3 +70,11 @@ examples:
         reg = <0x44 0x04>;
         period = <1000000>;
     };
+
+    rng@64 {
+        compatible = "timeriomem_rng";
+        reg = <0x64 0x01>;
+        period = <50000>;
+        width = <8>;
+        mask = <0xFF>;
+    };
-- 
2.54.0

[PATCH v3 2/2] hw_random: timeriomem-rng: add configurable read width and data mask

[Jad Keskes]

The TODO for supporting read sizes other than 32 bits and masking has
been sitting in this driver since 2009.  Implement it.

Add width (8, 16, or 32 bits) and mask properties to the platform data
and device tree bindings.  The read loop dispatches on width using
readb/readw/readl so a configured 8-bit access doesn't trigger a bus
error on hardware that rejects 32-bit reads to that address.  The mask
is ANDed with the value before storing.

These are platform properties, not runtime policy -- width depends on
SoC integration, mask reflects which output bits carry entropy.

The alignment check in probe is updated to verify the resource is
aligned to the configured width instead of hardcoding 4-byte alignment.

Signed-off-by: Jad Keskes <inasj268@gmail.com>
---
 drivers/char/hw_random/timeriomem-rng.c | 78 ++++++++++++++++++++-----
 include/linux/timeriomem-rng.h          | 12 ++++
 2 files changed, 77 insertions(+), 13 deletions(-)

diff --git a/drivers/char/hw_random/timeriomem-rng.c b/drivers/char/hw_random/timeriomem-rng.c
index e61f06393209..4557326618c9 100644
--- a/drivers/char/hw_random/timeriomem-rng.c
+++ b/drivers/char/hw_random/timeriomem-rng.c
@@ -14,7 +14,9 @@
  *   has to do is provide the address and 'wait time' that new data becomes
  *   available.
  *
- * TODO: add support for reading sizes other than 32bits and masking
+ * The read width (8, 16, or 32 bits) and an optional data mask can be
+ * configured through platform data or device tree properties.  Default is
+ * 32-bit reads with no mask.
  */
 
 #include <linux/completion.h>
@@ -34,6 +36,8 @@ struct timeriomem_rng_private {
 	void __iomem		*io_base;
 	ktime_t			period;
 	unsigned int		present:1;
+	unsigned int		width;
+	u32			mask;
 
 	struct hrtimer		timer;
 	struct completion	completion;
@@ -48,6 +52,7 @@ static int timeriomem_rng_read(struct hwrng *hwrng, void *data,
 		container_of(hwrng, struct timeriomem_rng_private, rng_ops);
 	int retval = 0;
 	int period_us = ktime_to_us(priv->period);
+	int chunk = priv->width / 8;
 
 	/*
 	 * There may not have been enough time for new data to be generated
@@ -71,11 +76,28 @@ static int timeriomem_rng_read(struct hwrng *hwrng, void *data,
 			usleep_range(period_us,
 					period_us + max(1, period_us / 100));
 
-		*(u32 *)data = readl(priv->io_base);
-		retval += sizeof(u32);
-		data += sizeof(u32);
-		max -= sizeof(u32);
-	} while (wait && max > sizeof(u32));
+		switch (priv->width) {
+		case 8: {
+			u8 val = readb(priv->io_base) & priv->mask;
+			*(u8 *)data = val;
+			break;
+		}
+		case 16: {
+			u16 val = readw(priv->io_base) & priv->mask;
+			*(u16 *)data = val;
+			break;
+		}
+		case 32: {
+			u32 val = readl(priv->io_base) & priv->mask;
+			*(u32 *)data = val;
+			break;
+		}
+		}
+
+		retval += chunk;
+		data += chunk;
+		max -= chunk;
+	} while (wait && max > chunk);
 
 	/*
 	 * Block any new callers until the RNG has had time to generate new
@@ -125,11 +147,8 @@ static int timeriomem_rng_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->io_base))
 		return PTR_ERR(priv->io_base);
 
-	if (res->start % 4 != 0 || resource_size(res) < 4) {
-		dev_err(&pdev->dev,
-			"address must be at least four bytes wide and 32-bit aligned\n");
-		return -EINVAL;
-	}
+	priv->width = 32;
+	priv->mask = 0xFFFFFFFF;
 
 	if (pdev->dev.of_node) {
 		int i;
@@ -145,9 +164,42 @@ static int timeriomem_rng_probe(struct platform_device *pdev)
 		if (!of_property_read_u32(pdev->dev.of_node,
 						"quality", &i))
 			priv->rng_ops.quality = i;
+
+		of_property_read_u32(pdev->dev.of_node,
+				     "width", &priv->width);
+		of_property_read_u32(pdev->dev.of_node,
+				     "mask", &priv->mask);
 	} else {
 		period = pdata->period;
 		priv->rng_ops.quality = pdata->quality;
+
+		if (pdata->width_set)
+			priv->width = pdata->width;
+		if (pdata->mask_set)
+			priv->mask = pdata->mask;
+	}
+
+	if (priv->width == 0)
+		priv->width = 32;
+
+	switch (priv->width) {
+	case 8:
+	case 16:
+	case 32:
+		break;
+	default:
+		dev_err(&pdev->dev, "invalid width %u, must be 8, 16, or 32\n",
+			priv->width);
+		return -EINVAL;
+	}
+
+	if (!IS_ALIGNED(res->start, priv->width / 8) ||
+	    resource_size(res) < priv->width / 8) {
+		dev_err(&pdev->dev,
+			"address must be at least %u-bit aligned (%u byte%s)\n",
+			priv->width, priv->width / 8,
+			priv->width / 8 > 1 ? "s" : "");
+		return -EINVAL;
 	}
 
 	priv->period = us_to_ktime(period);
@@ -167,8 +219,8 @@ static int timeriomem_rng_probe(struct platform_device *pdev)
 		return err;
 	}
 
-	dev_info(&pdev->dev, "32bits from 0x%p @ %dus\n",
-			priv->io_base, period);
+	dev_info(&pdev->dev, "%ubit from %p @ %dus\n",
+		 priv->width, priv->io_base, period);
 
 	return 0;
 }
diff --git a/include/linux/timeriomem-rng.h b/include/linux/timeriomem-rng.h
index 672df7fbf6c1..b4202ad2f507 100644
--- a/include/linux/timeriomem-rng.h
+++ b/include/linux/timeriomem-rng.h
@@ -16,6 +16,18 @@ struct timeriomem_rng_data {
 
 	/* bits of entropy per 1024 bits read */
 	unsigned int		quality;
+
+	/* read width (8, 16, or 32), 0 means 32 */
+	unsigned int		width;
+
+	/* set to true if width is explicitly provided */
+	bool			width_set;
+
+	/* mask applied to raw read value */
+	u32			mask;
+
+	/* set to true if mask is explicitly provided */
+	bool			mask_set;
 };
 
 #endif /* _LINUX_TIMERIOMEM_RNG_H */
-- 
2.54.0

Re: [PATCH v3 2/2] hw_random: timeriomem-rng: add configurable read width and data mask

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Use-After-Free / Deadlock in the device removal path due to incorrect devres ordering.
--

commit 4300c70f186763a1c545b060ff78e036f3434d38
Author: Jad Keskes <inasj268@gmail.com>

    hw_random: timeriomem-rng: add configurable read width and data mask

This commit implements configurable read widths (8, 16, or 32 bits) and data
masking for the timeriomem-rng driver. It updates the read loop to dispatch
based on width to avoid bus errors, and uses properties from platform data or
device tree.

> diff --git a/drivers/char/hw_random/timeriomem-rng.c b/drivers/char/hw_random/timeriomem-rng.c
> index e61f063932090..4557326618c90 100644
> --- a/drivers/char/hw_random/timeriomem-rng.c
> +++ b/drivers/char/hw_random/timeriomem-rng.c
[ ... ]
> @@ -167,8 +219,8 @@ static int timeriomem_rng_probe(struct platform_device *pdev)
>  		return err;
>  	}
>  
> -	dev_info(&pdev->dev, "32bits from 0x%p @ %dus\n",
> -			priv->io_base, period);
> +	dev_info(&pdev->dev, "%ubit from %p @ %dus\n",
> +		 priv->width, priv->io_base, period);
>  
>  	return 0;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but looking at the probe function
above, it uses devm_hwrng_register() to register the RNG device. This means
that hwrng_unregister() is automatically called by devres after
timeriomem_rng_remove() returns.

In timeriomem_rng_remove(), the driver manually calls
hrtimer_cancel(&priv->timer). At this point, the RNG is still registered and
fully exposed to concurrent readers. This ordering creates two distinct issues:

1. Deadlock: If a concurrent reader is inside timeriomem_rng_read() and
blocking on wait_for_completion(&priv->completion), it will wait forever
because the timer was just cancelled. When devres subsequently calls
hwrng_unregister(), it waits indefinitely for the reader's reference count to
drop, deadlocking the unbind process.

2. Use-After-Free: If a concurrent reader finishes its read loop just after
remove() cancelled the timer, it unconditionally executes
hrtimer_restart(&priv->timer) in the read path:

timeriomem_rng_read() {
    ...
    priv->present = 0;
    reinit_completion(&priv->completion);
    hrtimer_forward_now(&priv->timer, priv->period);
    hrtimer_restart(&priv->timer);
    ...
}

The reader then completes, remove() finishes, and devres frees the priv
structure. The re-armed timer will then fire and dereference the freed priv
pointer.

Could this devres ordering issue be addressed by manually unregistering the
hwrng before cancelling the timer in remove(), or by using a custom devm
action to ensure correct teardown order?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260617112642.1897096-1-inasj268@gmail.com?part=2

Re: [PATCH v3 1/2] dt-bindings: rng: timeriomem_rng: add width and mask properties

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Use of non-standard `width` property instead of `reg-io-width`.
--

commit 7dac0db93dcf3017652753bebd5c0a2a7a2a06b5
Author: Jad Keskes <inasj268@gmail.com>

dt-bindings: rng: timeriomem_rng: add width and mask properties

Add optional width (8, 16, 32) and mask properties to the binding. The width
selects the bus access size for reads, while the mask allows filtering out
non-entropy-bearing bits.

> diff --git a/Documentation/devicetree/bindings/rng/timeriomem_rng.yaml b/Documentation/devicetree/bindings/rng/timeriomem_rng.yaml
> index 4754174e9849b..636305f211c88 100644
> --- a/Documentation/devicetree/bindings/rng/timeriomem_rng.yaml
> +++ b/Documentation/devicetree/bindings/rng/timeriomem_rng.yaml

[ ... ]

> @@ -26,16 +40,26 @@ properties:
>        instead.  Note that the default quality is usually zero which disables
>        using this rng to automatically fill the kernel's entropy pool.
>  
> -  reg:
> -    maxItems: 1
> +  width:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    default: 32
> +    enum: [8, 16, 32]
>      description:
> -      Base address to sample from. Currently 'reg' must be at least four bytes
> -      wide and 32-bit aligned.
> +      Access width in bits.  Determines whether the read is performed as
> +      an 8-bit, 16-bit, or 32-bit bus access.

[Severity: Low]
Should this binding use the standard reg-io-width property (representing the
width in bytes) instead of introducing a custom width property in bits?

Using the established reg-io-width standard prevents fragmentation of the
device tree API conventions for specifying memory-mapped I/O access width.

> +
> +  mask:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    default: 0xFFFFFFFF
> +    description:
> +      Mask applied to the value read from the register.  Bits set to 0 in
> +      the mask are cleared in the output data.  Default (no mask) passes
> +      all bits through.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260617112642.1897096-1-inasj268@gmail.com?part=1