On 1/4/2025 11:49 AM, Dmitry Baryshkov wrote:
> If the NVMEM specifies a stride to access data, reading particular cell
> might require bit offset that is bigger than one byte. Rework NVMEM core
> code to support bit offsets of more than 8 bits.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> ---
> drivers/nvmem/core.c | 24 +++++++++++++++++-------
> 1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index d6494dfc20a7324bde6415776dcabbb0bfdd334b..c0af43a37195c3869507a203b370615309aeee67 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -834,7 +834,9 @@ static int nvmem_add_cells_from_dt(struct nvmem_device *nvmem, struct device_nod
> if (addr && len == (2 * sizeof(u32))) {
> info.bit_offset = be32_to_cpup(addr++);
> info.nbits = be32_to_cpup(addr);
> - if (info.bit_offset >= BITS_PER_BYTE || info.nbits < 1) {
> + if (info.bit_offset >= BITS_PER_BYTE * info.bytes ||
> + info.nbits < 1 ||
> + info.bit_offset + info.nbits >= BITS_PER_BYTE * info.bytes) {
Should it be ">" check instead of ">=" check here?
For eg: bit_offset = 7, nbits = 1 and info.bytes = 1 is valid, isn't it?
-Akhil
> dev_err(dev, "nvmem: invalid bits on %pOF\n", child);
> of_node_put(child);
> return -EINVAL;
> @@ -1627,21 +1629,29 @@ EXPORT_SYMBOL_GPL(nvmem_cell_put);
> static void nvmem_shift_read_buffer_in_place(struct nvmem_cell_entry *cell, void *buf)
> {
> u8 *p, *b;
> - int i, extra, bit_offset = cell->bit_offset;
> + int i, extra, bytes_offset;
> + int bit_offset = cell->bit_offset;
>
> p = b = buf;
> - if (bit_offset) {
> +
> + bytes_offset = bit_offset / BITS_PER_BYTE;
> + b += bytes_offset;
> + bit_offset %= BITS_PER_BYTE;
> +
> + if (bit_offset % BITS_PER_BYTE) {
> /* First shift */
> - *b++ >>= bit_offset;
> + *p = *b++ >> bit_offset;
>
> /* setup rest of the bytes if any */
> for (i = 1; i < cell->bytes; i++) {
> /* Get bits from next byte and shift them towards msb */
> - *p |= *b << (BITS_PER_BYTE - bit_offset);
> + *p++ |= *b << (BITS_PER_BYTE - bit_offset);
>
> - p = b;
> - *b++ >>= bit_offset;
> + *p = *b++ >> bit_offset;
> }
> + } else if (p != b) {
> + memmove(p, b, cell->bytes - bytes_offset);
> + p += cell->bytes - 1;
> } else {
> /* point to the msb */
> p += cell->bytes - 1;
>
