1. Patchew
  2. linux
  3. [PATCH v9 00/11] iio: accel: adxl345: add interrupt based sensor events
  4. View patch

[PATCH v9 05/11] iio: accel: adxl345: simplify reading the FIFO

Lothar Rubusch posted 11 patches 4 months ago
There is a newer version of this series
[PATCH v9 05/11] iio: accel: adxl345: simplify reading the FIFO
Posted by Lothar Rubusch 4 months ago 
Bulk reading from the FIFO can be simplified. Remove unnecessary
variables and simplify reading sets of x-, y- and z-axis measurements.

This is a refactoring change and should not impact functionality.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl345_core.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index 18c625d323ba..dcfbfe4cac0f 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -884,15 +884,13 @@ static int adxl345_get_samples(struct adxl345_state *st)
  */
 static int adxl345_fifo_transfer(struct adxl345_state *st, int samples)
 {
-	size_t count;
 	int i, ret = 0;
 
-	/* count is the 3x the fifo_buf element size, hence 6B */
-	count = sizeof(st->fifo_buf[0]) * ADXL345_DIRS;
 	for (i = 0; i < samples; i++) {
 		/* read 3x 2 byte elements from base address into next fifo_buf position */
 		ret = regmap_bulk_read(st->regmap, ADXL345_REG_XYZ_BASE,
-				       st->fifo_buf + (i * count / 2), count);
+				       st->fifo_buf + (i * ADXL345_DIRS),
+				       2 * ADXL345_DIRS);
 		if (ret)
 			return ret;
 
-- 
2.39.5
Re: [PATCH v9 05/11] iio: accel: adxl345: simplify reading the FIFO
Posted by Jonathan Cameron 3 months, 4 weeks ago 
On Tue, 10 Jun 2025 21:59:27 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> Bulk reading from the FIFO can be simplified. Remove unnecessary
> variables and simplify reading sets of x-, y- and z-axis measurements.
> 
> This is a refactoring change and should not impact functionality.
> 
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
> ---
>  drivers/iio/accel/adxl345_core.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> index 18c625d323ba..dcfbfe4cac0f 100644
> --- a/drivers/iio/accel/adxl345_core.c
> +++ b/drivers/iio/accel/adxl345_core.c
> @@ -884,15 +884,13 @@ static int adxl345_get_samples(struct adxl345_state *st)
>   */
>  static int adxl345_fifo_transfer(struct adxl345_state *st, int samples)
>  {
> -	size_t count;
>  	int i, ret = 0;
>  
> -	/* count is the 3x the fifo_buf element size, hence 6B */
> -	count = sizeof(st->fifo_buf[0]) * ADXL345_DIRS;
>  	for (i = 0; i < samples; i++) {
>  		/* read 3x 2 byte elements from base address into next fifo_buf position */
>  		ret = regmap_bulk_read(st->regmap, ADXL345_REG_XYZ_BASE,
> -				       st->fifo_buf + (i * count / 2), count);
> +				       st->fifo_buf + (i * ADXL345_DIRS),
> +				       2 * ADXL345_DIRS);
				       sizeof(st->fifo_buf[0]) * ADXL345_DIRS);

As it's not locally obvious where that 2 comes from.

>  		if (ret)
>  			return ret;
>
Re: [PATCH v9 05/11] iio: accel: adxl345: simplify reading the FIFO
Posted by Lothar Rubusch 3 months, 3 weeks ago 
Hi Jonathan,

On Sat, Jun 14, 2025 at 3:47 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Tue, 10 Jun 2025 21:59:27 +0000
> Lothar Rubusch <l.rubusch@gmail.com> wrote:
>
> > Bulk reading from the FIFO can be simplified. Remove unnecessary
> > variables and simplify reading sets of x-, y- and z-axis measurements.
> >
> > This is a refactoring change and should not impact functionality.
> >
> > Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
> > ---
> >  drivers/iio/accel/adxl345_core.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> > index 18c625d323ba..dcfbfe4cac0f 100644
> > --- a/drivers/iio/accel/adxl345_core.c
> > +++ b/drivers/iio/accel/adxl345_core.c
> > @@ -884,15 +884,13 @@ static int adxl345_get_samples(struct adxl345_state *st)
> >   */
> >  static int adxl345_fifo_transfer(struct adxl345_state *st, int samples)
> >  {
> > -     size_t count;
> >       int i, ret = 0;
> >
> > -     /* count is the 3x the fifo_buf element size, hence 6B */
> > -     count = sizeof(st->fifo_buf[0]) * ADXL345_DIRS;
> >       for (i = 0; i < samples; i++) {
> >               /* read 3x 2 byte elements from base address into next fifo_buf position */
> >               ret = regmap_bulk_read(st->regmap, ADXL345_REG_XYZ_BASE,
> > -                                    st->fifo_buf + (i * count / 2), count);
> > +                                    st->fifo_buf + (i * ADXL345_DIRS),
> > +                                    2 * ADXL345_DIRS);
>                                        sizeof(st->fifo_buf[0]) * ADXL345_DIRS);
>
> As it's not locally obvious where that 2 comes from.

Hm, I left the comment some lines above: "read 3x 2 byte elements from
base address into next fifo_buf position" - so, the 2 comes from the 2
bytes. 3x because of the 3-axis sensor, so 3 axis times 2 bytes. How
can I improve it? The patch is just a simplification, removing that
unnecessary count variable.

>
> >               if (ret)
> >                       return ret;
> >
>
Re: [PATCH v9 05/11] iio: accel: adxl345: simplify reading the FIFO
Posted by Jonathan Cameron 3 months, 3 weeks ago 
On Mon, 16 Jun 2025 00:14:47 +0200
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> Hi Jonathan,
> 
> On Sat, Jun 14, 2025 at 3:47 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Tue, 10 Jun 2025 21:59:27 +0000
> > Lothar Rubusch <l.rubusch@gmail.com> wrote:
> >  
> > > Bulk reading from the FIFO can be simplified. Remove unnecessary
> > > variables and simplify reading sets of x-, y- and z-axis measurements.
> > >
> > > This is a refactoring change and should not impact functionality.
> > >
> > > Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
> > > ---
> > >  drivers/iio/accel/adxl345_core.c | 6 ++----
> > >  1 file changed, 2 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> > > index 18c625d323ba..dcfbfe4cac0f 100644
> > > --- a/drivers/iio/accel/adxl345_core.c
> > > +++ b/drivers/iio/accel/adxl345_core.c
> > > @@ -884,15 +884,13 @@ static int adxl345_get_samples(struct adxl345_state *st)
> > >   */
> > >  static int adxl345_fifo_transfer(struct adxl345_state *st, int samples)
> > >  {
> > > -     size_t count;
> > >       int i, ret = 0;
> > >
> > > -     /* count is the 3x the fifo_buf element size, hence 6B */
> > > -     count = sizeof(st->fifo_buf[0]) * ADXL345_DIRS;
> > >       for (i = 0; i < samples; i++) {
> > >               /* read 3x 2 byte elements from base address into next fifo_buf position */
> > >               ret = regmap_bulk_read(st->regmap, ADXL345_REG_XYZ_BASE,
> > > -                                    st->fifo_buf + (i * count / 2), count);
> > > +                                    st->fifo_buf + (i * ADXL345_DIRS),
> > > +                                    2 * ADXL345_DIRS);  
> >                                        sizeof(st->fifo_buf[0]) * ADXL345_DIRS);
> >
> > As it's not locally obvious where that 2 comes from.  
> 
> Hm, I left the comment some lines above: "read 3x 2 byte elements from
> base address into next fifo_buf position" - so, the 2 comes from the 2
> bytes. 3x because of the 3-axis sensor, so 3 axis times 2 bytes. How
> can I improve it? The patch is just a simplification, removing that
> unnecessary count variable.

 sizeof(st->fifo_buf[0]) * ADXL345_DIRS);

improves it by making it clear where the 2 comes from.
Getting rid of count is good, but that remaining 2 * 
should be replaced by using sizeof() the buffer element.

The comment would then be unnecessary as we are sizing it directly from
the fifo storage.  Fine to leave it if you like though.

Jonathan


> 
> >  
> > >               if (ret)
> > >                       return ret;
> > >  
> >

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.