Re: [PATCH 1/5] iio: st-sensors: add configuration for WhoAmI address

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

 



Hi Giuseppe, Jonathan,

my comments inline.


On 07/19/2015 07:03 PM, Jonathan Cameron wrote:
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;
found_it is very ugly :). No need to use the variable, if (n != ST_SENSORS_MAX_4WAI) it means sensor found.

+	}
+	if (!found_it) {
here just if (i == num_sensors_list)

+		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;
No need. All sensors must be filled with wai_addr in the static array. You forget to fill pressure sensors...

+
  	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.c
index 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



[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