Re: [PATCH 4/4] iio: imu: st_lsm6dsx: add FIFO ops data structure

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> On Wed, 27 Sep 2017 21:29:28 +0200
> Lorenzo Bianconi <lorenzo.bianconi83@xxxxxxxxx> wrote:
>
>> Introduce FIFO ops data structure to contain FIFO related parameter
>> in order to properly support more devices in st_lsm6dsx driver
>>
>> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@xxxxxx>
>
> One nitpick inline.

Ack, I will fix in v2.

>
> Normally I'm reluctant to take refactors that are required for new support
> until the new support is also posted, but these are straight forward and
> it is obvious why they might be made.  Plus you deliver when you way
> you are going to send new support - so I'll make an exception if you
> give me a vague timescale for expected new support and it is fairly short...
>
> Jonathan
>

Honestly I have no an answer at the moment, that patchset is a
preliminary work and it works properly on current supported devices.
Up to you, if you want I can send a v2 now or I can queue it up and
send it later with other patches.

Regards,
Lorenzo

>> ---
>>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h        | 23 ++++++++++++++++--
>>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 23 ++++++++----------
>>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c   | 33 ++++++++++++++++++++++++++
>>  3 files changed, 64 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
>> index 052db1fbb46e..57c7e8e1f915 100644
>> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
>> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
>> @@ -29,8 +29,6 @@ enum st_lsm6dsx_hw_id {
>>
>>  #define ST_LSM6DSX_CHAN_SIZE         2
>>  #define ST_LSM6DSX_SAMPLE_SIZE               6
>> -#define ST_LSM6DSX_SAMPLE_DEPTH              (ST_LSM6DSX_SAMPLE_SIZE / \
>> -                                      ST_LSM6DSX_CHAN_SIZE)
>>
>>  #if defined(CONFIG_SPI_MASTER)
>>  #define ST_LSM6DSX_RX_MAX_LENGTH     256
>> @@ -52,18 +50,39 @@ struct st_lsm6dsx_reg {
>>       u8 mask;
>>  };
>>
>> +/**
>> + * struct st_lsm6dsx_fifo_ops - ST IMU FIFO settings
>> + * @fifo_th: FIFO threshold register info (addr + mask).
>> + * @fifo_diff: FIFO diff status register info (addr + mask).
>> + * @th_wl: FIFO threshold word length.
>> + *
>
> Drop this blank line.. Doesn't add anything useful ;)
>
>> + */
>> +struct st_lsm6dsx_fifo_ops {
>> +     struct {
>> +             u8 addr;
>> +             u16 mask;
>> +     } fifo_th;
>> +     struct {
>> +             u8 addr;
>> +             u16 mask;
>> +     } fifo_diff;
>> +     u8 th_wl;
>> +};
>> +
>>  /**
>>   * struct st_lsm6dsx_settings - ST IMU sensor settings
>>   * @wai: Sensor WhoAmI default value.
>>   * @max_fifo_size: Sensor max fifo length in FIFO words.
>>   * @id: List of hw id supported by the driver configuration.
>>   * @decimator: List of decimator register info (addr + mask).
>> + * @fifo_ops: Sensor hw FIFO parameters.
>>   */
>>  struct st_lsm6dsx_settings {
>>       u8 wai;
>>       u16 max_fifo_size;
>>       enum st_lsm6dsx_hw_id id[ST_LSM6DSX_MAX_ID];
>>       struct st_lsm6dsx_reg decimator[ST_LSM6DSX_MAX_ID];
>> +     struct st_lsm6dsx_fifo_ops fifo_ops;
>>  };
>>
>>  enum st_lsm6dsx_sensor_id {
>> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
>> index cb4f8558a98f..755c472e8a05 100644
>> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
>> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
>> @@ -35,9 +35,6 @@
>>
>>  #include "st_lsm6dsx.h"
>>
>> -#define ST_LSM6DSX_REG_FIFO_THL_ADDR         0x06
>> -#define ST_LSM6DSX_REG_FIFO_THH_ADDR         0x07
>> -#define ST_LSM6DSX_FIFO_TH_MASK                      GENMASK(11, 0)
>>  #define ST_LSM6DSX_REG_HLACTIVE_ADDR         0x12
>>  #define ST_LSM6DSX_REG_HLACTIVE_MASK         BIT(5)
>>  #define ST_LSM6DSX_REG_PP_OD_ADDR            0x12
>> @@ -45,8 +42,6 @@
>>  #define ST_LSM6DSX_REG_FIFO_MODE_ADDR                0x0a
>>  #define ST_LSM6DSX_FIFO_MODE_MASK            GENMASK(2, 0)
>>  #define ST_LSM6DSX_FIFO_ODR_MASK             GENMASK(6, 3)
>> -#define ST_LSM6DSX_REG_FIFO_DIFFL_ADDR               0x3a
>> -#define ST_LSM6DSX_FIFO_DIFF_MASK            GENMASK(11, 0)
>>  #define ST_LSM6DSX_FIFO_EMPTY_MASK           BIT(12)
>>  #define ST_LSM6DSX_REG_FIFO_OUTL_ADDR                0x3e
>>
>> @@ -165,7 +160,7 @@ static int st_lsm6dsx_set_fifo_odr(struct st_lsm6dsx_sensor *sensor,
>>
>>  int st_lsm6dsx_update_watermark(struct st_lsm6dsx_sensor *sensor, u16 watermark)
>>  {
>> -     u16 fifo_watermark = ~0, cur_watermark, sip = 0;
>> +     u16 fifo_watermark = ~0, cur_watermark, sip = 0, fifo_th_mask;
>>       struct st_lsm6dsx_hw *hw = sensor->hw;
>>       struct st_lsm6dsx_sensor *cur_sensor;
>>       __le16 wdata;
>> @@ -190,20 +185,21 @@ int st_lsm6dsx_update_watermark(struct st_lsm6dsx_sensor *sensor, u16 watermark)
>>
>>       fifo_watermark = max_t(u16, fifo_watermark, sip);
>>       fifo_watermark = (fifo_watermark / sip) * sip;
>> -     fifo_watermark = fifo_watermark * ST_LSM6DSX_SAMPLE_DEPTH;
>> +     fifo_watermark = fifo_watermark * hw->settings->fifo_ops.th_wl;
>>
>>       mutex_lock(&hw->lock);
>>
>> -     err = hw->tf->read(hw->dev, ST_LSM6DSX_REG_FIFO_THH_ADDR,
>> +     err = hw->tf->read(hw->dev, hw->settings->fifo_ops.fifo_th.addr + 1,
>>                          sizeof(data), &data);
>>       if (err < 0)
>>               goto out;
>>
>> -     fifo_watermark = ((data << 8) & ~ST_LSM6DSX_FIFO_TH_MASK) |
>> -                      (fifo_watermark & ST_LSM6DSX_FIFO_TH_MASK);
>> +     fifo_th_mask = hw->settings->fifo_ops.fifo_th.mask;
>> +     fifo_watermark = ((data << 8) & ~fifo_th_mask) |
>> +                      (fifo_watermark & fifo_th_mask);
>>
>>       wdata = cpu_to_le16(fifo_watermark);
>> -     err = hw->tf->write(hw->dev, ST_LSM6DSX_REG_FIFO_THL_ADDR,
>> +     err = hw->tf->write(hw->dev, hw->settings->fifo_ops.fifo_th.addr,
>>                           sizeof(wdata), (u8 *)&wdata);
>>  out:
>>       mutex_unlock(&hw->lock);
>> @@ -222,6 +218,7 @@ int st_lsm6dsx_update_watermark(struct st_lsm6dsx_sensor *sensor, u16 watermark)
>>  static int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
>>  {
>>       u16 fifo_len, pattern_len = hw->sip * ST_LSM6DSX_SAMPLE_SIZE;
>> +     u16 fifo_diff_mask = hw->settings->fifo_ops.fifo_diff.mask;
>>       int err, acc_sip, gyro_sip, read_len, samples, offset;
>>       struct st_lsm6dsx_sensor *acc_sensor, *gyro_sensor;
>>       s64 acc_ts, acc_delta_ts, gyro_ts, gyro_delta_ts;
>> @@ -229,7 +226,7 @@ static int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
>>       u8 buff[pattern_len];
>>       __le16 fifo_status;
>>
>> -     err = hw->tf->read(hw->dev, ST_LSM6DSX_REG_FIFO_DIFFL_ADDR,
>> +     err = hw->tf->read(hw->dev, hw->settings->fifo_ops.fifo_diff.addr,
>>                          sizeof(fifo_status), (u8 *)&fifo_status);
>>       if (err < 0)
>>               return err;
>> @@ -237,7 +234,7 @@ static int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
>>       if (fifo_status & cpu_to_le16(ST_LSM6DSX_FIFO_EMPTY_MASK))
>>               return 0;
>>
>> -     fifo_len = (le16_to_cpu(fifo_status) & ST_LSM6DSX_FIFO_DIFF_MASK) *
>> +     fifo_len = (le16_to_cpu(fifo_status) & fifo_diff_mask) *
>>                  ST_LSM6DSX_CHAN_SIZE;
>>       samples = fifo_len / ST_LSM6DSX_SAMPLE_SIZE;
>>       fifo_len = (fifo_len / pattern_len) * pattern_len;
>> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
>> index 4532671df1be..239c735242be 100644
>> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
>> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
>> @@ -168,6 +168,17 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>>                               .mask = GENMASK(5, 3),
>>                       },
>>               },
>> +             .fifo_ops = {
>> +                     .fifo_th = {
>> +                             .addr = 0x06,
>> +                             .mask = GENMASK(11, 0),
>> +                     },
>> +                     .fifo_diff = {
>> +                             .addr = 0x3a,
>> +                             .mask = GENMASK(11, 0),
>> +                     },
>> +                     .th_wl = 3, /* 1LSB = 2B */
>> +             },
>>       },
>>       {
>>               .wai = 0x69,
>> @@ -185,6 +196,17 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>>                               .mask = GENMASK(5, 3),
>>                       },
>>               },
>> +             .fifo_ops = {
>> +                     .fifo_th = {
>> +                             .addr = 0x06,
>> +                             .mask = GENMASK(11, 0),
>> +                     },
>> +                     .fifo_diff = {
>> +                             .addr = 0x3a,
>> +                             .mask = GENMASK(11, 0),
>> +                     },
>> +                     .th_wl = 3, /* 1LSB = 2B */
>> +             },
>>       },
>>       {
>>               .wai = 0x6a,
>> @@ -203,6 +225,17 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>>                               .mask = GENMASK(5, 3),
>>                       },
>>               },
>> +             .fifo_ops = {
>> +                     .fifo_th = {
>> +                             .addr = 0x06,
>> +                             .mask = GENMASK(11, 0),
>> +                     },
>> +                     .fifo_diff = {
>> +                             .addr = 0x3a,
>> +                             .mask = GENMASK(11, 0),
>> +                     },
>> +                     .th_wl = 3, /* 1LSB = 2B */
>> +             },
>>       },
>>  };
>>
>



-- 
UNIX is Sexy: who | grep -i blonde | talk; cd ~; wine; talk; touch;
unzip; touch; strip; gasp; finger; gasp; mount; fsck; more; yes; gasp;
umount; make clean; sleep
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux