> On Mon, 3 Jul 2017 20:07:11 +0200
> Lorenzo Bianconi <lorenzo.bianconi83@xxxxxxxxx> wrote:
>
>> Add sensor interface mode (SIM) lookup table to support devices
>> (like LSM303AGR accel sensor) that support just SPI-3wire
>> communication mode. SIM mode has to be configured before any
>> other operation since it is not enabled by default and the driver
>> is not able to read without that configuration
>>
>> Fixes: ddc05fa28606 (iio: st-accel: add support for lsm303agr accel)
>> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@xxxxxx>
> Just checked and the 3-wire device tree property is documented in spi-bus.txt
>
> So, that's all fine. However, I'm not keen on introducing a special look up
> table for this when we already have a convenient one covering all the other
> per device registers etc.
Hi Jonathan,
thanks for the review
>
> Another approach suggested inline...
>> ---
>> drivers/iio/accel/st_accel_core.c | 37 +++++++++++++++++++++++++
>> drivers/iio/common/st_sensors/st_sensors_core.c | 36 ++++++++++++++++++++++++
>> include/linux/iio/common/st_sensors.h | 11 ++++++++
>> include/linux/platform_data/st_sensors_pdata.h | 2 ++
>> 4 files changed, 86 insertions(+)
>>
>> diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
>> index 07d1489cd457..195966e82516 100644
>> --- a/drivers/iio/accel/st_accel_core.c
>> +++ b/drivers/iio/accel/st_accel_core.c
>> @@ -619,6 +619,38 @@ static const struct st_sensor_settings st_accel_sensors_settings[] = {
>> },
>> };
>>
>> +static const struct st_sensor_sim st_accel_sim_table[] = {
>> + {
>> + .sensors = {
>> + [0] = LSM303AGR_ACCEL_DEV_NAME,
>> + [1] = LIS3DH_ACCEL_DEV_NAME,
>> + [2] = LIS2DH12_ACCEL_DEV_NAME,
>> + [3] = LIS331DLH_ACCEL_DEV_NAME,
>> + [4] = LNG2DM_ACCEL_DEV_NAME,
>> + [5] = LSM330D_ACCEL_DEV_NAME,
>> + [6] = LSM330DL_ACCEL_DEV_NAME,
>> + [7] = LSM330DLC_ACCEL_DEV_NAME,
> The index has no meaning as we really have an unordered
> list so perhaps drop it and let it automatically place them.
Ack
>
> Doing it by name is particularly ugly given we already
> have a big look up table for device differences.
>
> One option would be to split the st_sensors_check_device_support
> function into two parts - the first does the match by name
> and the second does the wai check. That way you could
> then stick your configuration in between the two and
> put this info in with all the other device specific
> characteristics.
>
> To make the transition easy you could add the two split
> functions and call them from the original. Then you can
> introduce the split as needed into the various st sensor
> drivers.
Fine. I guess it is enough to add a static version of
st_sensors_init_interface_mode
with a reference to the right entry in st_sensor_settings from the top
half of st_sensors_check_device_support.
If the everything is ok with SIM configuration we will go forward to
wai check in st_sensors_check_device_support bottom half.
Maybe it is not necessary to split st_sensors_check_device_support in
two separate functions. Do you agree?
Regards,
Lorenzo
>> + },
>> + .addr = 0x23,
>> + .val = BIT(0),
>> + },
>> + {
>> + .sensors = {
>> + [0] = LSM330_ACCEL_DEV_NAME,
>> + },
>> + .addr = 0x24,
>> + .val = BIT(0),
>> + },
>> + {
>> + .sensors = {
>> + [0] = LIS3L02DQ_ACCEL_DEV_NAME,
>> + [1] = LIS3LV02DL_ACCEL_DEV_NAME,
>> + },
>> + .addr = 0x21,
>> + .val = BIT(1),
>> + },
>> +};
>> +
>> static int st_accel_read_raw(struct iio_dev *indio_dev,
>> struct iio_chan_spec const *ch, int *val,
>> int *val2, long mask)
>> @@ -719,6 +751,11 @@ int st_accel_common_probe(struct iio_dev *indio_dev)
>> indio_dev->info = &accel_info;
>> mutex_init(&adata->tb.buf_lock);
>>
>> + err = st_sensors_init_interface_mode(indio_dev, st_accel_sim_table,
>> + ARRAY_SIZE(st_accel_sim_table));
>> + if (err < 0)
>> + return err;
>> +
>> err = st_sensors_power_enable(indio_dev);
>> if (err)
>> return err;
>> diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
>> index 274868100fd0..50c281b6286d 100644
>> --- a/drivers/iio/common/st_sensors/st_sensors_core.c
>> +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
>> @@ -384,6 +384,42 @@ static struct st_sensors_platform_data *st_sensors_of_probe(struct device *dev,
>> }
>> #endif
>>
>> +int st_sensors_init_interface_mode(struct iio_dev *indio_dev,
>> + const struct st_sensor_sim *sim_table,
>> + int sim_len)
>> +{
>> + struct st_sensor_data *sdata = iio_priv(indio_dev);
>> + struct device_node *np = sdata->dev->of_node;
>> + struct st_sensors_platform_data *pdata;
>> + int err = 0;
>> +
>> + pdata = (struct st_sensors_platform_data *)sdata->dev->platform_data;
>> + if ((np && of_property_read_bool(np, "spi-3wire")) ||
>> + (pdata && pdata->spi_3wire)) {
>> + int i, j;
>> +
>> + for (i = 0; i < sim_len; i++) {
>> + for (j = 0; j < ST_SENSORS_MAX_SIM; j++) {
>> + if (!strcmp(sim_table[i].sensors[j],
>> + indio_dev->name))
>> + break;
>> + }
>> + if (j < ST_SENSORS_MAX_SIM)
>> + break;
>> + }
>> +
>> + if (i == sim_len)
>> + return -EINVAL;
>> +
>> + err = sdata->tf->write_byte(&sdata->tb, sdata->dev,
>> + sim_table[i].addr,
>> + sim_table[i].val);
>> + }
>> +
>> + return err;
>> +}
>> +EXPORT_SYMBOL(st_sensors_init_interface_mode);
>> +
>> int st_sensors_init_sensor(struct iio_dev *indio_dev,
>> struct st_sensors_platform_data *pdata)
>> {
>> diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
>> index 1f8211b6438b..15ce0cb360d7 100644
>> --- a/include/linux/iio/common/st_sensors.h
>> +++ b/include/linux/iio/common/st_sensors.h
>> @@ -41,6 +41,7 @@
>>
>> #define ST_SENSORS_MAX_NAME 17
>> #define ST_SENSORS_MAX_4WAI 7
>> +#define ST_SENSORS_MAX_SIM 9
>>
>> #define ST_SENSORS_LSM_CHANNELS(device_type, mask, index, mod, \
>> ch2, s, endian, rbits, sbits, addr) \
>> @@ -105,6 +106,12 @@ struct st_sensor_fullscale {
>> struct st_sensor_fullscale_avl fs_avl[ST_SENSORS_FULLSCALE_AVL_MAX];
>> };
>>
>> +struct st_sensor_sim {
>> + char sensors[ST_SENSORS_MAX_SIM][ST_SENSORS_MAX_NAME];
>> + u8 addr;
>> + u8 val;
>> +};
>> +
>> /**
>> * struct st_sensor_bdu - ST sensor device block data update
>> * @addr: address of the register.
>> @@ -325,6 +332,10 @@ ssize_t st_sensors_sysfs_sampling_frequency_avail(struct device *dev,
>> ssize_t st_sensors_sysfs_scale_avail(struct device *dev,
>> struct device_attribute *attr, char *buf);
>>
>> +int st_sensors_init_interface_mode(struct iio_dev *indio_dev,
>> + const struct st_sensor_sim *sim_table,
>> + int sim_len);
>> +
>> #ifdef CONFIG_OF
>> void st_sensors_of_name_probe(struct device *dev,
>> const struct of_device_id *match,
>> diff --git a/include/linux/platform_data/st_sensors_pdata.h b/include/linux/platform_data/st_sensors_pdata.h
>> index 79b0e4cdb814..f8274b0c6888 100644
>> --- a/include/linux/platform_data/st_sensors_pdata.h
>> +++ b/include/linux/platform_data/st_sensors_pdata.h
>> @@ -17,10 +17,12 @@
>> * Available only for accelerometer and pressure sensors.
>> * Accelerometer DRDY on LSM330 available only on pin 1 (see datasheet).
>> * @open_drain: set the interrupt line to be open drain if possible.
>> + * @spi_3wire: enable spi-3wire mode.
>> */
>> struct st_sensors_platform_data {
>> u8 drdy_int_pin;
>> bool open_drain;
>> + bool spi_3wire;
>> };
>>
>> #endif /* ST_SENSORS_PDATA_H */
>
--
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
