Re: [PATCH 1/1] drivers: imu: adis16475.c: Remove scan index from delta channels

[Jonathan Cameron]

On Thu, 12 Oct 2023 17:35:02 +0200
Nuno Sá <noname.nuno@xxxxxxxxx> wrote:

> On Thu, 2023-10-12 at 17:09 +0200, Nuno Sá wrote:
> > Hi Ramona,
> > 
> > There are some odd stuff...
> > 
> > On Thu, 2023-10-12 at 15:24 +0300, Ramona Gradinariu wrote:  
> > > Some devices do not support delta angle and delta velocity
> > > burst readings, which means there should be no buffer support
> > > for these types of channels.
> > > A new list of channels is created which contains the delta
> > > channels structures with the scan index equal to -1 to allow
> > > for raw register readings, without buffer support.
> > > This list of channels is assigned to the devices which
> > > do not support delta angle and delta velocity burst
> > > readings.
> > > 
> > > Fixes: 8f6bc87d67c0 ("iio: imu: adis16475.c: Add delta angle and delta velocity
> > > channels")
> > > Signed-off-by: Ramona Gradinariu <ramona.gradinariu@xxxxxxxxxx>
> > > ---
> > >  drivers/iio/imu/adis16475.c | 67 +++++++++++++++++++++++++------------
> > >  1 file changed, 46 insertions(+), 21 deletions(-)
> > > 
> > > diff --git a/drivers/iio/imu/adis16475.c b/drivers/iio/imu/adis16475.c
> > > index 9af07fec0d89..b7cbe1565aee 100644
> > > --- a/drivers/iio/imu/adis16475.c
> > > +++ b/drivers/iio/imu/adis16475.c
> > > @@ -604,7 +604,15 @@ static int adis16475_write_raw(struct iio_dev *indio_dev,
> > >         ADIS16475_MOD_CHAN_DELTA(IIO_DELTA_VELOCITY, IIO_MOD_ ## _mod, \
> > >                            ADIS16475_REG_ ## _mod ## _DELTVEL_L,
> > > ADIS16475_SCAN_DELTVEL_ ## _mod, 32, 32)
> > >  
> > > -static const struct iio_chan_spec adis16475_channels[] = {
> > > +#define ADIS16475_DELTANG_CHAN_NO_SCAN(_mod) \
> > > +       ADIS16475_MOD_CHAN_DELTA(IIO_DELTA_ANGL, IIO_MOD_ ## _mod, \
> > > +                          ADIS16475_REG_ ## _mod ## _DELTANG_L, -1, 32, 32)
> > > +
> > > +#define ADIS16475_DELTVEL_CHAN_NO_SCAN(_mod) \
> > > +       ADIS16475_MOD_CHAN_DELTA(IIO_DELTA_VELOCITY, IIO_MOD_ ## _mod, \
> > > +                          ADIS16475_REG_ ## _mod ## _DELTVEL_L, -1, 32, 32)
> > > +
> > > +static const struct iio_chan_spec adis16477_channels[] = {
> > >         ADIS16475_GYRO_CHANNEL(X),
> > >         ADIS16475_GYRO_CHANNEL(Y),
> > >         ADIS16475_GYRO_CHANNEL(Z),
> > > @@ -621,6 +629,23 @@ static const struct iio_chan_spec adis16475_channels[] = {
> > >         IIO_CHAN_SOFT_TIMESTAMP(13)
> > >  };
> > >  
> > > +static const struct iio_chan_spec adis16475_channels[] = {  
> > 
> > I guess you mean adis16477_channels?
> >   
> > > +       ADIS16475_GYRO_CHANNEL(X),
> > > +       ADIS16475_GYRO_CHANNEL(Y),
> > > +       ADIS16475_GYRO_CHANNEL(Z),
> > > +       ADIS16475_ACCEL_CHANNEL(X),
> > > +       ADIS16475_ACCEL_CHANNEL(Y),
> > > +       ADIS16475_ACCEL_CHANNEL(Z),
> > > +       ADIS16475_TEMP_CHANNEL(),
> > > +       ADIS16475_DELTANG_CHAN_NO_SCAN(X),
> > > +       ADIS16475_DELTANG_CHAN_NO_SCAN(Y),
> > > +       ADIS16475_DELTANG_CHAN_NO_SCAN(Z),
> > > +       ADIS16475_DELTVEL_CHAN_NO_SCAN(X),
> > > +       ADIS16475_DELTVEL_CHAN_NO_SCAN(Y),
> > > +       ADIS16475_DELTVEL_CHAN_NO_SCAN(Z),
> > > +       IIO_CHAN_SOFT_TIMESTAMP(7)
> > > +};
> > > +
> > >  enum adis16475_variant {
> > >         ADIS16470,
> > >         ADIS16475_1,
> > > @@ -782,8 +807,8 @@ static const struct adis16475_chip_info adis16475_chip_info[]
> > > =
> > > {
> > >         },
> > >         [ADIS16477_1] = {
> > >                 .name = "adis16477-1",
> > > -               .num_channels = ARRAY_SIZE(adis16475_channels),
> > > -               .channels = adis16475_channels,
> > > +               .num_channels = ARRAY_SIZE(adis16477_channels),
> > > +               .channels = adis16477_channels,
> > >                 .gyro_max_val = 1,
> > >                 .gyro_max_scale = IIO_RAD_TO_DEGREE(160 << 16),
> > >                 .accel_max_val = 1,
> > > @@ -800,8 +825,8 @@ static const struct adis16475_chip_info adis16475_chip_info[]
> > > =
> > > {
> > >         },
> > >         [ADIS16477_2] = {
> > >                 .name = "adis16477-2",
> > > -               .num_channels = ARRAY_SIZE(adis16475_channels),
> > > -               .channels = adis16475_channels,
> > > +               .num_channels = ARRAY_SIZE(adis16477_channels),
> > > +               .channels = adis16477_channels,
> > >                 .gyro_max_val = 1,
> > >                 .gyro_max_scale = IIO_RAD_TO_DEGREE(40 << 16),
> > >                 .accel_max_val = 1,
> > > @@ -818,8 +843,8 @@ static const struct adis16475_chip_info adis16475_chip_info[]
> > > =
> > > {
> > >         },
> > >         [ADIS16477_3] = {
> > >                 .name = "adis16477-3",
> > > -               .num_channels = ARRAY_SIZE(adis16475_channels),
> > > -               .channels = adis16475_channels,
> > > +               .num_channels = ARRAY_SIZE(adis16477_channels),
> > > +               .channels = adis16477_channels,
> > >                 .gyro_max_val = 1,
> > >                 .gyro_max_scale = IIO_RAD_TO_DEGREE(10 << 16),
> > >                 .accel_max_val = 1,
> > > @@ -938,8 +963,8 @@ static const struct adis16475_chip_info adis16475_chip_info[]
> > > =
> > > {
> > >         },
> > >         [ADIS16500] = {
> > >                 .name = "adis16500",
> > > -               .num_channels = ARRAY_SIZE(adis16475_channels),
> > > -               .channels = adis16475_channels,
> > > +               .num_channels = ARRAY_SIZE(adis16477_channels),
> > > +               .channels = adis16477_channels,
> > >                 .gyro_max_val = 1,
> > >                 .gyro_max_scale = IIO_RAD_TO_DEGREE(10 << 16),
> > >                 .accel_max_val = 392,
> > > @@ -957,8 +982,8 @@ static const struct adis16475_chip_info adis16475_chip_info[]
> > > =
> > > {
> > >         },
> > >         [ADIS16505_1] = {
> > >                 .name = "adis16505-1",
> > > -               .num_channels = ARRAY_SIZE(adis16475_channels),
> > > -               .channels = adis16475_channels,
> > > +               .num_channels = ARRAY_SIZE(adis16477_channels),
> > > +               .channels = adis16477_channels,  
> > 
> > AFAICS, this supports delta channels in burst mode... right? Please make sure to go
> > again through the datasheets so we make this right...
> >   
> 
> Alright, I just saw I missed the diff renaming adis16475_channels >
> adis16477_channels. MAybe you could state why you did it in the commit message.
> Anyhow:
> 
> Reviewed-by: Nuno Sa <nuno.sa@xxxxxxxxxx>
> 
Applied.