| Message ID | 20240823181714.64545-8-vassilisamir@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | pressure: bmp280: Minor cleanup and interrupt support | expand |
On Fri, 23 Aug 2024 20:17:14 +0200 Vasileios Amoiridis <vassilisamir@gmail.com> wrote: > This commit intends to add the old BMP085 sensor to the new IRQ interface > of the sensor consistence. No functional changes intended. interface of the driver for consistence > > The BMP085 sensor is equivalent with the BMP180 with the only difference of > BMP085 having an extra interrupt pin to inform about an End of Conversion. > > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com> > --- > drivers/iio/pressure/bmp280-core.c | 68 ++++++++++++++++++++++-------- > drivers/iio/pressure/bmp280-i2c.c | 4 +- > drivers/iio/pressure/bmp280-spi.c | 4 +- > drivers/iio/pressure/bmp280.h | 1 + > 4 files changed, 56 insertions(+), 21 deletions(-) > > diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c > index f5268a13bfdb..1d27777d8a2c 100644 > --- a/drivers/iio/pressure/bmp280-core.c > +++ b/drivers/iio/pressure/bmp280-core.c > @@ -3058,13 +3058,19 @@ static irqreturn_t bmp085_eoc_irq(int irq, void *d) > return IRQ_HANDLED; > } > > -static int bmp085_fetch_eoc_irq(struct device *dev, > - const char *name, > - int irq, > - struct bmp280_data *data) > +static int bmp085_trigger_probe(struct iio_dev *indio_dev) > { > + struct bmp280_data *data = iio_priv(indio_dev); > + struct device *dev = data->dev; > + struct fwnode_handle *fwnode; > unsigned long irq_trig; > - int ret; > + int ret, irq; > + > + fwnode = dev_fwnode(data->dev); > + if (!fwnode) > + return -ENODEV; > + > + irq = fwnode_irq_get(fwnode, 0); Should check this + as Andy pointed out in earlier review this call will return an error if fwnode not present anyway so can skip earlier check. see fwnode_call_int_op() definition. Otherwise, Andy has given some detailed review. I wouldn't suggest applying the style cleanup to existing code but he's entirely correct that we can make the stuff being touched anyway easier to read. The more functional stuff maybe needs to be looked at in other drivers though. Jonathan
On Sat, Aug 24, 2024 at 11:02:42AM +0100, Jonathan Cameron wrote: > On Fri, 23 Aug 2024 20:17:14 +0200 > Vasileios Amoiridis <vassilisamir@gmail.com> wrote: > > > This commit intends to add the old BMP085 sensor to the new IRQ interface > > of the sensor consistence. No functional changes intended. > > interface of the driver for consistence > ACK. > > > > The BMP085 sensor is equivalent with the BMP180 with the only difference of > > BMP085 having an extra interrupt pin to inform about an End of Conversion. > > > > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com> > > --- > > drivers/iio/pressure/bmp280-core.c | 68 ++++++++++++++++++++++-------- > > drivers/iio/pressure/bmp280-i2c.c | 4 +- > > drivers/iio/pressure/bmp280-spi.c | 4 +- > > drivers/iio/pressure/bmp280.h | 1 + > > 4 files changed, 56 insertions(+), 21 deletions(-) > > > > diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c > > index f5268a13bfdb..1d27777d8a2c 100644 > > --- a/drivers/iio/pressure/bmp280-core.c > > +++ b/drivers/iio/pressure/bmp280-core.c > > @@ -3058,13 +3058,19 @@ static irqreturn_t bmp085_eoc_irq(int irq, void *d) > > return IRQ_HANDLED; > > } > > > > -static int bmp085_fetch_eoc_irq(struct device *dev, > > - const char *name, > > - int irq, > > - struct bmp280_data *data) > > +static int bmp085_trigger_probe(struct iio_dev *indio_dev) > > { > > + struct bmp280_data *data = iio_priv(indio_dev); > > + struct device *dev = data->dev; > > + struct fwnode_handle *fwnode; > > unsigned long irq_trig; > > - int ret; > > + int ret, irq; > > + > > + fwnode = dev_fwnode(data->dev); > > + if (!fwnode) > > + return -ENODEV; > > + > > + irq = fwnode_irq_get(fwnode, 0); > > Should check this + as Andy pointed out in earlier review this > call will return an error if fwnode not present anyway so can skip > earlier check. > see fwnode_call_int_op() definition. > > Otherwise, Andy has given some detailed review. I wouldn't suggest > applying the style cleanup to existing code but he's entirely > correct that we can make the stuff being touched anyway easier > to read. > > The more functional stuff maybe needs to be looked at in other > drivers though. > > Jonathan Hi Jonathan, Thanks for the feedback. Indeed Andy has given a very detailed review feedback, I will also check what you said. Cheers, Vasilis
diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c index f5268a13bfdb..1d27777d8a2c 100644 --- a/drivers/iio/pressure/bmp280-core.c +++ b/drivers/iio/pressure/bmp280-core.c @@ -3058,13 +3058,19 @@ static irqreturn_t bmp085_eoc_irq(int irq, void *d) return IRQ_HANDLED; } -static int bmp085_fetch_eoc_irq(struct device *dev, - const char *name, - int irq, - struct bmp280_data *data) +static int bmp085_trigger_probe(struct iio_dev *indio_dev) { + struct bmp280_data *data = iio_priv(indio_dev); + struct device *dev = data->dev; + struct fwnode_handle *fwnode; unsigned long irq_trig; - int ret; + int ret, irq; + + fwnode = dev_fwnode(data->dev); + if (!fwnode) + return -ENODEV; + + irq = fwnode_irq_get(fwnode, 0); irq_trig = irqd_get_trigger_type(irq_get_irq_data(irq)); if (irq_trig != IRQF_TRIGGER_RISING) { @@ -3074,13 +3080,8 @@ static int bmp085_fetch_eoc_irq(struct device *dev, init_completion(&data->done); - ret = devm_request_threaded_irq(dev, - irq, - bmp085_eoc_irq, - NULL, - irq_trig, - name, - data); + ret = devm_request_irq(dev, irq, bmp085_eoc_irq, irq_trig, + indio_dev->name, data); if (ret) { /* Bail out without IRQ but keep the driver in place */ dev_err(dev, "unable to request DRDY IRQ\n"); @@ -3091,6 +3092,44 @@ static int bmp085_fetch_eoc_irq(struct device *dev, return 0; } +/* Identical to bmp180_chip_info + bmp085_trigger_probe */ +const struct bmp280_chip_info bmp085_chip_info = { + .id_reg = BMP280_REG_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, + .num_channels = ARRAY_SIZE(bmp280_channels), + .avail_scan_masks = bmp280_avail_scan_masks, + + .oversampling_temp_avail = bmp180_oversampling_temp_avail, + .num_oversampling_temp_avail = + ARRAY_SIZE(bmp180_oversampling_temp_avail), + .oversampling_temp_default = 0, + + .oversampling_press_avail = bmp180_oversampling_press_avail, + .num_oversampling_press_avail = + ARRAY_SIZE(bmp180_oversampling_press_avail), + .oversampling_press_default = BMP180_MEAS_PRESS_8X, + + .temp_coeffs = bmp180_temp_coeffs, + .temp_coeffs_type = IIO_VAL_FRACTIONAL, + .press_coeffs = bmp180_press_coeffs, + .press_coeffs_type = IIO_VAL_FRACTIONAL, + + .chip_config = bmp180_chip_config, + .read_temp = bmp180_read_temp, + .read_press = bmp180_read_press, + .read_calib = bmp180_read_calib, + .set_mode = bmp180_set_mode, + .wait_conv = bmp180_wait_conv, + + .trigger_probe = bmp085_trigger_probe, + .trigger_handler = bmp180_trigger_handler, +}; +EXPORT_SYMBOL_NS(bmp085_chip_info, IIO_BMP280); + static int bmp280_buffer_preenable(struct iio_dev *indio_dev) { struct bmp280_data *data = iio_priv(indio_dev); @@ -3262,11 +3301,6 @@ int bmp280_common_probe(struct device *dev, * so we look for an IRQ if we have that. */ if (irq > 0) { - if (chip_id == BMP180_CHIP_ID) { - ret = bmp085_fetch_eoc_irq(dev, name, irq, data); - if (ret) - return ret; - } if (data->chip_info->trigger_probe) { ret = data->chip_info->trigger_probe(indio_dev); if (ret) diff --git a/drivers/iio/pressure/bmp280-i2c.c b/drivers/iio/pressure/bmp280-i2c.c index 5c3a63b4327c..2f7b25984c7b 100644 --- a/drivers/iio/pressure/bmp280-i2c.c +++ b/drivers/iio/pressure/bmp280-i2c.c @@ -27,7 +27,7 @@ static int bmp280_i2c_probe(struct i2c_client *client) } static const struct of_device_id bmp280_of_i2c_match[] = { - { .compatible = "bosch,bmp085", .data = &bmp180_chip_info }, + { .compatible = "bosch,bmp085", .data = &bmp085_chip_info }, { .compatible = "bosch,bmp180", .data = &bmp180_chip_info }, { .compatible = "bosch,bmp280", .data = &bmp280_chip_info }, { .compatible = "bosch,bme280", .data = &bme280_chip_info }, @@ -38,7 +38,7 @@ static const struct of_device_id bmp280_of_i2c_match[] = { MODULE_DEVICE_TABLE(of, bmp280_of_i2c_match); static const struct i2c_device_id bmp280_i2c_id[] = { - {"bmp085", (kernel_ulong_t)&bmp180_chip_info }, + {"bmp085", (kernel_ulong_t)&bmp085_chip_info }, {"bmp180", (kernel_ulong_t)&bmp180_chip_info }, {"bmp280", (kernel_ulong_t)&bmp280_chip_info }, {"bme280", (kernel_ulong_t)&bme280_chip_info }, diff --git a/drivers/iio/pressure/bmp280-spi.c b/drivers/iio/pressure/bmp280-spi.c index d18549d9bb64..49aa8c2cd85b 100644 --- a/drivers/iio/pressure/bmp280-spi.c +++ b/drivers/iio/pressure/bmp280-spi.c @@ -114,7 +114,7 @@ static int bmp280_spi_probe(struct spi_device *spi) } static const struct of_device_id bmp280_of_spi_match[] = { - { .compatible = "bosch,bmp085", .data = &bmp180_chip_info }, + { .compatible = "bosch,bmp085", .data = &bmp085_chip_info }, { .compatible = "bosch,bmp180", .data = &bmp180_chip_info }, { .compatible = "bosch,bmp181", .data = &bmp180_chip_info }, { .compatible = "bosch,bmp280", .data = &bmp280_chip_info }, @@ -126,7 +126,7 @@ 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[] = { - { "bmp085", (kernel_ulong_t)&bmp180_chip_info }, + { "bmp085", (kernel_ulong_t)&bmp085_chip_info }, { "bmp180", (kernel_ulong_t)&bmp180_chip_info }, { "bmp181", (kernel_ulong_t)&bmp180_chip_info }, { "bmp280", (kernel_ulong_t)&bmp280_chip_info }, diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h index 0c32b6430677..e0e47bf22472 100644 --- a/drivers/iio/pressure/bmp280.h +++ b/drivers/iio/pressure/bmp280.h @@ -535,6 +535,7 @@ struct bmp280_chip_info { }; /* Chip infos for each variant */ +extern const struct bmp280_chip_info bmp085_chip_info; extern const struct bmp280_chip_info bmp180_chip_info; extern const struct bmp280_chip_info bmp280_chip_info; extern const struct bmp280_chip_info bme280_chip_info;
This commit intends to add the old BMP085 sensor to the new IRQ interface of the sensor consistence. No functional changes intended. The BMP085 sensor is equivalent with the BMP180 with the only difference of BMP085 having an extra interrupt pin to inform about an End of Conversion. Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com> --- drivers/iio/pressure/bmp280-core.c | 68 ++++++++++++++++++++++-------- drivers/iio/pressure/bmp280-i2c.c | 4 +- drivers/iio/pressure/bmp280-spi.c | 4 +- drivers/iio/pressure/bmp280.h | 1 + 4 files changed, 56 insertions(+), 21 deletions(-)