Hi Aswin,
On 2026-03-30T17:14:12, Aswin Murugan <[email protected]> wrote:
> diff --git a/drivers/misc/nvmem.c b/drivers/misc/nvmem.c
> @@ -121,13 +222,27 @@ int nvmem_cell_get_by_index(...)
> - dev_dbg(cell->nvmem, "missing address or size for %s\n",
> + dev_err(cell->nvmem, "missing address or size for %s\n",
This change from 'dev_dbg' to 'dev_err' is unrelated to bit field
support. Please can you either drop it or split it into its own patch?
> diff --git a/drivers/misc/nvmem.c b/drivers/misc/nvmem.c
> @@ -12,55 +12,156 @@
> +int nvmem_cell_write(struct nvmem_cell *cell, const void *buf, size_t size)
> +{
> ...
> + value = *(u32 *)buf;
> + value &= (1U << cell->nbits) - 1;
> + value <<= cell->bit_offset;
> +
> + mask = ((1U << cell->nbits) - 1) << cell->bit_offset;
> +
> + *(u32 *)buf = (current & ~mask) | value;
You should not write to a const pointer.
Second, when 'nbits == 32' the expression '(1U << cell->nbits) - 1' is
undefined behaviour (shift equal to type width). I suspect you want
something like 'GENMASK(cell->nbits - 1, 0)' for both read and write.
> diff --git a/drivers/misc/nvmem.c b/drivers/misc/nvmem.c
> @@ -12,55 +12,156 @@
> + if ((cell->nbits && size < cell->size) || (!cell->nbits && size !=
> cell->size)) {
This condition is checked identically in both 'nvmem_cell_read' and
'nvmem_cell_write'. For the bit field path you then require 'size ==
MAX_NVMEM_CELL_SIZE' a few lines later, so 'size < cell->size' is
always subsumed by that second check. Hard to follow How about
consolidating the size checks so the requirements for each mode are
clear in one place?
Regards,
Simon
