| Message ID | f0368a3f460707417110dffdb0166824da20af15.1672062380.git.ang.iglesiasg@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add support for pressure sensor Bosch BMP580 | expand |
On Mon, Dec 26, 2022 at 03:29:20PM +0100, Angel Iglesias wrote: > Adds enumeration to improve handling the different supported sensors > on driver initialization. This avoid collisions if different variants > share the same device idetifier on ID register. As per v1, use pointers in the ID tables.
On Mon, 26 Dec 2022 15:29:20 +0100 Angel Iglesias <ang.iglesiasg@gmail.com> wrote: > Adds enumeration to improve handling the different supported sensors > on driver initialization. This avoid collisions if different variants > share the same device idetifier on ID register. If we get two parts with the same ID that need different handling then we should probably be shouting at Bosch. Still I don't mind tidying this up anyway as using the CHIP_IDs is messy - however... Please be careful to make sure you have responded (either by changes, or by replying to review to say why you aren't making changes). If you are not receiving all emails, then you can always check lore.kernel.org to make sure you didn't miss any reviews. As Andy suggested, switch over to using pointers to the chip_info structure as the data element in *_device_id tables. It usually ends up neater in the end than messing around with an indirection via an enum value. > > 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 c0aff78489b4..46959a91408f 100644 > --- a/drivers/iio/pressure/bmp280-core.c > +++ b/drivers/iio/pressure/bmp280-core.c > @@ -186,6 +186,7 @@ struct bmp280_data { > > struct bmp280_chip_info { > unsigned int id_reg; > + const unsigned int chip_id; > > const struct iio_chan_spec *channels; > int num_channels; > @@ -907,6 +908,7 @@ static const int bmp280_oversampling_avail[] = { 1, 2, 4, 8, 16 }; > > static const struct bmp280_chip_info bmp280_chip_info = { > .id_reg = BMP280_REG_ID, > + .chip_id = BMP280_CHIP_ID, > .start_up_time = 2000, > .channels = bmp280_channels, > .num_channels = 2, > @@ -955,6 +957,7 @@ static int bme280_chip_config(struct bmp280_data *data) > > static const struct bmp280_chip_info bme280_chip_info = { > .id_reg = BMP280_REG_ID, > + .chip_id = BME280_CHIP_ID, > .start_up_time = 2000, > .channels = bmp280_channels, > .num_channels = 3, > @@ -1321,6 +1324,7 @@ static const int bmp380_iir_filter_coeffs_avail[] = { 1, 2, 4, 8, 16, 32, 64, 12 > > static const struct bmp280_chip_info bmp380_chip_info = { > .id_reg = BMP380_REG_ID, > + .chip_id = BMP380_CHIP_ID, > .start_up_time = 2000, > .channels = bmp380_channels, > .num_channels = 2, > @@ -1581,6 +1585,7 @@ static const int bmp180_oversampling_press_avail[] = { 1, 2, 4, 8 }; > > static const struct bmp280_chip_info bmp180_chip_info = { > .id_reg = BMP280_REG_ID, > + .chip_id = BMP180_CHIP_ID, > .start_up_time = 2000, > .channels = bmp280_channels, > .num_channels = 2, > @@ -1685,16 +1690,16 @@ int bmp280_common_probe(struct device *dev, > indio_dev->modes = INDIO_DIRECT_MODE; > > switch (chip) { > - case BMP180_CHIP_ID: > + case BMP180: > chip_info = &bmp180_chip_info; > break; > - case BMP280_CHIP_ID: > + case BMP280: > chip_info = &bmp280_chip_info; > break; > - case BME280_CHIP_ID: > + case BME280: > chip_info = &bme280_chip_info; > break; > - case BMP380_CHIP_ID: > + case BMP380: > chip_info = &bmp380_chip_info; If you use a pointer directly then no need for this switch statement at all. > break; > default: > @@ -1751,9 +1756,9 @@ 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 != chip) { > + if (chip_id != data->chip_info->chip_id) { > dev_err(dev, "bad chip id: expected %x got %x\n", > - chip, chip_id); > + data->chip_info->chip_id, chip_id); > return -EINVAL; > } > > diff --git a/drivers/iio/pressure/bmp280-i2c.c b/drivers/iio/pressure/bmp280-i2c.c > index 14eab086d24a..59921e8cd592 100644 > --- a/drivers/iio/pressure/bmp280-i2c.c > +++ b/drivers/iio/pressure/bmp280-i2c.c > @@ -12,14 +12,14 @@ static int bmp280_i2c_probe(struct i2c_client *client) > const struct i2c_device_id *id = i2c_client_get_device_id(client); > > switch (id->driver_data) { > - case BMP180_CHIP_ID: > + case BMP180: > regmap_config = &bmp180_regmap_config; > break; > - case BMP280_CHIP_ID: > - case BME280_CHIP_ID: > + case BMP280: > + case BME280: > regmap_config = &bmp280_regmap_config; > break; > - case BMP380_CHIP_ID: > + case BMP380: > regmap_config = &bmp380_regmap_config; > break; > default: > @@ -40,21 +40,21 @@ static int bmp280_i2c_probe(struct i2c_client *client) > } > > static const struct of_device_id bmp280_of_i2c_match[] = { > - { .compatible = "bosch,bmp085", .data = (void *)BMP180_CHIP_ID }, > - { .compatible = "bosch,bmp180", .data = (void *)BMP180_CHIP_ID }, > - { .compatible = "bosch,bmp280", .data = (void *)BMP280_CHIP_ID }, > - { .compatible = "bosch,bme280", .data = (void *)BME280_CHIP_ID }, > - { .compatible = "bosch,bmp380", .data = (void *)BMP380_CHIP_ID }, > + { .compatible = "bosch,bmp085", .data = (void *)BMP180 }, > + { .compatible = "bosch,bmp180", .data = (void *)BMP180 }, > + { .compatible = "bosch,bmp280", .data = (void *)BMP280 }, > + { .compatible = "bosch,bme280", .data = (void *)BME280 }, > + { .compatible = "bosch,bmp380", .data = (void *)BMP380 }, > { }, > }; > MODULE_DEVICE_TABLE(of, bmp280_of_i2c_match); > > static const struct i2c_device_id bmp280_i2c_id[] = { > - {"bmp085", BMP180_CHIP_ID }, > - {"bmp180", BMP180_CHIP_ID }, > - {"bmp280", BMP280_CHIP_ID }, > - {"bme280", BME280_CHIP_ID }, > - {"bmp380", BMP380_CHIP_ID }, > + {"bmp085", BMP180 }, > + {"bmp180", BMP180 }, > + {"bmp280", BMP280 }, > + {"bme280", BME280 }, > + {"bmp380", BMP380 }, > { }, > }; > MODULE_DEVICE_TABLE(i2c, bmp280_i2c_id); > diff --git a/drivers/iio/pressure/bmp280-spi.c b/drivers/iio/pressure/bmp280-spi.c > index 011c68e07ebf..4a2df5b5d838 100644 > --- a/drivers/iio/pressure/bmp280-spi.c > +++ b/drivers/iio/pressure/bmp280-spi.c > @@ -59,14 +59,14 @@ static int bmp280_spi_probe(struct spi_device *spi) > } > > switch (id->driver_data) { > - case BMP180_CHIP_ID: > + case BMP180: > regmap_config = &bmp180_regmap_config; > break; > - case BMP280_CHIP_ID: > - case BME280_CHIP_ID: > + case BMP280: > + case BME280: > regmap_config = &bmp280_regmap_config; > break; > - case BMP380_CHIP_ID: > + case BMP380: > regmap_config = &bmp380_regmap_config; > break; > default: > @@ -101,11 +101,11 @@ static const struct of_device_id bmp280_of_spi_match[] = { > MODULE_DEVICE_TABLE(of, bmp280_of_spi_match); > > static const struct spi_device_id bmp280_spi_id[] = { > - { "bmp180", BMP180_CHIP_ID }, > - { "bmp181", BMP180_CHIP_ID }, > - { "bmp280", BMP280_CHIP_ID }, > - { "bme280", BME280_CHIP_ID }, > - { "bmp380", BMP380_CHIP_ID }, > + { "bmp180", BMP180 }, > + { "bmp181", BMP180 }, > + { "bmp280", BMP280 }, > + { "bme280", BME280 }, > + { "bmp380", BMP380 }, > { } > }; > MODULE_DEVICE_TABLE(spi, bmp280_spi_id); > diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h > index c791325c7416..efc31bc84708 100644 > --- a/drivers/iio/pressure/bmp280.h > +++ b/drivers/iio/pressure/bmp280.h > @@ -191,6 +191,14 @@ > #define BMP280_PRESS_SKIPPED 0x80000 > #define BMP280_HUMIDITY_SKIPPED 0x8000 > > +/* Enum with supported pressure sensor models */ > +enum bmp280_variant { > + BMP180, > + BMP280, > + BME280, > + BMP380, > +}; > + > /* Regmap configurations */ > extern const struct regmap_config bmp180_regmap_config; > extern const struct regmap_config bmp280_regmap_config;
On Fri, 2022-12-30 at 18:14 +0000, Jonathan Cameron wrote: > On Mon, 26 Dec 2022 15:29:20 +0100 > Angel Iglesias <ang.iglesiasg@gmail.com> wrote: > > > Adds enumeration to improve handling the different supported sensors > > on driver initialization. This avoid collisions if different variants > > share the same device idetifier on ID register. > > If we get two parts with the same ID that need different handling > then we should probably be shouting at Bosch. Still I don't mind > tidying this up anyway as using the CHIP_IDs is messy - however... > > Please be careful to make sure you have responded (either by changes, or by > replying to review to say why you aren't making changes). If you are not > receiving > all emails, then you can always check lore.kernel.org to make sure you didn't > miss > any reviews. I've been away for the week visiting family and friends. Sorry for leaving your reviews lingering unanswered. Yeah, I kinda rushed this part of the patch to send it before christmas, not my brightest call. I like Andy suggestion, but I have a few questions on handling the regmap and the chip config using the pointers, please refer to the reply I sent to Andy review for the details. Thanks for your time, Angel > As Andy suggested, switch over to using pointers to the chip_info structure as > the > data element in *_device_id tables. It usually ends up neater in the end than > messing around with an indirection via an enum value. > > > > > 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 c0aff78489b4..46959a91408f 100644 > > --- a/drivers/iio/pressure/bmp280-core.c > > +++ b/drivers/iio/pressure/bmp280-core.c > > @@ -186,6 +186,7 @@ struct bmp280_data { > > > > struct bmp280_chip_info { > > unsigned int id_reg; > > + const unsigned int chip_id; > > > > const struct iio_chan_spec *channels; > > int num_channels; > > @@ -907,6 +908,7 @@ static const int bmp280_oversampling_avail[] = { 1, 2, > > 4, 8, 16 }; > > > > static const struct bmp280_chip_info bmp280_chip_info = { > > .id_reg = BMP280_REG_ID, > > + .chip_id = BMP280_CHIP_ID, > > .start_up_time = 2000, > > .channels = bmp280_channels, > > .num_channels = 2, > > @@ -955,6 +957,7 @@ static int bme280_chip_config(struct bmp280_data *data) > > > > static const struct bmp280_chip_info bme280_chip_info = { > > .id_reg = BMP280_REG_ID, > > + .chip_id = BME280_CHIP_ID, > > .start_up_time = 2000, > > .channels = bmp280_channels, > > .num_channels = 3, > > @@ -1321,6 +1324,7 @@ static const int bmp380_iir_filter_coeffs_avail[] = { > > 1, 2, 4, 8, 16, 32, 64, 12 > > > > static const struct bmp280_chip_info bmp380_chip_info = { > > .id_reg = BMP380_REG_ID, > > + .chip_id = BMP380_CHIP_ID, > > .start_up_time = 2000, > > .channels = bmp380_channels, > > .num_channels = 2, > > @@ -1581,6 +1585,7 @@ static const int bmp180_oversampling_press_avail[] = { > > 1, 2, 4, 8 }; > > > > static const struct bmp280_chip_info bmp180_chip_info = { > > .id_reg = BMP280_REG_ID, > > + .chip_id = BMP180_CHIP_ID, > > .start_up_time = 2000, > > .channels = bmp280_channels, > > .num_channels = 2, > > @@ -1685,16 +1690,16 @@ int bmp280_common_probe(struct device *dev, > > indio_dev->modes = INDIO_DIRECT_MODE; > > > > switch (chip) { > > - case BMP180_CHIP_ID: > > + case BMP180: > > chip_info = &bmp180_chip_info; > > break; > > - case BMP280_CHIP_ID: > > + case BMP280: > > chip_info = &bmp280_chip_info; > > break; > > - case BME280_CHIP_ID: > > + case BME280: > > chip_info = &bme280_chip_info; > > break; > > - case BMP380_CHIP_ID: > > + case BMP380: > > chip_info = &bmp380_chip_info; > > If you use a pointer directly then no need for this switch statement at all. > > > break; > > default: > > @@ -1751,9 +1756,9 @@ 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 != chip) { > > + if (chip_id != data->chip_info->chip_id) { > > dev_err(dev, "bad chip id: expected %x got %x\n", > > - chip, chip_id); > > + data->chip_info->chip_id, chip_id); > > return -EINVAL; > > } > > > > diff --git a/drivers/iio/pressure/bmp280-i2c.c > > b/drivers/iio/pressure/bmp280-i2c.c > > index 14eab086d24a..59921e8cd592 100644 > > --- a/drivers/iio/pressure/bmp280-i2c.c > > +++ b/drivers/iio/pressure/bmp280-i2c.c > > @@ -12,14 +12,14 @@ static int bmp280_i2c_probe(struct i2c_client *client) > > const struct i2c_device_id *id = i2c_client_get_device_id(client); > > > > switch (id->driver_data) { > > - case BMP180_CHIP_ID: > > + case BMP180: > > regmap_config = &bmp180_regmap_config; > > break; > > - case BMP280_CHIP_ID: > > - case BME280_CHIP_ID: > > + case BMP280: > > + case BME280: > > regmap_config = &bmp280_regmap_config; > > break; > > - case BMP380_CHIP_ID: > > + case BMP380: > > regmap_config = &bmp380_regmap_config; > > break; > > default: > > @@ -40,21 +40,21 @@ static int bmp280_i2c_probe(struct i2c_client *client) > > } > > > > static const struct of_device_id bmp280_of_i2c_match[] = { > > - { .compatible = "bosch,bmp085", .data = (void *)BMP180_CHIP_ID }, > > - { .compatible = "bosch,bmp180", .data = (void *)BMP180_CHIP_ID }, > > - { .compatible = "bosch,bmp280", .data = (void *)BMP280_CHIP_ID }, > > - { .compatible = "bosch,bme280", .data = (void *)BME280_CHIP_ID }, > > - { .compatible = "bosch,bmp380", .data = (void *)BMP380_CHIP_ID }, > > + { .compatible = "bosch,bmp085", .data = (void *)BMP180 }, > > + { .compatible = "bosch,bmp180", .data = (void *)BMP180 }, > > + { .compatible = "bosch,bmp280", .data = (void *)BMP280 }, > > + { .compatible = "bosch,bme280", .data = (void *)BME280 }, > > + { .compatible = "bosch,bmp380", .data = (void *)BMP380 }, > > { }, > > }; > > MODULE_DEVICE_TABLE(of, bmp280_of_i2c_match); > > > > static const struct i2c_device_id bmp280_i2c_id[] = { > > - {"bmp085", BMP180_CHIP_ID }, > > - {"bmp180", BMP180_CHIP_ID }, > > - {"bmp280", BMP280_CHIP_ID }, > > - {"bme280", BME280_CHIP_ID }, > > - {"bmp380", BMP380_CHIP_ID }, > > + {"bmp085", BMP180 }, > > + {"bmp180", BMP180 }, > > + {"bmp280", BMP280 }, > > + {"bme280", BME280 }, > > + {"bmp380", BMP380 }, > > { }, > > }; > > MODULE_DEVICE_TABLE(i2c, bmp280_i2c_id); > > diff --git a/drivers/iio/pressure/bmp280-spi.c > > b/drivers/iio/pressure/bmp280-spi.c > > index 011c68e07ebf..4a2df5b5d838 100644 > > --- a/drivers/iio/pressure/bmp280-spi.c > > +++ b/drivers/iio/pressure/bmp280-spi.c > > @@ -59,14 +59,14 @@ static int bmp280_spi_probe(struct spi_device *spi) > > } > > > > switch (id->driver_data) { > > - case BMP180_CHIP_ID: > > + case BMP180: > > regmap_config = &bmp180_regmap_config; > > break; > > - case BMP280_CHIP_ID: > > - case BME280_CHIP_ID: > > + case BMP280: > > + case BME280: > > regmap_config = &bmp280_regmap_config; > > break; > > - case BMP380_CHIP_ID: > > + case BMP380: > > regmap_config = &bmp380_regmap_config; > > break; > > default: > > @@ -101,11 +101,11 @@ static const struct of_device_id bmp280_of_spi_match[] > > = { > > MODULE_DEVICE_TABLE(of, bmp280_of_spi_match); > > > > static const struct spi_device_id bmp280_spi_id[] = { > > - { "bmp180", BMP180_CHIP_ID }, > > - { "bmp181", BMP180_CHIP_ID }, > > - { "bmp280", BMP280_CHIP_ID }, > > - { "bme280", BME280_CHIP_ID }, > > - { "bmp380", BMP380_CHIP_ID }, > > + { "bmp180", BMP180 }, > > + { "bmp181", BMP180 }, > > + { "bmp280", BMP280 }, > > + { "bme280", BME280 }, > > + { "bmp380", BMP380 }, > > { } > > }; > > MODULE_DEVICE_TABLE(spi, bmp280_spi_id); > > diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h > > index c791325c7416..efc31bc84708 100644 > > --- a/drivers/iio/pressure/bmp280.h > > +++ b/drivers/iio/pressure/bmp280.h > > @@ -191,6 +191,14 @@ > > #define BMP280_PRESS_SKIPPED 0x80000 > > #define BMP280_HUMIDITY_SKIPPED 0x8000 > > > > +/* Enum with supported pressure sensor models */ > > +enum bmp280_variant { > > + BMP180, > > + BMP280, > > + BME280, > > + BMP380, > > +}; > > + > > /* Regmap configurations */ > > extern const struct regmap_config bmp180_regmap_config; > > extern const struct regmap_config bmp280_regmap_config; >
On Tue, 2022-12-27 at 23:37 +0200, Andy Shevchenko wrote: > On Mon, Dec 26, 2022 at 03:29:20PM +0100, Angel Iglesias wrote: > > Adds enumeration to improve handling the different supported sensors > > on driver initialization. This avoid collisions if different variants > > share the same device idetifier on ID register. > > As per v1, use pointers in the ID tables. > Taking your suggestion and Jonathan's remarks into account seems to me like the best approach here is using chip_info pointer for each driver as the pointer set on the id tables. As in the i2c and spi drivers, the enum is used to fetch the correct regmap configuration, and later in the shared probe, the chip_info. The logical follow-up would be adding the regmap configuration to the chip_info, right? Or is there a better solution I'm not seeing right now? Thanks for your time, Angel
On Sun, 01 Jan 2023 12:04:40 +0100 Angel Iglesias <ang.iglesiasg@gmail.com> wrote: > On Tue, 2022-12-27 at 23:37 +0200, Andy Shevchenko wrote: > > On Mon, Dec 26, 2022 at 03:29:20PM +0100, Angel Iglesias wrote: > > > Adds enumeration to improve handling the different supported sensors > > > on driver initialization. This avoid collisions if different variants > > > share the same device idetifier on ID register. > > > > As per v1, use pointers in the ID tables. > > > > Taking your suggestion and Jonathan's remarks into account seems to me like the > best approach here is using chip_info pointer for each driver as the pointer set > on the id tables. Yes. > As in the i2c and spi drivers, the enum is used to fetch the > correct regmap configuration, and later in the shared probe, the chip_info. The > logical follow-up would be adding the regmap configuration to the chip_info, > right? Makes sense. > Or is there a better solution I'm not seeing right now? If I'm understanding what you have above, then this is exactly what we would want to do here. Thanks, Jonathan > > Thanks for your time, > Angel
diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c index c0aff78489b4..46959a91408f 100644 --- a/drivers/iio/pressure/bmp280-core.c +++ b/drivers/iio/pressure/bmp280-core.c @@ -186,6 +186,7 @@ struct bmp280_data { struct bmp280_chip_info { unsigned int id_reg; + const unsigned int chip_id; const struct iio_chan_spec *channels; int num_channels; @@ -907,6 +908,7 @@ static const int bmp280_oversampling_avail[] = { 1, 2, 4, 8, 16 }; static const struct bmp280_chip_info bmp280_chip_info = { .id_reg = BMP280_REG_ID, + .chip_id = BMP280_CHIP_ID, .start_up_time = 2000, .channels = bmp280_channels, .num_channels = 2, @@ -955,6 +957,7 @@ static int bme280_chip_config(struct bmp280_data *data) static const struct bmp280_chip_info bme280_chip_info = { .id_reg = BMP280_REG_ID, + .chip_id = BME280_CHIP_ID, .start_up_time = 2000, .channels = bmp280_channels, .num_channels = 3, @@ -1321,6 +1324,7 @@ static const int bmp380_iir_filter_coeffs_avail[] = { 1, 2, 4, 8, 16, 32, 64, 12 static const struct bmp280_chip_info bmp380_chip_info = { .id_reg = BMP380_REG_ID, + .chip_id = BMP380_CHIP_ID, .start_up_time = 2000, .channels = bmp380_channels, .num_channels = 2, @@ -1581,6 +1585,7 @@ static const int bmp180_oversampling_press_avail[] = { 1, 2, 4, 8 }; static const struct bmp280_chip_info bmp180_chip_info = { .id_reg = BMP280_REG_ID, + .chip_id = BMP180_CHIP_ID, .start_up_time = 2000, .channels = bmp280_channels, .num_channels = 2, @@ -1685,16 +1690,16 @@ int bmp280_common_probe(struct device *dev, indio_dev->modes = INDIO_DIRECT_MODE; switch (chip) { - case BMP180_CHIP_ID: + case BMP180: chip_info = &bmp180_chip_info; break; - case BMP280_CHIP_ID: + case BMP280: chip_info = &bmp280_chip_info; break; - case BME280_CHIP_ID: + case BME280: chip_info = &bme280_chip_info; break; - case BMP380_CHIP_ID: + case BMP380: chip_info = &bmp380_chip_info; break; default: @@ -1751,9 +1756,9 @@ 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 != chip) { + if (chip_id != data->chip_info->chip_id) { dev_err(dev, "bad chip id: expected %x got %x\n", - chip, chip_id); + data->chip_info->chip_id, chip_id); return -EINVAL; } diff --git a/drivers/iio/pressure/bmp280-i2c.c b/drivers/iio/pressure/bmp280-i2c.c index 14eab086d24a..59921e8cd592 100644 --- a/drivers/iio/pressure/bmp280-i2c.c +++ b/drivers/iio/pressure/bmp280-i2c.c @@ -12,14 +12,14 @@ static int bmp280_i2c_probe(struct i2c_client *client) const struct i2c_device_id *id = i2c_client_get_device_id(client); switch (id->driver_data) { - case BMP180_CHIP_ID: + case BMP180: regmap_config = &bmp180_regmap_config; break; - case BMP280_CHIP_ID: - case BME280_CHIP_ID: + case BMP280: + case BME280: regmap_config = &bmp280_regmap_config; break; - case BMP380_CHIP_ID: + case BMP380: regmap_config = &bmp380_regmap_config; break; default: @@ -40,21 +40,21 @@ static int bmp280_i2c_probe(struct i2c_client *client) } static const struct of_device_id bmp280_of_i2c_match[] = { - { .compatible = "bosch,bmp085", .data = (void *)BMP180_CHIP_ID }, - { .compatible = "bosch,bmp180", .data = (void *)BMP180_CHIP_ID }, - { .compatible = "bosch,bmp280", .data = (void *)BMP280_CHIP_ID }, - { .compatible = "bosch,bme280", .data = (void *)BME280_CHIP_ID }, - { .compatible = "bosch,bmp380", .data = (void *)BMP380_CHIP_ID }, + { .compatible = "bosch,bmp085", .data = (void *)BMP180 }, + { .compatible = "bosch,bmp180", .data = (void *)BMP180 }, + { .compatible = "bosch,bmp280", .data = (void *)BMP280 }, + { .compatible = "bosch,bme280", .data = (void *)BME280 }, + { .compatible = "bosch,bmp380", .data = (void *)BMP380 }, { }, }; MODULE_DEVICE_TABLE(of, bmp280_of_i2c_match); static const struct i2c_device_id bmp280_i2c_id[] = { - {"bmp085", BMP180_CHIP_ID }, - {"bmp180", BMP180_CHIP_ID }, - {"bmp280", BMP280_CHIP_ID }, - {"bme280", BME280_CHIP_ID }, - {"bmp380", BMP380_CHIP_ID }, + {"bmp085", BMP180 }, + {"bmp180", BMP180 }, + {"bmp280", BMP280 }, + {"bme280", BME280 }, + {"bmp380", BMP380 }, { }, }; MODULE_DEVICE_TABLE(i2c, bmp280_i2c_id); diff --git a/drivers/iio/pressure/bmp280-spi.c b/drivers/iio/pressure/bmp280-spi.c index 011c68e07ebf..4a2df5b5d838 100644 --- a/drivers/iio/pressure/bmp280-spi.c +++ b/drivers/iio/pressure/bmp280-spi.c @@ -59,14 +59,14 @@ static int bmp280_spi_probe(struct spi_device *spi) } switch (id->driver_data) { - case BMP180_CHIP_ID: + case BMP180: regmap_config = &bmp180_regmap_config; break; - case BMP280_CHIP_ID: - case BME280_CHIP_ID: + case BMP280: + case BME280: regmap_config = &bmp280_regmap_config; break; - case BMP380_CHIP_ID: + case BMP380: regmap_config = &bmp380_regmap_config; break; default: @@ -101,11 +101,11 @@ static const struct of_device_id bmp280_of_spi_match[] = { MODULE_DEVICE_TABLE(of, bmp280_of_spi_match); static const struct spi_device_id bmp280_spi_id[] = { - { "bmp180", BMP180_CHIP_ID }, - { "bmp181", BMP180_CHIP_ID }, - { "bmp280", BMP280_CHIP_ID }, - { "bme280", BME280_CHIP_ID }, - { "bmp380", BMP380_CHIP_ID }, + { "bmp180", BMP180 }, + { "bmp181", BMP180 }, + { "bmp280", BMP280 }, + { "bme280", BME280 }, + { "bmp380", BMP380 }, { } }; MODULE_DEVICE_TABLE(spi, bmp280_spi_id); diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h index c791325c7416..efc31bc84708 100644 --- a/drivers/iio/pressure/bmp280.h +++ b/drivers/iio/pressure/bmp280.h @@ -191,6 +191,14 @@ #define BMP280_PRESS_SKIPPED 0x80000 #define BMP280_HUMIDITY_SKIPPED 0x8000 +/* Enum with supported pressure sensor models */ +enum bmp280_variant { + BMP180, + BMP280, + BME280, + BMP380, +}; + /* Regmap configurations */ extern const struct regmap_config bmp180_regmap_config; extern const struct regmap_config bmp280_regmap_config;
Adds enumeration to improve handling the different supported sensors on driver initialization. This avoid collisions if different variants share the same device idetifier on ID register. Signed-off-by: Angel Iglesias <ang.iglesiasg@gmail.com>