bme680 cleanup

Signed-off-by David Frey <[email protected]>

Quoted 1 line · David Frey

Signed-off-by David Frey <[email protected]>

Reviewed-by Himanshu Jha <[email protected]>

Tested-by Himanshu Jha <[email protected]>

Also, 0-day tested with build success!

Thanks

Quoted 5 lines · On Sat, 18 Aug 2018 16:33:23 +0530 Himanshu Jha <[email protected]>…

Quoted 1 line · David Frey

Signed-off-by David Frey <[email protected]>

Reviewed-by Himanshu Jha <[email protected]>

Tested-by Himanshu Jha <[email protected]>

Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to play with it.

Quoted 10 lines · Jonathan Cameron

Quoted 5 lines · On Sat, 18 Aug 2018 16:33:23 +0530 Himanshu Jha <[email protected]>…

Quoted 1 line · David Frey

Signed-off-by David Frey <[email protected]>

Reviewed-by Himanshu Jha <[email protected]>

Tested-by Himanshu Jha <[email protected]>

Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to play with it.

Hi,

I'm trying to understand the linux-iio mailing list workflow a bit better. I have
git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git as my "iio" remote in my Linux kernel repository. From that remote, I can see remotes/iio/togreg. When I do a "git fetch", the togreg branch hasn't been updated. Did Jonathan mean that he applied the patch to the togreg branch in his local repository, but hasn't yet pushed it to the one that I have listed above? Are the "autobuilders" specific to IIO? Is this infrastructure publicly accessible or is it only available to specific users? If it's public, where can I view it/learn more about it?

Quoted 4 lines · Jonathan Cameron

Quoted 1 line · On Sat, 18 Aug 2018 16:33:23 +0530 Himanshu Jha <[email protected]>…

On Fri, Aug 17, 2018 at 12:03:13PM -0700, David Frey wrote:

Quoted 6 lines

Quoted 4 lines

Quoted 1 line

Signed-off-by David Frey <[email protected]>

Reviewed-by Himanshu Jha <[email protected]>

Tested-by Himanshu Jha <[email protected]>

Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to play with it.

Hi,

I'm trying to understand the linux-iio mailing list workflow a bit better. I have
git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git as my "iio" remote in my Linux kernel repository. From that remote, I can see remotes/iio/togreg. When I do a "git fetch", the togreg branch hasn't been updated. Did Jonathan mean that he applied the patch to the togreg
branch in his local repository, but hasn't yet pushed it to the one that
I have listed above?

Are the "autobuilders" specific to IIO?

Is this
infrastructure publicly accessible or is it only available to specific users? If it's public, where can I view it/learn more about it?

Thanks,
David

Signed-off-by David Frey <[email protected]>

---
 drivers/iio/chemical/bme680_core.c | 36 ++++++++++++------------------------
 1 file changed, 12 insertions(+), 24 deletions(-)

drivers/iio/chemical/bme680_core.c +8 -16

index 35cbcb16c9f9..cde08d57e7d5 100644
@@ -102,16 +102,14 @@ static int bme680_read_calib(struct bme680_data *data,
 	__le16 buf;
 
 	/* Temperature related coefficients */
-	ret = regmap_bulk_read(data->regmap, BME680_T1_LSB_REG,
-			       (u8 *) &buf, 2);
+	ret = regmap_bulk_read(data->regmap, BME680_T1_LSB_REG, (u8 *) &buf, 2);
 	if (ret < 0) {
 		dev_err(dev, "failed to read BME680_T1_LSB_REG\n");
 		return ret;
 	}
 	calib->par_t1 = le16_to_cpu(buf);
 
-	ret = regmap_bulk_read(data->regmap, BME680_T2_LSB_REG,
-			       (u8 *) &buf, 2);
+	ret = regmap_bulk_read(data->regmap, BME680_T2_LSB_REG, (u8 *) &buf, 2);
 	if (ret < 0) {
 		dev_err(dev, "failed to read BME680_T2_LSB_REG\n");
 		return ret;

@@ -126,16 +124,14 @@ static int bme680_read_calib(struct bme680_data *data,
 	calib->par_t3 = tmp;
 
 	/* Pressure related coefficients */
-	ret = regmap_bulk_read(data->regmap, BME680_P1_LSB_REG,
-			       (u8 *) &buf, 2);
+	ret = regmap_bulk_read(data->regmap, BME680_P1_LSB_REG, (u8 *) &buf, 2);
 	if (ret < 0) {
 		dev_err(dev, "failed to read BME680_P1_LSB_REG\n");
 		return ret;
 	}
 	calib->par_p1 = le16_to_cpu(buf);
 
-	ret = regmap_bulk_read(data->regmap, BME680_P2_LSB_REG,
-			       (u8 *) &buf, 2);
+	ret = regmap_bulk_read(data->regmap, BME680_P2_LSB_REG, (u8 *) &buf, 2);
 	if (ret < 0) {
 		dev_err(dev, "failed to read BME680_P2_LSB_REG\n");
 		return ret;

@@ -149,16 +145,14 @@ static int bme680_read_calib(struct bme680_data *data,
 	}
 	calib->par_p3 = tmp;
 
-	ret = regmap_bulk_read(data->regmap, BME680_P4_LSB_REG,
-			       (u8 *) &buf, 2);
+	ret = regmap_bulk_read(data->regmap, BME680_P4_LSB_REG, (u8 *) &buf, 2);
 	if (ret < 0) {
 		dev_err(dev, "failed to read BME680_P4_LSB_REG\n");
 		return ret;
 	}
 	calib->par_p4 = le16_to_cpu(buf);
 
-	ret = regmap_bulk_read(data->regmap, BME680_P5_LSB_REG,
-			       (u8 *) &buf, 2);
+	ret = regmap_bulk_read(data->regmap, BME680_P5_LSB_REG, (u8 *) &buf, 2);
 	if (ret < 0) {
 		dev_err(dev, "failed to read BME680_P5_LSB_REG\n");
 		return ret;

@@ -179,16 +173,14 @@ static int bme680_read_calib(struct bme680_data *data,
 	}
 	calib->par_p7 = tmp;
 
-	ret = regmap_bulk_read(data->regmap, BME680_P8_LSB_REG,
-			       (u8 *) &buf, 2);
+	ret = regmap_bulk_read(data->regmap, BME680_P8_LSB_REG, (u8 *) &buf, 2);
 	if (ret < 0) {
 		dev_err(dev, "failed to read BME680_P8_LSB_REG\n");
 		return ret;
 	}
 	calib->par_p8 = le16_to_cpu(buf);
 
-	ret = regmap_bulk_read(data->regmap, BME680_P9_LSB_REG,
-			       (u8 *) &buf, 2);
+	ret = regmap_bulk_read(data->regmap, BME680_P9_LSB_REG, (u8 *) &buf, 2);
 	if (ret < 0) {
 		dev_err(dev, "failed to read BME680_P9_LSB_REG\n");
 		return ret;

@@ -208,30 +200,26 @@ static int bme680_read_calib(struct bme680_data *data,
 		dev_err(dev, "failed to read BME680_H1_MSB_REG\n");
 		return ret;
 	}

drivers/iio/chemical/bme680_core.c +0 -0

 	ret = regmap_read(data->regmap, BME680_H2_MSB_REG, &tmp_msb);
 	if (ret < 0) {
 		dev_err(dev, "failed to read BME680_H2_MSB_REG\n");
 		return ret;
 	}

Quoted 1 line · David Frey

Signed-off-by David Frey <[email protected]>

Reviewed-by Himanshu Jha <[email protected]>

Tested-by Himanshu Jha <[email protected]>

Also, 0-day tested with build success!

Thanks

One minor comment below.

Maybe Jonathan would fix while applying, so don't need a new version for this.

Quoted 89 lines

---
 drivers/iio/chemical/bme680_core.c | 36 ++++++++++++------------------------
 1 file changed, 12 insertions(+), 24 deletions(-)

drivers/iio/chemical/bme680_core.c +8 -16

index 35cbcb16c9f9..cde08d57e7d5 100644
@@ -102,16 +102,14 @@ static int bme680_read_calib(struct bme680_data *data,
 	__le16 buf;
 
 	/* Temperature related coefficients */
-	ret = regmap_bulk_read(data->regmap, BME680_T1_LSB_REG,
-			       (u8 *) &buf, 2);
+	ret = regmap_bulk_read(data->regmap, BME680_T1_LSB_REG, (u8 *) &buf, 2);
 	if (ret < 0) {
 		dev_err(dev, "failed to read BME680_T1_LSB_REG\n");
 		return ret;
 	}
 	calib->par_t1 = le16_to_cpu(buf);
 
-	ret = regmap_bulk_read(data->regmap, BME680_T2_LSB_REG,
-			       (u8 *) &buf, 2);
+	ret = regmap_bulk_read(data->regmap, BME680_T2_LSB_REG, (u8 *) &buf, 2);
 	if (ret < 0) {
 		dev_err(dev, "failed to read BME680_T2_LSB_REG\n");
 		return ret;

@@ -126,16 +124,14 @@ static int bme680_read_calib(struct bme680_data *data,
 	calib->par_t3 = tmp;
 
 	/* Pressure related coefficients */
-	ret = regmap_bulk_read(data->regmap, BME680_P1_LSB_REG,
-			       (u8 *) &buf, 2);
+	ret = regmap_bulk_read(data->regmap, BME680_P1_LSB_REG, (u8 *) &buf, 2);
 	if (ret < 0) {
 		dev_err(dev, "failed to read BME680_P1_LSB_REG\n");
 		return ret;
 	}
 	calib->par_p1 = le16_to_cpu(buf);
 
-	ret = regmap_bulk_read(data->regmap, BME680_P2_LSB_REG,
-			       (u8 *) &buf, 2);
+	ret = regmap_bulk_read(data->regmap, BME680_P2_LSB_REG, (u8 *) &buf, 2);
 	if (ret < 0) {
 		dev_err(dev, "failed to read BME680_P2_LSB_REG\n");
 		return ret;

@@ -149,16 +145,14 @@ static int bme680_read_calib(struct bme680_data *data,
 	}
 	calib->par_p3 = tmp;
 
-	ret = regmap_bulk_read(data->regmap, BME680_P4_LSB_REG,
-			       (u8 *) &buf, 2);
+	ret = regmap_bulk_read(data->regmap, BME680_P4_LSB_REG, (u8 *) &buf, 2);
 	if (ret < 0) {
 		dev_err(dev, "failed to read BME680_P4_LSB_REG\n");
 		return ret;
 	}
 	calib->par_p4 = le16_to_cpu(buf);
 
-	ret = regmap_bulk_read(data->regmap, BME680_P5_LSB_REG,
-			       (u8 *) &buf, 2);
+	ret = regmap_bulk_read(data->regmap, BME680_P5_LSB_REG, (u8 *) &buf, 2);
 	if (ret < 0) {
 		dev_err(dev, "failed to read BME680_P5_LSB_REG\n");
 		return ret;

@@ -179,16 +173,14 @@ static int bme680_read_calib(struct bme680_data *data,
 	}
 	calib->par_p7 = tmp;
 
-	ret = regmap_bulk_read(data->regmap, BME680_P8_LSB_REG,
-			       (u8 *) &buf, 2);
+	ret = regmap_bulk_read(data->regmap, BME680_P8_LSB_REG, (u8 *) &buf, 2);
 	if (ret < 0) {
 		dev_err(dev, "failed to read BME680_P8_LSB_REG\n");
 		return ret;
 	}
 	calib->par_p8 = le16_to_cpu(buf);
 
-	ret = regmap_bulk_read(data->regmap, BME680_P9_LSB_REG,
-			       (u8 *) &buf, 2);
+	ret = regmap_bulk_read(data->regmap, BME680_P9_LSB_REG, (u8 *) &buf, 2);
 	if (ret < 0) {
 		dev_err(dev, "failed to read BME680_P9_LSB_REG\n");
 		return ret;

@@ -208,30 +200,26 @@ static int bme680_read_calib(struct bme680_data *data,
 		dev_err(dev, "failed to read BME680_H1_MSB_REG\n");
 		return ret;
 	}

drivers/iio/chemical/bme680_core.c static int bme680_read_calib(struct bme680_data *data, · line 209

Quoted 10 lines

drivers/iio/chemical/bme680_core.c +1 -3

-
 	ret = regmap_read(data->regmap, BME680_H1_LSB_REG, &tmp_lsb);
 	if (ret < 0) {
 		dev_err(dev, "failed to read BME680_H1_LSB_REG\n");
 		return ret;
 	}
-
 	calib->par_h1 = (tmp_msb << BME680_HUM_REG_SHIFT_VAL) |
-				(tmp_lsb & BME680_BIT_H1_DATA_MSK);
+	                (tmp_lsb & BME680_BIT_H1_DATA_MSK);  

Prefer tabs instead of spaces!
Please run checkpatch on the patch to get those warnings.

Quoted 5 lines

drivers/iio/chemical/bme680_core.c +0 -0

 	ret = regmap_read(data->regmap, BME680_H2_MSB_REG, &tmp_msb);
 	if (ret < 0) {
 		dev_err(dev, "failed to read BME680_H2_MSB_REG\n");
 		return ret;
 	}

drivers/iio/chemical/bme680_core.c static int bme680_read_calib(struct bme680_data *data, · line 222

Quoted 10 lines

drivers/iio/chemical/bme680_core.c +1 -3

-
 	ret = regmap_read(data->regmap, BME680_H2_LSB_REG, &tmp_lsb);
 	if (ret < 0) {
 		dev_err(dev, "failed to read BME680_H2_LSB_REG\n");
 		return ret;
 	}
-
 	calib->par_h2 = (tmp_msb << BME680_HUM_REG_SHIFT_VAL) |
-				(tmp_lsb >> BME680_HUM_REG_SHIFT_VAL);
+	                (tmp_lsb >> BME680_HUM_REG_SHIFT_VAL);  

Same here.

Quoted 10 lines · On Sat, 18 Aug 2018 16:36:55 +0530 Himanshu Jha <[email protected]>…

Quoted 1 line · David Frey

Signed-off-by David Frey <[email protected]>

Reviewed-by Himanshu Jha <[email protected]>

Tested-by Himanshu Jha <[email protected]>

Also, 0-day tested with build success!

Thanks

Quoted 4 lines

One minor comment below.

Maybe Jonathan would fix while applying, so don't need a new version for this.

I've fixed them up.

I would have liked to see a little more detail in the description of what was being changed and why though...

Quoted 1 line · David Frey

Signed-off-by David Frey <[email protected]>

One minor comment below.

Quoted 4 lines

drivers/iio/chemical/bme680_core.c +0 -0

index 35cbcb16c9f9..cde08d57e7d5 100644

Quoted 9 lines

drivers/iio/chemical/bme680_core.c +0 -1

@@ -208,30 +200,26 @@ static int bme680_read_calib(struct bme680_data *data,
 		dev_err(dev, "failed to read BME680_H1_MSB_REG\n");
 		return ret;
 	}
-
 	ret = regmap_read(data->regmap, BME680_H1_LSB_REG, &tmp_lsb);
 	if (ret < 0) {
 		dev_err(dev, "failed to read BME680_H1_LSB_REG\n");
 		return ret;

drivers/iio/chemical/bme680_core.c static int bme680_read_calib(struct bme680_data *data, · line 209

Quoted 5 lines

drivers/iio/chemical/bme680_core.c +1 -2

 	}
-
 	calib->par_h1 = (tmp_msb << BME680_HUM_REG_SHIFT_VAL) |
-				(tmp_lsb & BME680_BIT_H1_DATA_MSK);
+	                (tmp_lsb & BME680_BIT_H1_DATA_MSK);

Prefer tabs instead of spaces!
Please run checkpatch on the patch to get those warnings.

Quoted 4 lines · Himanshu Jha

Quoted 1 line · David Frey

Signed-off-by David Frey <[email protected]>

One minor comment below.

<snip>

Quoted 4 lines

Quoted 4 lines

drivers/iio/chemical/bme680_core.c +0 -0

index 35cbcb16c9f9..cde08d57e7d5 100644

<snip>

Quoted 9 lines

Quoted 9 lines

drivers/iio/chemical/bme680_core.c +0 -1

@@ -208,30 +200,26 @@ static int bme680_read_calib(struct bme680_data *data,
 		dev_err(dev, "failed to read BME680_H1_MSB_REG\n");
 		return ret;
 	}
-
 	ret = regmap_read(data->regmap, BME680_H1_LSB_REG, &tmp_lsb);
 	if (ret < 0) {
 		dev_err(dev, "failed to read BME680_H1_LSB_REG\n");
 		return ret;

Quoted 8 lines

drivers/iio/chemical/bme680_core.c static int bme680_read_calib(struct bme680_data *data, · line 209

Quoted 5 lines

drivers/iio/chemical/bme680_core.c +1 -2

 	}
-
 	calib->par_h1 = (tmp_msb << BME680_HUM_REG_SHIFT_VAL) |
-				(tmp_lsb & BME680_BIT_H1_DATA_MSK);
+	                (tmp_lsb & BME680_BIT_H1_DATA_MSK);

Prefer tabs instead of spaces!
Please run checkpatch on the patch to get those warnings.

Thanks for telling me about the existence of checkpatch.pl. The official docs
(https://www.kernel.org/doc/html/v4.18/process/coding-style.html) are pretty light on information about indentation/alignment, but there is this relevant quote: "Outside of comments, documentation and except in Kconfig, spaces are never used for indentation, and the above example is deliberately broken."

I thought that the way I indented it was optimal because I used spaces a tab for indentation and then spaces for alignment, but checkpatch.pl seems to disagree. Based on a quick read of the code, it seems that it will complain about more than 7 spaces in a row.

I guess you could achieve visually identical alignment (where "------->" is tab and "_" is space) and this would still pass checkpatch.pl. Note that I changed the identifier name slightly to demonstrate a mix of tabs and spaces.

------->calib->par_h123 = (tmp_msb << BME680_HUM_REG_SHIFT_VAL) | ------->------->------->__(tmp_lsb & BME680_BIT_H1_DATA_MSK);

What I don't understand is the logic used to justify the original indentation:
------->calib->par_h1 = (tmp_msb << BME680_HUM_REG_SHIFT_VAL) | ------->------->------->------->(tmp_lsb & BME680_BIT_H1_DATA_MSK);

Why not?
------->calib->par_h1 = (tmp_msb << BME680_HUM_REG_SHIFT_VAL) | ------->------->------->(tmp_lsb & BME680_BIT_H1_DATA_MSK);

Or?
------->calib->par_h1 = (tmp_msb << BME680_HUM_REG_SHIFT_VAL) | ------->------->(tmp_lsb & BME680_BIT_H1_DATA_MSK);

I'm not trying to turn this into a crazy bike shedding thread. I just want to understand the rules a bit better.

Signed-off-by David Frey <[email protected]>

Quoted 1 line · David Frey

Signed-off-by David Frey <[email protected]>

Reviewed-by Himanshu Jha <[email protected]>

Tested-by Himanshu Jha <[email protected]>

Also, 0-day tested with build success!

Thanks

Quoted 11 lines · On Sat, 18 Aug 2018 16:37:25 +0530 Himanshu Jha <[email protected]>…

Quoted 1 line · David Frey

Signed-off-by David Frey <[email protected]>

Reviewed-by Himanshu Jha <[email protected]>

Tested-by Himanshu Jha <[email protected]>

Also, 0-day tested with build success!

Thanks

Applied,

There is one more suspicious bit of indenting in here
#define BME680_REG_CTRL_GAS_1 0x71
#define BME680_RUN_GAS_MASK BIT(4)
#define BME680_NB_CONV_MASK GENMASK(3, 0)
#define BME680_RUN_GAS_EN_BIT BIT(4)
#define BME680_NB_CONV_0_VAL 0

I think RUN_GAS_EN_BIT should be one level lower?

Quoted 24 lines · Jonathan Cameron

Quoted 11 lines · On Sat, 18 Aug 2018 16:37:25 +0530 Himanshu Jha <[email protected]>…

Quoted 1 line · David Frey

Signed-off-by David Frey <[email protected]>

Reviewed-by Himanshu Jha <[email protected]>

Tested-by Himanshu Jha <[email protected]>

Also, 0-day tested with build success!

Thanks

Applied,

There is one more suspicious bit of indenting in here
#define BME680_REG_CTRL_GAS_1 0x71
#define BME680_RUN_GAS_MASK BIT(4)
#define BME680_NB_CONV_MASK GENMASK(3, 0)
#define BME680_RUN_GAS_EN_BIT BIT(4)
#define BME680_NB_CONV_0_VAL 0

Quoted 1 line

I think RUN_GAS_EN_BIT should be one level lower?

No, its fine. It is a value and third level indent is expected. It should be set to 1 to initiate measurement. And the total mask value is

	0b00001000 == BME680_RUN_GAS_EN_BIT | BME680_NB_CONV_0_VAL

Remove BME680_RUN_GAS_EN_BIT and BME680_NB_CONV_0_VAL field value definitions because the fields are simply boolean and integer respectively.

Signed-off-by David Frey <[email protected]>

---
This patch applies on top of my "indent #defines consistently" v3 patch. Appologies if I should have submitted this patch in a different way. If I should have submitted this differently, I would appreciate a pointer on what I should have done in this case.

BME680_RUN_GAS_EN_BIT was indeed somewhat wrongly formatted, but the issue was not the indentation level, but rather that I should have followed immediately after BME680_RUN_GAS_MASK. Once I moved it there, I realized that neither this definition nor BME680_NB_CONV_0_VAL really added any value and hence I removed both in this patch.

Quoted 5 lines · David Frey

Remove BME680_RUN_GAS_EN_BIT and BME680_NB_CONV_0_VAL field value definitions because the fields are simply boolean and integer respectively.

Signed-off-by David Frey <[email protected]>

Reviewed-by Himanshu Jha <[email protected]>

Tested-by Himanshu Jha <[email protected]>

Quoted 6 lines

---
This patch applies on top of my "indent #defines consistently" v3 patch. Appologies if I should have submitted this patch in a different way. If I should have submitted this differently, I would appreciate a pointer on what I should have done in this case.

This applies cleanly so no worries I guess.
Would have been better to send this patch as a separate thread since thread becomes complex and its hard to find the new patch in the nested series of replies.

Quoted 5 lines

BME680_RUN_GAS_EN_BIT was indeed somewhat wrongly formatted, but the issue was not the indentation level, but rather that I should have followed immediately after BME680_RUN_GAS_MASK. Once I moved it there, I realized that neither this definition nor BME680_NB_CONV_0_VAL really added any value and hence I removed both in this patch.

My main intention was to make it explicit that we are selected NB_CONV_0 set point, and I didn't knew about FIELD_PREP helper macro until you pointed me out in the early review cycle.

Now, it is much appropriate.

Thanks

Quoted 25 lines · On Sun, 19 Aug 2018 22:58:44 +0530 Himanshu Jha <[email protected]>…

Quoted 24 lines · Jonathan Cameron

Quoted 11 lines · On Sat, 18 Aug 2018 16:37:25 +0530 Himanshu Jha <[email protected]>…

Quoted 1 line · David Frey

Signed-off-by David Frey <[email protected]>

Reviewed-by Himanshu Jha <[email protected]>

Tested-by Himanshu Jha <[email protected]>

Also, 0-day tested with build success!

Thanks

Applied,

There is one more suspicious bit of indenting in here
#define BME680_REG_CTRL_GAS_1 0x71
#define BME680_RUN_GAS_MASK BIT(4)
#define BME680_NB_CONV_MASK GENMASK(3, 0)
#define BME680_RUN_GAS_EN_BIT BIT(4)
#define BME680_NB_CONV_0_VAL 0

Quoted 7 lines

Quoted 1 line

I think RUN_GAS_EN_BIT should be one level lower?

No, its fine. It is a value and third level indent is expected. It should be set to 1 to initiate measurement. And the total mask value is

	0b00001000 == BME680_RUN_GAS_EN_BIT | BME680_NB_CONV_0_VAL

Ah, then I'd argue it's name is wrong. BIT elsewhere has been used to indicate a field in the register, not a value.

Should probably just be BME680_RUN_GAS_EN?

Convert all defines to use "MASK" instead of a mix of "MSK" and "MASK"

Signed-off-by David Frey <[email protected]>

Quoted 3 lines · David Frey

Convert all defines to use "MASK" instead of a mix of "MSK" and "MASK"

Signed-off-by David Frey <[email protected]>

Reviewed-by Himanshu Jha <[email protected]>

Tested-by Himanshu Jha <[email protected]>

Also, 0-day tested with build success!

Thanks

drivers/iio/chemical/bme680_core.c static int bme680_read_calib(struct bme680_data *data, · line 209

Quoted 2 lines

drivers/iio/chemical/bme680_core.c +1 -1

-	                (tmp_lsb & BME680_BIT_H1_DATA_MSK);
+	                (tmp_lsb & BME680_BIT_H1_DATA_MASK);  

Its here again so, perhaps better to send a new version instead.

Replace hardcoded bit masks with GENMASK macro

Signed-off-by David Frey <[email protected]>

Quoted 3 lines · David Frey

Replace hardcoded bit masks with GENMASK macro

Signed-off-by David Frey <[email protected]>

Reviewed-by Himanshu Jha <[email protected]>

Tested-by Himanshu Jha <[email protected]>

Also, 0-day tested with build success!

Thanks

Use the FIELD_GET macro instead of explicit mask and shift.

Signed-off-by David Frey <[email protected]>

Quoted 3 lines · David Frey

Use the FIELD_GET macro instead of explicit mask and shift.

Signed-off-by David Frey <[email protected]>

Reviewed-by Himanshu Jha <[email protected]>

Tested-by Himanshu Jha <[email protected]>

Also, 0-day tested with build success!

Thanks

Temperature, pressure and humidity all expose and oversampling setting that works in the same way. Provide common handling for the oversampling sysfs attributes.

Signed-off-by David Frey <[email protected]>

Quoted 5 lines · David Frey

Temperature, pressure and humidity all expose and oversampling setting that works in the same way. Provide common handling for the oversampling sysfs attributes.

Signed-off-by David Frey <[email protected]>

Reviewed-by Himanshu Jha <[email protected]>

Tested-by Himanshu Jha <[email protected]>

Also, 0-day tested with build success!

Thanks

With this I don't think my patch:
https://lore.kernel.org/lkml/[email protected]/
is useful any more since bme680_is_valid_oversampling() does the work pehaps.

The best part of this patch is supplying direct values and removes the amiguity around ilog2(), especially at probe, when supplying default values.

Thanks David for the series.