Hi Giuseppe, Jonathan, my comments inline. On 07/19/2015 07:03 PM, Jonathan Cameron wrote:
found_it is very ugly :). No need to use the variable, if (n != ST_SENSORS_MAX_4WAI) it means sensor found.On 16/07/15 10:17, Giuseppe Barba wrote:This patch permits to configure the WhoAmI register address because some device could doesn't have a standard address for this register. Signed-off-by: Giuseppe Barba <giuseppe.barba@xxxxxx>Hmm. I'd be tempted to rename the DEFAULT WAIT ADDRESS as it's clearly only the default in the sense it was there first. Still probably not worth the hassle.. Looks good to me, but will leave this (and the rest of the series) on the list for a few days for others to comment. cc'd Denis for comments. Ideally patches should also cc the reviewers listed in MAINTAINERS (I don't care if you cc me as I have filters set to squash stuff that comes to me and the list into one).drivers/iio/accel/st_accel_core.c | 4 +++ drivers/iio/common/st_sensors/st_sensors_core.c | 44 +++++++++++++++---------- drivers/iio/gyro/st_gyro_core.c | 3 ++ drivers/iio/magnetometer/st_magn_core.c | 2 ++ include/linux/iio/common/st_sensors.h | 2 ++ 5 files changed, 37 insertions(+), 18 deletions(-) diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c index 4002e64..7ce027b 100644 --- a/drivers/iio/accel/st_accel_core.c +++ b/drivers/iio/accel/st_accel_core.c @@ -226,6 +226,7 @@ static const struct iio_chan_spec st_accel_16bit_channels[] = { static const struct st_sensor_settings st_accel_sensors_settings[] = { { .wai = ST_ACCEL_1_WAI_EXP, + .wai_addr = ST_SENSORS_DEFAULT_WAI_ADDRESS, .sensors_supported = { [0] = LIS3DH_ACCEL_DEV_NAME, [1] = LSM303DLHC_ACCEL_DEV_NAME, @@ -297,6 +298,7 @@ static const struct st_sensor_settings st_accel_sensors_settings[] = { }, { .wai = ST_ACCEL_2_WAI_EXP, + .wai_addr = ST_SENSORS_DEFAULT_WAI_ADDRESS, .sensors_supported = { [0] = LIS331DLH_ACCEL_DEV_NAME, [1] = LSM303DL_ACCEL_DEV_NAME, @@ -359,6 +361,7 @@ static const struct st_sensor_settings st_accel_sensors_settings[] = { }, { .wai = ST_ACCEL_3_WAI_EXP, + .wai_addr = ST_SENSORS_DEFAULT_WAI_ADDRESS, .sensors_supported = { [0] = LSM330_ACCEL_DEV_NAME, }, @@ -437,6 +440,7 @@ static const struct st_sensor_settings st_accel_sensors_settings[] = { }, { .wai = ST_ACCEL_4_WAI_EXP, + .wai_addr = ST_SENSORS_DEFAULT_WAI_ADDRESS, .sensors_supported = { [0] = LIS3LV02DL_ACCEL_DEV_NAME, }, diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c index 8086cbc..c0a611e 100644 --- a/drivers/iio/common/st_sensors/st_sensors_core.c +++ b/drivers/iio/common/st_sensors/st_sensors_core.c @@ -479,35 +479,43 @@ int st_sensors_check_device_support(struct iio_dev *indio_dev, int num_sensors_list, const struct st_sensor_settings *sensor_settings) { - u8 wai; int i, n, err; + u8 wai, wai_addr, found_it; struct st_sensor_data *sdata = iio_priv(indio_dev);+ found_it = 0;+ for (i = 0; i < num_sensors_list; i++) { + for (n = 0; n < ST_SENSORS_MAX_4WAI; n++) { + if (strcmp(indio_dev->name, + sensor_settings[i].sensors_supported[n]) == 0) { + found_it = 1; + break; + } + } + if (found_it) + break;
+ } + if (!found_it) {
here just if (i == num_sensors_list)
No need. All sensors must be filled with wai_addr in the static array. You forget to fill pressure sensors...+ dev_err(&indio_dev->dev, "device name %s not recognized.\n", + indio_dev->name); + goto sensor_name_mismatch; + } + + if (sensor_settings[i].wai_addr != 0) + wai_addr = sensor_settings[i].wai_addr; + else + wai_addr = ST_SENSORS_DEFAULT_WAI_ADDRESS;
+ err = sdata->tf->read_byte(&sdata->tb, sdata->dev, - ST_SENSORS_DEFAULT_WAI_ADDRESS, &wai); + wai_addr, &wai); if (err < 0) { dev_err(&indio_dev->dev, "failed to read Who-Am-I register.\n"); goto read_wai_error; }- for (i = 0; i < num_sensors_list; i++) {- if (sensor_settings[i].wai == wai) - break; - } - if (i == num_sensors_list) + if (sensor_settings[i].wai != wai) goto device_not_supported;- for (n = 0; n < ARRAY_SIZE(sensor_settings[i].sensors_supported); n++) {- if (strcmp(indio_dev->name, - &sensor_settings[i].sensors_supported[n][0]) == 0) - break; - } - if (n == ARRAY_SIZE(sensor_settings[i].sensors_supported)) { - dev_err(&indio_dev->dev, "device name \"%s\" and WhoAmI (0x%02x) mismatch", - indio_dev->name, wai); - goto sensor_name_mismatch; - } - sdata->sensor_settings = (struct st_sensor_settings *)&sensor_settings[i];diff --git a/drivers/iio/gyro/st_gyro_core.c b/drivers/iio/gyro/st_gyro_core.cindex ffe9664..4b993a5 100644 --- a/drivers/iio/gyro/st_gyro_core.c +++ b/drivers/iio/gyro/st_gyro_core.c @@ -131,6 +131,7 @@ static const struct iio_chan_spec st_gyro_16bit_channels[] = { static const struct st_sensor_settings st_gyro_sensors_settings[] = { { .wai = ST_GYRO_1_WAI_EXP, + .wai_addr = ST_SENSORS_DEFAULT_WAI_ADDRESS, .sensors_supported = { [0] = L3G4200D_GYRO_DEV_NAME, [1] = LSM330DL_GYRO_DEV_NAME, @@ -190,6 +191,7 @@ static const struct st_sensor_settings st_gyro_sensors_settings[] = { }, { .wai = ST_GYRO_2_WAI_EXP, + .wai_addr = ST_SENSORS_DEFAULT_WAI_ADDRESS, .sensors_supported = { [0] = L3GD20_GYRO_DEV_NAME, [1] = LSM330D_GYRO_DEV_NAME, @@ -252,6 +254,7 @@ static const struct st_sensor_settings st_gyro_sensors_settings[] = { }, { .wai = ST_GYRO_3_WAI_EXP, + .wai_addr = ST_SENSORS_DEFAULT_WAI_ADDRESS, .sensors_supported = { [0] = L3GD20_GYRO_DEV_NAME, }, diff --git a/drivers/iio/magnetometer/st_magn_core.c b/drivers/iio/magnetometer/st_magn_core.c index b4bcfb7..63da293 100644 --- a/drivers/iio/magnetometer/st_magn_core.c +++ b/drivers/iio/magnetometer/st_magn_core.c @@ -268,6 +268,7 @@ static const struct st_sensor_settings st_magn_sensors_settings[] = { }, { .wai = ST_MAGN_1_WAI_EXP, + .wai_addr = ST_SENSORS_DEFAULT_WAI_ADDRESS, .sensors_supported = { [0] = LSM303DLHC_MAGN_DEV_NAME, [1] = LSM303DLM_MAGN_DEV_NAME, @@ -346,6 +347,7 @@ static const struct st_sensor_settings st_magn_sensors_settings[] = { }, { .wai = ST_MAGN_2_WAI_EXP, + .wai_addr = ST_SENSORS_DEFAULT_WAI_ADDRESS, .sensors_supported = { [0] = LIS3MDL_MAGN_DEV_NAME, }, diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h index 2c476ac..f7c77b4 100644 --- a/include/linux/iio/common/st_sensors.h +++ b/include/linux/iio/common/st_sensors.h @@ -166,6 +166,7 @@ struct st_sensor_transfer_function { /** * struct st_sensor_settings - ST specific sensor settings * @wai: Contents of WhoAmI register. + * @wai_addr: The address of WhoAmI register
Forget . at the end.
* @sensors_supported: List of supported sensors by struct itself. * @ch: IIO channels for the sensor. * @odr: Output data rate register and ODR list available. @@ -179,6 +180,7 @@ struct st_sensor_transfer_function { */ struct st_sensor_settings { u8 wai; + u8 wai_addr; char sensors_supported[ST_SENSORS_MAX_4WAI][ST_SENSORS_MAX_NAME]; struct iio_chan_spec *ch; int num_ch;
-- 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