Skip to contentSkip to conversation

bmc150_magn fix and cleanup

PATCH Applied Hartmut Knaack 3 participants 27 messages Jul 17, 2015 – Aug 12, 2015 IIO

lore
Share & tools
lore

Hartmut Knaack

author
#0 lore raw

cover bmc150_magn fix and cleanup

Some issues I spotted during review of this driver. First three are quite obvious problems, the rest may be matter of taste or intended to stay in the original form. Opinions welcome.

Hartmut Knaack (8):
iio:magnetometer:bmc150_magn: add regmap dependency iio:magnetometer:bmc150_magn: sort entry alphabetically iio:magnetometer:bmc150_magn: output intended variable iio:magnetometer:bmc150_magn: replace magic value iio:magnetometer:bmc150_magn: add error handling on gpio probe iio:magnetometer:bmc150_magn: protect runtime_resume with mutex iio:magnetometer:bmc150_magn: expand mutex in trigger_handler iio:magnetometer:bmc150_magn: use descriptive name for mask

 drivers/iio/magnetometer/Kconfig       | 32 ++++++++++++++++++--------------
 drivers/iio/magnetometer/Makefile      |  3 +--
 drivers/iio/magnetometer/bmc150_magn.c | 31 ++++++++++++++++++++++---------
 3 files changed, 41 insertions(+), 25 deletions(-)
Signature · 1 line
2.4.3
author
#1 lore raw

PATCH 1/8 iio:magnetometer:bmc150_magn: add regmap dependency

bmc150_magn makes use of REGMAP_I2C, so select it to build always without errors.

Fixes c91746a2361d ("iio: magn: Add support for BMC150 magnetometer")
Signed-off-by Hartmut Knaack <[email protected]>
---
 drivers/iio/magnetometer/Kconfig | 1 +
 1 file changed, 1 insertion(+)
drivers/iio/magnetometer/Kconfig +1 -0
index dcadfc4f0661..efb9350b0d76 100644
@@ -90,6 +90,7 @@ config IIO_ST_MAGN_SPI_3AXIS
 config BMC150_MAGN
 	tristate "Bosch BMC150 Magnetometer Driver"
 	depends on I2C
+	select REGMAP_I2C
 	select IIO_BUFFER
 	select IIO_TRIGGERED_BUFFER
 	help
Signature · 1 line
2.4.3
maintainer Applied
#2 lore raw

Re: [PATCH 1/8] iio:magnetometer:bmc150_magn: add regmap dependency

Quoted 6 lines · Hartmut Knaack

bmc150_magn makes use of REGMAP_I2C, so select it to build always without errors.

Fixes c91746a2361d ("iio: magn: Add support for BMC150 magnetometer")
Signed-off-by Hartmut Knaack <[email protected]>

Applied to the fixes-togreg branch of iio.git

Thanks,

Jonathan

Quoted 17 lines
---
 drivers/iio/magnetometer/Kconfig | 1 +
 1 file changed, 1 insertion(+)
drivers/iio/magnetometer/Kconfig +1 -0
index dcadfc4f0661..efb9350b0d76 100644
@@ -90,6 +90,7 @@ config IIO_ST_MAGN_SPI_3AXIS
 config BMC150_MAGN
 	tristate "Bosch BMC150 Magnetometer Driver"
 	depends on I2C
+	select REGMAP_I2C
 	select IIO_BUFFER
 	select IIO_TRIGGERED_BUFFER
 	help
author
#3 lore raw

PATCH 2/8 iio:magnetometer:bmc150_magn: sort entry alphabetically

In reply to Hartmut Knaack

Sort the entry for bmc105_magn in Kconfig and Makefile to its correct position. Also add the minor module information for completeness.

Fixes c91746a2361d ("iio: magn: Add support for BMC150 magnetometer")
Signed-off-by Hartmut Knaack <[email protected]>
---
 drivers/iio/magnetometer/Kconfig  | 33 ++++++++++++++++++---------------
 drivers/iio/magnetometer/Makefile |  3 +--
 2 files changed, 19 insertions(+), 17 deletions(-)
drivers/iio/magnetometer/Kconfig +18 -15
index efb9350b0d76..868abada3409 100644
@@ -24,6 +24,24 @@ config AK09911
 	help
 	  Deprecated: AK09911 is now supported by AK8975 driver.
 
+config BMC150_MAGN
+	tristate "Bosch BMC150 Magnetometer Driver"
+	depends on I2C
+	select REGMAP_I2C
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+	help
+	  Say yes here to build support for the BMC150 magnetometer.
+
+	  Currently this only supports the device via an i2c interface.
+
+	  This is a combo module with both accelerometer and magnetometer.
+	  This driver is only implementing magnetometer part, which has
+	  its own address and register map.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called bmc150_magn.
+
 config MAG3110
 	tristate "Freescale MAG3110 3-Axis Magnetometer"
 	depends on I2C

@@ -87,19 +105,4 @@ config IIO_ST_MAGN_SPI_3AXIS
 	depends on IIO_ST_MAGN_3AXIS
 	depends on IIO_ST_SENSORS_SPI
 
-config BMC150_MAGN
-	tristate "Bosch BMC150 Magnetometer Driver"
-	depends on I2C
-	select REGMAP_I2C
-	select IIO_BUFFER
-	select IIO_TRIGGERED_BUFFER
-	help
-	  Say yes here to build support for the BMC150 magnetometer.
-
-	  Currently this only supports the device via an i2c interface.
-
-	  This is a combo module with both accelerometer and magnetometer.
-	  This driver is only implementing magnetometer part, which has
-	  its own address and register map.
-
 endmenu
drivers/iio/magnetometer/Makefile +1 -2
index 33b1d4d54ee7..2c72df458ec2 100644
@@ -4,6 +4,7 @@
 
 # When adding new entries keep the list in alphabetical order
 obj-$(CONFIG_AK8975)	+= ak8975.o
+obj-$(CONFIG_BMC150_MAGN) += bmc150_magn.o
 obj-$(CONFIG_MAG3110)	+= mag3110.o
 obj-$(CONFIG_HID_SENSOR_MAGNETOMETER_3D) += hid-sensor-magn-3d.o
 obj-$(CONFIG_MMC35240)	+= mmc35240.o

@@ -14,5 +15,3 @@ st_magn-$(CONFIG_IIO_BUFFER) += st_magn_buffer.o
 
 obj-$(CONFIG_IIO_ST_MAGN_I2C_3AXIS) += st_magn_i2c.o
 obj-$(CONFIG_IIO_ST_MAGN_SPI_3AXIS) += st_magn_spi.o
-
-obj-$(CONFIG_BMC150_MAGN) += bmc150_magn.o
Signature · 1 line
2.4.3
maintainer
#4 lore raw

Re: [PATCH 2/8] iio:magnetometer:bmc150_magn: sort entry alphabetically

Quoted 6 lines · Hartmut Knaack

Sort the entry for bmc105_magn in Kconfig and Makefile to its correct position. Also add the minor module information for completeness.

Fixes c91746a2361d ("iio: magn: Add support for BMC150 magnetometer")
Signed-off-by Hartmut Knaack <[email protected]>

Technically this isn't a fix so will have to go the slow way. Obviously it's dependent on the previous patch, so if I forget about it do poke me!

Jonathan

Quoted 73 lines
---
 drivers/iio/magnetometer/Kconfig  | 33 ++++++++++++++++++---------------
 drivers/iio/magnetometer/Makefile |  3 +--
 2 files changed, 19 insertions(+), 17 deletions(-)
drivers/iio/magnetometer/Kconfig +18 -15
index efb9350b0d76..868abada3409 100644
@@ -24,6 +24,24 @@ config AK09911
 	help
 	  Deprecated: AK09911 is now supported by AK8975 driver.
 
+config BMC150_MAGN
+	tristate "Bosch BMC150 Magnetometer Driver"
+	depends on I2C
+	select REGMAP_I2C
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+	help
+	  Say yes here to build support for the BMC150 magnetometer.
+
+	  Currently this only supports the device via an i2c interface.
+
+	  This is a combo module with both accelerometer and magnetometer.
+	  This driver is only implementing magnetometer part, which has
+	  its own address and register map.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called bmc150_magn.
+
 config MAG3110
 	tristate "Freescale MAG3110 3-Axis Magnetometer"
 	depends on I2C

@@ -87,19 +105,4 @@ config IIO_ST_MAGN_SPI_3AXIS
 	depends on IIO_ST_MAGN_3AXIS
 	depends on IIO_ST_SENSORS_SPI
 
-config BMC150_MAGN
-	tristate "Bosch BMC150 Magnetometer Driver"
-	depends on I2C
-	select REGMAP_I2C
-	select IIO_BUFFER
-	select IIO_TRIGGERED_BUFFER
-	help
-	  Say yes here to build support for the BMC150 magnetometer.
-
-	  Currently this only supports the device via an i2c interface.
-
-	  This is a combo module with both accelerometer and magnetometer.
-	  This driver is only implementing magnetometer part, which has
-	  its own address and register map.
-
 endmenu
drivers/iio/magnetometer/Makefile +1 -2
index 33b1d4d54ee7..2c72df458ec2 100644
@@ -4,6 +4,7 @@
 
 # When adding new entries keep the list in alphabetical order
 obj-$(CONFIG_AK8975)	+= ak8975.o
+obj-$(CONFIG_BMC150_MAGN) += bmc150_magn.o
 obj-$(CONFIG_MAG3110)	+= mag3110.o
 obj-$(CONFIG_HID_SENSOR_MAGNETOMETER_3D) += hid-sensor-magn-3d.o
 obj-$(CONFIG_MMC35240)	+= mmc35240.o

@@ -14,5 +15,3 @@ st_magn-$(CONFIG_IIO_BUFFER) += st_magn_buffer.o
 
 obj-$(CONFIG_IIO_ST_MAGN_I2C_3AXIS) += st_magn_i2c.o
 obj-$(CONFIG_IIO_ST_MAGN_SPI_3AXIS) += st_magn_spi.o
-
-obj-$(CONFIG_BMC150_MAGN) += bmc150_magn.o
maintainer Applied
#5 lore raw

Re: [PATCH 2/8] iio:magnetometer:bmc150_magn: sort entry alphabetically

Quoted 11 lines · Jonathan Cameron
Quoted 6 lines · Hartmut Knaack

Sort the entry for bmc105_magn in Kconfig and Makefile to its correct position. Also add the minor module information for completeness.

Fixes c91746a2361d ("iio: magn: Add support for BMC150 magnetometer")
Signed-off-by Hartmut Knaack <[email protected]>

Technically this isn't a fix so will have to go the slow way. Obviously it's dependent on the previous patch, so if I forget about it do poke me!

Jonathan

Applied.

Quoted 79 lines
Quoted 68 lines
---
 drivers/iio/magnetometer/Kconfig  | 33 ++++++++++++++++++---------------
 drivers/iio/magnetometer/Makefile |  3 +--
 2 files changed, 19 insertions(+), 17 deletions(-)
drivers/iio/magnetometer/Kconfig +18 -15
index efb9350b0d76..868abada3409 100644
@@ -24,6 +24,24 @@ config AK09911
 	help
 	  Deprecated: AK09911 is now supported by AK8975 driver.
 
+config BMC150_MAGN
+	tristate "Bosch BMC150 Magnetometer Driver"
+	depends on I2C
+	select REGMAP_I2C
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+	help
+	  Say yes here to build support for the BMC150 magnetometer.
+
+	  Currently this only supports the device via an i2c interface.
+
+	  This is a combo module with both accelerometer and magnetometer.
+	  This driver is only implementing magnetometer part, which has
+	  its own address and register map.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called bmc150_magn.
+
 config MAG3110
 	tristate "Freescale MAG3110 3-Axis Magnetometer"
 	depends on I2C

@@ -87,19 +105,4 @@ config IIO_ST_MAGN_SPI_3AXIS
 	depends on IIO_ST_MAGN_3AXIS
 	depends on IIO_ST_SENSORS_SPI
 
-config BMC150_MAGN
-	tristate "Bosch BMC150 Magnetometer Driver"
-	depends on I2C
-	select REGMAP_I2C
-	select IIO_BUFFER
-	select IIO_TRIGGERED_BUFFER
-	help
-	  Say yes here to build support for the BMC150 magnetometer.
-
-	  Currently this only supports the device via an i2c interface.
-
-	  This is a combo module with both accelerometer and magnetometer.
-	  This driver is only implementing magnetometer part, which has
-	  its own address and register map.
-
 endmenu
drivers/iio/magnetometer/Makefile +1 -0
index 33b1d4d54ee7..2c72df458ec2 100644
@@ -4,6 +4,7 @@
 
 # When adding new entries keep the list in alphabetical order
 obj-$(CONFIG_AK8975)	+= ak8975.o
+obj-$(CONFIG_BMC150_MAGN) += bmc150_magn.o
 obj-$(CONFIG_MAG3110)	+= mag3110.o
 obj-$(CONFIG_HID_SENSOR_MAGNETOMETER_3D) += hid-sensor-magn-3d.o
 obj-$(CONFIG_MMC35240)	+= mmc35240.o

drivers/iio/magnetometer/Makefile st_magn-$(CONFIG_IIO_BUFFER) += st_magn_buffer.o · line 18

Quoted 5 lines
drivers/iio/magnetometer/Makefile +0 -2
@@ -14,5 +15,3 @@ st_magn-$(CONFIG_IIO_BUFFER) += st_magn_buffer.o
 
 obj-$(CONFIG_IIO_ST_MAGN_I2C_3AXIS) += st_magn_i2c.o
 obj-$(CONFIG_IIO_ST_MAGN_SPI_3AXIS) += st_magn_spi.o
-
-obj-$(CONFIG_BMC150_MAGN) += bmc150_magn.o

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#6 lore raw

PATCH 3/8 iio:magnetometer:bmc150_magn: output intended variable

In reply to Hartmut Knaack

According to the debug/error string, the content of chip_id is supposed to be output, rather than the return value of the previous operation.

Fixes c91746a2361d ("iio: magn: Add support for BMC150 magnetometer")
Signed-off-by Hartmut Knaack <[email protected]>
---
 drivers/iio/magnetometer/bmc150_magn.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/iio/magnetometer/bmc150_magn.c +2 -2
index cd002710dd02..7da4ce9409e8 100644
@@ -694,11 +694,11 @@ static int bmc150_magn_init(struct bmc150_magn_data *data)
 		goto err_poweroff;
 	}
 	if (chip_id != BMC150_MAGN_CHIP_ID_VAL) {
-		dev_err(&data->client->dev, "Invalid chip id 0x%x\n", ret);
+		dev_err(&data->client->dev, "Invalid chip id 0x%x\n", chip_id);
 		ret = -ENODEV;
 		goto err_poweroff;
 	}
-	dev_dbg(&data->client->dev, "Chip id %x\n", ret);
+	dev_dbg(&data->client->dev, "Chip id %x\n", chip_id);
 
 	preset = bmc150_magn_presets_table[BMC150_MAGN_DEFAULT_PRESET];
 	ret = bmc150_magn_set_odr(data, preset.odr);
Signature · 1 line
2.4.3
maintainer Applied
#7 lore raw

Re: [PATCH 3/8] iio:magnetometer:bmc150_magn: output intended variable

Quoted 6 lines · Hartmut Knaack

According to the debug/error string, the content of chip_id is supposed to be output, rather than the return value of the previous operation.

Fixes c91746a2361d ("iio: magn: Add support for BMC150 magnetometer")
Signed-off-by Hartmut Knaack <[email protected]>

Applied to the fixes-togreg branch of iio.git.

Thanks,

Jonathan

Quoted 23 lines
---
 drivers/iio/magnetometer/bmc150_magn.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/iio/magnetometer/bmc150_magn.c +2 -2
index cd002710dd02..7da4ce9409e8 100644
@@ -694,11 +694,11 @@ static int bmc150_magn_init(struct bmc150_magn_data *data)
 		goto err_poweroff;
 	}
 	if (chip_id != BMC150_MAGN_CHIP_ID_VAL) {
-		dev_err(&data->client->dev, "Invalid chip id 0x%x\n", ret);
+		dev_err(&data->client->dev, "Invalid chip id 0x%x\n", chip_id);
 		ret = -ENODEV;
 		goto err_poweroff;
 	}
-	dev_dbg(&data->client->dev, "Chip id %x\n", ret);
+	dev_dbg(&data->client->dev, "Chip id %x\n", chip_id);
 
 	preset = bmc150_magn_presets_table[BMC150_MAGN_DEFAULT_PRESET];
 	ret = bmc150_magn_set_odr(data, preset.odr);
author
#8 lore raw

PATCH 4/8 iio:magnetometer:bmc150_magn: replace magic value

In reply to Hartmut Knaack

Construct the scanmask using its descriptive axis names (as used in iio_chan_spec) instead of a 'magic' value.

Signed-off-by Hartmut Knaack <[email protected]>

---
Does this look better?

 drivers/iio/magnetometer/bmc150_magn.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/iio/magnetometer/bmc150_magn.c +3 -1
index 7da4ce9409e8..f1b6a4dc40ea 100644
@@ -651,7 +651,9 @@ static const struct iio_info bmc150_magn_info = {
 	.driver_module = THIS_MODULE,
 };
 
-static const unsigned long bmc150_magn_scan_masks[] = {0x07, 0};
+static const unsigned long bmc150_magn_scan_masks[] = {
+					BIT(AXIS_X) | BIT(AXIS_Y) | BIT(AXIS_Z),
+					0};
 
 static irqreturn_t bmc150_magn_trigger_handler(int irq, void *p)
 {
Signature · 1 line
2.4.3
maintainer Applied
#9 lore raw

Re: [PATCH 4/8] iio:magnetometer:bmc150_magn: replace magic value

Quoted 4 lines · Hartmut Knaack

Construct the scanmask using its descriptive axis names (as used in iio_chan_spec) instead of a 'magic' value.

Signed-off-by Hartmut Knaack <[email protected]>

Applied to the togreg branch of iio.git, pushed out as testing for the autobuilders to play with it.

Thanks,

Jonathan

Quoted 2 lines

---
Does this look better?

Certainly slightly clearer what is going on.

Quoted 20 lines
 drivers/iio/magnetometer/bmc150_magn.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/iio/magnetometer/bmc150_magn.c +3 -1
index 7da4ce9409e8..f1b6a4dc40ea 100644
@@ -651,7 +651,9 @@ static const struct iio_info bmc150_magn_info = {
 	.driver_module = THIS_MODULE,
 };
 
-static const unsigned long bmc150_magn_scan_masks[] = {0x07, 0};
+static const unsigned long bmc150_magn_scan_masks[] = {
+					BIT(AXIS_X) | BIT(AXIS_Y) | BIT(AXIS_Z),
+					0};
 
 static irqreturn_t bmc150_magn_trigger_handler(int irq, void *p)
 {
author
#10 lore raw

PATCH 5/8 iio:magnetometer:bmc150_magn: add error handling on gpio probe

In reply to Hartmut Knaack

bmc150_magn_gpio_probe() may return some error codes, so better check them.

Signed-off-by Hartmut Knaack <[email protected]>
---
 drivers/iio/magnetometer/bmc150_magn.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/iio/magnetometer/bmc150_magn.c +6 -1
index f1b6a4dc40ea..e1f804b2b244 100644
@@ -920,8 +920,13 @@ static int bmc150_magn_probe(struct i2c_client *client,
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->info = &bmc150_magn_info;
 
-	if (client->irq <= 0)
+	if (client->irq <= 0) {
 		client->irq = bmc150_magn_gpio_probe(client);
+		if (client->irq < 0) {
+			ret = client->irq;
+			goto err_poweroff;
+		}
+	}
 
 	if (client->irq > 0) {
 		data->dready_trig = devm_iio_trigger_alloc(&client->dev,
Signature · 1 line
2.4.3
maintainer
#11 lore raw

Re: [PATCH 5/8] iio:magnetometer:bmc150_magn: add error handling on gpio probe

Quoted 4 lines · Hartmut Knaack

bmc150_magn_gpio_probe() may return some error codes, so better check them.

Signed-off-by Hartmut Knaack <[email protected]>

Another kind of obvious improvement. Obviously comments still welcome from others. Applied to the togreg branch of iio.git

Thanks,

Jonathan

Quoted 24 lines
---
 drivers/iio/magnetometer/bmc150_magn.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/iio/magnetometer/bmc150_magn.c +6 -1
index f1b6a4dc40ea..e1f804b2b244 100644
@@ -920,8 +920,13 @@ static int bmc150_magn_probe(struct i2c_client *client,
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->info = &bmc150_magn_info;
 
-	if (client->irq <= 0)
+	if (client->irq <= 0) {
 		client->irq = bmc150_magn_gpio_probe(client);
+		if (client->irq < 0) {
+			ret = client->irq;
+			goto err_poweroff;
+		}
+	}
 
 	if (client->irq > 0) {
 		data->dready_trig = devm_iio_trigger_alloc(&client->dev,
#12 lore raw

RE: [PATCH 5/8] iio:magnetometer:bmc150_magn: add error handling on gpio probe

In reply to Hartmut Knaack
Quoted 23 lines

-----Original Message-----
From: Hartmut Knaack [mailto:[email protected]]
Sent: 17 July, 2015 11:52
To: [email protected]
Cc: Jonathan Cameron; Lars-Peter Clausen; Peter Meerwald; Tirdea, Irina; Dogaru, Vlad
Subject: [PATCH 5/8] iio:magnetometer:bmc150_magn: add error handling on gpio probe

bmc150_magn_gpio_probe() may return some error codes, so better check them.

Signed-off-by Hartmut Knaack <[email protected]>
---
 drivers/iio/magnetometer/bmc150_magn.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/iio/magnetometer/bmc150_magn.c +0 -0
index f1b6a4dc40ea..e1f804b2b244 100644
@@ -920,8 +920,13 @@ static int bmc150_magn_probe(struct i2c_client *client,
 	indio_dev->modes = INDIO_DIRECT_MODE;

drivers/iio/magnetometer/bmc150_magn.c static int bmc150_magn_probe(struct i2c_client *client, · line 929

Quoted 10 lines
drivers/iio/magnetometer/bmc150_magn.c +6 -1
 	indio_dev->info = &bmc150_magn_info;
 
-	if (client->irq <= 0)
+	if (client->irq <= 0) {
 		client->irq = bmc150_magn_gpio_probe(client);
+		if (client->irq < 0) {
+			ret = client->irq;
+			goto err_poweroff;
+		}
+	}

The error code for client->irq was not checked on purpose. We would like to be able to use this driver even if the interrupt pin is not connected (reading from sysfs in poll mode or by using iio hrtimers).

If the i2c core does not initialize the irq, we try to initialize it through ACPI in bmc150_magn_gpio_probe and if that fails too we do not register the interrupt and iio trigger (see check for client->irq >0 below). In this way we could still use the device even if we don't have interrupts.

Quoted 5 lines
drivers/iio/magnetometer/bmc150_magn.c +0 -1
 	if (client->irq > 0) {
 		data->dready_trig = devm_iio_trigger_alloc(&client->dev,
--

2.4.3

maintainer
#13 lore raw

Re: [PATCH 5/8] iio:magnetometer:bmc150_magn: add error handling on gpio probe

Quoted 37 lines · Irina
Quoted 23 lines

-----Original Message-----
From: Hartmut Knaack [mailto:[email protected]]
Sent: 17 July, 2015 11:52
To: [email protected]
Cc: Jonathan Cameron; Lars-Peter Clausen; Peter Meerwald; Tirdea, Irina; Dogaru, Vlad
Subject: [PATCH 5/8] iio:magnetometer:bmc150_magn: add error handling on gpio probe

bmc150_magn_gpio_probe() may return some error codes, so better check them.

Signed-off-by Hartmut Knaack <[email protected]>
---
 drivers/iio/magnetometer/bmc150_magn.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/iio/magnetometer/bmc150_magn.c +0 -0
index f1b6a4dc40ea..e1f804b2b244 100644
@@ -920,8 +920,13 @@ static int bmc150_magn_probe(struct i2c_client *client,
 	indio_dev->modes = INDIO_DIRECT_MODE;

drivers/iio/magnetometer/bmc150_magn.c static int bmc150_magn_probe(struct i2c_client *client, · line 929

Quoted 10 lines
drivers/iio/magnetometer/bmc150_magn.c +6 -1
 	indio_dev->info = &bmc150_magn_info;
 
-	if (client->irq <= 0)
+	if (client->irq <= 0) {
 		client->irq = bmc150_magn_gpio_probe(client);
+		if (client->irq < 0) {
+			ret = client->irq;
+			goto err_poweroff;
+		}
+	}

The error code for client->irq was not checked on purpose. We would like to be able to use this driver even if the interrupt pin is not connected (reading from sysfs in poll mode or by using iio hrtimers).

Quoted 4 lines

If the i2c core does not initialize the irq, we try to initialize it through ACPI in bmc150_magn_gpio_probe and if that fails too we do not register the interrupt and iio trigger (see check for client->irq >0 below). In this way we could still use the device even if we don't have interrupts.

One of the advantages of a pre push to testing is I can still rebase when I jump the gun and mess up like this ;)

Anyhow, patch dropped and testing re-pushed out.

Sorry about that, should have left these a while on the list before applying.

Jonathan

Quoted 11 lines
Quoted 5 lines
drivers/iio/magnetometer/bmc150_magn.c +0 -1
 	if (client->irq > 0) {
 		data->dready_trig = devm_iio_trigger_alloc(&client->dev,
--

2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#14 lore raw

PATCH 6/8 iio:magnetometer:bmc150_magn: protect runtime_resume with mutex

In reply to Hartmut Knaack

Protect bmc150_magn_runtime_resume() with a mutex, as done with other suspend/resume functions.

Signed-off-by Hartmut Knaack <[email protected]>
---
 drivers/iio/magnetometer/bmc150_magn.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
drivers/iio/magnetometer/bmc150_magn.c +7 -2
index e1f804b2b244..ac0cdbde813b 100644
@@ -1052,9 +1052,14 @@ static int bmc150_magn_runtime_resume(struct device *dev)
 {
 	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
 	struct bmc150_magn_data *data = iio_priv(indio_dev);
+	int ret;
 
-	return bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_NORMAL,
-					  true);
+	mutex_lock(&data->mutex);
+	ret = bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_NORMAL,
+					 true);
+	mutex_unlock(&data->mutex);
+
+	return ret;
 }
 #endif
Signature · 1 line
2.4.3
maintainer
#15 lore raw

Re: [PATCH 6/8] iio:magnetometer:bmc150_magn: protect runtime_resume with mutex

Quoted 4 lines · Hartmut Knaack

Protect bmc150_magn_runtime_resume() with a mutex, as done with other suspend/resume functions.

Signed-off-by Hartmut Knaack <[email protected]>

Looks fine to me, but given we are getting into more complex patches, I'll let this sit on the list for Irina and others to have a look at.

Jonathan

Quoted 26 lines
---
 drivers/iio/magnetometer/bmc150_magn.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
drivers/iio/magnetometer/bmc150_magn.c +7 -2
index e1f804b2b244..ac0cdbde813b 100644
@@ -1052,9 +1052,14 @@ static int bmc150_magn_runtime_resume(struct device *dev)
 {
 	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
 	struct bmc150_magn_data *data = iio_priv(indio_dev);
+	int ret;
 
-	return bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_NORMAL,
-					  true);
+	mutex_lock(&data->mutex);
+	ret = bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_NORMAL,
+					 true);
+	mutex_unlock(&data->mutex);
+
+	return ret;
 }
 #endif
#16 lore raw

RE: [PATCH 6/8] iio:magnetometer:bmc150_magn: protect runtime_resume with mutex

In reply to Hartmut Knaack
Quoted 7 lines

-----Original Message-----
From: Hartmut Knaack [mailto:[email protected]]
Sent: 17 July, 2015 11:52
To: [email protected]
Cc: Jonathan Cameron; Lars-Peter Clausen; Peter Meerwald; Tirdea, Irina; =

Dogaru, Vlad

Quoted 1 line

Subject: [PATCH 6/8] iio:magnetometer:bmc150_magn: protect runtime_resume=

with mutex

drivers/iio/magneto=

Quoted 10 lines

=20
Protect bmc150_magn_runtime_resume() with a mutex, as done with other suspend/resume functions.
=20

Signed-off-by Hartmut Knaack <[email protected]>
---
 drivers/iio/magnetometer/bmc150_magn.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

=20

drivers/iio/magneto= renamed from drivers/iio/magnetometer/bmc150_magn.c +0 -0

meter/bmc150_magn.c

drivers/iio/magnetometer/bmc150_magn.c static int bmc150_magn_runtime_resume(struct devic=

Quoted 4 lines

index e1f804b2b244..ac0cdbde813b 100644

drivers/iio/magnetometer/bmc150_magn.c +0 -0
@@ -1052,9 +1052,14 @@ static int bmc150_magn_runtime_resume(struct devic=

e *dev)

drivers/iio/magnetometer/bmc150_magn.c static int bmc150_magn_runtime_resume(struct device *dev) · line 1062

Quoted 13 lines
drivers/iio/magnetometer/bmc150_magn.c +1 -0
 {
 	struct iio_dev *indio_dev =3D i2c_get_clientdata(to_i2c_client(dev));
 	struct bmc150_magn_data *data =3D iio_priv(indio_dev);
+	int ret;

=20

drivers/iio/magnetometer/bmc150_magn.c +6 -2
-	return bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_NORMAL,
-					  true);
+	mutex_lock(&data->mutex);
+	ret =3D bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_NORMAL,
+					 true);
+	mutex_unlock(&data->mutex);
+
+	return ret;

This function does not have mutex_lock/unlock because it is called with dat= a->mutex locked.
I should have added a comment to specify this.

Whenever we wake up the device, bmc150_magn_set_power_state is called after=
locking
data->mutex. This in turn calls pm_runtime_get_sync which will immediately = call
bmc150_magn_runtime_resume. Trying to lock the mutex here would lead to a d= eadlock
(e.g. when trying to read a value through sysfs _raw).

Since this driver uses autosuspend delay, the situation is different for ru= ntime suspend since
that is called after 2 seconds in a separate thread. This is why there is a=
mutex locked in runtime
suspend but not in runtime resume.

Quoted 5 lines

}
#endif
=20
--
2.4.3

author
#17 lore raw

Re: [PATCH 6/8] iio:magnetometer:bmc150_magn: protect runtime_resume with mutex

Tirdea, Irina schrieb am 20.07.2015 um 19:45:

Quoted 39 lines
Quoted 25 lines

-----Original Message-----
From: Hartmut Knaack [mailto:[email protected]]
Sent: 17 July, 2015 11:52
To: [email protected]
Cc: Jonathan Cameron; Lars-Peter Clausen; Peter Meerwald; Tirdea, Irina; Dogaru, Vlad
Subject: [PATCH 6/8] iio:magnetometer:bmc150_magn: protect runtime_resume with mutex

Protect bmc150_magn_runtime_resume() with a mutex, as done with other suspend/resume functions.

Signed-off-by Hartmut Knaack <[email protected]>
---
 drivers/iio/magnetometer/bmc150_magn.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
drivers/iio/magnetometer/bmc150_magn.c +0 -0
index e1f804b2b244..ac0cdbde813b 100644
@@ -1052,9 +1052,14 @@ static int bmc150_magn_runtime_resume(struct device *dev)
 {
 	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
 	struct bmc150_magn_data *data = iio_priv(indio_dev);

drivers/iio/magnetometer/bmc150_magn.c static int bmc150_magn_runtime_resume(struct device *dev) · line 1062

Quoted 10 lines
drivers/iio/magnetometer/bmc150_magn.c +7 -2
+	int ret;
 
-	return bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_NORMAL,
-					  true);
+	mutex_lock(&data->mutex);
+	ret = bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_NORMAL,
+					 true);
+	mutex_unlock(&data->mutex);
+
+	return ret;

This function does not have mutex_lock/unlock because it is called with data->mutex locked. I should have added a comment to specify this.

Quoted 9 lines

Whenever we wake up the device, bmc150_magn_set_power_state is called after locking data->mutex. This in turn calls pm_runtime_get_sync which will immediately call bmc150_magn_runtime_resume. Trying to lock the mutex here would lead to a deadlock (e.g. when trying to read a value through sysfs _raw).

Since this driver uses autosuspend delay, the situation is different for runtime suspend since that is called after 2 seconds in a separate thread. This is why there is a mutex locked in runtime suspend but not in runtime resume.

Yes, please add such a comment. As for me, it seems I have to dig a bit deeper into power management magic - maybe another day ;-) Thanks,

Hartmut

Quoted 11 lines
Quoted 5 lines
drivers/iio/magnetometer/bmc150_magn.c +0 -1
 }
 #endif
 
--

2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#18 lore raw

PATCH 7/8 iio:magnetometer:bmc150_magn: expand mutex in trigger_handler

In reply to Hartmut Knaack

Keep the mutex locked, until the content of data->buffer has been pushed out.

Signed-off-by Hartmut Knaack <[email protected]>
---
 drivers/iio/magnetometer/bmc150_magn.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iio/magnetometer/bmc150_magn.c +1 -1
index ac0cdbde813b..bdcf49ee75df 100644
@@ -664,7 +664,6 @@ static irqreturn_t bmc150_magn_trigger_handler(int irq, void *p)
 
 	mutex_lock(&data->mutex);
 	ret = bmc150_magn_read_xyz(data, data->buffer);
-	mutex_unlock(&data->mutex);
 	if (ret < 0)
 		goto err;
 

@@ -672,6 +671,7 @@ static irqreturn_t bmc150_magn_trigger_handler(int irq, void *p)
 					   pf->timestamp);
 
 err:
+	mutex_unlock(&data->mutex);
 	iio_trigger_notify_done(indio_dev->trig);
 
 	return IRQ_HANDLED;
Signature · 1 line
2.4.3
Acked-by on v1 7/8
#19 lore raw

RE: [PATCH 7/8] iio:magnetometer:bmc150_magn: expand mutex in trigger_handler

Quoted 9 lines

-----Original Message-----
From: Hartmut Knaack [mailto:[email protected]]
Sent: 17 July, 2015 11:53
To: [email protected]
Cc: Jonathan Cameron; Lars-Peter Clausen; Peter Meerwald; Tirdea, Irina; Dogaru, Vlad
Subject: [PATCH 7/8] iio:magnetometer:bmc150_magn: expand mutex in trigger_handler

Quoted 3 lines

Keep the mutex locked, until the content of data->buffer has been pushed out.

Looks good to me.

Acked-by Irina Tirdea <[email protected]>
Quoted 27 lines
Signed-off-by Hartmut Knaack <[email protected]>
---
 drivers/iio/magnetometer/bmc150_magn.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iio/magnetometer/bmc150_magn.c +1 -2
index ac0cdbde813b..bdcf49ee75df 100644
@@ -664,7 +664,6 @@ static irqreturn_t bmc150_magn_trigger_handler(int irq, void *p)
 
 	mutex_lock(&data->mutex);
 	ret = bmc150_magn_read_xyz(data, data->buffer);
-	mutex_unlock(&data->mutex);
 	if (ret < 0)
 		goto err;
 

@@ -672,6 +671,7 @@ static irqreturn_t bmc150_magn_trigger_handler(int irq, void *p)
 					   pf->timestamp);
 
 err:
+	mutex_unlock(&data->mutex);
 	iio_trigger_notify_done(indio_dev->trig);
 
 	return IRQ_HANDLED;
--

2.4.3

maintainer Applied
#20 lore raw

Re: [PATCH 7/8] iio:magnetometer:bmc150_magn: expand mutex in trigger_handler

Quoted 9 lines · Irina
Quoted 9 lines

-----Original Message-----
From: Hartmut Knaack [mailto:[email protected]]
Sent: 17 July, 2015 11:53
To: [email protected]
Cc: Jonathan Cameron; Lars-Peter Clausen; Peter Meerwald; Tirdea, Irina; Dogaru, Vlad
Subject: [PATCH 7/8] iio:magnetometer:bmc150_magn: expand mutex in trigger_handler

Quoted 7 lines
Quoted 3 lines

Keep the mutex locked, until the content of data->buffer has been pushed out.

Looks good to me.

Acked-by Irina Tirdea <[email protected]>

Applied to the togreg branch of iio.git.

Thanks,

Quoted 34 lines
Quoted 27 lines
Signed-off-by Hartmut Knaack <[email protected]>
---
 drivers/iio/magnetometer/bmc150_magn.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iio/magnetometer/bmc150_magn.c +1 -2
index ac0cdbde813b..bdcf49ee75df 100644
@@ -664,7 +664,6 @@ static irqreturn_t bmc150_magn_trigger_handler(int irq, void *p)
 
 	mutex_lock(&data->mutex);
 	ret = bmc150_magn_read_xyz(data, data->buffer);
-	mutex_unlock(&data->mutex);
 	if (ret < 0)
 		goto err;
 

@@ -672,6 +671,7 @@ static irqreturn_t bmc150_magn_trigger_handler(int irq, void *p)
 					   pf->timestamp);
 
 err:
+	mutex_unlock(&data->mutex);
 	iio_trigger_notify_done(indio_dev->trig);
 
 	return IRQ_HANDLED;
--
Quoted 1 line

2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#21 lore raw

PATCH 8/8 iio:magnetometer:bmc150_magn: use descriptive name for mask

In reply to Hartmut Knaack

Define and use a descriptive name for the repetition registers data mask, instead of a 'magic' value.

Signed-off-by Hartmut Knaack <[email protected]>
---
 drivers/iio/magnetometer/bmc150_magn.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/iio/magnetometer/bmc150_magn.c +3 -2
index bdcf49ee75df..e8cf08ee2d15 100644
@@ -85,6 +85,7 @@
 #define BMC150_MAGN_REG_HIGH_THRESH		0x50
 #define BMC150_MAGN_REG_REP_XY			0x51
 #define BMC150_MAGN_REG_REP_Z			0x52
+#define BMC150_MAGN_REG_REP_DATAMASK		GENMASK(7, 0)
 
 #define BMC150_MAGN_REG_TRIM_START		0x5D
 #define BMC150_MAGN_REG_TRIM_END		0x71

@@ -559,7 +560,7 @@ static int bmc150_magn_write_raw(struct iio_dev *indio_dev,
 			}
 			ret = regmap_update_bits(data->regmap,
 						 BMC150_MAGN_REG_REP_XY,
-						 0xFF,
+						 BMC150_MAGN_REG_REP_DATAMASK,
 						 BMC150_MAGN_REPXY_TO_REGVAL
 						 (val));
 			mutex_unlock(&data->mutex);

@@ -575,7 +576,7 @@ static int bmc150_magn_write_raw(struct iio_dev *indio_dev,
 			}
 			ret = regmap_update_bits(data->regmap,
 						 BMC150_MAGN_REG_REP_Z,
-						 0xFF,
+						 BMC150_MAGN_REG_REP_DATAMASK,
 						 BMC150_MAGN_REPZ_TO_REGVAL
 						 (val));
 			mutex_unlock(&data->mutex);
Signature · 1 line
2.4.3
Acked-by on v1 8/8
#22 lore raw

RE: [PATCH 8/8] iio:magnetometer:bmc150_magn: use descriptive name for mask

Quoted 9 lines

-----Original Message-----
From: Hartmut Knaack [mailto:[email protected]]
Sent: 17 July, 2015 11:53
To: [email protected]
Cc: Jonathan Cameron; Lars-Peter Clausen; Peter Meerwald; Tirdea, Irina; Dogaru, Vlad
Subject: [PATCH 8/8] iio:magnetometer:bmc150_magn: use descriptive name for mask

Quoted 3 lines

Define and use a descriptive name for the repetition registers data mask, instead of a 'magic' value.

Missed this hardcoded value. Thanks for catching this!

Acked-by Irina Tirdea <[email protected]>
Quoted 37 lines
Signed-off-by Hartmut Knaack <[email protected]>
---
 drivers/iio/magnetometer/bmc150_magn.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/iio/magnetometer/bmc150_magn.c +3 -3
index bdcf49ee75df..e8cf08ee2d15 100644
@@ -85,6 +85,7 @@
 #define BMC150_MAGN_REG_HIGH_THRESH		0x50
 #define BMC150_MAGN_REG_REP_XY			0x51
 #define BMC150_MAGN_REG_REP_Z			0x52
+#define BMC150_MAGN_REG_REP_DATAMASK		GENMASK(7, 0)
 
 #define BMC150_MAGN_REG_TRIM_START		0x5D
 #define BMC150_MAGN_REG_TRIM_END		0x71

@@ -559,7 +560,7 @@ static int bmc150_magn_write_raw(struct iio_dev *indio_dev,
 			}
 			ret = regmap_update_bits(data->regmap,
 						 BMC150_MAGN_REG_REP_XY,
-						 0xFF,
+						 BMC150_MAGN_REG_REP_DATAMASK,
 						 BMC150_MAGN_REPXY_TO_REGVAL
 						 (val));
 			mutex_unlock(&data->mutex);

@@ -575,7 +576,7 @@ static int bmc150_magn_write_raw(struct iio_dev *indio_dev,
 			}
 			ret = regmap_update_bits(data->regmap,
 						 BMC150_MAGN_REG_REP_Z,
-						 0xFF,
+						 BMC150_MAGN_REG_REP_DATAMASK,
 						 BMC150_MAGN_REPZ_TO_REGVAL
 						 (val));
 			mutex_unlock(&data->mutex);
--

2.4.3

maintainer Applied
#23 lore raw

Re: [PATCH 8/8] iio:magnetometer:bmc150_magn: use descriptive name for mask

Quoted 9 lines · Irina
Quoted 9 lines

-----Original Message-----
From: Hartmut Knaack [mailto:[email protected]]
Sent: 17 July, 2015 11:53
To: [email protected]
Cc: Jonathan Cameron; Lars-Peter Clausen; Peter Meerwald; Tirdea, Irina; Dogaru, Vlad
Subject: [PATCH 8/8] iio:magnetometer:bmc150_magn: use descriptive name for mask

Quoted 7 lines
Quoted 3 lines

Define and use a descriptive name for the repetition registers data mask, instead of a 'magic' value.

Missed this hardcoded value. Thanks for catching this!

Acked-by Irina Tirdea <[email protected]>

Applied, thanks!

Jonathan

Quoted 44 lines
Quoted 37 lines
Signed-off-by Hartmut Knaack <[email protected]>
---
 drivers/iio/magnetometer/bmc150_magn.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/iio/magnetometer/bmc150_magn.c +3 -3
index bdcf49ee75df..e8cf08ee2d15 100644
@@ -85,6 +85,7 @@
 #define BMC150_MAGN_REG_HIGH_THRESH		0x50
 #define BMC150_MAGN_REG_REP_XY			0x51
 #define BMC150_MAGN_REG_REP_Z			0x52
+#define BMC150_MAGN_REG_REP_DATAMASK		GENMASK(7, 0)
 
 #define BMC150_MAGN_REG_TRIM_START		0x5D
 #define BMC150_MAGN_REG_TRIM_END		0x71

@@ -559,7 +560,7 @@ static int bmc150_magn_write_raw(struct iio_dev *indio_dev,
 			}
 			ret = regmap_update_bits(data->regmap,
 						 BMC150_MAGN_REG_REP_XY,
-						 0xFF,
+						 BMC150_MAGN_REG_REP_DATAMASK,
 						 BMC150_MAGN_REPXY_TO_REGVAL
 						 (val));
 			mutex_unlock(&data->mutex);

@@ -575,7 +576,7 @@ static int bmc150_magn_write_raw(struct iio_dev *indio_dev,
 			}
 			ret = regmap_update_bits(data->regmap,
 						 BMC150_MAGN_REG_REP_Z,
-						 0xFF,
+						 BMC150_MAGN_REG_REP_DATAMASK,
 						 BMC150_MAGN_REPZ_TO_REGVAL
 						 (val));
 			mutex_unlock(&data->mutex);
--
Quoted 1 line

2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#24 lore raw
In reply to Hartmut Knaack
Quoted 9 lines

-----Original Message-----
From: Hartmut Knaack [mailto:[email protected]]
Sent: 17 July, 2015 11:52
To: [email protected]
Cc: Jonathan Cameron; Lars-Peter Clausen; Peter Meerwald; Tirdea, Irina; Dogaru, Vlad
Subject: [PATCH 0/8] bmc150_magn fix and cleanup

Quoted 4 lines

Some issues I spotted during review of this driver. First three are quite obvious problems, the rest may be matter of taste or intended to stay in the original form. Opinions welcome.

Hi Hartmut,

Thanks for taking the time to cleanup these issues. I know you did not get to review this code before merge, so I was expecting your feedback :)

I see Jonathan has already merged some of the changes, so I'm just going to reply to the rest.
I am fine with the first 4 changes which already got merged anyway.

Thanks,
Irina

Quoted 17 lines

Hartmut Knaack (8):
iio:magnetometer:bmc150_magn: add regmap dependency iio:magnetometer:bmc150_magn: sort entry alphabetically iio:magnetometer:bmc150_magn: output intended variable iio:magnetometer:bmc150_magn: replace magic value iio:magnetometer:bmc150_magn: add error handling on gpio probe iio:magnetometer:bmc150_magn: protect runtime_resume with mutex iio:magnetometer:bmc150_magn: expand mutex in trigger_handler iio:magnetometer:bmc150_magn: use descriptive name for mask

 drivers/iio/magnetometer/Kconfig       | 32 ++++++++++++++++++--------------
 drivers/iio/magnetometer/Makefile      |  3 +--
 drivers/iio/magnetometer/bmc150_magn.c | 31 ++++++++++++++++++++++---------
 3 files changed, 41 insertions(+), 25 deletions(-)

--
2.4.3

maintainer
#25 lore raw
Quoted 14 lines · Irina
Quoted 9 lines

-----Original Message-----
From: Hartmut Knaack [mailto:[email protected]]
Sent: 17 July, 2015 11:52
To: [email protected]
Cc: Jonathan Cameron; Lars-Peter Clausen; Peter Meerwald; Tirdea, Irina; Dogaru, Vlad
Subject: [PATCH 0/8] bmc150_magn fix and cleanup

Quoted 4 lines

Some issues I spotted during review of this driver. First three are quite obvious problems, the rest may be matter of taste or intended to stay in the original form. Opinions welcome.

Quoted 8 lines

Hi Hartmut,

Thanks for taking the time to cleanup these issues. I know you did not get to review this code before merge, so I was expecting your feedback :)

I see Jonathan has already merged some of the changes, so I'm just going to reply to the rest.
I am fine with the first 4 changes which already got merged anyway.

Feel free to offer comments on the rest as well. I was on a bit of a roll and not giving sufficient time to others to review these. The 'obviously' correct are not always so obvious given patch 5!

Sorry about that

Jonathan

Quoted 27 lines

Thanks,
Irina

Quoted 10 lines

Hartmut Knaack (8):
iio:magnetometer:bmc150_magn: add regmap dependency iio:magnetometer:bmc150_magn: sort entry alphabetically iio:magnetometer:bmc150_magn: output intended variable iio:magnetometer:bmc150_magn: replace magic value iio:magnetometer:bmc150_magn: add error handling on gpio probe iio:magnetometer:bmc150_magn: protect runtime_resume with mutex iio:magnetometer:bmc150_magn: expand mutex in trigger_handler iio:magnetometer:bmc150_magn: use descriptive name for mask

Quoted 7 lines
 drivers/iio/magnetometer/Kconfig       | 32 ++++++++++++++++++--------------
 drivers/iio/magnetometer/Makefile      |  3 +--
 drivers/iio/magnetometer/bmc150_magn.c | 31 ++++++++++++++++++++++---------
 3 files changed, 41 insertions(+), 25 deletions(-)

--
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#26 lore raw
Quoted 4 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]=

.org] On Behalf Of Jonathan Cameron

Quoted 13 lines

Sent: 20 July, 2015 20:49
To: Tirdea, Irina; Hartmut Knaack; [email protected]
Cc: Lars-Peter Clausen; Peter Meerwald; Dogaru, Vlad
Subject: Re: [PATCH 0/8] bmc150_magn fix and cleanup =20

Quoted 7 lines · Irina
Quoted 7 lines

-----Original Message-----
From: Hartmut Knaack [mailto:[email protected]]
Sent: 17 July, 2015 11:52
To: [email protected]
Cc: Jonathan Cameron; Lars-Peter Clausen; Peter Meerwald; Tirdea, Irin=

a; Dogaru, Vlad

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

Subject: [PATCH 0/8] bmc150_magn fix and cleanup

Some issues I spotted during review of this driver. First three are qu=

ite

Quoted 7 lines
Quoted 7 lines
Quoted 3 lines

obvious problems, the rest may be matter of taste or intended to stay in the original form. Opinions welcome.

Hi Hartmut,

Thanks for taking the time to cleanup these issues. I know you did not =

get

Quoted 3 lines
Quoted 3 lines

to review this code before merge, so I was expecting your feedback :)

I see Jonathan has already merged some of the changes, so I'm just goin=

g

Quoted 3 lines
Quoted 2 lines

to reply to the rest.
I am fine with the first 4 changes which already got merged anyway.

Feel free to offer comments on the rest as well. I was on a bit of a rol=

l

Quoted 5 lines

and not giving sufficient time to others to review these. The 'obviously' correct are not always so obvious given patch 5! =20
Sorry about that
=20

No problem, Jonathan.=20
I did review the rest of the patches as well and they look good to me.

Thanks,
Irina

Quoted 16 lines

Jonathan

Quoted 15 lines

Thanks,
Irina

Quoted 11 lines

Hartmut Knaack (8):
iio:magnetometer:bmc150_magn: add regmap dependency iio:magnetometer:bmc150_magn: sort entry alphabetically iio:magnetometer:bmc150_magn: output intended variable iio:magnetometer:bmc150_magn: replace magic value iio:magnetometer:bmc150_magn: add error handling on gpio probe iio:magnetometer:bmc150_magn: protect runtime_resume with mutex iio:magnetometer:bmc150_magn: expand mutex in trigger_handler iio:magnetometer:bmc150_magn: use descriptive name for mask

drivers/iio/magnetometer/Kconfig | 32 ++++++++++++++++++-------=

-------

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
 drivers/iio/magnetometer/Makefile      |  3 +--

drivers/iio/magnetometer/bmc150_magn.c | 31 ++++++++++++++++++++++---=

------

Quoted 15 lines
Quoted 10 lines
Quoted 4 lines
 3 files changed, 41 insertions(+), 25 deletions(-)

--
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

=20
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html