diff mbox series

[v3,3/5] iio: pressure: bmp280: simplify driver initialization logic

Message ID 20220710091708.15967-1-ang.iglesiasg@gmail.com (mailing list archive)
State Changes Requested
Headers show
Series Add support for pressure sensor Bosch BMP380 | expand

Commit Message

Angel Iglesias July 10, 2022, 9:17 a.m. UTC 
Simplified common initialization logic of different sensor types
unifying calibration and initial configuration recovery.

Default config param values of each sensor type are stored inside
chip_info structure and used to initialize sensor data struct instance.

The auxiliar functions for read each sensor type calibration are converted
to a callback available on the chip_info struct.

Signed-off-by: Angel Iglesias <ang.iglesiasg@gmail.com>
Suggested-by: Jonathan Cameron <jic23@kernel.org>
---
 drivers/iio/pressure/bmp280-core.c | 91 ++++++++++++++++++------------
 1 file changed, 54 insertions(+), 37 deletions(-)

Comments

andriy.shevchenko@linux.intel.com July 10, 2022, 6:41 p.m. UTC | #1 
It seems you somehow managed to send patches separately from email perspective.
Make sure you pass parameter --thread to `git format-patch`, so it will create
a proper chain of emails.

On Sun, Jul 10, 2022 at 11:17:06AM +0200, Angel Iglesias wrote:
> Simplified common initialization logic of different sensor types
> unifying calibration and initial configuration recovery.
> 
> Default config param values of each sensor type are stored inside
> chip_info structure and used to initialize sensor data struct instance.
> 
> The auxiliar functions for read each sensor type calibration are converted
> to a callback available on the chip_info struct.

> Signed-off-by: Angel Iglesias <ang.iglesiasg@gmail.com>
> Suggested-by: Jonathan Cameron <jic23@kernel.org>

Usually this kind of tag goes before your SoB, because it's chronologically
correct.
Angel Iglesias July 11, 2022, 12:45 p.m. UTC | #2 
On dom, 2022-07-10 at 21:41 +0300, Andy Shevchenko wrote:
> It seems you somehow managed to send patches separately from email
> perspective.
> Make sure you pass parameter --thread to `git format-patch`, so it will create
> a proper chain of emails.

How should I proceed to fix this issue? Should I wait for more feedback and send
a new revision of the patches, now as a proper mail chain? Or should I resend
current patchset as a chain? I don't want to spam the mailing list more due to
my inexperience contributing.

> On Sun, Jul 10, 2022 at 11:17:06AM +0200, Angel Iglesias wrote:
> > Simplified common initialization logic of different sensor types
> > unifying calibration and initial configuration recovery.
> > 
> > Default config param values of each sensor type are stored inside
> > chip_info structure and used to initialize sensor data struct instance.
> > 
> > The auxiliar functions for read each sensor type calibration are converted
> > to a callback available on the chip_info struct.
> 
> > Signed-off-by: Angel Iglesias <ang.iglesiasg@gmail.com>
> > Suggested-by: Jonathan Cameron <jic23@kernel.org>
> 
> Usually this kind of tag goes before your SoB, because it's chronologically
> correct.
> 
Ok, makes sense.

Thank you for your time and help,

Best regards,
Angel
Andy Shevchenko July 11, 2022, 1:11 p.m. UTC | #3 
On Mon, Jul 11, 2022 at 2:46 PM Angel Iglesias <ang.iglesiasg@gmail.com> wrote:
> On dom, 2022-07-10 at 21:41 +0300, Andy Shevchenko wrote:

> > It seems you somehow managed to send patches separately from email
> > perspective.
> > Make sure you pass parameter --thread to `git format-patch`, so it will create
> > a proper chain of emails.
>
> How should I proceed to fix this issue? Should I wait for more feedback and send
> a new revision of the patches, now as a proper mail chain? Or should I resend
> current patchset as a chain? I don't want to spam the mailing list more due to
> my inexperience contributing.

Since there are already comments and more might come, wait a few days
and send a new version.
diff mbox series

Patch

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index bf8167f43c56..59e9c5fb4ab4 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -107,19 +107,28 @@  struct bmp280_data {
 };
 
 struct bmp280_chip_info {
+	unsigned int id_reg;
+
+	int num_channels;
+	unsigned int start_up_time;
+
 	const int *oversampling_temp_avail;
 	int num_oversampling_temp_avail;
+	int oversampling_temp_default;
 
 	const int *oversampling_press_avail;
 	int num_oversampling_press_avail;
+	int oversampling_press_default;
 
 	const int *oversampling_humid_avail;
 	int num_oversampling_humid_avail;
+	int oversampling_humid_default;
 
 	int (*chip_config)(struct bmp280_data *);
 	int (*read_temp)(struct bmp280_data *, int *);
 	int (*read_press)(struct bmp280_data *, int *, int *);
 	int (*read_humid)(struct bmp280_data *, int *, int *);
+	int (*read_calib)(struct bmp280_data *, unsigned int);
 };
 
 /*
@@ -147,15 +156,14 @@  static const struct iio_chan_spec bmp280_channels[] = {
 	},
 };
 
-static int bmp280_read_calib(struct bmp280_data *data,
-			     struct bmp280_calib *calib,
-			     unsigned int chip)
+static int bmp280_read_calib(struct bmp280_data *data, unsigned int chip)
 {
 	int ret;
 	unsigned int tmp;
 	__le16 l16;
 	__be16 b16;
 	struct device *dev = data->dev;
+	struct bmp280_calib *calib = &data->calib.bmp280;
 	__le16 t_buf[BMP280_COMP_TEMP_REG_COUNT / 2];
 	__le16 p_buf[BMP280_COMP_PRESS_REG_COUNT / 2];
 
@@ -640,15 +648,22 @@  static int bmp280_chip_config(struct bmp280_data *data)
 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,
+	.start_up_time = 2000,
+	.num_channels = 2,
+
 	.oversampling_temp_avail = bmp280_oversampling_avail,
 	.num_oversampling_temp_avail = ARRAY_SIZE(bmp280_oversampling_avail),
+	.oversampling_temp_default = ilog2(2),
 
 	.oversampling_press_avail = bmp280_oversampling_avail,
 	.num_oversampling_press_avail = ARRAY_SIZE(bmp280_oversampling_avail),
+	.oversampling_press_default = ilog2(16),
 
 	.chip_config = bmp280_chip_config,
 	.read_temp = bmp280_read_temp,
 	.read_press = bmp280_read_press,
+	.read_calib = bmp280_read_calib,
 };
 
 static int bme280_chip_config(struct bmp280_data *data)
@@ -670,19 +685,27 @@  static int bme280_chip_config(struct bmp280_data *data)
 }
 
 static const struct bmp280_chip_info bme280_chip_info = {
+	.id_reg = BMP280_REG_ID,
+	.start_up_time = 2000,
+	.num_channels = 3,
+
 	.oversampling_temp_avail = bmp280_oversampling_avail,
 	.num_oversampling_temp_avail = ARRAY_SIZE(bmp280_oversampling_avail),
+	.oversampling_temp_default = ilog2(2),
 
 	.oversampling_press_avail = bmp280_oversampling_avail,
 	.num_oversampling_press_avail = ARRAY_SIZE(bmp280_oversampling_avail),
+	.oversampling_press_default = ilog2(16),
 
 	.oversampling_humid_avail = bmp280_oversampling_avail,
 	.num_oversampling_humid_avail = ARRAY_SIZE(bmp280_oversampling_avail),
+	.oversampling_humid_default = ilog2(16),
 
 	.chip_config = bme280_chip_config,
 	.read_temp = bmp280_read_temp,
 	.read_press = bmp280_read_press,
 	.read_humid = bmp280_read_humid,
+	.read_calib = bmp280_read_calib,
 };
 
 static int bmp180_measure(struct bmp280_data *data, u8 ctrl_meas)
@@ -748,11 +771,11 @@  static int bmp180_read_adc_temp(struct bmp280_data *data, int *val)
 	return 0;
 }
 
-static int bmp180_read_calib(struct bmp280_data *data,
-			     struct bmp180_calib *calib)
+static int bmp180_read_calib(struct bmp280_data *data, unsigned int chip)
 {
 	int ret;
 	int i;
+	struct bmp180_calib *calib = &data->calib.bmp180;
 	__be16 buf[BMP180_REG_CALIB_COUNT / 2];
 
 	ret = regmap_bulk_read(data->regmap, BMP180_REG_CALIB_START, buf,
@@ -913,17 +936,24 @@  static const int bmp180_oversampling_temp_avail[] = { 1 };
 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,
+	.start_up_time = 2000,
+	.num_channels = 2,
+
 	.oversampling_temp_avail = bmp180_oversampling_temp_avail,
 	.num_oversampling_temp_avail =
 		ARRAY_SIZE(bmp180_oversampling_temp_avail),
+	.oversampling_temp_default = ilog2(1),
 
 	.oversampling_press_avail = bmp180_oversampling_press_avail,
 	.num_oversampling_press_avail =
 		ARRAY_SIZE(bmp180_oversampling_press_avail),
+	.oversampling_press_default = ilog2(8),
 
 	.chip_config = bmp180_chip_config,
 	.read_temp = bmp180_read_temp,
 	.read_press = bmp180_read_press,
+	.read_calib = bmp180_read_calib,
 };
 
 static irqreturn_t bmp085_eoc_irq(int irq, void *d)
@@ -993,6 +1023,7 @@  int bmp280_common_probe(struct device *dev,
 	int ret;
 	struct iio_dev *indio_dev;
 	struct bmp280_data *data;
+	const struct bmp280_chip_info *chip_info;
 	unsigned int chip_id;
 	struct gpio_desc *gpiod;
 
@@ -1011,30 +1042,25 @@  int bmp280_common_probe(struct device *dev,
 
 	switch (chip) {
 	case BMP180_CHIP_ID:
-		indio_dev->num_channels = 2;
-		data->chip_info = &bmp180_chip_info;
-		data->oversampling_press = ilog2(8);
-		data->oversampling_temp = ilog2(1);
-		data->start_up_time = 10000;
+		chip_info = &bmp180_chip_info;
 		break;
 	case BMP280_CHIP_ID:
-		indio_dev->num_channels = 2;
-		data->chip_info = &bmp280_chip_info;
-		data->oversampling_press = ilog2(16);
-		data->oversampling_temp = ilog2(2);
-		data->start_up_time = 2000;
+		chip_info = &bmp280_chip_info;
 		break;
 	case BME280_CHIP_ID:
-		indio_dev->num_channels = 3;
-		data->chip_info = &bme280_chip_info;
-		data->oversampling_press = ilog2(16);
-		data->oversampling_humid = ilog2(16);
-		data->oversampling_temp = ilog2(2);
-		data->start_up_time = 2000;
+		chip_info = &bme280_chip_info;
 		break;
 	default:
 		return -EINVAL;
 	}
+	data->chip_info = chip_info;
+
+	/* apply initial values from chip info structure */
+	indio_dev->num_channels = chip_info->num_channels;
+	data->oversampling_press = chip_info->oversampling_press_default;
+	data->oversampling_humid = chip_info->oversampling_humid_default;
+	data->oversampling_temp = chip_info->oversampling_temp_default;
+	data->start_up_time = chip_info->start_up_time;
 
 	/* Bring up regulators */
 	regulator_bulk_set_supply_names(data->supplies,
@@ -1071,7 +1097,8 @@  int bmp280_common_probe(struct device *dev,
 	}
 
 	data->regmap = regmap;
-	ret = regmap_read(regmap, BMP280_REG_ID, &chip_id);
+
+	ret = regmap_read(regmap, data->chip_info->id_reg, &chip_id);
 	if (ret < 0)
 		return ret;
 	if (chip_id != chip) {
@@ -1091,21 +1118,11 @@  int bmp280_common_probe(struct device *dev,
 	 * non-volatile memory during production". Let's read them out at probe
 	 * time once. They will not change.
 	 */
-	if (chip_id  == BMP180_CHIP_ID) {
-		ret = bmp180_read_calib(data, &data->calib.bmp180);
-		if (ret < 0) {
-			dev_err(data->dev,
-				"failed to read calibration coefficients\n");
-			return ret;
-		}
-	} else if (chip_id == BMP280_CHIP_ID || chip_id == BME280_CHIP_ID) {
-		ret = bmp280_read_calib(data, &data->calib.bmp280, chip_id);
-		if (ret < 0) {
-			dev_err(data->dev,
-				"failed to read calibration coefficients\n");
-			return ret;
-		}
-	}
+
+	ret = data->chip_info->read_calib(data, chip_id);
+	if (ret < 0)
+		return dev_err_probe(data->dev, ret,
+				     "failed to read calibration coefficients\n");
 
 	/*
 	 * Attempt to grab an optional EOC IRQ - only the BMP085 has this