linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] Add support for BMP390 and various driver cleanups
@ 2023-10-15 15:16 Angel Iglesias
  2023-10-15 15:16 ` [PATCH 1/5] iio: pressure: bmp280: Use i2c_get_match_data() Angel Iglesias
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Angel Iglesias @ 2023-10-15 15:16 UTC (permalink / raw)
  To: linux-iio
  Cc: Biju Das, linux-kernel, Angel Iglesias, Jonathan Cameron,
	Lars-Peter Clausen, Andy Shevchenko, Uwe Kleine-König

Add support for the Bosch BMP390 pressure sensors and includes minor
driver code cleanups.
Patches 1 and 2 update I2C and SPI driver matching logic using newer
helper functions available.
Patch 3 fixes minor coding style incoherences.
Patches 4 and 5 introduce support for the new BMP390 sensor allowing
sensor families to have a list of known device IDs to improve detection

This patch series is aggregates patches from various series:
https://lore.kernel.org/all/20230812175808.236405-1-biju.das.jz@bp.renesas.com/
https://lore.kernel.org/all/cover.1691952005.git.ang.iglesiasg@gmail.com/
https://lore.kernel.org/all/cover.1692805377.git.ang.iglesiasg@gmail.com/

Angel Iglesias (4):
  iio: pressure: bmp280: Use spi_get_device_match_data()
  iio: pressure: bmp280: Rearrange vars in reverse xmas tree order
  iio: pressure: bmp280: Allow multiple chips id per family of devices
  iio: pressure: bmp280: Add support for BMP390

Biju Das (1):
  iio: pressure: bmp280: Use i2c_get_match_data()

 drivers/iio/pressure/bmp280-core.c | 56 ++++++++++++++++++++++++------
 drivers/iio/pressure/bmp280-i2c.c  |  8 ++---
 drivers/iio/pressure/bmp280-spi.c  | 10 ++----
 drivers/iio/pressure/bmp280.h      |  4 ++-
 4 files changed, 54 insertions(+), 24 deletions(-)


base-commit: 73006239ef2313f1986f86af86f8b06150a807e9
-- 
2.42.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/5] iio: pressure: bmp280: Use i2c_get_match_data()
  2023-10-15 15:16 [PATCH 0/5] Add support for BMP390 and various driver cleanups Angel Iglesias
@ 2023-10-15 15:16 ` Angel Iglesias
  2023-10-15 15:16 ` [PATCH 2/5] iio: pressure: bmp280: Use spi_get_device_match_data() Angel Iglesias
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Angel Iglesias @ 2023-10-15 15:16 UTC (permalink / raw)
  To: linux-iio
  Cc: Biju Das, linux-kernel, Jonathan Cameron, Lars-Peter Clausen,
	Angel Iglesias, Andy Shevchenko, Uwe Kleine-König

From: Biju Das <biju.das.jz@bp.renesas.com>

Replace device_get_match_data() and id lookup for retrieving match data
by i2c_get_match_data().

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

diff --git a/drivers/iio/pressure/bmp280-i2c.c b/drivers/iio/pressure/bmp280-i2c.c
index dbe630ad05b5..b3e069730f97 100644
--- a/drivers/iio/pressure/bmp280-i2c.c
+++ b/drivers/iio/pressure/bmp280-i2c.c
@@ -11,9 +11,7 @@ static int bmp280_i2c_probe(struct i2c_client *client)
 	const struct bmp280_chip_info *chip_info;
 	const struct i2c_device_id *id = i2c_client_get_device_id(client);
 
-	chip_info = device_get_match_data(&client->dev);
-	if (!chip_info)
-		chip_info = (const struct bmp280_chip_info *) id->driver_data;
+	chip_info = i2c_get_match_data(client);
 
 	regmap = devm_regmap_init_i2c(client, chip_info->regmap_config);
 	if (IS_ERR(regmap)) {
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/5] iio: pressure: bmp280: Use spi_get_device_match_data()
  2023-10-15 15:16 [PATCH 0/5] Add support for BMP390 and various driver cleanups Angel Iglesias
  2023-10-15 15:16 ` [PATCH 1/5] iio: pressure: bmp280: Use i2c_get_match_data() Angel Iglesias
@ 2023-10-15 15:16 ` Angel Iglesias
  2023-10-15 15:16 ` [PATCH 3/5] iio: pressure: bmp280: Rearrange vars in reverse xmas tree order Angel Iglesias
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Angel Iglesias @ 2023-10-15 15:16 UTC (permalink / raw)
  To: linux-iio
  Cc: Biju Das, linux-kernel, Angel Iglesias, Jonathan Cameron,
	Lars-Peter Clausen, Andy Shevchenko, Uwe Kleine-König

Use the spi_get_device_match_data() helper instead of
device_get_match_data() and the fallback match_id logic.

Signed-off-by: Angel Iglesias <ang.iglesiasg@gmail.com>

diff --git a/drivers/iio/pressure/bmp280-spi.c b/drivers/iio/pressure/bmp280-spi.c
index 1dff9bb7c4e9..2eed483a8cc4 100644
--- a/drivers/iio/pressure/bmp280-spi.c
+++ b/drivers/iio/pressure/bmp280-spi.c
@@ -58,9 +58,7 @@ static int bmp280_spi_probe(struct spi_device *spi)
 		return ret;
 	}
 
-	chip_info = device_get_match_data(&spi->dev);
-	if (!chip_info)
-		chip_info = (const struct bmp280_chip_info *) id->driver_data;
+	chip_info = spi_get_device_match_data(spi);
 
 	regmap = devm_regmap_init(&spi->dev,
 				  &bmp280_regmap_bus,
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/5] iio: pressure: bmp280: Rearrange vars in reverse xmas tree order
  2023-10-15 15:16 [PATCH 0/5] Add support for BMP390 and various driver cleanups Angel Iglesias
  2023-10-15 15:16 ` [PATCH 1/5] iio: pressure: bmp280: Use i2c_get_match_data() Angel Iglesias
  2023-10-15 15:16 ` [PATCH 2/5] iio: pressure: bmp280: Use spi_get_device_match_data() Angel Iglesias
@ 2023-10-15 15:16 ` Angel Iglesias
  2023-10-16  8:40   ` Jonathan Cameron
  2023-10-15 15:16 ` [PATCH 4/5] iio: pressure: bmp280: Allow multiple chips id per family of devices Angel Iglesias
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 10+ messages in thread
From: Angel Iglesias @ 2023-10-15 15:16 UTC (permalink / raw)
  To: linux-iio
  Cc: Biju Das, linux-kernel, Angel Iglesias, Jonathan Cameron,
	Lars-Peter Clausen, Andy Shevchenko, Uwe Kleine-König

Small cleanup reordering local variable declarations following reverse
christmas tree convention.

Signed-off-by: Angel Iglesias <ang.iglesiasg@gmail.com>

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index 6089f3f9d8f4..ea02a623bb58 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -766,8 +766,8 @@ static const struct iio_info bmp280_info = {
 
 static int bmp280_chip_config(struct bmp280_data *data)
 {
-	u8 osrs = FIELD_PREP(BMP280_OSRS_TEMP_MASK, data->oversampling_temp + 1) |
-		  FIELD_PREP(BMP280_OSRS_PRESS_MASK, data->oversampling_press + 1);
+	u8 osrs = FIELD_PREP(BMP280_OSRS_PRESS_MASK, data->oversampling_press + 1) |
+		  FIELD_PREP(BMP280_OSRS_TEMP_MASK, data->oversampling_temp + 1);
 	int ret;
 
 	ret = regmap_write_bits(data->regmap, BMP280_REG_CTRL_MEAS,
diff --git a/drivers/iio/pressure/bmp280-i2c.c b/drivers/iio/pressure/bmp280-i2c.c
index b3e069730f97..34e3bc758493 100644
--- a/drivers/iio/pressure/bmp280-i2c.c
+++ b/drivers/iio/pressure/bmp280-i2c.c
@@ -7,9 +7,9 @@
 
 static int bmp280_i2c_probe(struct i2c_client *client)
 {
-	struct regmap *regmap;
-	const struct bmp280_chip_info *chip_info;
 	const struct i2c_device_id *id = i2c_client_get_device_id(client);
+	const struct bmp280_chip_info *chip_info;
+	struct regmap *regmap;
 
 	chip_info = i2c_get_match_data(client);
 
diff --git a/drivers/iio/pressure/bmp280-spi.c b/drivers/iio/pressure/bmp280-spi.c
index 2eed483a8cc4..433d6fac83c4 100644
--- a/drivers/iio/pressure/bmp280-spi.c
+++ b/drivers/iio/pressure/bmp280-spi.c
@@ -14,8 +14,7 @@
 static int bmp280_regmap_spi_write(void *context, const void *data,
                                    size_t count)
 {
-	struct device *dev = context;
-	struct spi_device *spi = to_spi_device(dev);
+	struct spi_device *spi = to_spi_device(context);
 	u8 buf[2];
 
 	memcpy(buf, data, 2);
@@ -31,8 +30,7 @@ static int bmp280_regmap_spi_write(void *context, const void *data,
 static int bmp280_regmap_spi_read(void *context, const void *reg,
                                   size_t reg_size, void *val, size_t val_size)
 {
-	struct device *dev = context;
-	struct spi_device *spi = to_spi_device(dev);
+	struct spi_device *spi = to_spi_device(context);
 
 	return spi_write_then_read(spi, reg, reg_size, val, val_size);
 }
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/5] iio: pressure: bmp280: Allow multiple chips id per family of devices
  2023-10-15 15:16 [PATCH 0/5] Add support for BMP390 and various driver cleanups Angel Iglesias
                   ` (2 preceding siblings ...)
  2023-10-15 15:16 ` [PATCH 3/5] iio: pressure: bmp280: Rearrange vars in reverse xmas tree order Angel Iglesias
@ 2023-10-15 15:16 ` Angel Iglesias
  2023-10-16  7:53   ` Andy Shevchenko
  2023-10-16  8:38   ` Jonathan Cameron
  2023-10-15 15:16 ` [PATCH 5/5] iio: pressure: bmp280: Add support for BMP390 Angel Iglesias
  2023-10-16  7:54 ` [PATCH 0/5] Add support for BMP390 and various driver cleanups Andy Shevchenko
  5 siblings, 2 replies; 10+ messages in thread
From: Angel Iglesias @ 2023-10-15 15:16 UTC (permalink / raw)
  To: linux-iio
  Cc: Biju Das, linux-kernel, Angel Iglesias, Jonathan Cameron,
	Lars-Peter Clausen, Andy Shevchenko, Uwe Kleine-König

Improve device detection in certain chip families known to have various
chip ids.

Signed-off-by: Angel Iglesias <ang.iglesiasg@gmail.com>

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index ea02a623bb58..e3bb4d7906a9 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -38,6 +38,7 @@
 #include <linux/interrupt.h>
 #include <linux/irq.h> /* For irq_get_irq_data() */
 #include <linux/completion.h>
+#include <linux/overflow.h>
 #include <linux/pm_runtime.h>
 #include <linux/random.h>
 
@@ -794,10 +795,12 @@ static int bmp280_chip_config(struct bmp280_data *data)
 }
 
 static const int bmp280_oversampling_avail[] = { 1, 2, 4, 8, 16 };
+static const u8 bmp280_chip_ids[] = { BMP280_CHIP_ID };
 
 const struct bmp280_chip_info bmp280_chip_info = {
 	.id_reg = BMP280_REG_ID,
-	.chip_id = BMP280_CHIP_ID,
+	.chip_id = bmp280_chip_ids,
+	.num_chip_id = ARRAY_SIZE(bmp280_chip_ids),
 	.regmap_config = &bmp280_regmap_config,
 	.start_up_time = 2000,
 	.channels = bmp280_channels,
@@ -846,9 +849,12 @@ static int bme280_chip_config(struct bmp280_data *data)
 	return bmp280_chip_config(data);
 }
 
+static const u8 bme280_chip_ids[] = { BME280_CHIP_ID };
+
 const struct bmp280_chip_info bme280_chip_info = {
 	.id_reg = BMP280_REG_ID,
-	.chip_id = BME280_CHIP_ID,
+	.chip_id = bme280_chip_ids,
+	.num_chip_id = ARRAY_SIZE(bme280_chip_ids),
 	.regmap_config = &bmp280_regmap_config,
 	.start_up_time = 2000,
 	.channels = bmp280_channels,
@@ -1220,10 +1226,12 @@ static int bmp380_chip_config(struct bmp280_data *data)
 
 static const int bmp380_oversampling_avail[] = { 1, 2, 4, 8, 16, 32 };
 static const int bmp380_iir_filter_coeffs_avail[] = { 1, 2, 4, 8, 16, 32, 64, 128};
+static const u8 bmp380_chip_ids[] = { BMP380_CHIP_ID };
 
 const struct bmp280_chip_info bmp380_chip_info = {
 	.id_reg = BMP380_REG_ID,
-	.chip_id = BMP380_CHIP_ID,
+	.chip_id = bmp380_chip_ids,
+	.num_chip_id = ARRAY_SIZE(bmp380_chip_ids),
 	.regmap_config = &bmp380_regmap_config,
 	.start_up_time = 2000,
 	.channels = bmp380_channels,
@@ -1720,10 +1728,12 @@ static int bmp580_chip_config(struct bmp280_data *data)
 }
 
 static const int bmp580_oversampling_avail[] = { 1, 2, 4, 8, 16, 32, 64, 128 };
+static const u8 bmp580_chip_ids[] = { BMP580_CHIP_ID, BMP580_CHIP_ID_ALT };
 
 const struct bmp280_chip_info bmp580_chip_info = {
 	.id_reg = BMP580_REG_CHIP_ID,
-	.chip_id = BMP580_CHIP_ID,
+	.chip_id = bmp580_chip_ids,
+	.num_chip_id = ARRAY_SIZE(bmp580_chip_ids),
 	.regmap_config = &bmp580_regmap_config,
 	.start_up_time = 2000,
 	.channels = bmp380_channels,
@@ -1983,10 +1993,12 @@ static int bmp180_chip_config(struct bmp280_data *data)
 
 static const int bmp180_oversampling_temp_avail[] = { 1 };
 static const int bmp180_oversampling_press_avail[] = { 1, 2, 4, 8 };
+static const u8 bmp180_chip_ids[] = { BMP180_CHIP_ID };
 
 const struct bmp280_chip_info bmp180_chip_info = {
 	.id_reg = BMP280_REG_ID,
-	.chip_id = BMP180_CHIP_ID,
+	.chip_id = bmp180_chip_ids,
+	.num_chip_id = ARRAY_SIZE(bmp180_chip_ids),
 	.regmap_config = &bmp180_regmap_config,
 	.start_up_time = 2000,
 	.channels = bmp280_channels,
@@ -2077,6 +2089,7 @@ int bmp280_common_probe(struct device *dev,
 	struct bmp280_data *data;
 	struct gpio_desc *gpiod;
 	unsigned int chip_id;
+	unsigned int i;
 	int ret;
 
 	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
@@ -2142,10 +2155,30 @@ int bmp280_common_probe(struct device *dev,
 	ret = regmap_read(regmap, data->chip_info->id_reg, &chip_id);
 	if (ret < 0)
 		return ret;
-	if (chip_id != data->chip_info->chip_id) {
-		dev_err(dev, "bad chip id: expected %x got %x\n",
-			data->chip_info->chip_id, chip_id);
-		return -EINVAL;
+
+	for (i = 0; i < data->chip_info->num_chip_id; i++) {
+		if (chip_id == data->chip_info->chip_id[i])
+			break;
+	}
+
+	if (i == data->chip_info->num_chip_id) {
+		size_t nbuf;
+		char *buf;
+
+		// 0x<id>, so four chars per number plus one space + ENDL
+		if (check_mul_overflow(data->chip_info->num_chip_id, 5, &nbuf))
+			return ret;
+
+		buf = kmalloc_array(data->chip_info->num_chip_id, 5, GFP_KERNEL);
+		if (!buf)
+			return ret;
+
+		for (i = 0; i < data->chip_info->num_chip_id; i++)
+			snprintf(&buf[i*5], nbuf - i*5, "0x%x ", data->chip_info->chip_id[i]);
+
+		dev_err(dev, "bad chip id: expected one of [ %s ] got 0x%x\n", buf, chip_id);
+		kfree(buf);
+		return ret;
 	}
 
 	if (data->chip_info->preinit) {
diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
index 5c0563ce7572..a230fcfc4a85 100644
--- a/drivers/iio/pressure/bmp280.h
+++ b/drivers/iio/pressure/bmp280.h
@@ -418,7 +418,8 @@ struct bmp280_data {
 
 struct bmp280_chip_info {
 	unsigned int id_reg;
-	const unsigned int chip_id;
+	const u8 *chip_id;
+	int num_chip_id;
 
 	const struct regmap_config *regmap_config;
 
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 5/5] iio: pressure: bmp280: Add support for BMP390
  2023-10-15 15:16 [PATCH 0/5] Add support for BMP390 and various driver cleanups Angel Iglesias
                   ` (3 preceding siblings ...)
  2023-10-15 15:16 ` [PATCH 4/5] iio: pressure: bmp280: Allow multiple chips id per family of devices Angel Iglesias
@ 2023-10-15 15:16 ` Angel Iglesias
  2023-10-16  7:54 ` [PATCH 0/5] Add support for BMP390 and various driver cleanups Andy Shevchenko
  5 siblings, 0 replies; 10+ messages in thread
From: Angel Iglesias @ 2023-10-15 15:16 UTC (permalink / raw)
  To: linux-iio
  Cc: Biju Das, linux-kernel, Angel Iglesias, Jonathan Cameron,
	Lars-Peter Clausen, Andy Shevchenko, Uwe Kleine-König

Add BMP390 device ID to the supported IDs on bmp380 sensor family

Signed-off-by: Angel Iglesias <ang.iglesiasg@gmail.com>

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index e3bb4d7906a9..d2345956ebc8 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -13,6 +13,7 @@
  * https://www.bosch-sensortec.com/media/boschsensortec/downloads/datasheets/bst-bmp280-ds001.pdf
  * https://www.bosch-sensortec.com/media/boschsensortec/downloads/datasheets/bst-bme280-ds002.pdf
  * https://www.bosch-sensortec.com/media/boschsensortec/downloads/datasheets/bst-bmp388-ds001.pdf
+ * https://www.bosch-sensortec.com/media/boschsensortec/downloads/datasheets/bst-bmp390-ds002.pdf
  * https://www.bosch-sensortec.com/media/boschsensortec/downloads/datasheets/bst-bmp581-ds004.pdf
  *
  * Notice:
@@ -1226,7 +1227,7 @@ static int bmp380_chip_config(struct bmp280_data *data)
 
 static const int bmp380_oversampling_avail[] = { 1, 2, 4, 8, 16, 32 };
 static const int bmp380_iir_filter_coeffs_avail[] = { 1, 2, 4, 8, 16, 32, 64, 128};
-static const u8 bmp380_chip_ids[] = { BMP380_CHIP_ID };
+static const u8 bmp380_chip_ids[] = { BMP380_CHIP_ID, BMP390_CHIP_ID };
 
 const struct bmp280_chip_info bmp380_chip_info = {
 	.id_reg = BMP380_REG_ID,
diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
index a230fcfc4a85..2971bf58f802 100644
--- a/drivers/iio/pressure/bmp280.h
+++ b/drivers/iio/pressure/bmp280.h
@@ -292,6 +292,7 @@
 #define BMP580_CHIP_ID_ALT		0x51
 #define BMP180_CHIP_ID			0x55
 #define BMP280_CHIP_ID			0x58
+#define BMP390_CHIP_ID			0x60
 #define BME280_CHIP_ID			0x60
 #define BMP280_SOFT_RESET_VAL		0xB6
 
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/5] iio: pressure: bmp280: Allow multiple chips id per family of devices
  2023-10-15 15:16 ` [PATCH 4/5] iio: pressure: bmp280: Allow multiple chips id per family of devices Angel Iglesias
@ 2023-10-16  7:53   ` Andy Shevchenko
  2023-10-16  8:38   ` Jonathan Cameron
  1 sibling, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2023-10-16  7:53 UTC (permalink / raw)
  To: Angel Iglesias
  Cc: linux-iio, Biju Das, linux-kernel, Jonathan Cameron,
	Lars-Peter Clausen, Uwe Kleine-König

On Sun, Oct 15, 2023 at 05:16:26PM +0200, Angel Iglesias wrote:
> Improve device detection in certain chip families known to have various
> chip ids.

...

> +#include <linux/overflow.h>

Probably you don't need this, see below.

...

>  	ret = regmap_read(regmap, data->chip_info->id_reg, &chip_id);
>  	if (ret < 0)
>  		return ret;

> +	if (i == data->chip_info->num_chip_id) {
> +		size_t nbuf;
> +		char *buf;
> +
> +		// 0x<id>, so four chars per number plus one space + ENDL
> +		if (check_mul_overflow(data->chip_info->num_chip_id, 5, &nbuf))
> +			return ret;

First of all, it _implicitly_ returns 0 here...

> +		buf = kmalloc_array(data->chip_info->num_chip_id, 5, GFP_KERNEL);
> +		if (!buf)
> +			return ret;

...and here.

Second, kmalloc_array() has that check inside.

Third, define this magic 5 either as strlen(), or a constant (in latter case
with the comment of its meaning).

> +		for (i = 0; i < data->chip_info->num_chip_id; i++)
> +			snprintf(&buf[i*5], nbuf - i*5, "0x%x ", data->chip_info->chip_id[i]);

Fourth, use incremental position, i.e. use retuned value from snprintf().

> +		dev_err(dev, "bad chip id: expected one of [ %s ] got 0x%x\n", buf, chip_id);
> +		kfree(buf);

> +		return ret;

As per "first" and "second" above.

>  	}

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/5] Add support for BMP390 and various driver cleanups
  2023-10-15 15:16 [PATCH 0/5] Add support for BMP390 and various driver cleanups Angel Iglesias
                   ` (4 preceding siblings ...)
  2023-10-15 15:16 ` [PATCH 5/5] iio: pressure: bmp280: Add support for BMP390 Angel Iglesias
@ 2023-10-16  7:54 ` Andy Shevchenko
  5 siblings, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2023-10-16  7:54 UTC (permalink / raw)
  To: Angel Iglesias
  Cc: linux-iio, Biju Das, linux-kernel, Jonathan Cameron,
	Lars-Peter Clausen, Uwe Kleine-König

On Sun, Oct 15, 2023 at 05:16:22PM +0200, Angel Iglesias wrote:
> Add support for the Bosch BMP390 pressure sensors and includes minor
> driver code cleanups.
> Patches 1 and 2 update I2C and SPI driver matching logic using newer
> helper functions available.
> Patch 3 fixes minor coding style incoherences.
> Patches 4 and 5 introduce support for the new BMP390 sensor allowing
> sensor families to have a list of known device IDs to improve detection
> 
> This patch series is aggregates patches from various series:
> https://lore.kernel.org/all/20230812175808.236405-1-biju.das.jz@bp.renesas.com/
> https://lore.kernel.org/all/cover.1691952005.git.ang.iglesiasg@gmail.com/
> https://lore.kernel.org/all/cover.1692805377.git.ang.iglesiasg@gmail.com/

For the patches 1-3,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/5] iio: pressure: bmp280: Allow multiple chips id per family of devices
  2023-10-15 15:16 ` [PATCH 4/5] iio: pressure: bmp280: Allow multiple chips id per family of devices Angel Iglesias
  2023-10-16  7:53   ` Andy Shevchenko
@ 2023-10-16  8:38   ` Jonathan Cameron
  1 sibling, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2023-10-16  8:38 UTC (permalink / raw)
  To: Angel Iglesias
  Cc: linux-iio, Biju Das, linux-kernel, Lars-Peter Clausen,
	Andy Shevchenko, Uwe Kleine-König

On Sun, 15 Oct 2023 17:16:26 +0200
Angel Iglesias <ang.iglesiasg@gmail.com> wrote:

> Improve device detection in certain chip families known to have various
> chip ids.
> 
> Signed-off-by: Angel Iglesias <ang.iglesiasg@gmail.com>

A few minor things inline.

J
> 
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index ea02a623bb58..e3bb4d7906a9 100644
> --- a/drivers/iio/pressure/bmp280-core.c
> +++ b/drivers/iio/pressure/bmp280-core.c
> @@ -38,6 +38,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/irq.h> /* For irq_get_irq_data() */
>  #include <linux/completion.h>
> +#include <linux/overflow.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/random.h>
>  
> @@ -794,10 +795,12 @@ static int bmp280_chip_config(struct bmp280_data *data)
>  }
>  
>  static const int bmp280_oversampling_avail[] = { 1, 2, 4, 8, 16 };
> +static const u8 bmp280_chip_ids[] = { BMP280_CHIP_ID };
>  
>  const struct bmp280_chip_info bmp280_chip_info = {
>  	.id_reg = BMP280_REG_ID,
> -	.chip_id = BMP280_CHIP_ID,
> +	.chip_id = bmp280_chip_ids,
> +	.num_chip_id = ARRAY_SIZE(bmp280_chip_ids),
>  	.regmap_config = &bmp280_regmap_config,
>  	.start_up_time = 2000,
>  	.channels = bmp280_channels,
> @@ -846,9 +849,12 @@ static int bme280_chip_config(struct bmp280_data *data)
>  	return bmp280_chip_config(data);
>  }
>  
> +static const u8 bme280_chip_ids[] = { BME280_CHIP_ID };
> +
>  const struct bmp280_chip_info bme280_chip_info = {
>  	.id_reg = BMP280_REG_ID,
> -	.chip_id = BME280_CHIP_ID,
> +	.chip_id = bme280_chip_ids,
> +	.num_chip_id = ARRAY_SIZE(bme280_chip_ids),
>  	.regmap_config = &bmp280_regmap_config,
>  	.start_up_time = 2000,
>  	.channels = bmp280_channels,
> @@ -1220,10 +1226,12 @@ static int bmp380_chip_config(struct bmp280_data *data)
>  
>  static const int bmp380_oversampling_avail[] = { 1, 2, 4, 8, 16, 32 };
>  static const int bmp380_iir_filter_coeffs_avail[] = { 1, 2, 4, 8, 16, 32, 64, 128};
> +static const u8 bmp380_chip_ids[] = { BMP380_CHIP_ID };
>  
>  const struct bmp280_chip_info bmp380_chip_info = {
>  	.id_reg = BMP380_REG_ID,
> -	.chip_id = BMP380_CHIP_ID,
> +	.chip_id = bmp380_chip_ids,
> +	.num_chip_id = ARRAY_SIZE(bmp380_chip_ids),
>  	.regmap_config = &bmp380_regmap_config,
>  	.start_up_time = 2000,
>  	.channels = bmp380_channels,
> @@ -1720,10 +1728,12 @@ static int bmp580_chip_config(struct bmp280_data *data)
>  }
>  
>  static const int bmp580_oversampling_avail[] = { 1, 2, 4, 8, 16, 32, 64, 128 };
> +static const u8 bmp580_chip_ids[] = { BMP580_CHIP_ID, BMP580_CHIP_ID_ALT };
>  
>  const struct bmp280_chip_info bmp580_chip_info = {
>  	.id_reg = BMP580_REG_CHIP_ID,
> -	.chip_id = BMP580_CHIP_ID,
> +	.chip_id = bmp580_chip_ids,
> +	.num_chip_id = ARRAY_SIZE(bmp580_chip_ids),
>  	.regmap_config = &bmp580_regmap_config,
>  	.start_up_time = 2000,
>  	.channels = bmp380_channels,
> @@ -1983,10 +1993,12 @@ static int bmp180_chip_config(struct bmp280_data *data)
>  
>  static const int bmp180_oversampling_temp_avail[] = { 1 };
>  static const int bmp180_oversampling_press_avail[] = { 1, 2, 4, 8 };
> +static const u8 bmp180_chip_ids[] = { BMP180_CHIP_ID };
>  
>  const struct bmp280_chip_info bmp180_chip_info = {
>  	.id_reg = BMP280_REG_ID,
> -	.chip_id = BMP180_CHIP_ID,
> +	.chip_id = bmp180_chip_ids,
> +	.num_chip_id = ARRAY_SIZE(bmp180_chip_ids),
>  	.regmap_config = &bmp180_regmap_config,
>  	.start_up_time = 2000,
>  	.channels = bmp280_channels,
> @@ -2077,6 +2089,7 @@ int bmp280_common_probe(struct device *dev,
>  	struct bmp280_data *data;
>  	struct gpio_desc *gpiod;
>  	unsigned int chip_id;
> +	unsigned int i;
>  	int ret;
>  
>  	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> @@ -2142,10 +2155,30 @@ int bmp280_common_probe(struct device *dev,
>  	ret = regmap_read(regmap, data->chip_info->id_reg, &chip_id);
>  	if (ret < 0)
>  		return ret;
> -	if (chip_id != data->chip_info->chip_id) {
> -		dev_err(dev, "bad chip id: expected %x got %x\n",
> -			data->chip_info->chip_id, chip_id);
> -		return -EINVAL;
> +
> +	for (i = 0; i < data->chip_info->num_chip_id; i++) {
> +		if (chip_id == data->chip_info->chip_id[i])
> +			break;
> +	}
> +
> +	if (i == data->chip_info->num_chip_id) {
> +		size_t nbuf;
> +		char *buf;
> +
> +		// 0x<id>, so four chars per number plus one space + ENDL

Trivial but /* */ comment syntax for IIO please.

> +		if (check_mul_overflow(data->chip_info->num_chip_id, 5, &nbuf))
> +			return ret;
> +
> +		buf = kmalloc_array(data->chip_info->num_chip_id, 5, GFP_KERNEL);
> +		if (!buf)
> +			return ret;
> +
> +		for (i = 0; i < data->chip_info->num_chip_id; i++)
> +			snprintf(&buf[i*5], nbuf - i*5, "0x%x ", data->chip_info->chip_id[i]);

I'd uses multiple dev_err() lines and skip this complexity. Doesn't need to be as pretty as this
given it's an error reprot :)  However, we should support fallback compatibles. So this should
be a dev_warn at most and we should go on with whatever firmware said.  Note the original code didn't
do this (I used to get this wrong in reviews) but it would be good to improve it whilst here.

> +
> +		dev_err(dev, "bad chip id: expected one of [ %s ] got 0x%x\n", buf, chip_id);
In probe so dev_err_probe() preferred and with the autocleanup this can be return dev_err_probe()

> +		kfree(buf);

The new autocleanup stuff is nice for this sort of case.

char *buf __free(kfree) = NULL;



> +		return ret;
>  	}
>  
>  	if (data->chip_info->preinit) {
> diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
> index 5c0563ce7572..a230fcfc4a85 100644
> --- a/drivers/iio/pressure/bmp280.h
> +++ b/drivers/iio/pressure/bmp280.h
> @@ -418,7 +418,8 @@ struct bmp280_data {
>  
>  struct bmp280_chip_info {
>  	unsigned int id_reg;
> -	const unsigned int chip_id;
> +	const u8 *chip_id;
> +	int num_chip_id;
>  
>  	const struct regmap_config *regmap_config;
>  


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/5] iio: pressure: bmp280: Rearrange vars in reverse xmas tree order
  2023-10-15 15:16 ` [PATCH 3/5] iio: pressure: bmp280: Rearrange vars in reverse xmas tree order Angel Iglesias
@ 2023-10-16  8:40   ` Jonathan Cameron
  0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2023-10-16  8:40 UTC (permalink / raw)
  To: Angel Iglesias
  Cc: linux-iio, Biju Das, linux-kernel, Lars-Peter Clausen,
	Andy Shevchenko, Uwe Kleine-König

On Sun, 15 Oct 2023 17:16:25 +0200
Angel Iglesias <ang.iglesiasg@gmail.com> wrote:

> Small cleanup reordering local variable declarations following reverse
> christmas tree convention.
> 
> Signed-off-by: Angel Iglesias <ang.iglesiasg@gmail.com>
> 
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index 6089f3f9d8f4..ea02a623bb58 100644
> --- a/drivers/iio/pressure/bmp280-core.c
> +++ b/drivers/iio/pressure/bmp280-core.c
> @@ -766,8 +766,8 @@ static const struct iio_info bmp280_info = {
>  
>  static int bmp280_chip_config(struct bmp280_data *data)
>  {
> -	u8 osrs = FIELD_PREP(BMP280_OSRS_TEMP_MASK, data->oversampling_temp + 1) |
> -		  FIELD_PREP(BMP280_OSRS_PRESS_MASK, data->oversampling_press + 1);
> +	u8 osrs = FIELD_PREP(BMP280_OSRS_PRESS_MASK, data->oversampling_press + 1) |
> +		  FIELD_PREP(BMP280_OSRS_TEMP_MASK, data->oversampling_temp + 1);

For fields within a register, don't bother.
Normally the order is dictated by someone reading the fields on a datasheet.  
Feels like reorganzing them is just noise to me.

The rest look like good little tidy ups to me.

>  	int ret;
>  
>  	ret = regmap_write_bits(data->regmap, BMP280_REG_CTRL_MEAS,
> diff --git a/drivers/iio/pressure/bmp280-i2c.c b/drivers/iio/pressure/bmp280-i2c.c
> index b3e069730f97..34e3bc758493 100644
> --- a/drivers/iio/pressure/bmp280-i2c.c
> +++ b/drivers/iio/pressure/bmp280-i2c.c
> @@ -7,9 +7,9 @@
>  
>  static int bmp280_i2c_probe(struct i2c_client *client)
>  {
> -	struct regmap *regmap;
> -	const struct bmp280_chip_info *chip_info;
>  	const struct i2c_device_id *id = i2c_client_get_device_id(client);
> +	const struct bmp280_chip_info *chip_info;
> +	struct regmap *regmap;
>  
>  	chip_info = i2c_get_match_data(client);
>  
> diff --git a/drivers/iio/pressure/bmp280-spi.c b/drivers/iio/pressure/bmp280-spi.c
> index 2eed483a8cc4..433d6fac83c4 100644
> --- a/drivers/iio/pressure/bmp280-spi.c
> +++ b/drivers/iio/pressure/bmp280-spi.c
> @@ -14,8 +14,7 @@
>  static int bmp280_regmap_spi_write(void *context, const void *data,
>                                     size_t count)
>  {
> -	struct device *dev = context;
> -	struct spi_device *spi = to_spi_device(dev);
> +	struct spi_device *spi = to_spi_device(context);
>  	u8 buf[2];
>  
>  	memcpy(buf, data, 2);
> @@ -31,8 +30,7 @@ static int bmp280_regmap_spi_write(void *context, const void *data,
>  static int bmp280_regmap_spi_read(void *context, const void *reg,
>                                    size_t reg_size, void *val, size_t val_size)
>  {
> -	struct device *dev = context;
> -	struct spi_device *spi = to_spi_device(dev);
> +	struct spi_device *spi = to_spi_device(context);
>  
>  	return spi_write_then_read(spi, reg, reg_size, val, val_size);
>  }


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-10-16  8:40 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-15 15:16 [PATCH 0/5] Add support for BMP390 and various driver cleanups Angel Iglesias
2023-10-15 15:16 ` [PATCH 1/5] iio: pressure: bmp280: Use i2c_get_match_data() Angel Iglesias
2023-10-15 15:16 ` [PATCH 2/5] iio: pressure: bmp280: Use spi_get_device_match_data() Angel Iglesias
2023-10-15 15:16 ` [PATCH 3/5] iio: pressure: bmp280: Rearrange vars in reverse xmas tree order Angel Iglesias
2023-10-16  8:40   ` Jonathan Cameron
2023-10-15 15:16 ` [PATCH 4/5] iio: pressure: bmp280: Allow multiple chips id per family of devices Angel Iglesias
2023-10-16  7:53   ` Andy Shevchenko
2023-10-16  8:38   ` Jonathan Cameron
2023-10-15 15:16 ` [PATCH 5/5] iio: pressure: bmp280: Add support for BMP390 Angel Iglesias
2023-10-16  7:54 ` [PATCH 0/5] Add support for BMP390 and various driver cleanups Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).