diff mbox series

[v5,05/10] iio: pressure: bmp280: Make return values consistent

Message ID 20240429190046.24252-6-vassilisamir@gmail.com (mailing list archive)
State Changes Requested
Headers show
Series iio: pressure: bmp280: Driver cleanup and add triggered buffer support | expand

Commit Message

Vasileios Amoiridis April 29, 2024, 7 p.m. UTC 
Throughout the driver there are quite a few places were return
values are treated as errors if they are negative or not-zero.
This commit tries to make the return values of those functions
consistent and treat them as errors in case there is a negative
value since the vast majority of the functions are returning
erorrs coming from regmap_*() functions.

While at it, add error messages that were not implemented before.

Finally, remove any extra error checks that are dead code.

Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
 drivers/iio/pressure/bmp280-core.c | 175 ++++++++++++++++-------------
 1 file changed, 96 insertions(+), 79 deletions(-)

Comments

Jonathan Cameron May 5, 2024, 7:08 p.m. UTC | #1 
On Mon, 29 Apr 2024 21:00:41 +0200
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:

> Throughout the driver there are quite a few places were return
> values are treated as errors if they are negative or not-zero.
> This commit tries to make the return values of those functions
> consistent and treat them as errors in case there is a negative
> value since the vast majority of the functions are returning
> erorrs coming from regmap_*() functions.

The changes are fine, but that argument isn't correct.
regmap_*() functions never (that I can recall) return positive
values, so if (ret) would be valid for those and I'd have expected
the exact opposite outcome if you are looking at regmap*() return
values to make the decision.

The if (ret) pattern is sometimes used throughout because it
makes
	return function()

consistent without needing to do

	ret = function();
	if (ret < 0)
		return ret;

	return 0;

That pattern isn't particularly common in this driver (there are few cases).
We also tend not to worry too much about that slight inconsistency though
in a few cases it has lead to compilers failing to detect that some paths
are not possible and reporting false warnings.

However, all arguments about which is 'better' aside, key is that consistency
(either choice) is better than a mix.  So I'm fine with ret < 0 on basis
it's the most common in this driver being your justification. Just don't
blame regmap*() return values!

> 
> While at it, add error messages that were not implemented before.
> 
> Finally, remove any extra error checks that are dead code.

Ideally this would be broken up a little more as, whilst all error
code related, these aren't all the same thing.

I'd have preferred:
1) Dead code removal.
2) Message updates.
3) Switch to consistent ret handling.

However it isn't that bad as a single patch, so just address the question
above and I think this will be fine as one patch.

> 
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>

Jonathan
Vasileios Amoiridis May 5, 2024, 11:08 p.m. UTC | #2 
On Sun, May 05, 2024 at 08:08:18PM +0100, Jonathan Cameron wrote:
> On Mon, 29 Apr 2024 21:00:41 +0200
> Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
> 
> > Throughout the driver there are quite a few places were return
> > values are treated as errors if they are negative or not-zero.
> > This commit tries to make the return values of those functions
> > consistent and treat them as errors in case there is a negative
> > value since the vast majority of the functions are returning
> > erorrs coming from regmap_*() functions.
> 
> The changes are fine, but that argument isn't correct.
> regmap_*() functions never (that I can recall) return positive
> values, so if (ret) would be valid for those and I'd have expected
> the exact opposite outcome if you are looking at regmap*() return
> values to make the decision.
> 
> The if (ret) pattern is sometimes used throughout because it
> makes
> 	return function()
> 
> consistent without needing to do
> 
> 	ret = function();
> 	if (ret < 0)
> 		return ret;
> 
> 	return 0;
> 
> That pattern isn't particularly common in this driver (there are few cases).
> We also tend not to worry too much about that slight inconsistency though
> in a few cases it has lead to compilers failing to detect that some paths
> are not possible and reporting false warnings.
> 
> However, all arguments about which is 'better' aside, key is that consistency
> (either choice) is better than a mix.  So I'm fine with ret < 0 on basis
> it's the most common in this driver being your justification. Just don't
> blame regmap*() return values!
> 

Hi Jonathan!

Thank you once again for the valueable feedback!

Of course, if (ret) would be valid for the return values of the regmap_*()
functions. I was just trying to understand which of the 2 options is more
widely used in other drivers and I tried to implement that. In general,
the if (ret) is used 65 times while the if (ret < 0) only 20. So, in
terms of noise, changing the if (ret < 0) to if (ret) will create less
noise. I chose the if (ret < 0) because I saw other people using it
and it felt better in my eyes. I could check if if (ret) applies
everywhere and update it in the v6.
 
> > 
> > While at it, add error messages that were not implemented before.
> > 
> > Finally, remove any extra error checks that are dead code.
> 
> Ideally this would be broken up a little more as, whilst all error
> code related, these aren't all the same thing.
> 
> I'd have preferred:
> 1) Dead code removal.
> 2) Message updates.
> 3) Switch to consistent ret handling.
> 
> However it isn't that bad as a single patch, so just address the question
> above and I think this will be fine as one patch.
> 

Since from your comments in the next patches a v6 is for sure, I could split
this as well!

Cheers,
Vasilis

> > 
> > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> 
> Jonathan
diff mbox series

Patch

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index ed49e0779d41..8290028824e9 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -222,10 +222,8 @@  static int bme280_read_calib(struct bmp280_data *data)
 
 	/* Load shared calibration params with bmp280 first */
 	ret = bmp280_read_calib(data);
-	if  (ret < 0) {
-		dev_err(dev, "failed to read calibration parameters\n");
+	if  (ret < 0)
 		return ret;
-	}
 
 	/*
 	 * Read humidity calibration values.
@@ -552,7 +550,7 @@  static int bme280_write_oversampling_ratio_humid(struct bmp280_data *data,
 			data->oversampling_humid = ilog2(val);
 
 			ret = data->chip_info->chip_config(data);
-			if (ret) {
+			if (ret < 0) {
 				data->oversampling_humid = prev;
 				data->chip_info->chip_config(data);
 				return ret;
@@ -577,7 +575,7 @@  static int bmp280_write_oversampling_ratio_temp(struct bmp280_data *data,
 			data->oversampling_temp = ilog2(val);
 
 			ret = data->chip_info->chip_config(data);
-			if (ret) {
+			if (ret < 0) {
 				data->oversampling_temp = prev;
 				data->chip_info->chip_config(data);
 				return ret;
@@ -602,7 +600,7 @@  static int bmp280_write_oversampling_ratio_press(struct bmp280_data *data,
 			data->oversampling_press = ilog2(val);
 
 			ret = data->chip_info->chip_config(data);
-			if (ret) {
+			if (ret < 0) {
 				data->oversampling_press = prev;
 				data->chip_info->chip_config(data);
 				return ret;
@@ -627,7 +625,7 @@  static int bmp280_write_sampling_frequency(struct bmp280_data *data,
 			data->sampling_freq = i;
 
 			ret = data->chip_info->chip_config(data);
-			if (ret) {
+			if (ret < 0) {
 				data->sampling_freq = prev;
 				data->chip_info->chip_config(data);
 				return ret;
@@ -651,7 +649,7 @@  static int bmp280_write_iir_filter_coeffs(struct bmp280_data *data, int val)
 			data->iir_filter_coeff = i;
 
 			ret = data->chip_info->chip_config(data);
-			if (ret) {
+			if (ret < 0) {
 				data->iir_filter_coeff = prev;
 				data->chip_info->chip_config(data);
 				return ret;
@@ -841,8 +839,10 @@  static int bme280_chip_config(struct bmp280_data *data)
 	 */
 	ret = regmap_update_bits(data->regmap, BME280_REG_CTRL_HUMIDITY,
 				 BME280_OSRS_HUMIDITY_MASK, osrs);
-	if (ret < 0)
+	if (ret < 0) {
+		dev_err(data->dev, "failed to set humidity oversampling");
 		return ret;
+	}
 
 	return bmp280_chip_config(data);
 }
@@ -892,7 +892,7 @@  static int bmp380_cmd(struct bmp280_data *data, u8 cmd)
 
 	/* Check if device is ready to process a command */
 	ret = regmap_read(data->regmap, BMP380_REG_STATUS, &reg);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(data->dev, "failed to read error register\n");
 		return ret;
 	}
@@ -903,7 +903,7 @@  static int bmp380_cmd(struct bmp280_data *data, u8 cmd)
 
 	/* Send command to process */
 	ret = regmap_write(data->regmap, BMP380_REG_CMD, cmd);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(data->dev, "failed to send command to device\n");
 		return ret;
 	}
@@ -911,7 +911,7 @@  static int bmp380_cmd(struct bmp280_data *data, u8 cmd)
 	usleep_range(data->start_up_time, data->start_up_time + 100);
 	/* Check for command processing error */
 	ret = regmap_read(data->regmap, BMP380_REG_ERROR, &reg);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(data->dev, "error reading ERROR reg\n");
 		return ret;
 	}
@@ -1003,7 +1003,7 @@  static int bmp380_read_temp(struct bmp280_data *data, int *val, int *val2)
 
 	ret = regmap_bulk_read(data->regmap, BMP380_REG_TEMP_XLSB,
 			       data->buf, sizeof(data->buf));
-	if (ret) {
+	if (ret < 0) {
 		dev_err(data->dev, "failed to read temperature\n");
 		return ret;
 	}
@@ -1036,12 +1036,12 @@  static int bmp380_read_press(struct bmp280_data *data, int *val, int *val2)
 
 	/* Read and compensate for temperature so we get a reading of t_fine */
 	ret = bmp380_read_temp(data, NULL, NULL);
-	if (ret)
+	if (ret < 0)
 		return ret;
 
 	ret = regmap_bulk_read(data->regmap, BMP380_REG_PRESS_XLSB,
 			       data->buf, sizeof(data->buf));
-	if (ret) {
+	if (ret < 0) {
 		dev_err(data->dev, "failed to read pressure\n");
 		return ret;
 	}
@@ -1069,9 +1069,9 @@  static int bmp380_read_calib(struct bmp280_data *data)
 	ret = regmap_bulk_read(data->regmap, BMP380_REG_CALIB_TEMP_START,
 			       data->bmp380_cal_buf,
 			       sizeof(data->bmp380_cal_buf));
-	if (ret) {
+	if (ret < 0) {
 		dev_err(data->dev,
-			"failed to read temperature calibration parameters\n");
+			"failed to read calibration parameters\n");
 		return ret;
 	}
 
@@ -1137,7 +1137,7 @@  static int bmp380_chip_config(struct bmp280_data *data)
 				 BMP380_CTRL_SENSORS_MASK,
 				 BMP380_CTRL_SENSORS_PRESS_EN |
 				 BMP380_CTRL_SENSORS_TEMP_EN);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(data->dev,
 			"failed to write operation control register\n");
 		return ret;
@@ -1151,7 +1151,7 @@  static int bmp380_chip_config(struct bmp280_data *data)
 				       BMP380_OSRS_TEMP_MASK |
 				       BMP380_OSRS_PRESS_MASK,
 				       osrs, &aux);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(data->dev, "failed to write oversampling register\n");
 		return ret;
 	}
@@ -1161,7 +1161,7 @@  static int bmp380_chip_config(struct bmp280_data *data)
 	ret = regmap_update_bits_check(data->regmap, BMP380_REG_ODR,
 				       BMP380_ODRS_MASK, data->sampling_freq,
 				       &aux);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(data->dev, "failed to write ODR selection register\n");
 		return ret;
 	}
@@ -1171,7 +1171,7 @@  static int bmp380_chip_config(struct bmp280_data *data)
 	ret = regmap_update_bits_check(data->regmap, BMP380_REG_CONFIG, BMP380_FILTER_MASK,
 				       FIELD_PREP(BMP380_FILTER_MASK, data->iir_filter_coeff),
 				       &aux);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(data->dev, "failed to write config register\n");
 		return ret;
 	}
@@ -1190,7 +1190,7 @@  static int bmp380_chip_config(struct bmp280_data *data)
 		ret = regmap_write_bits(data->regmap, BMP380_REG_POWER_CONTROL,
 					BMP380_MODE_MASK,
 					FIELD_PREP(BMP380_MODE_MASK, BMP380_MODE_SLEEP));
-		if (ret) {
+		if (ret < 0) {
 			dev_err(data->dev, "failed to set sleep mode\n");
 			return ret;
 		}
@@ -1198,7 +1198,7 @@  static int bmp380_chip_config(struct bmp280_data *data)
 		ret = regmap_write_bits(data->regmap, BMP380_REG_POWER_CONTROL,
 					BMP380_MODE_MASK,
 					FIELD_PREP(BMP380_MODE_MASK, BMP380_MODE_NORMAL));
-		if (ret) {
+		if (ret < 0) {
 			dev_err(data->dev, "failed to set normal mode\n");
 			return ret;
 		}
@@ -1211,7 +1211,7 @@  static int bmp380_chip_config(struct bmp280_data *data)
 
 		/* Check config error flag */
 		ret = regmap_read(data->regmap, BMP380_REG_ERROR, &tmp);
-		if (ret) {
+		if (ret < 0) {
 			dev_err(data->dev, "failed to read error register\n");
 			return ret;
 		}
@@ -1269,7 +1269,7 @@  static int bmp580_soft_reset(struct bmp280_data *data)
 	int ret;
 
 	ret = regmap_write(data->regmap, BMP580_REG_CMD, BMP580_CMD_SOFT_RESET);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(data->dev, "failed to send reset command to device\n");
 		return ret;
 	}
@@ -1277,13 +1277,13 @@  static int bmp580_soft_reset(struct bmp280_data *data)
 
 	/* Dummy read of chip_id */
 	ret = regmap_read(data->regmap, BMP580_REG_CHIP_ID, &reg);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(data->dev, "failed to reestablish comms after reset\n");
 		return ret;
 	}
 
 	ret = regmap_read(data->regmap, BMP580_REG_INT_STATUS, &reg);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(data->dev, "error reading interrupt status register\n");
 		return ret;
 	}
@@ -1308,7 +1308,7 @@  static int bmp580_nvm_operation(struct bmp280_data *data, bool is_write)
 
 	/* Check NVM ready flag */
 	ret = regmap_read(data->regmap, BMP580_REG_STATUS, &reg);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(data->dev, "failed to check nvm status\n");
 		return ret;
 	}
@@ -1320,7 +1320,7 @@  static int bmp580_nvm_operation(struct bmp280_data *data, bool is_write)
 	/* Start NVM operation sequence */
 	ret = regmap_write(data->regmap, BMP580_REG_CMD,
 			   BMP580_CMD_NVM_OP_SEQ_0);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(data->dev,
 			"failed to send nvm operation's first sequence\n");
 		return ret;
@@ -1329,7 +1329,7 @@  static int bmp580_nvm_operation(struct bmp280_data *data, bool is_write)
 		/* Send NVM write sequence */
 		ret = regmap_write(data->regmap, BMP580_REG_CMD,
 				   BMP580_CMD_NVM_WRITE_SEQ_1);
-		if (ret) {
+		if (ret < 0) {
 			dev_err(data->dev,
 				"failed to send nvm write sequence\n");
 			return ret;
@@ -1341,7 +1341,7 @@  static int bmp580_nvm_operation(struct bmp280_data *data, bool is_write)
 		/* Send NVM read sequence */
 		ret = regmap_write(data->regmap, BMP580_REG_CMD,
 				   BMP580_CMD_NVM_READ_SEQ_1);
-		if (ret) {
+		if (ret < 0) {
 			dev_err(data->dev,
 				"failed to send nvm read sequence\n");
 			return ret;
@@ -1350,16 +1350,12 @@  static int bmp580_nvm_operation(struct bmp280_data *data, bool is_write)
 		poll = 50;
 		timeout = 400;
 	}
-	if (ret) {
-		dev_err(data->dev, "failed to write command sequence\n");
-		return -EIO;
-	}
 
 	/* Wait until NVM is ready again */
 	ret = regmap_read_poll_timeout(data->regmap, BMP580_REG_STATUS, reg,
 				       (reg & BMP580_STATUS_NVM_RDY_MASK),
 				       poll, timeout);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(data->dev, "error checking nvm operation status\n");
 		return ret;
 	}
@@ -1386,7 +1382,7 @@  static int bmp580_read_temp(struct bmp280_data *data, int *val, int *val2)
 
 	ret = regmap_bulk_read(data->regmap, BMP580_REG_TEMP_XLSB, data->buf,
 			       sizeof(data->buf));
-	if (ret) {
+	if (ret < 0) {
 		dev_err(data->dev, "failed to read temperature\n");
 		return ret;
 	}
@@ -1414,7 +1410,7 @@  static int bmp580_read_press(struct bmp280_data *data, int *val, int *val2)
 
 	ret = regmap_bulk_read(data->regmap, BMP580_REG_PRESS_XLSB, data->buf,
 			       sizeof(data->buf));
-	if (ret) {
+	if (ret < 0) {
 		dev_err(data->dev, "failed to read pressure\n");
 		return ret;
 	}
@@ -1485,7 +1481,7 @@  static int bmp580_nvmem_read(void *priv, unsigned int offset, void *val,
 				 BMP580_MODE_MASK | BMP580_ODR_DEEPSLEEP_DIS,
 				 BMP580_ODR_DEEPSLEEP_DIS |
 				 FIELD_PREP(BMP580_MODE_MASK, BMP580_MODE_SLEEP));
-	if (ret) {
+	if (ret < 0) {
 		dev_err(data->dev, "failed to change sensor to standby mode\n");
 		goto exit;
 	}
@@ -1497,18 +1493,18 @@  static int bmp580_nvmem_read(void *priv, unsigned int offset, void *val,
 
 		ret = regmap_write(data->regmap, BMP580_REG_NVM_ADDR,
 				   FIELD_PREP(BMP580_NVM_ROW_ADDR_MASK, addr));
-		if (ret) {
+		if (ret < 0) {
 			dev_err(data->dev, "error writing nvm address\n");
 			goto exit;
 		}
 
 		ret = bmp580_nvm_operation(data, false);
-		if (ret)
+		if (ret < 0)
 			goto exit;
 
 		ret = regmap_bulk_read(data->regmap, BMP580_REG_NVM_DATA_LSB,
 				       &data->le16, sizeof(data->le16));
-		if (ret) {
+		if (ret < 0) {
 			dev_err(data->dev, "error reading nvm data regs\n");
 			goto exit;
 		}
@@ -1541,7 +1537,7 @@  static int bmp580_nvmem_write(void *priv, unsigned int offset, void *val,
 				 BMP580_MODE_MASK | BMP580_ODR_DEEPSLEEP_DIS,
 				 BMP580_ODR_DEEPSLEEP_DIS |
 				 FIELD_PREP(BMP580_MODE_MASK, BMP580_MODE_SLEEP));
-	if (ret) {
+	if (ret < 0) {
 		dev_err(data->dev, "failed to change sensor to standby mode\n");
 		goto exit;
 	}
@@ -1554,7 +1550,7 @@  static int bmp580_nvmem_write(void *priv, unsigned int offset, void *val,
 		ret = regmap_write(data->regmap, BMP580_REG_NVM_ADDR,
 				   BMP580_NVM_PROG_EN |
 				   FIELD_PREP(BMP580_NVM_ROW_ADDR_MASK, addr));
-		if (ret) {
+		if (ret < 0) {
 			dev_err(data->dev, "error writing nvm address\n");
 			goto exit;
 		}
@@ -1562,19 +1558,19 @@  static int bmp580_nvmem_write(void *priv, unsigned int offset, void *val,
 
 		ret = regmap_bulk_write(data->regmap, BMP580_REG_NVM_DATA_LSB,
 					&data->le16, sizeof(data->le16));
-		if (ret) {
+		if (ret < 0) {
 			dev_err(data->dev, "error writing LSB NVM data regs\n");
 			goto exit;
 		}
 
 		ret = bmp580_nvm_operation(data, true);
-		if (ret)
+		if (ret < 0)
 			goto exit;
 
 		/* Disable programming mode bit */
 		ret = regmap_update_bits(data->regmap, BMP580_REG_NVM_ADDR,
 					 BMP580_NVM_PROG_EN, 0);
-		if (ret) {
+		if (ret < 0) {
 			dev_err(data->dev, "error resetting nvm write\n");
 			goto exit;
 		}
@@ -1608,25 +1604,29 @@  static int bmp580_preinit(struct bmp280_data *data)
 
 	/* Issue soft-reset command */
 	ret = bmp580_soft_reset(data);
-	if (ret)
+	if (ret < 0)
 		return ret;
 
 	/* Post powerup sequence */
 	ret = regmap_read(data->regmap, BMP580_REG_CHIP_ID, &reg);
-	if (ret)
+	if (ret < 0) {
+		dev_err(data->dev, "failed to establish comms with the chip\n");
 		return ret;
+	}
 
 	/* Print warn message if we don't know the chip id */
 	if (reg != BMP580_CHIP_ID && reg != BMP580_CHIP_ID_ALT)
-		dev_warn(data->dev, "preinit: unexpected chip_id\n");
+		dev_warn(data->dev, "unexpected chip_id\n");
 
 	ret = regmap_read(data->regmap, BMP580_REG_STATUS, &reg);
-	if (ret)
+	if (ret < 0) {
+		dev_err(data->dev, "failed to read nvm status\n");
 		return ret;
+	}
 
 	/* Check nvm status */
 	if (!(reg & BMP580_STATUS_NVM_RDY_MASK) || (reg & BMP580_STATUS_NVM_ERR_MASK)) {
-		dev_err(data->dev, "preinit: nvm error on powerup sequence\n");
+		dev_err(data->dev, "nvm error on powerup sequence\n");
 		return -EIO;
 	}
 
@@ -1646,7 +1646,7 @@  static int bmp580_chip_config(struct bmp280_data *data)
 				 BMP580_MODE_MASK | BMP580_ODR_DEEPSLEEP_DIS,
 				 BMP580_ODR_DEEPSLEEP_DIS |
 				 FIELD_PREP(BMP580_MODE_MASK, BMP580_MODE_SLEEP));
-	if (ret) {
+	if (ret < 0) {
 		dev_err(data->dev, "failed to change sensor to standby mode\n");
 		return ret;
 	}
@@ -1661,6 +1661,10 @@  static int bmp580_chip_config(struct bmp280_data *data)
 				 BMP580_DSP_COMP_MASK |
 				 BMP580_DSP_SHDW_IIR_TEMP_EN |
 				 BMP580_DSP_SHDW_IIR_PRESS_EN, reg_val);
+	if (ret < 0) {
+		dev_err(data->dev, "failed to change DSP mode settings\n");
+		return ret;
+	}
 
 	/* Configure oversampling */
 	reg_val = FIELD_PREP(BMP580_OSR_TEMP_MASK, data->oversampling_temp) |
@@ -1672,7 +1676,7 @@  static int bmp580_chip_config(struct bmp280_data *data)
 				       BMP580_OSR_PRESS_MASK |
 				       BMP580_OSR_PRESS_EN,
 				       reg_val, &aux);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(data->dev, "failed to write oversampling register\n");
 		return ret;
 	}
@@ -1682,7 +1686,7 @@  static int bmp580_chip_config(struct bmp280_data *data)
 	ret = regmap_update_bits_check(data->regmap, BMP580_REG_ODR_CONFIG, BMP580_ODR_MASK,
 				       FIELD_PREP(BMP580_ODR_MASK, data->sampling_freq),
 				       &aux);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(data->dev, "failed to write ODR configuration register\n");
 		return ret;
 	}
@@ -1696,7 +1700,7 @@  static int bmp580_chip_config(struct bmp280_data *data)
 				       BMP580_DSP_IIR_PRESS_MASK |
 				       BMP580_DSP_IIR_TEMP_MASK,
 				       reg_val, &aux);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(data->dev, "failed to write config register\n");
 		return ret;
 	}
@@ -1706,7 +1710,7 @@  static int bmp580_chip_config(struct bmp280_data *data)
 	ret = regmap_write_bits(data->regmap, BMP580_REG_ODR_CONFIG,
 				BMP580_MODE_MASK,
 				FIELD_PREP(BMP580_MODE_MASK, BMP580_MODE_NORMAL));
-	if (ret) {
+	if (ret < 0) {
 		dev_err(data->dev, "failed to set normal mode\n");
 		return ret;
 	}
@@ -1719,7 +1723,7 @@  static int bmp580_chip_config(struct bmp280_data *data)
 		 * operating in a degraded mode.
 		 */
 		ret = regmap_read(data->regmap, BMP580_REG_EFF_OSR, &tmp);
-		if (ret) {
+		if (ret < 0) {
 			dev_err(data->dev,
 				"error reading effective OSR register\n");
 			return ret;
@@ -1782,8 +1786,10 @@  static int bmp180_wait_for_eoc(struct bmp280_data *data, u8 ctrl_meas)
 		reinit_completion(&data->done);
 
 	ret = regmap_write(data->regmap, BMP280_REG_CTRL_MEAS, ctrl_meas);
-	if (ret)
+	if (ret < 0) {
+		dev_err(data->dev, "failed to write crtl_meas register\n");
 		return ret;
+	}
 
 	if (data->use_eoc) {
 		/*
@@ -1806,12 +1812,16 @@  static int bmp180_wait_for_eoc(struct bmp280_data *data, u8 ctrl_meas)
 	}
 
 	ret = regmap_read(data->regmap, BMP280_REG_CTRL_MEAS, &ctrl);
-	if (ret)
+	if (ret < 0) {
+		dev_err(data->dev, "failed to read ctrl_meas register\n");
 		return ret;
+	}
 
 	/* The value of this bit reset to "0" after conversion is complete */
-	if (ctrl & BMP180_MEAS_SCO)
+	if (ctrl & BMP180_MEAS_SCO) {
+		dev_err(data->dev, "conversion didn't complete\n");
 		return -EIO;
+	}
 
 	return 0;
 }
@@ -1823,13 +1833,15 @@  static int bmp180_read_adc_temp(struct bmp280_data *data, int *val)
 	ret = bmp180_wait_for_eoc(data,
 				  FIELD_PREP(BMP180_MEAS_CTRL_MASK, BMP180_MEAS_TEMP) |
 				  BMP180_MEAS_SCO);
-	if (ret)
+	if (ret < 0)
 		return ret;
 
 	ret = regmap_bulk_read(data->regmap, BMP180_REG_OUT_MSB,
 			       &data->be16, sizeof(data->be16));
-	if (ret)
+	if (ret < 0) {
+		dev_err(data->dev, "failed to read temperature\n");
 		return ret;
+	}
 
 	*val = be16_to_cpu(data->be16);
 
@@ -1844,9 +1856,10 @@  static int bmp180_read_calib(struct bmp280_data *data)
 
 	ret = regmap_bulk_read(data->regmap, BMP180_REG_CALIB_START,
 			       data->bmp180_cal_buf, sizeof(data->bmp180_cal_buf));
-
-	if (ret < 0)
+	if (ret < 0) {
+		dev_err(data->dev, "failed to read calibration parameters\n");
 		return ret;
+	}
 
 	/* None of the words has the value 0 or 0xFFFF */
 	for (i = 0; i < ARRAY_SIZE(data->bmp180_cal_buf); i++) {
@@ -1898,7 +1911,7 @@  static int bmp180_read_temp(struct bmp280_data *data, int *val, int *val2)
 	int ret;
 
 	ret = bmp180_read_adc_temp(data, &adc_temp);
-	if (ret)
+	if (ret < 0)
 		return ret;
 
 	comp_temp = bmp180_compensate_temp(data, adc_temp);
@@ -1924,13 +1937,15 @@  static int bmp180_read_adc_press(struct bmp280_data *data, int *val)
 				  FIELD_PREP(BMP180_MEAS_CTRL_MASK, BMP180_MEAS_PRESS) |
 				  FIELD_PREP(BMP180_OSRS_PRESS_MASK, oss) |
 				  BMP180_MEAS_SCO);
-	if (ret)
+	if (ret < 0)
 		return ret;
 
 	ret = regmap_bulk_read(data->regmap, BMP180_REG_OUT_MSB,
 			       data->buf, sizeof(data->buf));
-	if (ret)
+	if (ret < 0) {
+		dev_err(data->dev, "failed to read pressure\n");
 		return ret;
+	}
 
 	*val = get_unaligned_be24(data->buf) >> (8 - oss);
 
@@ -1980,11 +1995,11 @@  static int bmp180_read_press(struct bmp280_data *data, int *val, int *val2)
 
 	/* Read and compensate temperature so we get a reading of t_fine. */
 	ret = bmp180_read_temp(data, NULL, NULL);
-	if (ret)
+	if (ret < 0)
 		return ret;
 
 	ret = bmp180_read_adc_press(data, &adc_press);
-	if (ret)
+	if (ret < 0)
 		return ret;
 
 	comp_press = bmp180_compensate_press(data, adc_press);
@@ -2062,7 +2077,7 @@  static int bmp085_fetch_eoc_irq(struct device *dev,
 			irq_trig,
 			name,
 			data);
-	if (ret) {
+	if (ret < 0) {
 		/* Bail out without IRQ but keep the driver in place */
 		dev_err(dev, "unable to request DRDY IRQ\n");
 		return 0;
@@ -2132,20 +2147,20 @@  int bmp280_common_probe(struct device *dev,
 
 	ret = devm_regulator_bulk_get(dev,
 				      BMP280_NUM_SUPPLIES, data->supplies);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(dev, "failed to get regulators\n");
 		return ret;
 	}
 
 	ret = regulator_bulk_enable(BMP280_NUM_SUPPLIES, data->supplies);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(dev, "failed to enable regulators\n");
 		return ret;
 	}
 
 	ret = devm_add_action_or_reset(dev, bmp280_regulators_disable,
 				       data->supplies);
-	if (ret)
+	if (ret < 0)
 		return ret;
 
 	/* Wait to make sure we started up properly */
@@ -2162,8 +2177,10 @@  int bmp280_common_probe(struct device *dev,
 	data->regmap = regmap;
 
 	ret = regmap_read(regmap, data->chip_info->id_reg, &chip_id);
-	if (ret < 0)
+	if (ret < 0) {
+		dev_err(data->dev, "failed to read chip id\n");
 		return ret;
+	}
 
 	for (i = 0; i < data->chip_info->num_chip_id; i++) {
 		if (chip_id == data->chip_info->chip_id[i]) {
@@ -2177,7 +2194,7 @@  int bmp280_common_probe(struct device *dev,
 
 	if (data->chip_info->preinit) {
 		ret = data->chip_info->preinit(data);
-		if (ret)
+		if (ret < 0)
 			return dev_err_probe(data->dev, ret,
 					     "error running preinit tasks\n");
 	}
@@ -2208,7 +2225,7 @@  int bmp280_common_probe(struct device *dev,
 	 */
 	if (irq > 0 && (chip_id  == BMP180_CHIP_ID)) {
 		ret = bmp085_fetch_eoc_irq(dev, name, irq, data);
-		if (ret)
+		if (ret < 0)
 			return ret;
 	}
 
@@ -2225,7 +2242,7 @@  int bmp280_common_probe(struct device *dev,
 	pm_runtime_put(dev);
 
 	ret = devm_add_action_or_reset(dev, bmp280_pm_disable, dev);
-	if (ret)
+	if (ret < 0)
 		return ret;
 
 	return devm_iio_device_register(dev, indio_dev);
@@ -2247,7 +2264,7 @@  static int bmp280_runtime_resume(struct device *dev)
 	int ret;
 
 	ret = regulator_bulk_enable(BMP280_NUM_SUPPLIES, data->supplies);
-	if (ret)
+	if (ret < 0)
 		return ret;
 
 	usleep_range(data->start_up_time, data->start_up_time + 100);