[v3,7/8] iio: magnetometer: yas530: Apply minor cleanups

Commit Message

Jakob Hauser June 18, 2022, 12:13 a.m. UTC

This commit gathers minor cleanups in the current driver.

In the device examples, "Xiaomi" is too generic, specific devices should be
listed here. E.g. Xiaomi Redmi 2 seems to have YAS537 but it's not fully clear
if this applies to all its variants. Samsung Galaxy S7 is often quoted in
conjunction with YAS537.

In the function yas530_get_calibration_data(), the debug dump was extended to
16 elements as this is the size of the calibration data array of YAS530.
---
 drivers/iio/magnetometer/yamaha-yas530.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

Comments

Andy Shevchenko June 18, 2022, 9:53 a.m. UTC | #1

On Sat, Jun 18, 2022 at 2:15 AM Jakob Hauser <[e-mail]> wrote:
>
> This commit gathers minor cleanups in the current driver.
>
> In the device examples, "Xiaomi" is too generic, specific devices should be
> listed here. E.g. Xiaomi Redmi 2 seems to have YAS537 but it's not fully clear
> if this applies to all its variants. Samsung Galaxy S7 is often quoted in
> conjunction with YAS537.
>
> In the function yas530_get_calibration_data(), the debug dump was extended to
> 16 elements as this is the size of the calibration data array of YAS530.

We do not accept or even consider contributions w/o SoB tag, sorry.

P.S. Reading briefly, split this to two patches:
1) moving %*ph parameters out from stack;
2) documentation and indentation fixes.

Jakob Hauser June 21, 2022, 12:57 a.m. UTC | #2

Hi Andy,

On 18.06.22 11:53, Andy Shevchenko wrote:
>
> On Sat, Jun 18, 2022 at 2:15 AM Jakob Hauser <[e-mail]> wrote:
>>
>> This commit gathers minor cleanups in the current driver.
>>
>> In the device examples, "Xiaomi" is too generic, specific devices should be
>> listed here. E.g. Xiaomi Redmi 2 seems to have YAS537 but it's not fully clear
>> if this applies to all its variants. Samsung Galaxy S7 is often quoted in
>> conjunction with YAS537.
>>
>> In the function yas530_get_calibration_data(), the debug dump was extended to
>> 16 elements as this is the size of the calibration data array of YAS530.
> 
> We do not accept or even consider contributions w/o SoB tag, sorry.

I'm not used to the routines yet, sorry...

> P.S. Reading briefly, split this to two patches:
> 1) moving %*ph parameters out from stack;
> 2) documentation and indentation fixes.

There is also the change on memchr_inv(). Additionally on that line, I
would also add the "if (a && b)" that you suggested in patch 8.

So I'd split this patch into two, as recommended by you, but the second
one would be "documentation and style fixes" including the changes on
memchr_inv().

Kind regards,
Jakob

Patch

diff --git a/drivers/iio/magnetometer/yamaha-yas530.c b/drivers/iio/magnetometer/yamaha-yas530.c
index 98c8d365fab7..72a75dc57e11 100644
--- a/drivers/iio/magnetometer/yamaha-yas530.c
+++ b/drivers/iio/magnetometer/yamaha-yas530.c
@@ -10,7 +10,7 @@ 
  * (YAS534 is a magnetic switch, not handled)
  * YAS535 MS-6C
  * YAS536 MS-3W
- * YAS537 MS-3T (2015 Samsung Galaxy S6, Note 5, Xiaomi)
+ * YAS537 MS-3T (2015 Samsung Galaxy S6, Note 5, Galaxy S7)
  * YAS539 MS-3S (2018 Samsung Galaxy A7 SM-A750FN)
  *
  * Code functions found in the MPU3050 YAS530 and YAS532 drivers
@@ -322,7 +322,7 @@  static int yas530_532_get_measure(struct yas5xx *yas5xx, s32 *to,
 {
 	struct yas5xx_calibration *c = &yas5xx->calibration;
 	u16 t_ref, t, x, y1, y2;
-	/* These are "signed x, signed y1 etc */
+	/* These are signed x, signed y1 etc */
 	s32 sx, sy1, sy2, sy, sz;
 	int ret;
 
@@ -656,9 +656,12 @@  static int yas530_get_calibration_data(struct yas5xx *yas5xx)
 	ret = regmap_bulk_read(yas5xx->map, YAS530_532_CAL, data, sizeof(data));
 	if (ret)
 		return ret;
-	dev_dbg(yas5xx->dev, "calibration data: %*ph\n", 14, data);
+	dev_dbg(yas5xx->dev, "calibration data: %16ph\n", data);
 
+	/* Contribute calibration data to the input pool for kernel entropy */
 	add_device_randomness(data, sizeof(data));
+
+	/* Extract version */
 	yas5xx->version = data[15] & GENMASK(1, 0);
 
 	/* Extract the calibration from the bitfield */
@@ -685,6 +688,7 @@  static int yas530_get_calibration_data(struct yas5xx *yas5xx)
 	c->r[0] = sign_extend32(FIELD_GET(GENMASK(28, 23), val), 5);
 	c->r[1] = sign_extend32(FIELD_GET(GENMASK(20, 15), val), 5);
 	c->r[2] = sign_extend32(FIELD_GET(GENMASK(12, 7), val), 5);
+
 	return 0;
 }
 
@@ -703,15 +707,17 @@  static int yas532_get_calibration_data(struct yas5xx *yas5xx)
 	ret = regmap_bulk_read(yas5xx->map, YAS530_532_CAL, data, sizeof(data));
 	if (ret)
 		return ret;
-	dev_dbg(yas5xx->dev, "calibration data: %*ph\n", 14, data);
+	dev_dbg(yas5xx->dev, "calibration data: %14ph\n", data);
 
 	/* Sanity check, is this all zeroes? */
-	if (memchr_inv(data, 0x00, 13) == NULL) {
+	if (!memchr_inv(data, 0x00, 13)) {
 		if (!(data[13] & BIT(7)))
 			dev_warn(yas5xx->dev, "calibration is blank!\n");
 	}
 
+	/* Contribute calibration data to the input pool for kernel entropy */
 	add_device_randomness(data, sizeof(data));
+
 	/* Only one bit of version info reserved here as far as we know */
 	yas5xx->version = data[13] & BIT(0);
 
@@ -720,6 +726,7 @@  static int yas532_get_calibration_data(struct yas5xx *yas5xx)
 	c->Cy1 = data[1] * 10 - 1280;
 	c->Cy2 = data[2] * 10 - 1280;
 	yas530_532_extract_calibration(&data[3], c);
+
 	/*
 	 * Extract linearization:
 	 * Linearization layout in the 32 bits at byte 10: