On Fri, 7 Jul 2017 11:12:44 +0200
Lorenzo Bianconi <lorenzo.bianconi83@xxxxxxxxx> wrote:
> > On Wed, 5 Jul 2017 20:30:01 +0200
> > Lorenzo Bianconi <lorenzo.bianconi83@xxxxxxxxx> wrote:
> >
> >> Add SPI Serial Interface Mode (SIM) register information
> >> in st_sensor_settings look up table to support devices
> >> (like LSM303AGR accel sensor) that allow 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>
> > Applied to the fixes-togreg branch and marked for stable.
> >
> > I added a brief note for stable maintainers saying that whilst
> > substantial logic was simple and better to have a proper fix than
> > a work around.
>
> Ack. I was wondering what is a better way to fix that issue. Do you
> mean to not use sensor name for look-up in st_accel_sensors_settings?
> Is my understanding correct?
The only alternative that immediately came to mind as a minimal fix would
be to have enforced 3 wire for the one part that doens't work any other way.
Would have turned it in two a couple of lines.
Adding 3 wire support for other devices is technically a feature enhancement.
Who cares though - fix is good as far as I'm concerned. Up to the stable
maintainers on whether they want to pick this up or not.
Jonathan
> If so we can migrate the lookup in st_{sensor}_sensors_settings to
> st_{sensor}_type like we did in st_lsm6dsx.
> Anyway we can't get any information from the device to enforce lookup
> before configuring the SIM.
>
> Regards,
> Lorenzo
>
> >
> > Jonathan
> >> ---
> >> Changes since v1:
> >> - move SIM register information in st_sensor_settings look up table
> >> - make st_sensors_init_interface_mode static and use name lookup
> >> performed by st_sensors_check_device_support
> >> ---
> >> drivers/iio/accel/st_accel_core.c | 32 +++++++++++++++++++++++++
> >> drivers/iio/common/st_sensors/st_sensors_core.c | 29 ++++++++++++++++++++++
> >> include/linux/iio/common/st_sensors.h | 7 ++++++
> >> include/linux/platform_data/st_sensors_pdata.h | 2 ++
> >> 4 files changed, 70 insertions(+)
> >>
> >> diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
> >> index 07d1489cd457..e44f62bf9caa 100644
> >> --- a/drivers/iio/accel/st_accel_core.c
> >> +++ b/drivers/iio/accel/st_accel_core.c
> >> @@ -166,6 +166,10 @@ static const struct st_sensor_settings st_accel_sensors_settings[] = {
> >> .mask_ihl = 0x02,
> >> .addr_stat_drdy = ST_SENSORS_DEFAULT_STAT_ADDR,
> >> },
> >> + .sim = {
> >> + .addr = 0x23,
> >> + .value = BIT(0),
> >> + },
> >> .multi_read_bit = true,
> >> .bootime = 2,
> >> },
> >> @@ -234,6 +238,10 @@ static const struct st_sensor_settings st_accel_sensors_settings[] = {
> >> .mask_od = 0x40,
> >> .addr_stat_drdy = ST_SENSORS_DEFAULT_STAT_ADDR,
> >> },
> >> + .sim = {
> >> + .addr = 0x23,
> >> + .value = BIT(0),
> >> + },
> >> .multi_read_bit = true,
> >> .bootime = 2,
> >> },
> >> @@ -316,6 +324,10 @@ static const struct st_sensor_settings st_accel_sensors_settings[] = {
> >> .en_mask = 0x08,
> >> },
> >> },
> >> + .sim = {
> >> + .addr = 0x24,
> >> + .value = BIT(0),
> >> + },
> >> .multi_read_bit = false,
> >> .bootime = 2,
> >> },
> >> @@ -379,6 +391,10 @@ static const struct st_sensor_settings st_accel_sensors_settings[] = {
> >> .mask_int1 = 0x04,
> >> .addr_stat_drdy = ST_SENSORS_DEFAULT_STAT_ADDR,
> >> },
> >> + .sim = {
> >> + .addr = 0x21,
> >> + .value = BIT(1),
> >> + },
> >> .multi_read_bit = true,
> >> .bootime = 2, /* guess */
> >> },
> >> @@ -437,6 +453,10 @@ static const struct st_sensor_settings st_accel_sensors_settings[] = {
> >> .mask_od = 0x40,
> >> .addr_stat_drdy = ST_SENSORS_DEFAULT_STAT_ADDR,
> >> },
> >> + .sim = {
> >> + .addr = 0x21,
> >> + .value = BIT(7),
> >> + },
> >> .multi_read_bit = false,
> >> .bootime = 2, /* guess */
> >> },
> >> @@ -499,6 +519,10 @@ static const struct st_sensor_settings st_accel_sensors_settings[] = {
> >> .addr_ihl = 0x22,
> >> .mask_ihl = 0x80,
> >> },
> >> + .sim = {
> >> + .addr = 0x23,
> >> + .value = BIT(0),
> >> + },
> >> .multi_read_bit = true,
> >> .bootime = 2,
> >> },
> >> @@ -547,6 +571,10 @@ static const struct st_sensor_settings st_accel_sensors_settings[] = {
> >> .mask_int1 = 0x04,
> >> .addr_stat_drdy = ST_SENSORS_DEFAULT_STAT_ADDR,
> >> },
> >> + .sim = {
> >> + .addr = 0x21,
> >> + .value = BIT(1),
> >> + },
> >> .multi_read_bit = false,
> >> .bootime = 2,
> >> },
> >> @@ -614,6 +642,10 @@ static const struct st_sensor_settings st_accel_sensors_settings[] = {
> >> .mask_ihl = 0x02,
> >> .addr_stat_drdy = ST_SENSORS_DEFAULT_STAT_ADDR,
> >> },
> >> + .sim = {
> >> + .addr = 0x23,
> >> + .value = BIT(0),
> >> + },
> >> .multi_read_bit = true,
> >> .bootime = 2,
> >> },
> >> diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
> >> index 274868100fd0..d99bb1460fe2 100644
> >> --- a/drivers/iio/common/st_sensors/st_sensors_core.c
> >> +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
> >> @@ -581,6 +581,31 @@ int st_sensors_read_info_raw(struct iio_dev *indio_dev,
> >> }
> >> EXPORT_SYMBOL(st_sensors_read_info_raw);
> >>
> >> +static int st_sensors_init_interface_mode(struct iio_dev *indio_dev,
> >> + const struct st_sensor_settings *sensor_settings)
> >> +{
> >> + struct st_sensor_data *sdata = iio_priv(indio_dev);
> >> + struct device_node *np = sdata->dev->of_node;
> >> + struct st_sensors_platform_data *pdata;
> >> +
> >> + pdata = (struct st_sensors_platform_data *)sdata->dev->platform_data;
> >> + if (((np && of_property_read_bool(np, "spi-3wire")) ||
> >> + (pdata && pdata->spi_3wire)) && sensor_settings->sim.addr) {
> >> + int err;
> >> +
> >> + err = sdata->tf->write_byte(&sdata->tb, sdata->dev,
> >> + sensor_settings->sim.addr,
> >> + sensor_settings->sim.value);
> >> + if (err < 0) {
> >> + dev_err(&indio_dev->dev,
> >> + "failed to init interface mode\n");
> >> + return err;
> >> + }
> >> + }
> >> +
> >> + return 0;
> >> +}
> >> +
> >> int st_sensors_check_device_support(struct iio_dev *indio_dev,
> >> int num_sensors_list,
> >> const struct st_sensor_settings *sensor_settings)
> >> @@ -605,6 +630,10 @@ int st_sensors_check_device_support(struct iio_dev *indio_dev,
> >> return -ENODEV;
> >> }
> >>
> >> + err = st_sensors_init_interface_mode(indio_dev, &sensor_settings[i]);
> >> + if (err < 0)
> >> + return err;
> >> +
> >> if (sensor_settings[i].wai_addr) {
> >> err = sdata->tf->read_byte(&sdata->tb, sdata->dev,
> >> sensor_settings[i].wai_addr, &wai);
> >> diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
> >> index 1f8211b6438b..7b0fa8b5c120 100644
> >> --- a/include/linux/iio/common/st_sensors.h
> >> +++ b/include/linux/iio/common/st_sensors.h
> >> @@ -105,6 +105,11 @@ struct st_sensor_fullscale {
> >> struct st_sensor_fullscale_avl fs_avl[ST_SENSORS_FULLSCALE_AVL_MAX];
> >> };
> >>
> >> +struct st_sensor_sim {
> >> + u8 addr;
> >> + u8 value;
> >> +};
> >> +
> >> /**
> >> * struct st_sensor_bdu - ST sensor device block data update
> >> * @addr: address of the register.
> >> @@ -197,6 +202,7 @@ struct st_sensor_transfer_function {
> >> * @bdu: Block data update register.
> >> * @das: Data Alignment Selection register.
> >> * @drdy_irq: Data ready register of the sensor.
> >> + * @sim: SPI serial interface mode register of the sensor.
> >> * @multi_read_bit: Use or not particular bit for [I2C/SPI] multi-read.
> >> * @bootime: samples to discard when sensor passing from power-down to power-up.
> >> */
> >> @@ -213,6 +219,7 @@ struct st_sensor_settings {
> >> struct st_sensor_bdu bdu;
> >> struct st_sensor_das das;
> >> struct st_sensor_data_ready_irq drdy_irq;
> >> + struct st_sensor_sim sim;
> >> bool multi_read_bit;
> >> unsigned int bootime;
> >> };
> >> 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 */
> >
>
>
>
--
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
