1. Patchew
  2. linux
  3. [PATCH v3 0/5] nvmem: qfprom: add Qualcomm SAR2130P support
  4. View patch

[PATCH v3 1/5] nvmem: core: fix bit offsets of more than one byte

Dmitry Baryshkov posted 5 patches 1 year, 1 month ago
There is a newer version of this series
[PATCH v3 1/5] nvmem: core: fix bit offsets of more than one byte
Posted by Dmitry Baryshkov 1 year, 1 month ago 
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@linaro.org>
---
 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) {
 				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;

-- 
2.39.5
Re: [PATCH v3 1/5] nvmem: core: fix bit offsets of more than one byte
Posted by Akhil P Oommen 1 year, 1 month ago 
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@linaro.org>
> ---
>  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;
>
Re: [PATCH v3 1/5] nvmem: core: fix bit offsets of more than one byte
Posted by Dmitry Baryshkov 1 year, 1 month ago 
On Thu, Jan 09, 2025 at 03:17:08AM +0530, Akhil P Oommen wrote:
> 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@linaro.org>
> > ---
> >  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?

Indeed. I'll send v-next.

> 
> -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;
> > 
> 

-- 
With best wishes
Dmitry

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.