Hi Aswin, On 2026-02-13T11:27:11, Aswin Murugan <[email protected]> wrote: > misc: Add support for bit fields in NVMEM cells > > NVMEM cells currently only support byte-level access. Many hardware > registers pack multiple fields into single bytes, requiring bit-level > granularity. For example, Qualcomm PMIC PON registers store a 7-bit > reboot reason field within a single byte, with bit 0 reserved for other > purposes. > > Add support for the optional 'bits' property in NVMEM cell device tree > bindings. This property specifies <bit_offset num_bits> to define a bit > field within the cell's register space. > > Implement multi-byte bit field support by porting bit manipulation > algorithms from the Linux kernel driver [1]: > > 1. nvmem_shift_read_buffer_in_place() - Extract bit fields from raw > bytes by shifting and masking across byte boundaries. Handles fields > that span multiple bytes. > > 2. nvmem_cell_prepare_write_buffer() - Perform Read-Modify-Write to > [...] > > drivers/misc/nvmem.c | 241 ++++++++++++++++++++++++++++++++++++++++++++++++--- > include/nvmem.h | 4 + > 2 files changed, 231 insertions(+), 14 deletions(-)
Reviewed-by: Simon Glass <[email protected]> > diff --git a/include/nvmem.h b/include/nvmem.h > @@ -26,11 +26,15 @@ > * @nvmem: The backing storage device > * @offset: The offset of the cell from the start of @nvmem > * @size: The size of the cell, in bytes > + * @bit_offset: Bit offset within the cell (0 for byte-level access) > + * @nbits: Number of bits to use (0 for byte-level access) > */ The note on @bit_offset is misleading — a valid bit-field can have bit_offset == 0. It's nbits == 0 alone that selects byte-level access. Please reword. > diff --git a/drivers/misc/nvmem.c b/drivers/misc/nvmem.c > @@ -121,13 +320,27 @@ int nvmem_cell_get_by_index(struct udevice *dev, int > index, > + ret = ofnode_read_u32_index(args.node, 'bits', 0, &cell->bit_offset); > + if (ret) { > + cell->bit_offset = 0; > + cell->nbits = 0; > + } else { > + ret = ofnode_read_u32_index(args.node, 'bits', 1, &cell->nbits); > + if (ret) > + return -EINVAL; > + > + if (cell->bit_offset + cell->nbits > cell->size * 8) > + return -EINVAL; > + } The binding requires nbits >= 1 when 'bits' is present, but this accepts nbits == 0 (silently falling back to byte mode and confusing the read/write paths). Please also reject bit_offset >= 8 - the binding caps it at 0..7. Better to complain about malformed DT than to silently accept it. Regards, Simon
