diff mbox series

[v2,4/5] iio: pressure: bmp280: Allow multiple chips id per family of devices

Message ID eade22d11e9de4405ea19fdaa5a8249143ae94df.1697994521.git.ang.iglesiasg@gmail.com (mailing list archive)
State Accepted
Headers show
Series Add support for BMP390 and various driver cleanups | expand

Commit Message

Angel Iglesias Oct. 22, 2023, 5:22 p.m. UTC 
Improve device detection in certain chip families known to have various
chip ids.
When no known ids match, gives a warning but follows along what device
said on the firmware and tries to configure it.

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

Comments

Andy Shevchenko Oct. 23, 2023, 11:25 a.m. UTC | #1 
On Sun, Oct 22, 2023 at 07:22:20PM +0200, Angel Iglesias wrote:
> Improve device detection in certain chip families known to have various
> chip ids.
> When no known ids match, gives a warning but follows along what device
> said on the firmware and tries to configure it.

I would rephrase it a bit:

"Improve device detection in certain chip families known to have
various chip IDs. When no ID matches, give a warning but follow
along what device said on the firmware side and try to configure
it."

...

> +	for (i = 0; i < data->chip_info->num_chip_id; i++) {
> +		if (chip_id == data->chip_info->chip_id[i]) {
> +			dev_info(dev, "0x%x is a known chip id for %s\n", chip_id, name);
> +			break;
> +		}

> +		dev_warn(dev, "chip id 0x%x does not match known id 0x%x\n",
> +			 chip_id, data->chip_info->chip_id[i]);

If the matching ID is not the first one, user will have an unneeded warning here.

>  	}
Angel Iglesias Oct. 28, 2023, 11:23 a.m. UTC | #2 
On Fri, 2023-10-27 at 14:46 +0100, Jonathan Cameron wrote:
> On Fri, 27 Oct 2023 14:42:34 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:
> 
> > On Mon, 23 Oct 2023 14:25:42 +0300
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > 
> > > On Sun, Oct 22, 2023 at 07:22:20PM +0200, Angel Iglesias wrote:  
> > > > Improve device detection in certain chip families known to have various
> > > > chip ids.
> > > > When no known ids match, gives a warning but follows along what device
> > > > said on the firmware and tries to configure it.    
> > > 
> > > I would rephrase it a bit:
> > > 
> > > "Improve device detection in certain chip families known to have
> > > various chip IDs. When no ID matches, give a warning but follow
> > > along what device said on the firmware side and try to configure
> > > it."
> > > 
> > > ...
> > >   
> > > > +	for (i = 0; i < data->chip_info->num_chip_id; i++) {
> > > > +		if (chip_id == data->chip_info->chip_id[i]) {
> > > > +			dev_info(dev, "0x%x is a known chip id for
> > > > %s\n", chip_id, name);
> > > > +			break;
> > > > +		}    
> > >   
> > > > +		dev_warn(dev, "chip id 0x%x does not match known id
> > > > 0x%x\n",
> > > > +			 chip_id, data->chip_info->chip_id[i]);    
> > > 
> > > If the matching ID is not the first one, user will have an unneeded
> > > warning here.  
> > 
> > Could be a dev_dbg() but I'd just drop it entirely.
> > 
> Given that was all that came up, I've hopefully saved us all time by
> dropping the bring and changing the patch description as Andy suggested.
> 
> With that done, applied.
> 
> Jonathan
> 

Sorry for the extra work Jonathan. Next time I'll be quicker checking the inbox.

Thanks for your time guys.

Angel

> > 
> > >   
> > > >  	}    
> > >   
> > 
>
Jonathan Cameron Oct. 28, 2023, 1:07 p.m. UTC | #3 
On Sat, 28 Oct 2023 13:23:46 +0200
Angel Iglesias <ang.iglesiasg@gmail.com> wrote:

> On Fri, 2023-10-27 at 14:46 +0100, Jonathan Cameron wrote:
> > On Fri, 27 Oct 2023 14:42:34 +0100
> > Jonathan Cameron <jic23@kernel.org> wrote:
> >   
> > > On Mon, 23 Oct 2023 14:25:42 +0300
> > > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > >   
> > > > On Sun, Oct 22, 2023 at 07:22:20PM +0200, Angel Iglesias wrote:    
> > > > > Improve device detection in certain chip families known to have various
> > > > > chip ids.
> > > > > When no known ids match, gives a warning but follows along what device
> > > > > said on the firmware and tries to configure it.      
> > > > 
> > > > I would rephrase it a bit:
> > > > 
> > > > "Improve device detection in certain chip families known to have
> > > > various chip IDs. When no ID matches, give a warning but follow
> > > > along what device said on the firmware side and try to configure
> > > > it."
> > > > 
> > > > ...
> > > >     
> > > > > +	for (i = 0; i < data->chip_info->num_chip_id; i++) {
> > > > > +		if (chip_id == data->chip_info->chip_id[i]) {
> > > > > +			dev_info(dev, "0x%x is a known chip id for
> > > > > %s\n", chip_id, name);
> > > > > +			break;
> > > > > +		}      
> > > >     
> > > > > +		dev_warn(dev, "chip id 0x%x does not match known id
> > > > > 0x%x\n",
> > > > > +			 chip_id, data->chip_info->chip_id[i]);      
> > > > 
> > > > If the matching ID is not the first one, user will have an unneeded
> > > > warning here.    
> > > 
> > > Could be a dev_dbg() but I'd just drop it entirely.
> > >   
> > Given that was all that came up, I've hopefully saved us all time by
> > dropping the bring and changing the patch description as Andy suggested.
> > 
> > With that done, applied.
> > 
> > Jonathan
> >   
> 
> Sorry for the extra work Jonathan. Next time I'll be quicker checking the inbox.
Don't worry about it!  It's a bit random when I get to IIO stuff these days
so when I do, I like to clear as much as possible out in one go.

J 

> 
> Thanks for your time guys.
> 
> Angel
> 
> > >   
> > > >     
> > > > >  	}      
> > > >     
> > >   
> >   
>
diff mbox series

Patch

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index a2ef1373a274..deb336781b26 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -794,10 +794,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 +848,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 +1225,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 +1727,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 +1992,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 +2088,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,12 +2154,19 @@  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]) {
+			dev_info(dev, "0x%x is a known chip id for %s\n", chip_id, name);
+			break;
+		}
+		dev_warn(dev, "chip id 0x%x does not match known id 0x%x\n",
+			 chip_id, data->chip_info->chip_id[i]);
 	}
 
+	if (i == data->chip_info->num_chip_id)
+		dev_warn(dev, "bad chip id: 0x%x is not a known chip id\n", chip_id);
+
 	if (data->chip_info->preinit) {
 		ret = data->chip_info->preinit(data);
 		if (ret)
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;