linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V1 0/1] iio: accel: sca3300:  Extend the trigger buffer
@ 2022-07-01  2:30 LI Qingwu
  2022-07-01  2:30 ` [PATCH V1 1/1] iio: accel: sca3300: Extend the trigger buffer from 16 to 32 bytes LI Qingwu
  0 siblings, 1 reply; 5+ messages in thread
From: LI Qingwu @ 2022-07-01  2:30 UTC (permalink / raw)
  To: jic23, lars, tomas.melin, nuno.sa, linux-iio, linux-kernel, Qing-wu.Li
  Cc: bsp-development.geo

Fix the buffer reading issue for SCA3300:
  - Extend the buffer size from 16 to 32 bytes.
  - Change the buffer struct to a u8 array to adapt different sensors.
  - Readjustment the scan index to make it consistent with the buffer data.
  - Change sca3300_channels to indio_dev->channels.

LI Qingwu (1):
  iio: accel: sca3300: Extend the trigger buffer from 16 to 32 bytes

 drivers/iio/accel/sca3300.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH V1 1/1] iio: accel: sca3300: Extend the trigger buffer from 16 to 32 bytes
  2022-07-01  2:30 [PATCH V1 0/1] iio: accel: sca3300: Extend the trigger buffer LI Qingwu
@ 2022-07-01  2:30 ` LI Qingwu
  2022-07-01 16:46   ` Jonathan Cameron
  0 siblings, 1 reply; 5+ messages in thread
From: LI Qingwu @ 2022-07-01  2:30 UTC (permalink / raw)
  To: jic23, lars, tomas.melin, nuno.sa, linux-iio, linux-kernel, Qing-wu.Li
  Cc: bsp-development.geo

After added inclination angle channels, the trigger buffer size is
insufficient. Extend the buffer size from 16 to 32 bytes, and change
the trigger buffer from the struct to a u8 array to adapt the sensor
with/without inclination angles output.
New trigger buffer data:
  - SCA3300: 3 accel channels, temp, and timestamp.
  - SCL3300: 3 accel channels, temp, 3 incli channels, and timestamp.
Readjustment the scan index to make it consistent with the buffer data.

Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
---
 drivers/iio/accel/sca3300.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/accel/sca3300.c b/drivers/iio/accel/sca3300.c
index 3c4827bfef53..820dfb635bf1 100644
--- a/drivers/iio/accel/sca3300.c
+++ b/drivers/iio/accel/sca3300.c
@@ -47,12 +47,20 @@
 #define SCL3300_REG_ANG_CTRL 0x0C
 #define SCL3300_ANG_ENABLE   0x1F
 
+/*
+ * Buffer size max case:
+ * Three accel channels, two bytes per channel.
+ * Temperature channel, two bytes.
+ * Three incli channels, two bytes per channel.
+ * Timestamp channel, eight bytes.
+ */
+#define SCA3300_MAX_BUFFER_SIZE (ALIGN(2 * 7, sizeof(s64)) + sizeof(s64))
+
 enum sca3300_scan_indexes {
 	SCA3300_ACC_X = 0,
 	SCA3300_ACC_Y,
 	SCA3300_ACC_Z,
 	SCA3300_TEMP,
-	SCA3300_TIMESTAMP,
 	SCA3300_INCLI_X,
 	SCA3300_INCLI_Y,
 	SCA3300_INCLI_Z,
@@ -140,10 +148,10 @@ static const struct iio_chan_spec scl3300_channels[] = {
 	SCA3300_ACCEL_CHANNEL(SCA3300_ACC_Y, 0x2, Y),
 	SCA3300_ACCEL_CHANNEL(SCA3300_ACC_Z, 0x3, Z),
 	SCA3300_TEMP_CHANNEL(SCA3300_TEMP, 0x05),
-	IIO_CHAN_SOFT_TIMESTAMP(4),
 	SCA3300_INCLI_CHANNEL(SCA3300_INCLI_X, 0x09, X),
 	SCA3300_INCLI_CHANNEL(SCA3300_INCLI_Y, 0x0A, Y),
 	SCA3300_INCLI_CHANNEL(SCA3300_INCLI_Z, 0x0B, Z),
+	IIO_CHAN_SOFT_TIMESTAMP(7),
 };
 
 static const unsigned long sca3300_scan_masks[] = {
@@ -184,7 +192,9 @@ struct sca3300_chip_info {
  * @spi: SPI device structure
  * @lock: Data buffer lock
  * @chip: Sensor chip specific information
- * @scan: Triggered buffer. Four channel 16-bit data + 64-bit timestamp
+ * @buffer: Triggered buffer:
+ *          -SCA3300: 4 channel 16-bit data + 64-bit timestamp
+ *          -SCL3300: 7 channel 16-bit data + 64-bit timestamp
  * @txbuf: Transmit buffer
  * @rxbuf: Receive buffer
  */
@@ -192,10 +202,7 @@ struct sca3300_data {
 	struct spi_device *spi;
 	struct mutex lock;
 	const struct sca3300_chip_info *chip;
-	struct {
-		s16 channels[4];
-		s64 ts __aligned(sizeof(s64));
-	} scan;
+	u8 buffer[SCA3300_MAX_BUFFER_SIZE] __aligned(sizeof(s64));
 	u8 txbuf[4] __aligned(IIO_DMA_MINALIGN);
 	u8 rxbuf[4];
 };
@@ -484,21 +491,21 @@ static irqreturn_t sca3300_trigger_handler(int irq, void *p)
 	struct iio_dev *indio_dev = pf->indio_dev;
 	struct sca3300_data *data = iio_priv(indio_dev);
 	int bit, ret, val, i = 0;
+	s16 *channels = (s16 *)data->buffer;
 
 	for_each_set_bit(bit, indio_dev->active_scan_mask,
 			 indio_dev->masklength) {
-		ret = sca3300_read_reg(data, sca3300_channels[bit].address,
-				       &val);
+		ret = sca3300_read_reg(data, indio_dev->channels[bit].address, &val);
 		if (ret) {
 			dev_err_ratelimited(&data->spi->dev,
 				"failed to read register, error: %d\n", ret);
 			/* handled, but bailing out due to errors */
 			goto out;
 		}
-		data->scan.channels[i++] = val;
+		channels[i++] = val;
 	}
 
-	iio_push_to_buffers_with_timestamp(indio_dev, &data->scan,
+	iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
 					   iio_get_time_ns(indio_dev));
 out:
 	iio_trigger_notify_done(indio_dev->trig);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH V1 1/1] iio: accel: sca3300: Extend the trigger buffer from 16 to 32 bytes
  2022-07-01  2:30 ` [PATCH V1 1/1] iio: accel: sca3300: Extend the trigger buffer from 16 to 32 bytes LI Qingwu
@ 2022-07-01 16:46   ` Jonathan Cameron
  2022-07-04  1:01     ` LI Qingwu
  0 siblings, 1 reply; 5+ messages in thread
From: Jonathan Cameron @ 2022-07-01 16:46 UTC (permalink / raw)
  To: LI Qingwu
  Cc: lars, tomas.melin, nuno.sa, linux-iio, linux-kernel, bsp-development.geo

On Fri,  1 Jul 2022 02:30:30 +0000
LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn> wrote:

> After added inclination angle channels, the trigger buffer size is
> insufficient. Extend the buffer size from 16 to 32 bytes, and change
> the trigger buffer from the struct to a u8 array to adapt the sensor
> with/without inclination angles output.
> New trigger buffer data:
>   - SCA3300: 3 accel channels, temp, and timestamp.
>   - SCL3300: 3 accel channels, temp, 3 incli channels, and timestamp.
> Readjustment the scan index to make it consistent with the buffer data.
> 
> Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>

Hi.

Looks good. A trivial suggestion inline to make the code a little more
'self documenting'.  It's a minor change so if you are happy with
the suggestion I can tweak that whilst applying.

Thanks,

Jonathan

> ---
>  drivers/iio/accel/sca3300.c | 29 ++++++++++++++++++-----------
>  1 file changed, 18 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/iio/accel/sca3300.c b/drivers/iio/accel/sca3300.c
> index 3c4827bfef53..820dfb635bf1 100644
> --- a/drivers/iio/accel/sca3300.c
> +++ b/drivers/iio/accel/sca3300.c
> @@ -47,12 +47,20 @@
>  #define SCL3300_REG_ANG_CTRL 0x0C
>  #define SCL3300_ANG_ENABLE   0x1F
>  
> +/*
> + * Buffer size max case:
> + * Three accel channels, two bytes per channel.
> + * Temperature channel, two bytes.
> + * Three incli channels, two bytes per channel.
> + * Timestamp channel, eight bytes.
> + */
> +#define SCA3300_MAX_BUFFER_SIZE (ALIGN(2 * 7, sizeof(s64)) + sizeof(s64))

Instead of the 2 use sizeof(s16)
 Also now you don't have timestamp in your enum sca3000_scan_indexes
you could add a 'tail' element to the enum such as SCA3000_SCAN_MAX then
use that instead of the 7 here.  Hopefully that would make
this more self documenting.

> +
>  enum sca3300_scan_indexes {
>  	SCA3300_ACC_X = 0,
>  	SCA3300_ACC_Y,
>  	SCA3300_ACC_Z,
>  	SCA3300_TEMP,
> -	SCA3300_TIMESTAMP,
>  	SCA3300_INCLI_X,
>  	SCA3300_INCLI_Y,
>  	SCA3300_INCLI_Z,
> @@ -140,10 +148,10 @@ static const struct iio_chan_spec scl3300_channels[] = {
>  	SCA3300_ACCEL_CHANNEL(SCA3300_ACC_Y, 0x2, Y),
>  	SCA3300_ACCEL_CHANNEL(SCA3300_ACC_Z, 0x3, Z),
>  	SCA3300_TEMP_CHANNEL(SCA3300_TEMP, 0x05),
> -	IIO_CHAN_SOFT_TIMESTAMP(4),
>  	SCA3300_INCLI_CHANNEL(SCA3300_INCLI_X, 0x09, X),
>  	SCA3300_INCLI_CHANNEL(SCA3300_INCLI_Y, 0x0A, Y),
>  	SCA3300_INCLI_CHANNEL(SCA3300_INCLI_Z, 0x0B, Z),
> +	IIO_CHAN_SOFT_TIMESTAMP(7),
>  };
>  
>  static const unsigned long sca3300_scan_masks[] = {
> @@ -184,7 +192,9 @@ struct sca3300_chip_info {
>   * @spi: SPI device structure
>   * @lock: Data buffer lock
>   * @chip: Sensor chip specific information
> - * @scan: Triggered buffer. Four channel 16-bit data + 64-bit timestamp
> + * @buffer: Triggered buffer:
> + *          -SCA3300: 4 channel 16-bit data + 64-bit timestamp
> + *          -SCL3300: 7 channel 16-bit data + 64-bit timestamp
>   * @txbuf: Transmit buffer
>   * @rxbuf: Receive buffer
>   */
> @@ -192,10 +202,7 @@ struct sca3300_data {
>  	struct spi_device *spi;
>  	struct mutex lock;
>  	const struct sca3300_chip_info *chip;
> -	struct {
> -		s16 channels[4];
> -		s64 ts __aligned(sizeof(s64));
> -	} scan;
> +	u8 buffer[SCA3300_MAX_BUFFER_SIZE] __aligned(sizeof(s64));
>  	u8 txbuf[4] __aligned(IIO_DMA_MINALIGN);
>  	u8 rxbuf[4];
>  };
> @@ -484,21 +491,21 @@ static irqreturn_t sca3300_trigger_handler(int irq, void *p)
>  	struct iio_dev *indio_dev = pf->indio_dev;
>  	struct sca3300_data *data = iio_priv(indio_dev);
>  	int bit, ret, val, i = 0;
> +	s16 *channels = (s16 *)data->buffer;
>  
>  	for_each_set_bit(bit, indio_dev->active_scan_mask,
>  			 indio_dev->masklength) {
> -		ret = sca3300_read_reg(data, sca3300_channels[bit].address,
> -				       &val);
> +		ret = sca3300_read_reg(data, indio_dev->channels[bit].address, &val);
>  		if (ret) {
>  			dev_err_ratelimited(&data->spi->dev,
>  				"failed to read register, error: %d\n", ret);
>  			/* handled, but bailing out due to errors */
>  			goto out;
>  		}
> -		data->scan.channels[i++] = val;
> +		channels[i++] = val;
>  	}
>  
> -	iio_push_to_buffers_with_timestamp(indio_dev, &data->scan,
> +	iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
>  					   iio_get_time_ns(indio_dev));
>  out:
>  	iio_trigger_notify_done(indio_dev->trig);


^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH V1 1/1] iio: accel: sca3300: Extend the trigger buffer from 16 to 32 bytes
  2022-07-01 16:46   ` Jonathan Cameron
@ 2022-07-04  1:01     ` LI Qingwu
  2022-07-19  8:39       ` Jonathan Cameron
  0 siblings, 1 reply; 5+ messages in thread
From: LI Qingwu @ 2022-07-04  1:01 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: lars, tomas.melin, nuno.sa, linux-iio, linux-kernel,
	GEO-CHHER-bsp-development





> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Saturday, July 2, 2022 12:46 AM
> To: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
> Cc: lars@metafoo.de; tomas.melin@vaisala.com; nuno.sa@analog.com;
> linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org;
> GEO-CHHER-bsp-development <bsp-development.geo@leica-geosystems.com>
> Subject: Re: [PATCH V1 1/1] iio: accel: sca3300: Extend the trigger buffer from
> 16 to 32 bytes
> 
> [Some people who received this message don't often get email from
> jic23@kernel.org. Learn why this is important at
> https://aka.ms/LearnAboutSenderIdentification ]
> 
> This email is not from Hexagon's Office 365 instance. Please be careful while
> clicking links, opening attachments, or replying to this email.
> 
> 
> On Fri,  1 Jul 2022 02:30:30 +0000
> LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn> wrote:
> 
> > After added inclination angle channels, the trigger buffer size is
> > insufficient. Extend the buffer size from 16 to 32 bytes, and change
> > the trigger buffer from the struct to a u8 array to adapt the sensor
> > with/without inclination angles output.
> > New trigger buffer data:
> >   - SCA3300: 3 accel channels, temp, and timestamp.
> >   - SCL3300: 3 accel channels, temp, 3 incli channels, and timestamp.
> > Readjustment the scan index to make it consistent with the buffer data.
> >
> > Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
> 
> Hi.
> 
> Looks good. A trivial suggestion inline to make the code a little more 'self
> documenting'.  It's a minor change so if you are happy with the suggestion I
> can tweak that whilst applying.
> 
> Thanks,
> 
> Jonathan
> 

Yes, sure, absolutely agree, Thank you!

LI Qingwu

> > ---
> >  drivers/iio/accel/sca3300.c | 29 ++++++++++++++++++-----------
> >  1 file changed, 18 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/iio/accel/sca3300.c b/drivers/iio/accel/sca3300.c
> > index 3c4827bfef53..820dfb635bf1 100644
> > --- a/drivers/iio/accel/sca3300.c
> > +++ b/drivers/iio/accel/sca3300.c
> > @@ -47,12 +47,20 @@
> >  #define SCL3300_REG_ANG_CTRL 0x0C
> >  #define SCL3300_ANG_ENABLE   0x1F
> >
> > +/*
> > + * Buffer size max case:
> > + * Three accel channels, two bytes per channel.
> > + * Temperature channel, two bytes.
> > + * Three incli channels, two bytes per channel.
> > + * Timestamp channel, eight bytes.
> > + */
> > +#define SCA3300_MAX_BUFFER_SIZE (ALIGN(2 * 7, sizeof(s64)) +
> > +sizeof(s64))
> 
> Instead of the 2 use sizeof(s16)
>  Also now you don't have timestamp in your enum sca3000_scan_indexes you
> could add a 'tail' element to the enum such as SCA3000_SCAN_MAX then use
> that instead of the 7 here.  Hopefully that would make this more self
> documenting.


> 
> > +
> >  enum sca3300_scan_indexes {
> >       SCA3300_ACC_X = 0,
> >       SCA3300_ACC_Y,
> >       SCA3300_ACC_Z,
> >       SCA3300_TEMP,
> > -     SCA3300_TIMESTAMP,
> >       SCA3300_INCLI_X,
> >       SCA3300_INCLI_Y,
> >       SCA3300_INCLI_Z,
> > @@ -140,10 +148,10 @@ static const struct iio_chan_spec scl3300_channels[]
> = {
> >       SCA3300_ACCEL_CHANNEL(SCA3300_ACC_Y, 0x2, Y),
> >       SCA3300_ACCEL_CHANNEL(SCA3300_ACC_Z, 0x3, Z),
> >       SCA3300_TEMP_CHANNEL(SCA3300_TEMP, 0x05),
> > -     IIO_CHAN_SOFT_TIMESTAMP(4),
> >       SCA3300_INCLI_CHANNEL(SCA3300_INCLI_X, 0x09, X),
> >       SCA3300_INCLI_CHANNEL(SCA3300_INCLI_Y, 0x0A, Y),
> >       SCA3300_INCLI_CHANNEL(SCA3300_INCLI_Z, 0x0B, Z),
> > +     IIO_CHAN_SOFT_TIMESTAMP(7),
> >  };
> >
> >  static const unsigned long sca3300_scan_masks[] = { @@ -184,7 +192,9
> > @@ struct sca3300_chip_info {
> >   * @spi: SPI device structure
> >   * @lock: Data buffer lock
> >   * @chip: Sensor chip specific information
> > - * @scan: Triggered buffer. Four channel 16-bit data + 64-bit
> > timestamp
> > + * @buffer: Triggered buffer:
> > + *          -SCA3300: 4 channel 16-bit data + 64-bit timestamp
> > + *          -SCL3300: 7 channel 16-bit data + 64-bit timestamp
> >   * @txbuf: Transmit buffer
> >   * @rxbuf: Receive buffer
> >   */
> > @@ -192,10 +202,7 @@ struct sca3300_data {
> >       struct spi_device *spi;
> >       struct mutex lock;
> >       const struct sca3300_chip_info *chip;
> > -     struct {
> > -             s16 channels[4];
> > -             s64 ts __aligned(sizeof(s64));
> > -     } scan;
> > +     u8 buffer[SCA3300_MAX_BUFFER_SIZE] __aligned(sizeof(s64));
> >       u8 txbuf[4] __aligned(IIO_DMA_MINALIGN);
> >       u8 rxbuf[4];
> >  };
> > @@ -484,21 +491,21 @@ static irqreturn_t sca3300_trigger_handler(int irq,
> void *p)
> >       struct iio_dev *indio_dev = pf->indio_dev;
> >       struct sca3300_data *data = iio_priv(indio_dev);
> >       int bit, ret, val, i = 0;
> > +     s16 *channels = (s16 *)data->buffer;
> >
> >       for_each_set_bit(bit, indio_dev->active_scan_mask,
> >                        indio_dev->masklength) {
> > -             ret = sca3300_read_reg(data,
> sca3300_channels[bit].address,
> > -                                    &val);
> > +             ret = sca3300_read_reg(data,
> > + indio_dev->channels[bit].address, &val);
> >               if (ret) {
> >                       dev_err_ratelimited(&data->spi->dev,
> >                               "failed to read register, error: %d\n",
> ret);
> >                       /* handled, but bailing out due to errors */
> >                       goto out;
> >               }
> > -             data->scan.channels[i++] = val;
> > +             channels[i++] = val;
> >       }
> >
> > -     iio_push_to_buffers_with_timestamp(indio_dev, &data->scan,
> > +     iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
> >
> iio_get_time_ns(indio_dev));
> >  out:
> >       iio_trigger_notify_done(indio_dev->trig);


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH V1 1/1] iio: accel: sca3300: Extend the trigger buffer from 16 to 32 bytes
  2022-07-04  1:01     ` LI Qingwu
@ 2022-07-19  8:39       ` Jonathan Cameron
  0 siblings, 0 replies; 5+ messages in thread
From: Jonathan Cameron @ 2022-07-19  8:39 UTC (permalink / raw)
  To: LI Qingwu
  Cc: lars, tomas.melin, nuno.sa, linux-iio, linux-kernel,
	GEO-CHHER-bsp-development

On Mon, 4 Jul 2022 01:01:46 +0000
LI Qingwu <qing-wu.li@leica-geosystems.com.cn> wrote:

> > -----Original Message-----
> > From: Jonathan Cameron <jic23@kernel.org>
> > Sent: Saturday, July 2, 2022 12:46 AM
> > To: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
> > Cc: lars@metafoo.de; tomas.melin@vaisala.com; nuno.sa@analog.com;
> > linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org;
> > GEO-CHHER-bsp-development <bsp-development.geo@leica-geosystems.com>
> > Subject: Re: [PATCH V1 1/1] iio: accel: sca3300: Extend the trigger buffer from
> > 16 to 32 bytes
> > 
> > [Some people who received this message don't often get email from
> > jic23@kernel.org. Learn why this is important at
> > https://aka.ms/LearnAboutSenderIdentification ]
> > 
> > This email is not from Hexagon's Office 365 instance. Please be careful while
> > clicking links, opening attachments, or replying to this email.
> > 
> > 
> > On Fri,  1 Jul 2022 02:30:30 +0000
> > LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn> wrote:
> >   
> > > After added inclination angle channels, the trigger buffer size is
> > > insufficient. Extend the buffer size from 16 to 32 bytes, and change
> > > the trigger buffer from the struct to a u8 array to adapt the sensor
> > > with/without inclination angles output.
> > > New trigger buffer data:
> > >   - SCA3300: 3 accel channels, temp, and timestamp.
> > >   - SCL3300: 3 accel channels, temp, 3 incli channels, and timestamp.
> > > Readjustment the scan index to make it consistent with the buffer data.
> > >
> > > Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>  
> > 
> > Hi.
> > 
> > Looks good. A trivial suggestion inline to make the code a little more 'self
> > documenting'.  It's a minor change so if you are happy with the suggestion I
> > can tweak that whilst applying.
> > 
> > Thanks,
> > 
> > Jonathan
> >   
> 
> Yes, sure, absolutely agree, Thank you!
Looks like I should be able to sneak in a last pull request for this cycle
before the merge window (it was delayed a week).  Hence I've applied this
to the togreg branch of iio.git 

Thanks,

Jonathan

> 
> LI Qingwu
> 
> > > ---
> > >  drivers/iio/accel/sca3300.c | 29 ++++++++++++++++++-----------
> > >  1 file changed, 18 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/iio/accel/sca3300.c b/drivers/iio/accel/sca3300.c
> > > index 3c4827bfef53..820dfb635bf1 100644
> > > --- a/drivers/iio/accel/sca3300.c
> > > +++ b/drivers/iio/accel/sca3300.c
> > > @@ -47,12 +47,20 @@
> > >  #define SCL3300_REG_ANG_CTRL 0x0C
> > >  #define SCL3300_ANG_ENABLE   0x1F
> > >
> > > +/*
> > > + * Buffer size max case:
> > > + * Three accel channels, two bytes per channel.
> > > + * Temperature channel, two bytes.
> > > + * Three incli channels, two bytes per channel.
> > > + * Timestamp channel, eight bytes.
> > > + */
> > > +#define SCA3300_MAX_BUFFER_SIZE (ALIGN(2 * 7, sizeof(s64)) +
> > > +sizeof(s64))  
> > 
> > Instead of the 2 use sizeof(s16)
> >  Also now you don't have timestamp in your enum sca3000_scan_indexes you
> > could add a 'tail' element to the enum such as SCA3000_SCAN_MAX then use
> > that instead of the 7 here.  Hopefully that would make this more self
> > documenting.  
> 
> 
> >   
> > > +
> > >  enum sca3300_scan_indexes {
> > >       SCA3300_ACC_X = 0,
> > >       SCA3300_ACC_Y,
> > >       SCA3300_ACC_Z,
> > >       SCA3300_TEMP,
> > > -     SCA3300_TIMESTAMP,
> > >       SCA3300_INCLI_X,
> > >       SCA3300_INCLI_Y,
> > >       SCA3300_INCLI_Z,
> > > @@ -140,10 +148,10 @@ static const struct iio_chan_spec scl3300_channels[]  
> > = {  
> > >       SCA3300_ACCEL_CHANNEL(SCA3300_ACC_Y, 0x2, Y),
> > >       SCA3300_ACCEL_CHANNEL(SCA3300_ACC_Z, 0x3, Z),
> > >       SCA3300_TEMP_CHANNEL(SCA3300_TEMP, 0x05),
> > > -     IIO_CHAN_SOFT_TIMESTAMP(4),
> > >       SCA3300_INCLI_CHANNEL(SCA3300_INCLI_X, 0x09, X),
> > >       SCA3300_INCLI_CHANNEL(SCA3300_INCLI_Y, 0x0A, Y),
> > >       SCA3300_INCLI_CHANNEL(SCA3300_INCLI_Z, 0x0B, Z),
> > > +     IIO_CHAN_SOFT_TIMESTAMP(7),
> > >  };
> > >
> > >  static const unsigned long sca3300_scan_masks[] = { @@ -184,7 +192,9
> > > @@ struct sca3300_chip_info {
> > >   * @spi: SPI device structure
> > >   * @lock: Data buffer lock
> > >   * @chip: Sensor chip specific information
> > > - * @scan: Triggered buffer. Four channel 16-bit data + 64-bit
> > > timestamp
> > > + * @buffer: Triggered buffer:
> > > + *          -SCA3300: 4 channel 16-bit data + 64-bit timestamp
> > > + *          -SCL3300: 7 channel 16-bit data + 64-bit timestamp
> > >   * @txbuf: Transmit buffer
> > >   * @rxbuf: Receive buffer
> > >   */
> > > @@ -192,10 +202,7 @@ struct sca3300_data {
> > >       struct spi_device *spi;
> > >       struct mutex lock;
> > >       const struct sca3300_chip_info *chip;
> > > -     struct {
> > > -             s16 channels[4];
> > > -             s64 ts __aligned(sizeof(s64));
> > > -     } scan;
> > > +     u8 buffer[SCA3300_MAX_BUFFER_SIZE] __aligned(sizeof(s64));
> > >       u8 txbuf[4] __aligned(IIO_DMA_MINALIGN);
> > >       u8 rxbuf[4];
> > >  };
> > > @@ -484,21 +491,21 @@ static irqreturn_t sca3300_trigger_handler(int irq,  
> > void *p)  
> > >       struct iio_dev *indio_dev = pf->indio_dev;
> > >       struct sca3300_data *data = iio_priv(indio_dev);
> > >       int bit, ret, val, i = 0;
> > > +     s16 *channels = (s16 *)data->buffer;
> > >
> > >       for_each_set_bit(bit, indio_dev->active_scan_mask,
> > >                        indio_dev->masklength) {
> > > -             ret = sca3300_read_reg(data,  
> > sca3300_channels[bit].address,  
> > > -                                    &val);
> > > +             ret = sca3300_read_reg(data,
> > > + indio_dev->channels[bit].address, &val);
> > >               if (ret) {
> > >                       dev_err_ratelimited(&data->spi->dev,
> > >                               "failed to read register, error: %d\n",  
> > ret);  
> > >                       /* handled, but bailing out due to errors */
> > >                       goto out;
> > >               }
> > > -             data->scan.channels[i++] = val;
> > > +             channels[i++] = val;
> > >       }
> > >
> > > -     iio_push_to_buffers_with_timestamp(indio_dev, &data->scan,
> > > +     iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
> > >  
> > iio_get_time_ns(indio_dev));  
> > >  out:
> > >       iio_trigger_notify_done(indio_dev->trig);  
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-07-19  8:29 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-01  2:30 [PATCH V1 0/1] iio: accel: sca3300: Extend the trigger buffer LI Qingwu
2022-07-01  2:30 ` [PATCH V1 1/1] iio: accel: sca3300: Extend the trigger buffer from 16 to 32 bytes LI Qingwu
2022-07-01 16:46   ` Jonathan Cameron
2022-07-04  1:01     ` LI Qingwu
2022-07-19  8:39       ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).