[PATCH 0/2] nvmem: Add byte-mask support for non-contiguous byte selection

[PATCH 0/2] nvmem: Add byte-mask support for non-contiguous byte selection

[Hongmei Xu]

Some hardware stores NVMEM cell data in non-contiguous bytes. For
example, a MAC address cell may span 7 physical bytes with one byte
unused, requiring the driver to skip that byte during readout.

The existing 'bits' property only handles sub-byte granularity and
cannot express byte-level gaps. This series adds a 'byte-mask'
property to the fixed-cell binding and implements it in the nvmem
core, allowing consumers to describe and read such cells.

An example: assume a MAC address is assembled from efuse registers
0xFFFF4320[31:0] (bytes 0-3), 0xFFFF4324[7:0] (byte 4), and
0xFFFF4324[23:16] (byte 6), with 0xFFFF4324[15:8] unused (byte 5).
Map the efuse region as an 8-byte nvmem device and define the cell
skipping byte 5:

  efuse: efuse@ffff4320 {
    compatible = "...";
    reg = <0x0 0xffff4320 0x0 0x8>;
    #address-cells = <1>;
    #size-cells = <1>;

    mac_addr: mac-addr@0 {
      reg = <0x0 0x6>;
      byte-mask = <0x5f>;
    };
  };

  With byte-mask = 0x5f (0101_1111), bytes 0-4 and 6 are selected
  (byte 5 is skipped), forming the 6-byte MAC address output.

Signed-off-by: Hongmei Xu <hongmei.xu@oss.qualcomm.com>
---
Hongmei Xu (2):
      dt-bindings: nvmem: add byte-mask property for non-contiguous byte selection
      nvmem: core: add byte-mask support for non-contiguous cell bytes

 .../bindings/nvmem/layouts/fixed-cell.yaml         |  22 +++++
 drivers/nvmem/core.c                               | 101 +++++++++++++++++++--
 include/linux/nvmem-provider.h                     |   7 ++
 3 files changed, 123 insertions(+), 7 deletions(-)
---
base-commit: d26955e57475434b59d43ba372b2a50d33d5e0ed
change-id: 20261110-nvmem-byte-mask-for-split-macaddr-a4698a128057

Best regards,
--  
Hongmei Xu <hongmei.xu@oss.qualcomm.com>

[PATCH 1/2] dt-bindings: nvmem: add byte-mask property for non-contiguous byte selection

[Hongmei Xu]

Add the byte-mask property to the fixed-cell binding. This allows
selecting non-contiguous physical bytes from an NVMEM region to form
a logical cell value.

Bit N set in byte-mask means physical byte N relative to the cell's
base offset is included in the output; selected bytes are packed into
the output buffer in ascending byte order. If fewer bytes are selected
than the cell's logical size given by reg, the remaining output bytes
are zero-filled. The number of set bits must not exceed the cell's
logical size given by reg. byte-mask and bits are mutually exclusive.

Signed-off-by: Hongmei Xu <hongmei.xu@oss.qualcomm.com>
---
 .../bindings/nvmem/layouts/fixed-cell.yaml         | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/Documentation/devicetree/bindings/nvmem/layouts/fixed-cell.yaml b/Documentation/devicetree/bindings/nvmem/layouts/fixed-cell.yaml
index 38e3ad50ff4f..cddda3f70691 100644
--- a/Documentation/devicetree/bindings/nvmem/layouts/fixed-cell.yaml
+++ b/Documentation/devicetree/bindings/nvmem/layouts/fixed-cell.yaml
@@ -34,6 +34,21 @@ properties:
         description:
           Size in bit within the address range specified by reg.
 
+  byte-mask:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 1
+    description: >
+      Bitmask selecting which physical bytes within the NVMEM device region
+      contribute to this cell's output. Bit N set means physical byte N
+      relative to the cell's base offset is included; selected bytes are
+      packed into the output buffer in ascending byte order. If fewer bytes
+      are selected than the cell's logical size given by reg, the remaining
+      output bytes are zero-filled. Byte positions are relative to the cell's
+      base offset and limited to a 32-byte window. The highest set bit must
+      not reference a byte beyond the device size. The number of set bits must
+      not exceed the cell's logical size given by reg. Cannot be used
+      together with bits.
+
 allOf:
   - if:
       properties:
@@ -48,6 +63,13 @@ allOf:
           const: 1
       required:
         - "#nvmem-cell-cells"
+  - if:
+      required: [ byte-mask ]
+    then:
+      properties:
+        bits: false
+      description:
+        byte-mask and bits are mutually exclusive.
 
 required:
   - reg

-- 
2.34.1

Re: [PATCH 1/2] dt-bindings: nvmem: add byte-mask property for non-contiguous byte selection

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] dt-bindings: nvmem: byte-mask redefines reg size and accesses
  beyond reg
- [Low] dt-bindings: nvmem: missing hardware motivation for byte-mask in
  changelog
--

commit f8160f5bc1c29ed4ba48f6d458beb8ca96b09427
Author: Hongmei Xu <hongmei.xu@oss.qualcomm.com>

> dt-bindings: nvmem: add byte-mask property for non-contiguous byte selection
>
> Add the byte-mask property to the fixed-cell binding. This allows
> selecting non-contiguous physical bytes from an NVMEM region to form
> a logical cell value.
>
> Bit N set in byte-mask means physical byte N relative to the cell's
> base offset is included in the output; selected bytes are packed into
> the output buffer in ascending byte order. If fewer bytes are selected
> than the cell's logical size given by reg, the remaining output bytes
> are zero-filled. The number of set bits must not exceed the cell's
> logical size given by reg. byte-mask and bits are mutually exclusive.
>
> Signed-off-by: Hongmei Xu <hongmei.xu@oss.qualcomm.com>

[Severity: Low]
Could the commit message explain the hardware layout that motivates
byte-mask? It repeats the property rules from the schema, but does not
describe which SoC or NVMEM layout splits a logical cell across
non-contiguous physical bytes or why existing mechanisms such as multiple
reg entries or a device-specific layout are insufficient.

> diff --git a/Documentation/devicetree/bindings/nvmem/layouts/fixed-cell.yaml b/Documentation/devicetree/bindings/nvmem/layouts/fixed-cell.yaml
> index 38e3ad50ff4fb..cddda3f706917 100644
> --- a/Documentation/devicetree/bindings/nvmem/layouts/fixed-cell.yaml
> +++ b/Documentation/devicetree/bindings/nvmem/layouts/fixed-cell.yaml
> @@ -34,6 +34,21 @@ properties:
>          description:
>            Size in bit within the address range specified by reg.
>  
> +  byte-mask:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 1
> +    description: >
> +      Bitmask selecting which physical bytes within the NVMEM device region
> +      contribute to this cell's output. Bit N set means physical byte N
> +      relative to the cell's base offset is included; selected bytes are
> +      packed into the output buffer in ascending byte order. If fewer bytes
> +      are selected than the cell's logical size given by reg, the remaining
> +      output bytes are zero-filled. Byte positions are relative to the cell's
> +      base offset and limited to a 32-byte window. The highest set bit must
> +      not reference a byte beyond the device size. The number of set bits must
> +      not exceed the cell's logical size given by reg. Cannot be used
> +      together with bits.

[Severity: Medium]
Does byte-mask conflict with standard reg semantics by treating the reg
size as a logical output buffer size while reading physical bytes outside
the reg range?

Any non-contiguous selection of K bytes spans at least K + 1 physical
bytes (highest set bit H >= K). A Devicetree author must then either set
the reg size to K and access physical bytes up to offset + H outside the
declared reg range, hiding overlaps with sibling cells, or set the reg
size to the physical span M >= H + 1, which pads the output to M bytes and
breaks typed consumers like nvmem_cell_read_u32() that require the cell
size to match sizeof(*val).

Defining software zero-padding in reg also puts buffer formatting policy
into the hardware description, and the binding leaves write semantics for
cells with byte-mask undefined.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-nvmem-byte-mask-for-split-macaddr-v1-0-138f715f18c1@oss.qualcomm.com?part=1

[PATCH 2/2] nvmem: core: add byte-mask support for non-contiguous cell bytes

[Hongmei Xu]

Add support for a byte-mask cell property that selects non-contiguous
physical bytes from an NVMEM device region to form a logical cell value.

When byte-mask is set, the entire NVMEM device region is read starting
from the cell's base offset. Bit N set in the mask means physical byte N
is included; selected bytes are packed into the output buffer in
ascending byte order. If fewer bytes are selected than the cell's
logical size given by reg, the remaining output bytes are zero-filled.

Two checks are performed at cell registration:
- The number of set bits must not exceed the cell's logical size (reg),
  to prevent output buffer overflow.
- The highest set bit must not reference a byte beyond the NVMEM device
  size.

Signed-off-by: Hongmei Xu <hongmei.xu@oss.qualcomm.com>
---
 drivers/nvmem/core.c           | 101 ++++++++++++++++++++++++++++++++++++++---
 include/linux/nvmem-provider.h |   7 +++
 2 files changed, 101 insertions(+), 7 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 7d4cce8e2742..6c9994abce39 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -33,6 +33,7 @@ struct nvmem_cell_entry {
 	int			bytes;
 	int			bit_offset;
 	int			nbits;
+	u32			byte_mask;
 	nvmem_cell_post_process_t read_post_process;
 	void			*priv;
 	struct device_node	*np;
@@ -589,6 +590,47 @@ static void nvmem_cell_entry_add(struct nvmem_cell_entry *cell)
 	blocking_notifier_call_chain(&nvmem_notifier, NVMEM_CELL_ADD, cell);
 }
 
+static int nvmem_validate_byte_mask(struct nvmem_device *nvmem,
+				    const struct nvmem_cell_info *info)
+{
+	int out_bytes;
+
+	if (!info->byte_mask)
+		return 0;
+
+	if (info->nbits || info->bit_offset) {
+		dev_err(&nvmem->dev,
+			"cell %s byte-mask and bits are mutually exclusive\n",
+			info->name ?: "<unknown>");
+		return -EINVAL;
+	}
+
+	if (info->offset >= nvmem->size) {
+		dev_err(&nvmem->dev,
+			"cell %s offset %u exceeds device size %zu\n",
+			info->name ?: "<unknown>", info->offset, nvmem->size);
+		return -EINVAL;
+	}
+
+	out_bytes = hweight32(info->byte_mask);
+	if (out_bytes > info->bytes) {
+		dev_err(&nvmem->dev,
+			"cell %s byte-mask set-bit count %d exceeds reg size %d\n",
+			info->name ?: "<unknown>", out_bytes, info->bytes);
+		return -EINVAL;
+	}
+
+	if (fls(info->byte_mask) > nvmem->size - info->offset) {
+		dev_err(&nvmem->dev,
+			"cell %s byte-mask bit %d exceeds device size %zu\n",
+			info->name ?: "<unknown>",
+			fls(info->byte_mask) - 1, nvmem->size);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int nvmem_cell_info_to_nvmem_cell_entry_nodup(struct nvmem_device *nvmem,
 						     const struct nvmem_cell_info *info,
 						     struct nvmem_cell_entry *cell)
@@ -603,12 +645,19 @@ static int nvmem_cell_info_to_nvmem_cell_entry_nodup(struct nvmem_device *nvmem,
 
 	cell->bit_offset = info->bit_offset;
 	cell->nbits = info->nbits;
+	cell->byte_mask = info->byte_mask;
 	cell->np = info->np;
 
 	if (cell->nbits) {
 		cell->bytes = DIV_ROUND_UP(cell->nbits + cell->bit_offset,
 					   BITS_PER_BYTE);
 		cell->raw_len = ALIGN(cell->bytes, nvmem->word_size);
+	} else if (cell->byte_mask) {
+		int rc = nvmem_validate_byte_mask(nvmem, info);
+
+		if (rc)
+			return rc;
+		cell->raw_len = nvmem->size - cell->offset;
 	}
 
 	if (!IS_ALIGNED(cell->offset, nvmem->stride)) {
@@ -804,7 +853,7 @@ int nvmem_add_cells_from_dt(struct nvmem_device *nvmem, struct device_node *np)
 {
 	struct device *dev = &nvmem->dev;
 	const __be32 *addr;
-	int len, ret;
+	int len, ret = 0;
 
 	for_each_child_of_node_scoped(np, child) {
 		struct nvmem_cell_info info = {0};
@@ -829,16 +878,27 @@ int nvmem_add_cells_from_dt(struct nvmem_device *nvmem, struct device_node *np)
 			    info.nbits < 1 ||
 			    info.bit_offset + info.nbits > BITS_PER_BYTE * info.bytes) {
 				dev_err(dev, "nvmem: invalid bits on %pOF\n", child);
-				return -EINVAL;
+				ret = -EINVAL;
+				goto err_free_name;
 			}
 		}
 
+		of_property_read_u32(child, "byte-mask", &info.byte_mask);
+
 		info.np = of_node_get(child);
 
+		if (info.byte_mask) {
+			ret = nvmem_validate_byte_mask(nvmem, &info);
+			if (ret)
+				goto err_free_name;
+		}
+
 		if (nvmem->fixup_dt_cell_info)
 			nvmem->fixup_dt_cell_info(nvmem, &info);
 
 		ret = nvmem_add_one_cell(nvmem, &info);
+
+err_free_name:
 		kfree(info.name);
 		if (ret) {
 			of_node_put(info.np);
@@ -1648,28 +1708,51 @@ static int __nvmem_cell_read(struct nvmem_device *nvmem,
 			     struct nvmem_cell_entry *cell,
 			     void *buf, size_t *len, const char *id, int index)
 {
+	u8 *rawbuf = NULL;
+	void *readbuf = buf;
 	int rc;
 
-	rc = nvmem_reg_read(nvmem, cell->offset, buf, cell->raw_len);
+	if (cell->byte_mask) {
+		rawbuf = kzalloc(cell->raw_len, GFP_KERNEL);
+		if (!rawbuf)
+			return -ENOMEM;
+		readbuf = rawbuf;
+	}
+
+	rc = nvmem_reg_read(nvmem, cell->offset, readbuf, cell->raw_len);
 
 	if (rc)
-		return rc;
+		goto err_exit;
 
 	/* shift bits in-place */
 	if (cell->bit_offset || cell->nbits)
 		nvmem_shift_read_buffer_in_place(cell, buf);
 
+	/* compact non-contiguous bytes selected by byte_mask */
+	if (cell->byte_mask) {
+		int out_idx = 0;
+		int i;
+
+		for (i = 0; i < cell->raw_len && i < 32; i++) {
+			if (cell->byte_mask & BIT(i))
+				((u8 *)buf)[out_idx++] = rawbuf[i];
+		}
+	}
+
 	if (cell->read_post_process) {
 		rc = cell->read_post_process(cell->priv, id, index,
-					     cell->offset, buf, cell->raw_len);
+					     cell->offset, buf,
+					     cell->byte_mask ? cell->bytes : cell->raw_len);
 		if (rc)
-			return rc;
+			goto err_exit;
 	}
 
 	if (len)
 		*len = cell->bytes;
 
-	return 0;
+err_exit:
+	kfree(rawbuf);
+	return rc;
 }
 
 /**
@@ -1776,6 +1859,10 @@ static int __nvmem_cell_entry_write(struct nvmem_cell_entry *cell, void *buf, si
 	if (cell->read_post_process)
 		return -EINVAL;
 
+	/* byte-mask cells have no scatter-write implementation yet */
+	if (cell->byte_mask)
+		return -EINVAL;
+
 	if (cell->bit_offset || cell->nbits) {
 		if (len != BITS_TO_BYTES(cell->nbits) && len != cell->bytes)
 			return -EINVAL;
diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
index 6063fe5b7784..c9891595aac5 100644
--- a/include/linux/nvmem-provider.h
+++ b/include/linux/nvmem-provider.h
@@ -57,6 +57,12 @@ struct nvmem_keepout {
  * @bytes:	Length of the cell.
  * @bit_offset:	Bit offset if cell is smaller than a byte.
  * @nbits:	Number of bits.
+ * @byte_mask:   Bitmask selecting physical bytes to include in the output.
+ *               Bit N=1 means physical byte N relative to the cell's base
+ *               offset is packed into the output buffer; bit N=0 means skip.
+ *               The entire NVMEM device region from the cell's offset is read,
+ *               only bytes within the first 32 bytes may be selected. 
+ *               unselected output positions are zero-filled. Zero means no masking.
  * @np:		Optional device_node pointer.
  * @read_post_process:	Callback for optional post processing of cell data
  *			on reads.
@@ -69,6 +75,7 @@ struct nvmem_cell_info {
 	unsigned int		bytes;
 	unsigned int		bit_offset;
 	unsigned int		nbits;
+	u32			byte_mask;
 	struct device_node	*np;
 	nvmem_cell_post_process_t read_post_process;
 	void			*priv;

-- 
2.34.1

Re: [PATCH 2/2] nvmem: core: add byte-mask support for non-contiguous cell bytes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [High] nvmem: core: byte_mask raw_len reads full device and rounds
  past size
- [Medium] nvmem: core: byte_mask validation bypassed when nbits is
  non-zero
- [Medium] nvmem: core: missing zero-fill of trailing bytes in
  __nvmem_cell_read()
--

commit 2da8dac9e0c0313c6b2e42597b707a19ff863b3a
Author: Hongmei Xu <hongmei.xu@oss.qualcomm.com>

nvmem: core: add byte-mask support for non-contiguous cell bytes

This commit adds support for a byte-mask cell property that selects and packs
non-contiguous bytes from an NVMEM region into the logical cell value.

> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index 7d4cce8e27424..6c9994abce39f 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c

[ ... ]

> @@ -603,12 +645,19 @@ static int nvmem_cell_info_to_nvmem_cell_entry_nodup(struct nvmem_device *nvmem,
>  
>  	cell->bit_offset = info->bit_offset;
>  	cell->nbits = info->nbits;
> +	cell->byte_mask = info->byte_mask;
>  	cell->np = info->np;
>  
>  	if (cell->nbits) {
>  		cell->bytes = DIV_ROUND_UP(cell->nbits + cell->bit_offset,
>  					   BITS_PER_BYTE);
>  		cell->raw_len = ALIGN(cell->bytes, nvmem->word_size);
> +	} else if (cell->byte_mask) {
> +		int rc = nvmem_validate_byte_mask(nvmem, info);
> +
> +		if (rc)
> +			return rc;

[Severity: Medium]
Does placing nvmem_validate_byte_mask() in the else-if branch after
if (cell->nbits) bypass validation when both nbits and byte_mask are set?

cell->byte_mask is copied unconditionally above, so if info->nbits is
non-zero, nvmem_cell_info_to_nvmem_cell_entry_nodup() takes the
if (cell->nbits) branch and skips nvmem_validate_byte_mask(). In
nvmem_add_cells_from_dt(), nvmem_validate_byte_mask() runs before
nvmem->fixup_dt_cell_info(), which can set info.nbits and info.bit_offset and
adjust info.offset (as qfprom_fixup_dt_cell_info() does when a cell offset is
not 4-byte aligned) before nvmem_add_one_cell() runs.

During __nvmem_cell_read(), nvmem_reg_read() then reads into rawbuf while
nvmem_shift_read_buffer_in_place() shifts the unwritten buf, and the
byte_mask loop writes hweight32(cell->byte_mask) bytes into buf without
bounds validation, corrupting the cell data or overflowing buf.

> +		cell->raw_len = nvmem->size - cell->offset;
>  	}

[Severity: High]
Can setting cell->raw_len to nvmem->size - cell->offset cause out-of-bounds
reads past nvmem->size?

Right below this block, nvmem_cell_info_to_nvmem_cell_entry_nodup() aligns
cell->raw_len up to nvmem->word_size:

drivers/nvmem/core.c:nvmem_cell_info_to_nvmem_cell_entry_nodup() {
    ...
	if (!IS_ALIGNED(cell->raw_len, nvmem->word_size)) {
		dev_err(&nvmem->dev,
			"cell %s raw len %zd unaligned to nvmem word size %d\n",
			cell->name ?: "<unknown>", cell->raw_len,
			nvmem->word_size);

		if (info->raw_len)
			return -EINVAL;

		cell->raw_len = ALIGN(cell->raw_len, nvmem->word_size);
	}
    ...
}

When nvmem->size - cell->offset is not a multiple of nvmem->word_size,
rounding cell->raw_len up makes cell->offset + cell->raw_len exceed
nvmem->size, causing __nvmem_cell_read() to call nvmem_reg_read() past the
end of the device.

Even when aligned, setting cell->raw_len to the rest of the device rather
than the masked span (at most 32 bytes) forces every read to allocate and
read the entire trailing region, which fails on providers where reading
protected or unmapped trailing fuse rows returns an error (such as
sec-qfprom).

[ ... ]

> @@ -1648,28 +1708,51 @@ static int __nvmem_cell_read(struct nvmem_device *nvmem,
>  			     struct nvmem_cell_entry *cell,
>  			     void *buf, size_t *len, const char *id, int index)
>  {
> +	u8 *rawbuf = NULL;
> +	void *readbuf = buf;
>  	int rc;
>  
> -	rc = nvmem_reg_read(nvmem, cell->offset, buf, cell->raw_len);
> +	if (cell->byte_mask) {
> +		rawbuf = kzalloc(cell->raw_len, GFP_KERNEL);
> +		if (!rawbuf)
> +			return -ENOMEM;
> +		readbuf = rawbuf;
> +	}
> +
> +	rc = nvmem_reg_read(nvmem, cell->offset, readbuf, cell->raw_len);
>  
>  	if (rc)
> -		return rc;
> +		goto err_exit;
>  
>  	/* shift bits in-place */
>  	if (cell->bit_offset || cell->nbits)
>  		nvmem_shift_read_buffer_in_place(cell, buf);
>  
> +	/* compact non-contiguous bytes selected by byte_mask */
> +	if (cell->byte_mask) {
> +		int out_idx = 0;
> +		int i;
> +
> +		for (i = 0; i < cell->raw_len && i < 32; i++) {
> +			if (cell->byte_mask & BIT(i))
> +				((u8 *)buf)[out_idx++] = rawbuf[i];
> +		}
> +	}

[Severity: Medium]
When hweight32(cell->byte_mask) is smaller than cell->bytes, does this leave
the trailing bytes in buf uninitialized instead of zero-filling them?

Because nvmem_reg_read() reads into rawbuf, buf is only written up to
out_idx. Callers such as nvmem_device_cell_read() pass a caller-supplied
buffer directly into __nvmem_cell_read() without pre-zeroing it and return
cell->bytes, leaving stale or uninitialized memory in
buf[out_idx .. cell->bytes - 1] for both cell->read_post_process() and the
caller.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-nvmem-byte-mask-for-split-macaddr-v1-0-138f715f18c1@oss.qualcomm.com?part=2