All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] Add support for magnetometer Yamaha YAS537
       [not found] <cover.1654727058.git.jahau.ref@rocketmail.com>
@ 2022-06-08 23:36 ` Jakob Hauser
  2022-06-08 23:38   ` [PATCH 1/7] iio: magnetometer: yas530: Change range of data in volatile register Jakob Hauser
                     ` (6 more replies)
  0 siblings, 7 replies; 23+ messages in thread
From: Jakob Hauser @ 2022-06-08 23:36 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lars-Peter Clausen, Linus Walleij, linux-iio, phone-devel,
	~postmarketos/upstreaming, Jakob Hauser

This patchset adds YAS537 variant to the already existing driver for
Yamaha YAS magnetometers.

Patches 1-4 applies corrections that popped up when working on this.
Patches 5-6 are preparations for adding YAS537.
Patch 7 finally adds the YAS537 variant.

Jakob Hauser (7):
  iio: magnetometer: yas530: Change range of data in volatile register
  iio: magnetometer: yas530: Correct scaling of magnetic axes
  iio: magnetometer: yas530: Correct temperature handling
  iio: magnetometer: yas530: Change data type of hard_offsets to signed
  iio: magnetometer: yas530: Change data type of calibration
    coefficients
  iio: magnetometer: yas530: Remove redundant defaults on switch devid
  iio: magnetometer: yas530: Add YAS537 variant

 drivers/iio/magnetometer/Kconfig         |   4 +-
 drivers/iio/magnetometer/yamaha-yas530.c | 814 +++++++++++++++++++----
 2 files changed, 698 insertions(+), 120 deletions(-)

-- 
2.35.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 1/7] iio: magnetometer: yas530: Change range of data in volatile register
  2022-06-08 23:36 ` [PATCH 0/7] Add support for magnetometer Yamaha YAS537 Jakob Hauser
@ 2022-06-08 23:38   ` Jakob Hauser
  2022-06-09  8:50     ` Linus Walleij
  2022-06-08 23:38   ` [PATCH 2/7] iio: magnetometer: yas530: Correct scaling of magnetic axes Jakob Hauser
                     ` (5 subsequent siblings)
  6 siblings, 1 reply; 23+ messages in thread
From: Jakob Hauser @ 2022-06-08 23:38 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lars-Peter Clausen, Linus Walleij, linux-iio, phone-devel,
	~postmarketos/upstreaming, Jakob Hauser

In function yas5xx_volatile_reg(), the range for measure data should end at
"YAS5XX_MEASURE_DATA + 7" instead of "+ 8" as we count from 0 to 7 here.

This change is of low importance as the "+ 8" register isn't called.

Cc: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
---
 drivers/iio/magnetometer/yamaha-yas530.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/magnetometer/yamaha-yas530.c b/drivers/iio/magnetometer/yamaha-yas530.c
index 9ff7b0e56cf6..425281401324 100644
--- a/drivers/iio/magnetometer/yamaha-yas530.c
+++ b/drivers/iio/magnetometer/yamaha-yas530.c
@@ -527,7 +527,7 @@ static bool yas5xx_volatile_reg(struct device *dev, unsigned int reg)
 {
 	return reg == YAS5XX_ACTUATE_INIT_COIL ||
 		reg == YAS5XX_MEASURE ||
-		(reg >= YAS5XX_MEASURE_DATA && reg <= YAS5XX_MEASURE_DATA + 8);
+		(reg >= YAS5XX_MEASURE_DATA && reg <= YAS5XX_MEASURE_DATA + 7);
 }
 
 /* TODO: enable regmap cache, using mark dirty and sync at runtime resume */
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 2/7] iio: magnetometer: yas530: Correct scaling of magnetic axes
  2022-06-08 23:36 ` [PATCH 0/7] Add support for magnetometer Yamaha YAS537 Jakob Hauser
  2022-06-08 23:38   ` [PATCH 1/7] iio: magnetometer: yas530: Change range of data in volatile register Jakob Hauser
@ 2022-06-08 23:38   ` Jakob Hauser
  2022-06-09  8:52     ` Linus Walleij
  2022-06-08 23:38   ` [PATCH 3/7] iio: magnetometer: yas530: Correct temperature handling Jakob Hauser
                     ` (4 subsequent siblings)
  6 siblings, 1 reply; 23+ messages in thread
From: Jakob Hauser @ 2022-06-08 23:38 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lars-Peter Clausen, Linus Walleij, linux-iio, phone-devel,
	~postmarketos/upstreaming, Jakob Hauser

Looks like YAS530 raw values return picotesla and YAS532 nanotesla. Adapt
comments and scaling.

Cc: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
---
 drivers/iio/magnetometer/yamaha-yas530.c | 28 +++++++++++++++---------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/magnetometer/yamaha-yas530.c b/drivers/iio/magnetometer/yamaha-yas530.c
index 425281401324..5c7d0ac533ac 100644
--- a/drivers/iio/magnetometer/yamaha-yas530.c
+++ b/drivers/iio/magnetometer/yamaha-yas530.c
@@ -310,8 +310,6 @@ static s32 yas5xx_linearize(struct yas5xx *yas5xx, u16 val, int axis)
  * @yo: Y axis out
  * @zo: Z axis out
  * @return: 0 on success or error code
- *
- * Returned values are in nanotesla according to some code.
  */
 static int yas5xx_get_measure(struct yas5xx *yas5xx, s32 *to, s32 *xo, s32 *yo, s32 *zo)
 {
@@ -417,14 +415,24 @@ static int yas5xx_read_raw(struct iio_dev *indio_dev,
 			*val = 1;
 			return IIO_VAL_INT;
 		}
-		/*
-		 * The axis values are in nanotesla according to the vendor
-		 * drivers, but is clearly in microtesla according to
-		 * experiments. Since 1 uT = 0.01 Gauss, we need to divide
-		 * by 100000000 (10^8) to get to Gauss from the raw value.
-		 */
-		*val = 1;
-		*val2 = 100000000;
+		switch (yas5xx->devid) {
+		case YAS530_DEVICE_ID:
+			/*
+			 * Raw values of YAS530 are in picotesla. Devide by
+			 * 100000000 (10^8) to get Gauss.
+			 */
+			*val = 1;
+			*val2 = 100000000;
+			break;
+		case YAS532_DEVICE_ID:
+			/*
+			 * Raw values of YAS532 are in nanotesla. Devide by
+			 * 100000 (10^5) to get Gauss.
+			 */
+			*val = 1;
+			*val2 = 100000;
+			break;
+		}
 		return IIO_VAL_FRACTIONAL;
 	default:
 		/* Unknown request */
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 3/7] iio: magnetometer: yas530: Correct temperature handling
  2022-06-08 23:36 ` [PATCH 0/7] Add support for magnetometer Yamaha YAS537 Jakob Hauser
  2022-06-08 23:38   ` [PATCH 1/7] iio: magnetometer: yas530: Change range of data in volatile register Jakob Hauser
  2022-06-08 23:38   ` [PATCH 2/7] iio: magnetometer: yas530: Correct scaling of magnetic axes Jakob Hauser
@ 2022-06-08 23:38   ` Jakob Hauser
  2022-06-09  8:53     ` Linus Walleij
  2022-06-08 23:38   ` [PATCH 4/7] iio: magnetometer: yas530: Change data type of hard_offsets to signed Jakob Hauser
                     ` (3 subsequent siblings)
  6 siblings, 1 reply; 23+ messages in thread
From: Jakob Hauser @ 2022-06-08 23:38 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lars-Peter Clausen, Linus Walleij, linux-iio, phone-devel,
	~postmarketos/upstreaming, Jakob Hauser

The raw temperature value is a number of counts from a certain starting
point. The resolution of the temperature counts is different for the YAS
variants.

Temperature compensation for YAS532 version AC seems to be handled differently.
It uses the deviation from 20 degree Celsius [1] whereas YAS530 and older
versions of YAS532 apply solely the t value as a multiplier [2][3].

In funtion yas5xx_read_raw(), add case IIO_CHAN_INFO_PROCESSED. Remove scale
of temperature as this isn't applied.

Additionally correct sign of temperature channel in iio_chan_spec. It's already
defined that way in yas5xx_get_measure() function.

[1] https://github.com/msm8916-mainline/android_kernel_qcom_msm8916/blob/GT-I9195I/drivers/iio/magnetometer/yas_mag_drv-yas532.c#L442
[2] https://github.com/NovaFusion/android_kernel_samsung_golden/blob/cm-12.1/drivers/sensor/compass/yas_mag_driver-yas530.c#L881-L883
[3] https://github.com/LineageOS/android_kernel_samsung_msm8930-common/blob/lineage-18.1/drivers/sensors/geomagnetic/yas_mag_driver-yas53x.c#L856-L858

Cc: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
---
 drivers/iio/magnetometer/yamaha-yas530.c | 93 ++++++++++++++++++------
 1 file changed, 70 insertions(+), 23 deletions(-)

diff --git a/drivers/iio/magnetometer/yamaha-yas530.c b/drivers/iio/magnetometer/yamaha-yas530.c
index 5c7d0ac533ac..2e8d20b05217 100644
--- a/drivers/iio/magnetometer/yamaha-yas530.c
+++ b/drivers/iio/magnetometer/yamaha-yas530.c
@@ -77,6 +77,7 @@
 #define YAS530_DATA_BITS		12
 #define YAS530_DATA_CENTER		BIT(YAS530_DATA_BITS - 1)
 #define YAS530_DATA_OVERFLOW		(BIT(YAS530_DATA_BITS) - 1)
+#define YAS530_20DEGREES		182 /* Counts starting at -62 °C */
 
 #define YAS532_DEVICE_ID		0x02 /* YAS532/YAS533 (MS-3R/F) */
 #define YAS532_VERSION_AB		0 /* YAS532/533 AB (MS-3R/F AB) */
@@ -88,7 +89,7 @@
 #define YAS532_DATA_BITS		13
 #define YAS532_DATA_CENTER		BIT(YAS532_DATA_BITS - 1)
 #define YAS532_DATA_OVERFLOW		(BIT(YAS532_DATA_BITS) - 1)
-#define YAS532_20DEGREES		390 /* Looks like Kelvin */
+#define YAS532_20DEGREES		390 /* Counts starting at -50 °C */
 
 /* These variant IDs are known from code dumps */
 #define YAS537_DEVICE_ID		0x07 /* YAS537 (MS-3T) */
@@ -314,7 +315,7 @@ static s32 yas5xx_linearize(struct yas5xx *yas5xx, u16 val, int axis)
 static int yas5xx_get_measure(struct yas5xx *yas5xx, s32 *to, s32 *xo, s32 *yo, s32 *zo)
 {
 	struct yas5xx_calibration *c = &yas5xx->calibration;
-	u16 t, x, y1, y2;
+	u16 t_ref, t, x, y1, y2;
 	/* These are "signed x, signed y1 etc */
 	s32 sx, sy1, sy2, sy, sz;
 	int ret;
@@ -329,16 +330,43 @@ static int yas5xx_get_measure(struct yas5xx *yas5xx, s32 *to, s32 *xo, s32 *yo,
 	sy1 = yas5xx_linearize(yas5xx, y1, 1);
 	sy2 = yas5xx_linearize(yas5xx, y2, 2);
 
-	/*
-	 * Temperature compensation for x, y1, y2 respectively:
-	 *
-	 *          Cx * t
-	 * x' = x - ------
-	 *           100
-	 */
-	sx = sx - (c->Cx * t) / 100;
-	sy1 = sy1 - (c->Cy1 * t) / 100;
-	sy2 = sy2 - (c->Cy2 * t) / 100;
+	/* Set the temperature reference value (unit: counts) */
+	switch (yas5xx->devid) {
+	case YAS530_DEVICE_ID:
+		t_ref = YAS530_20DEGREES;
+		break;
+	case YAS532_DEVICE_ID:
+		t_ref = YAS532_20DEGREES;
+		break;
+	}
+
+	/* Temperature compensation for x, y1, y2 respectively */
+	if (yas5xx->devid == YAS532_DEVICE_ID &&
+	    yas5xx->version == YAS532_VERSION_AC) {
+		/*
+		 * YAS532 version AC uses the temperature deviation as a
+		 * multiplier.
+		 *
+		 *          Cx * (t - t_ref)
+		 * x' = x - ----------------
+		 *                100
+		 */
+		sx = sx - (c->Cx * (t - t_ref)) / 100;
+		sy1 = sy1 - (c->Cy1 * (t - t_ref)) / 100;
+		sy2 = sy2 - (c->Cy2 * (t - t_ref)) / 100;
+	} else {
+		/*
+		 * YAS530 and YAS532 version AB use solely the t value as a
+		 * multiplier.
+		 *
+		 *          Cx * t
+		 * x' = x - ------
+		 *           100
+		 */
+		sx = sx - (c->Cx * t) / 100;
+		sy1 = sy1 - (c->Cy1 * t) / 100;
+		sy2 = sy2 - (c->Cy2 * t) / 100;
+	}
 
 	/*
 	 * Break y1 and y2 into y and z, y1 and y2 are apparently encoding
@@ -347,11 +375,34 @@ static int yas5xx_get_measure(struct yas5xx *yas5xx, s32 *to, s32 *xo, s32 *yo,
 	sy = sy1 - sy2;
 	sz = -sy1 - sy2;
 
-	/*
-	 * FIXME: convert to Celsius? Just guessing this is given
-	 * as 1/10:s of degrees so multiply by 100 to get millicentigrades.
-	 */
-	*to = t * 100;
+	/* Process temperature readout */
+	switch (yas5xx->devid) {
+	case YAS530_DEVICE_ID:
+		/*
+		 * Raw temperature value t is number of counts starting at
+		 * -62 °C. Reference value t_ref is the number of counts
+		 * between -62 °C and 20 °C (82 °C range).
+		 *
+		 * Temperature in °C would be (82 / t_ref * t) - 62.
+		 *
+		 * Contrary to this, perform multiplication first and division
+		 * second due to calculating with integers.
+		 *
+		 * To get a nicer result, calculate with 1/10:s degrees Celsius
+		 * and finally multiply by 100 to return milli degrees Celsius.
+		 */
+		*to = ((820 * t / t_ref) - 620) * 100;
+		break;
+	case YAS532_DEVICE_ID:
+		/*
+		 * Actually same procedure for YAS532 but the starting point is
+		 * at -50 °C. Reference value t_ref is the number of counts
+		 * between -50 °C and 20 °C (70 °C range).
+		 */
+		*to = ((700 * t / t_ref) - 500) * 100;
+		break;
+	}
+
 	/*
 	 * Calibrate [x,y,z] with some formulas like this:
 	 *
@@ -384,6 +435,7 @@ static int yas5xx_read_raw(struct iio_dev *indio_dev,
 	int ret;
 
 	switch (mask) {
+	case IIO_CHAN_INFO_PROCESSED:
 	case IIO_CHAN_INFO_RAW:
 		pm_runtime_get_sync(yas5xx->dev);
 		ret = yas5xx_get_measure(yas5xx, &t, &x, &y, &z);
@@ -410,11 +462,6 @@ static int yas5xx_read_raw(struct iio_dev *indio_dev,
 		}
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
-		if (chan->address == 0) {
-			/* Temperature is unscaled */
-			*val = 1;
-			return IIO_VAL_INT;
-		}
 		switch (yas5xx->devid) {
 		case YAS530_DEVICE_ID:
 			/*
@@ -513,7 +560,7 @@ static const struct iio_chan_spec yas5xx_channels[] = {
 		.address = 0,
 		.scan_index = 0,
 		.scan_type = {
-			.sign = 'u',
+			.sign = 's',
 			.realbits = 32,
 			.storagebits = 32,
 			.endianness = IIO_CPU,
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 4/7] iio: magnetometer: yas530: Change data type of hard_offsets to signed
  2022-06-08 23:36 ` [PATCH 0/7] Add support for magnetometer Yamaha YAS537 Jakob Hauser
                     ` (2 preceding siblings ...)
  2022-06-08 23:38   ` [PATCH 3/7] iio: magnetometer: yas530: Correct temperature handling Jakob Hauser
@ 2022-06-08 23:38   ` Jakob Hauser
  2022-06-09  8:54     ` Linus Walleij
  2022-06-10 14:22     ` Andy Shevchenko
  2022-06-08 23:38   ` [PATCH 5/7] iio: magnetometer: yas530: Change data type of calibration coefficients Jakob Hauser
                     ` (2 subsequent siblings)
  6 siblings, 2 replies; 23+ messages in thread
From: Jakob Hauser @ 2022-06-08 23:38 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lars-Peter Clausen, Linus Walleij, linux-iio, phone-devel,
	~postmarketos/upstreaming, Jakob Hauser

The "hard_offsets" are currently unsigned u8 but they should be signed as they
can get negative. They are signed in function yas5xx_meaure_offsets() and in the
Yamaha drivers [1][2].

[1] https://github.com/NovaFusion/android_kernel_samsung_golden/blob/cm-12.1/drivers/sensor/compass/yas.h#L156
[2] https://github.com/msm8916-mainline/android_kernel_qcom_msm8916/blob/GT-I9195I/drivers/iio/magnetometer/yas_mag_drv-yas532.c#L91

Fixes: de8860b1ed47 ("iio: magnetometer: Add driver for Yamaha YAS530")
Cc: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
---
 drivers/iio/magnetometer/yamaha-yas530.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/magnetometer/yamaha-yas530.c b/drivers/iio/magnetometer/yamaha-yas530.c
index 2e8d20b05217..9bfb3b573907 100644
--- a/drivers/iio/magnetometer/yamaha-yas530.c
+++ b/drivers/iio/magnetometer/yamaha-yas530.c
@@ -133,7 +133,7 @@ struct yas5xx {
 	unsigned int version;
 	char name[16];
 	struct yas5xx_calibration calibration;
-	u8 hard_offsets[3];
+	s8 hard_offsets[3];
 	struct iio_mount_matrix orientation;
 	struct regmap *map;
 	struct regulator_bulk_data regs[2];
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 5/7] iio: magnetometer: yas530: Change data type of calibration coefficients
  2022-06-08 23:36 ` [PATCH 0/7] Add support for magnetometer Yamaha YAS537 Jakob Hauser
                     ` (3 preceding siblings ...)
  2022-06-08 23:38   ` [PATCH 4/7] iio: magnetometer: yas530: Change data type of hard_offsets to signed Jakob Hauser
@ 2022-06-08 23:38   ` Jakob Hauser
  2022-06-09  8:55     ` Linus Walleij
  2022-06-08 23:38   ` [PATCH 6/7] iio: magnetometer: yas530: Remove redundant defaults on switch devid Jakob Hauser
  2022-06-08 23:38   ` [PATCH 7/7] iio: magnetometer: yas530: Add YAS537 variant Jakob Hauser
  6 siblings, 1 reply; 23+ messages in thread
From: Jakob Hauser @ 2022-06-08 23:38 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lars-Peter Clausen, Linus Walleij, linux-iio, phone-devel,
	~postmarketos/upstreaming, Jakob Hauser

This is a preparation for adding YAS537 variant.

YAS537 uses other data types on the calibration coefficients [1] than YAS530 [2]
and YAS532 [3].

On YAS537, at least for a4 and a7 this could matter because 8-bit unsigned data
from the register gets stored into a signed data type, therefore this should be
8-bit as well.

For YAS530/532, on the other hand, it doesn't seem to matter. The size of a2-a9
and k is smaller than 8-bit at extraction, also the applied math is low. And
Cx/Cy1/Cy2, now being defined as signed 16-bit, are extracted as unsigned 8-bit
and undergo only minor math.

[1] https://github.com/msm8916-mainline/android_kernel_qcom_msm8916/blob/GT-I9195I/drivers/iio/magnetometer/yas_mag_drv-yas537.c#L76-L78
[2] https://github.com/NovaFusion/android_kernel_samsung_golden/blob/cm-12.1/drivers/sensor/compass/yas_mag_driver-yas530.c#L526-L527
[3] https://github.com/msm8916-mainline/android_kernel_qcom_msm8916/blob/GT-I9195I/drivers/iio/magnetometer/yas_mag_drv-yas532.c#L76-L77

Cc: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
---
 drivers/iio/magnetometer/yamaha-yas530.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/magnetometer/yamaha-yas530.c b/drivers/iio/magnetometer/yamaha-yas530.c
index 9bfb3b573907..31637a890b7f 100644
--- a/drivers/iio/magnetometer/yamaha-yas530.c
+++ b/drivers/iio/magnetometer/yamaha-yas530.c
@@ -103,9 +103,11 @@ struct yas5xx_calibration {
 	s32 r[3];
 	u32 f[3];
 	/* Temperature compensation calibration */
-	s32 Cx, Cy1, Cy2;
+	s16 Cx, Cy1, Cy2;
 	/* Misc calibration coefficients */
-	s32 a2, a3, a4, a5, a6, a7, a8, a9, k;
+	s8  a2, a3, a4, a6, a7, a8;
+	s16 a5, a9;
+	u8  k;
 	/* clock divider */
 	u8 dck;
 };
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 6/7] iio: magnetometer: yas530: Remove redundant defaults on switch devid
  2022-06-08 23:36 ` [PATCH 0/7] Add support for magnetometer Yamaha YAS537 Jakob Hauser
                     ` (4 preceding siblings ...)
  2022-06-08 23:38   ` [PATCH 5/7] iio: magnetometer: yas530: Change data type of calibration coefficients Jakob Hauser
@ 2022-06-08 23:38   ` Jakob Hauser
  2022-06-09  8:55     ` Linus Walleij
  2022-06-10 14:24     ` Andy Shevchenko
  2022-06-08 23:38   ` [PATCH 7/7] iio: magnetometer: yas530: Add YAS537 variant Jakob Hauser
  6 siblings, 2 replies; 23+ messages in thread
From: Jakob Hauser @ 2022-06-08 23:38 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lars-Peter Clausen, Linus Walleij, linux-iio, phone-devel,
	~postmarketos/upstreaming, Jakob Hauser

This is a preparation for adding YAS537 variant.

In function yas5xx_probe(), there is a switch statement checking for device
IDs. If the ID is unknown, it exits with a device error. In later functions,
it's not neccessary to check the validity of the device IDs again.

When adding YAS537 in a later patch, several of such switch statements will be
added. To make it more uniform, the redundant ones in YAS530/532 get herby
removed. This is done in a separate patch for better history control.

Cc: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
---
 drivers/iio/magnetometer/yamaha-yas530.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/iio/magnetometer/yamaha-yas530.c b/drivers/iio/magnetometer/yamaha-yas530.c
index 31637a890b7f..59844e1b794c 100644
--- a/drivers/iio/magnetometer/yamaha-yas530.c
+++ b/drivers/iio/magnetometer/yamaha-yas530.c
@@ -251,13 +251,9 @@ static int yas5xx_measure(struct yas5xx *yas5xx, u16 *t, u16 *x, u16 *y1, u16 *y
 		*y1 = yas532_extract_axis(&data[4]);
 		*y2 = yas532_extract_axis(&data[6]);
 		break;
-	default:
-		dev_err(yas5xx->dev, "unknown data format\n");
-		ret = -EINVAL;
-		break;
 	}
 
-	return ret;
+	return 0;
 
 out_unlock:
 	mutex_unlock(&yas5xx->lock);
@@ -289,9 +285,6 @@ static s32 yas5xx_linearize(struct yas5xx *yas5xx, u16 val, int axis)
 			/* Elaborate coefficients */
 			coef = yas532ac_coef[axis];
 		break;
-	default:
-		dev_err(yas5xx->dev, "unknown device type\n");
-		return val;
 	}
 	/*
 	 * Linearization formula:
@@ -798,9 +791,6 @@ static int yas5xx_meaure_offsets(struct yas5xx *yas5xx)
 	case YAS532_DEVICE_ID:
 		center = YAS532_DATA_CENTER;
 		break;
-	default:
-		dev_err(yas5xx->dev, "unknown device type\n");
-		return -EINVAL;
 	}
 
 	/*
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 7/7] iio: magnetometer: yas530: Add YAS537 variant
  2022-06-08 23:36 ` [PATCH 0/7] Add support for magnetometer Yamaha YAS537 Jakob Hauser
                     ` (5 preceding siblings ...)
  2022-06-08 23:38   ` [PATCH 6/7] iio: magnetometer: yas530: Remove redundant defaults on switch devid Jakob Hauser
@ 2022-06-08 23:38   ` Jakob Hauser
  2022-06-09  8:56     ` Linus Walleij
  2022-06-10 14:31     ` Andy Shevchenko
  6 siblings, 2 replies; 23+ messages in thread
From: Jakob Hauser @ 2022-06-08 23:38 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lars-Peter Clausen, Linus Walleij, linux-iio, phone-devel,
	~postmarketos/upstreaming, Jakob Hauser

This adds support for the magnetometer Yamaha YAS537. The additions are based
on comparison of Yamaha Android kernel drivers for YAS532 [1] and YAS537 [2].

Functions used by YAS530 & YAS532 only were renamed from yas5xx to yas530_532.
Registers were renamed accordingly.

In the Yamaha YAS537 Android driver, there is an overflow/underflow control
implemented. For regular usage, this seems not necessary. A similar overflow/
underflow control of Yamaha YAS530/532 Android driver isn't integrated in the
mainline driver. It is therefore skipped for YAS537 in mainline too.

Also in the Yamaha YAS537 Android driver, at the end of the reset_yas537()
function, a measurement is saved in "last_after_rcoil". Later on, this is
compared to current measurements. If the difference gets too big, a new
reset is intialized. The difference in measurements needs to be quite big,
it's hard to say if this is necessary for regular operation. Therefore this
isn't integrated in the mainline driver either.

[1] https://github.com/msm8916-mainline/android_kernel_qcom_msm8916/blob/GT-I9195I/drivers/iio/magnetometer/yas_mag_drv-yas532.c
[2] https://github.com/msm8916-mainline/android_kernel_qcom_msm8916/blob/GT-I9195I/drivers/iio/magnetometer/yas_mag_drv-yas537.c

Cc: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
---
 drivers/iio/magnetometer/Kconfig         |   4 +-
 drivers/iio/magnetometer/yamaha-yas530.c | 683 ++++++++++++++++++++---
 2 files changed, 609 insertions(+), 78 deletions(-)

diff --git a/drivers/iio/magnetometer/Kconfig b/drivers/iio/magnetometer/Kconfig
index 07eb619bcfe8..868128cee835 100644
--- a/drivers/iio/magnetometer/Kconfig
+++ b/drivers/iio/magnetometer/Kconfig
@@ -216,8 +216,8 @@ config YAMAHA_YAS530
 	select IIO_TRIGGERED_BUFFER
 	help
 	  Say Y here to add support for the Yamaha YAS530 series of
-	  3-Axis Magnetometers. Right now YAS530, YAS532 and YAS533 are
-	  fully supported.
+	  3-Axis Magnetometers. Right now YAS530, YAS532, YAS533 and
+	  YAS537 are fully supported.
 
 	  This driver can also be compiled as a module.
 	  To compile this driver as a module, choose M here: the module
diff --git a/drivers/iio/magnetometer/yamaha-yas530.c b/drivers/iio/magnetometer/yamaha-yas530.c
index 59844e1b794c..ca755b72f7c4 100644
--- a/drivers/iio/magnetometer/yamaha-yas530.c
+++ b/drivers/iio/magnetometer/yamaha-yas530.c
@@ -10,13 +10,16 @@
  * (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
  * named "inv_compass" in the Tegra Android kernel tree.
  * Copyright (C) 2012 InvenSense Corporation
  *
+ * Code functions for YAS537 based on Yamaha Android kernel driver.
+ * Copyright (c) 2014 Yamaha Corporation
+ *
  * Author: Linus Walleij <linus.walleij@linaro.org>
  */
 #include <linux/bitfield.h>
@@ -40,20 +43,39 @@
 
 #include <asm/unaligned.h>
 
-/* This register map covers YAS530 and YAS532 but differs in YAS 537 and YAS539 */
+/* Commonly used registers */
 #define YAS5XX_DEVICE_ID		0x80
-#define YAS5XX_ACTUATE_INIT_COIL	0x81
-#define YAS5XX_MEASURE			0x82
-#define YAS5XX_CONFIG			0x83
-#define YAS5XX_MEASURE_INTERVAL		0x84
-#define YAS5XX_OFFSET_X			0x85 /* [-31 .. 31] */
-#define YAS5XX_OFFSET_Y1		0x86 /* [-31 .. 31] */
-#define YAS5XX_OFFSET_Y2		0x87 /* [-31 .. 31] */
-#define YAS5XX_TEST1			0x88
-#define YAS5XX_TEST2			0x89
-#define YAS5XX_CAL			0x90
 #define YAS5XX_MEASURE_DATA		0xB0
 
+/* Registers used by YAS530 and YAS532 */
+#define YAS530_532_ACTUATE_INIT_COIL	0x81
+#define YAS530_532_MEASURE		0x82
+#define YAS530_532_CONFIG		0x83
+#define YAS530_532_MEASURE_INTERVAL	0x84
+#define YAS530_532_OFFSET_X		0x85 /* [-31 .. 31] */
+#define YAS530_532_OFFSET_Y1		0x86 /* [-31 .. 31] */
+#define YAS530_532_OFFSET_Y2		0x87 /* [-31 .. 31] */
+#define YAS530_532_TEST1		0x88
+#define YAS530_532_TEST2		0x89
+#define YAS530_532_CAL			0x90
+
+/* Registers used by YAS537 */
+#define YAS537_MEASURE			0x81 /* Originally YAS537_REG_CMDR */
+#define YAS537_CONFIG			0x82 /* Originally YAS537_REG_CONFR */
+#define YAS537_MEASURE_INTERVAL		0x83 /* Originally YAS537_REG_INTRVLR */
+#define YAS537_OFFSET_X			0x84 /* Originally YAS537_REG_OXR */
+#define YAS537_OFFSET_Y1		0x85 /* Originally YAS537_REG_OY1R */
+#define YAS537_OFFSET_Y2		0x86 /* Originally YAS537_REG_OY2R */
+#define YAS537_AVR			0x87
+#define YAS537_HCK			0x88
+#define YAS537_LCK			0x89
+#define YAS537_SRST			0x90
+#define YAS537_ADCCAL			0x91
+#define YAS537_MTC			0x93
+#define YAS537_OC			0x9E
+#define YAS537_TRM			0x9F
+#define YAS537_CAL			0xC0
+
 /* Bits in the YAS5xx config register */
 #define YAS5XX_CONFIG_INTON		BIT(0) /* Interrupt on? */
 #define YAS5XX_CONFIG_INTHACT		BIT(1) /* Interrupt active high? */
@@ -65,6 +87,7 @@
 #define YAS5XX_MEASURE_LDTC		BIT(1)
 #define YAS5XX_MEASURE_FORS		BIT(2)
 #define YAS5XX_MEASURE_DLYMES		BIT(4)
+#define YAS5XX_MEASURE_CONT		BIT(5)
 
 /* Bits in the measure data register */
 #define YAS5XX_MEASURE_DATA_BUSY	BIT(7)
@@ -91,8 +114,15 @@
 #define YAS532_DATA_OVERFLOW		(BIT(YAS532_DATA_BITS) - 1)
 #define YAS532_20DEGREES		390 /* Counts starting at -50 °C */
 
-/* These variant IDs are known from code dumps */
 #define YAS537_DEVICE_ID		0x07 /* YAS537 (MS-3T) */
+#define YAS537_VERSION_0		0 /* Version naming unknown */
+#define YAS537_VERSION_1		1 /* Version naming unknown */
+#define YAS537_MAG_AVERAGE_32_MASK	GENMASK(6, 4) /* corresponds to 0x70 */
+#define YAS537_MEASURE_TIME_WORST	1500 /* us */
+#define YAS537_DEFAULT_SENSOR_DELAY	50   /* ms */
+#define YAS537_MAG_RCOIL_TIME		65   /* us */
+#define YAS537_20DEGREES		8120 /* Counts starting at -386 °C */
+
 #define YAS539_DEVICE_ID		0x08 /* YAS539 (MS-3S) */
 
 /* Turn off device regulators etc after 5 seconds of inactivity */
@@ -182,7 +212,7 @@ static u16 yas532_extract_axis(u8 *data)
 }
 
 /**
- * yas5xx_measure() - Make a measure from the hardware
+ * yas530_532_measure() - Make a measure from the hardware
  * @yas5xx: The device state
  * @t: the raw temperature measurement
  * @x: the raw x axis measurement
@@ -190,7 +220,8 @@ static u16 yas532_extract_axis(u8 *data)
  * @y2: the y2 axis measurement
  * @return: 0 on success or error code
  */
-static int yas5xx_measure(struct yas5xx *yas5xx, u16 *t, u16 *x, u16 *y1, u16 *y2)
+static int yas530_532_measure(struct yas5xx *yas5xx, u16 *t,
+			      u16 *x, u16 *y1, u16 *y2)
 {
 	unsigned int busy;
 	u8 data[8];
@@ -198,7 +229,7 @@ static int yas5xx_measure(struct yas5xx *yas5xx, u16 *t, u16 *x, u16 *y1, u16 *y
 	u16 val;
 
 	mutex_lock(&yas5xx->lock);
-	ret = regmap_write(yas5xx->map, YAS5XX_MEASURE, YAS5XX_MEASURE_START);
+	ret = regmap_write(yas5xx->map, YAS530_532_MEASURE, YAS5XX_MEASURE_START);
 	if (ret < 0)
 		goto out_unlock;
 
@@ -260,7 +291,85 @@ static int yas5xx_measure(struct yas5xx *yas5xx, u16 *t, u16 *x, u16 *y1, u16 *y
 	return ret;
 }
 
-static s32 yas5xx_linearize(struct yas5xx *yas5xx, u16 val, int axis)
+/**
+ * yas537_measure() - Make a measure from the hardware
+ * @yas5xx: The device state
+ * @t: the raw temperature measurement
+ * @x: the raw x axis measurement
+ * @y1: the y1 axis measurement
+ * @y2: the y2 axis measurement
+ * @return: 0 on success or error code
+ */
+static int yas537_measure(struct yas5xx *yas5xx, u16 *t,
+			  u16 *x, u16 *y1, u16 *y2)
+{
+	struct yas5xx_calibration *c = &yas5xx->calibration;
+	unsigned int busy;
+	u8 data[8];
+	u16 xy1y2[3];
+	s32 h[3], s[3];
+	int i, ret;
+
+	mutex_lock(&yas5xx->lock);
+
+	/* Contrary to YAS530/532, also a "cont" bit is set, meaning unknown */
+	ret = regmap_write(yas5xx->map, YAS537_MEASURE, (YAS5XX_MEASURE_START |
+			   YAS5XX_MEASURE_CONT));
+	if (ret < 0)
+		goto out_unlock;
+
+	/* Use same timeout like YAS530/532 but the bit is in data row 2 */
+	ret = regmap_read_poll_timeout(yas5xx->map, YAS5XX_MEASURE_DATA + 2, busy,
+				       !(busy & YAS5XX_MEASURE_DATA_BUSY),
+				       500, 20000);
+	if (ret) {
+		dev_err(yas5xx->dev, "timeout waiting for measurement\n");
+		goto out_unlock;
+	}
+
+	/* Read data */
+	ret = regmap_bulk_read(yas5xx->map, YAS5XX_MEASURE_DATA,
+			       data, sizeof(data));
+	if (ret)
+		goto out_unlock;
+
+	mutex_unlock(&yas5xx->lock);
+
+	/* Arrange data */
+	*t = ((data[0] << 8) | data[1]);
+	xy1y2[0] = ((FIELD_GET(GENMASK(5, 0), data[2]) << 8) | data[3]);
+	xy1y2[1] = ((data[4] << 8) | data[5]);
+	xy1y2[2] = ((data[6] << 8) | data[7]);
+
+	/* The second version of YAS537 needs to include calibration coefficients */
+	if (yas5xx->version == YAS537_VERSION_1) {
+		for (i = 0; i < 3; i++)
+			s[i] = xy1y2[i] - 8192;
+		h[0] = (c->k *   (128 * s[0] + c->a2 * s[1] + c->a3 * s[2])) / 8192;
+		h[1] = (c->k * (c->a4 * s[0] + c->a5 * s[1] + c->a6 * s[2])) / 8192;
+		h[2] = (c->k * (c->a7 * s[0] + c->a8 * s[1] + c->a9 * s[2])) / 8192;
+		for (i = 0; i < 3; i++) {
+			if (h[i] < -8192)
+				h[i] = -8192;
+			if (h[i] > 8191)
+				h[i] = 8191;
+			xy1y2[i] = h[i] + 8192;
+		}
+	}
+
+	/* Assign data */
+	*x = xy1y2[0];
+	*y1 = xy1y2[1];
+	*y2 = xy1y2[2];
+
+	return 0;
+
+out_unlock:
+	mutex_unlock(&yas5xx->lock);
+	return ret;
+}
+
+static s32 yas530_532_linearize(struct yas5xx *yas5xx, u16 val, int axis)
 {
 	struct yas5xx_calibration *c = &yas5xx->calibration;
 	static const s32 yas532ac_coef[] = {
@@ -299,7 +408,7 @@ static s32 yas5xx_linearize(struct yas5xx *yas5xx, u16 val, int axis)
 }
 
 /**
- * yas5xx_get_measure() - Measure a sample of all axis and process
+ * yas530_532_get_measure() - Measure a sample of all axis and process
  * @yas5xx: The device state
  * @to: Temperature out
  * @xo: X axis out
@@ -307,23 +416,24 @@ static s32 yas5xx_linearize(struct yas5xx *yas5xx, u16 val, int axis)
  * @zo: Z axis out
  * @return: 0 on success or error code
  */
-static int yas5xx_get_measure(struct yas5xx *yas5xx, s32 *to, s32 *xo, s32 *yo, s32 *zo)
+static int yas530_532_get_measure(struct yas5xx *yas5xx, s32 *to,
+				  s32 *xo, s32 *yo, s32 *zo)
 {
 	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;
 
 	/* We first get raw data that needs to be translated to [x,y,z] */
-	ret = yas5xx_measure(yas5xx, &t, &x, &y1, &y2);
+	ret = yas530_532_measure(yas5xx, &t, &x, &y1, &y2);
 	if (ret)
 		return ret;
 
 	/* Do some linearization if available */
-	sx = yas5xx_linearize(yas5xx, x, 0);
-	sy1 = yas5xx_linearize(yas5xx, y1, 1);
-	sy2 = yas5xx_linearize(yas5xx, y2, 2);
+	sx = yas530_532_linearize(yas5xx, x, 0);
+	sy1 = yas530_532_linearize(yas5xx, y1, 1);
+	sy2 = yas530_532_linearize(yas5xx, y2, 2);
 
 	/* Set the temperature reference value (unit: counts) */
 	switch (yas5xx->devid) {
@@ -420,6 +530,57 @@ static int yas5xx_get_measure(struct yas5xx *yas5xx, s32 *to, s32 *xo, s32 *yo,
 	return 0;
 }
 
+/**
+ * yas537_get_measure() - Measure a sample of all axis and process
+ * @yas5xx: The device state
+ * @to: Temperature out
+ * @xo: X axis out
+ * @yo: Y axis out
+ * @zo: Z axis out
+ * @return: 0 on success or error code
+ */
+static int yas537_get_measure(struct yas5xx *yas5xx, s32 *to,
+			      s32 *xo, s32 *yo, s32 *zo)
+{
+	u16 t_ref, t, x, y1, y2;
+	int ret;
+
+	/* We first get raw data that needs to be translated to [x,y,z] */
+	ret = yas537_measure(yas5xx, &t, &x, &y1, &y2);
+	if (ret)
+		return ret;
+
+	/* Set the temperature reference value (unit: counts) */
+	t_ref = YAS537_20DEGREES;
+
+	/*
+	 * Raw temperature value t is number of counts. A product description
+	 * of YAS537 mentions a temperature resulution of 0.05 °C/count.
+	 * A readout of the t value at ca. 20 °C returns approx. 8120 counts.
+	 *
+	 * 8120 counts x 0.05 °C/count corresponds to a range of 406 °C.
+	 * 0 counts would be at theoretical -386 °C.
+	 *
+	 * The formula used for YAS530/532 needs to be adapted to this
+	 * theoretical starting temperature, again calculating with 1/10:s
+	 * of degrees Celsius and finally multiplying by 100 to get milli
+	 * degrees Celsius.
+	 */
+	*to = ((4060 * t / t_ref) - 3860) * 100;
+
+	/*
+	 * Unfortunately, no linearization or temperature compensation formulas
+	 * are known for YAS537.
+	 */
+
+	/* Calculate x, y, z from x, y1, y2 */
+	*xo = (x - 8192) * 300;
+	*yo = (y1 - y2) * 1732 / 10;
+	*zo = (-y1 - y2 + 16384) * 300;
+
+	return 0;
+}
+
 static int yas5xx_read_raw(struct iio_dev *indio_dev,
 			   struct iio_chan_spec const *chan,
 			   int *val, int *val2,
@@ -433,7 +594,15 @@ static int yas5xx_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_PROCESSED:
 	case IIO_CHAN_INFO_RAW:
 		pm_runtime_get_sync(yas5xx->dev);
-		ret = yas5xx_get_measure(yas5xx, &t, &x, &y, &z);
+		switch (yas5xx->devid) {
+		case YAS530_DEVICE_ID:
+		case YAS532_DEVICE_ID:
+			ret = yas530_532_get_measure(yas5xx, &t, &x, &y, &z);
+			break;
+		case YAS537_DEVICE_ID:
+			ret = yas537_get_measure(yas5xx, &t, &x, &y, &z);
+			break;
+		}
 		pm_runtime_mark_last_busy(yas5xx->dev);
 		pm_runtime_put_autosuspend(yas5xx->dev);
 		if (ret)
@@ -467,9 +636,10 @@ static int yas5xx_read_raw(struct iio_dev *indio_dev,
 			*val2 = 100000000;
 			break;
 		case YAS532_DEVICE_ID:
+		case YAS537_DEVICE_ID:
 			/*
-			 * Raw values of YAS532 are in nanotesla. Devide by
-			 * 100000 (10^5) to get Gauss.
+			 * Raw values of YAS532 and YAS537 are in nanotesla.
+			 * Devide by 100000 (10^5) to get Gauss.
 			 */
 			*val = 1;
 			*val2 = 100000;
@@ -489,7 +659,15 @@ static void yas5xx_fill_buffer(struct iio_dev *indio_dev)
 	int ret;
 
 	pm_runtime_get_sync(yas5xx->dev);
-	ret = yas5xx_get_measure(yas5xx, &t, &x, &y, &z);
+	switch (yas5xx->devid) {
+	case YAS530_DEVICE_ID:
+	case YAS532_DEVICE_ID:
+		ret = yas530_532_get_measure(yas5xx, &t, &x, &y, &z);
+		break;
+	case YAS537_DEVICE_ID:
+		ret = yas537_get_measure(yas5xx, &t, &x, &y, &z);
+		break;
+	}
 	pm_runtime_mark_last_busy(yas5xx->dev);
 	pm_runtime_put_autosuspend(yas5xx->dev);
 	if (ret) {
@@ -575,9 +753,30 @@ static const struct iio_info yas5xx_info = {
 
 static bool yas5xx_volatile_reg(struct device *dev, unsigned int reg)
 {
-	return reg == YAS5XX_ACTUATE_INIT_COIL ||
-		reg == YAS5XX_MEASURE ||
-		(reg >= YAS5XX_MEASURE_DATA && reg <= YAS5XX_MEASURE_DATA + 7);
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct yas5xx *yas5xx = iio_priv(indio_dev);
+
+	if (reg >= YAS5XX_MEASURE_DATA && reg <= YAS5XX_MEASURE_DATA + 7)
+		return true;
+
+	/*
+	 * YAS versions share different registers on the same address,
+	 * need to differentiate.
+	 */
+	switch (yas5xx->devid) {
+	case YAS530_DEVICE_ID:
+	case YAS532_DEVICE_ID:
+		if (reg == YAS530_532_ACTUATE_INIT_COIL ||
+		    reg == YAS530_532_MEASURE)
+			return true;
+		break;
+	case YAS537_DEVICE_ID:
+		if (reg == YAS537_MEASURE)
+			return true;
+		break;
+	}
+
+	return false;
 }
 
 /* TODO: enable regmap cache, using mark dirty and sync at runtime resume */
@@ -589,11 +788,11 @@ static const struct regmap_config yas5xx_regmap_config = {
 };
 
 /**
- * yas53x_extract_calibration() - extracts the a2-a9 and k calibration
+ * yas530_532_extract_calibration() - extracts the a2-a9 and k calibration
  * @data: the bitfield to use
  * @c: the calibration to populate
  */
-static void yas53x_extract_calibration(u8 *data, struct yas5xx_calibration *c)
+static void yas530_532_extract_calibration(u8 *data, struct yas5xx_calibration *c)
 {
 	u64 val = get_unaligned_be64(data);
 
@@ -631,24 +830,27 @@ static int yas530_get_calibration_data(struct yas5xx *yas5xx)
 	int ret;
 
 	/* Dummy read, first read is ALWAYS wrong */
-	ret = regmap_bulk_read(yas5xx->map, YAS5XX_CAL, data, sizeof(data));
+	ret = regmap_bulk_read(yas5xx->map, YAS530_532_CAL, data, sizeof(data));
 	if (ret)
 		return ret;
 
 	/* Actual calibration readout */
-	ret = regmap_bulk_read(yas5xx->map, YAS5XX_CAL, data, sizeof(data));
+	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);
 
+	/* 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 */
 	c->Cx = data[0] * 6 - 768;
 	c->Cy1 = data[1] * 6 - 768;
 	c->Cy2 = data[2] * 6 - 768;
-	yas53x_extract_calibration(&data[3], c);
+	yas530_532_extract_calibration(&data[3], c);
 
 	/*
 	 * Extract linearization:
@@ -668,6 +870,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;
 }
 
@@ -679,11 +882,11 @@ static int yas532_get_calibration_data(struct yas5xx *yas5xx)
 	int ret;
 
 	/* Dummy read, first read is ALWAYS wrong */
-	ret = regmap_bulk_read(yas5xx->map, YAS5XX_CAL, data, sizeof(data));
+	ret = regmap_bulk_read(yas5xx->map, YAS530_532_CAL, data, sizeof(data));
 	if (ret)
 		return ret;
 	/* Actual calibration readout */
-	ret = regmap_bulk_read(yas5xx->map, YAS5XX_CAL, data, sizeof(data));
+	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);
@@ -694,7 +897,9 @@ static int yas532_get_calibration_data(struct yas5xx *yas5xx)
 			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);
 
@@ -702,7 +907,8 @@ static int yas532_get_calibration_data(struct yas5xx *yas5xx)
 	c->Cx = data[0] * 10 - 1280;
 	c->Cy1 = data[1] * 10 - 1280;
 	c->Cy2 = data[2] * 10 - 1280;
-	yas53x_extract_calibration(&data[3], c);
+	yas530_532_extract_calibration(&data[3], c);
+
 	/*
 	 * Extract linearization:
 	 * Linearization layout in the 32 bits at byte 10:
@@ -725,7 +931,217 @@ static int yas532_get_calibration_data(struct yas5xx *yas5xx)
 	return 0;
 }
 
-static void yas5xx_dump_calibration(struct yas5xx *yas5xx)
+static int yas537_get_calibration_data(struct yas5xx *yas5xx)
+{
+	struct yas5xx_calibration *c = &yas5xx->calibration;
+	u8 data[17];
+	u32 val1, val2, val3, val4;
+	int i, ret;
+
+	/* Writing SRST register, the exact meaning is unknown */
+	ret = regmap_write(yas5xx->map, YAS537_SRST, BIT(1));
+	if (ret)
+		return ret;
+
+	/* Calibration readout, YAS537 needs one readout only */
+	ret = regmap_bulk_read(yas5xx->map, YAS537_CAL, data, sizeof(data));
+	if (ret)
+		return ret;
+	dev_dbg(yas5xx->dev, "calibration data: %*ph\n", 17, data);
+
+	/* Sanity check, is this all zeroes? */
+	if (memchr_inv(data, 0x00, 16) == NULL) {
+		if (FIELD_GET(GENMASK(5, 0), data[16]) == 0)
+			dev_warn(yas5xx->dev, "calibration is blank!\n");
+	}
+
+	/* Contribute calibration data to the input pool for kernel entropy */
+	add_device_randomness(data, sizeof(data));
+
+	/* Extract version information */
+	yas5xx->version = FIELD_GET(GENMASK(7, 6), data[16]);
+
+	/* There are two versions of YAS537 behaving differently */
+	switch (yas5xx->version) {
+
+	case YAS537_VERSION_0:
+
+		/*
+		 * The first version simply writes data back into registers:
+		 *
+		 * data[0]  YAS537_MTC		0x93
+		 * data[1]			0x94
+		 * data[2]			0x95
+		 * data[3]			0x96
+		 * data[4]			0x97
+		 * data[5]			0x98
+		 * data[6]			0x99
+		 * data[7]			0x9a
+		 * data[8]			0x9b
+		 * data[9]			0x9c
+		 * data[10]			0x9d
+		 * data[11] YAS537_OC		0x9e
+		 *
+		 * data[12] YAS537_OFFSET_X	0x84
+		 * data[13] YAS537_OFFSET_Y1	0x85
+		 * data[14] YAS537_OFFSET_Y2	0x86
+		 *
+		 * data[15] YAS537_HCK		0x88
+		 * data[16] YAS537_LCK		0x89
+		 */
+
+		for (i = 0; i < 17; i++) {
+			if (i < 12) {
+				ret = regmap_write(yas5xx->map,
+						   YAS537_MTC + i,
+						   data[i]);
+				if (ret)
+					return ret;
+			} else if (i < 15) {
+				ret = regmap_write(yas5xx->map,
+						   YAS537_OFFSET_X + i - 12,
+						   data[i]);
+				if (ret)
+					return ret;
+				yas5xx->hard_offsets[i - 12] = data[i];
+			} else {
+				ret = regmap_write(yas5xx->map,
+						   YAS537_HCK + i - 15,
+						   data[i]);
+				if (ret)
+					return ret;
+			}
+		}
+
+		break;
+
+	case YAS537_VERSION_1:
+
+		/*
+		 * The second version writes some data into registers but also
+		 * extracts calibration coefficients.
+		 *
+		 * Registers being written:
+		 *
+		 * data[0]  YAS537_MTC			0x93
+		 * data[1]  YAS537_MTC+1		0x94
+		 * data[2]  YAS537_MTC+2		0x95
+		 * data[3]  YAS537_MTC+3 (partially)	0x96
+		 *
+		 * data[12] YAS537_OFFSET_X		0x84
+		 * data[13] YAS537_OFFSET_Y1		0x85
+		 * data[14] YAS537_OFFSET_Y2		0x86
+		 *
+		 * data[15] YAS537_HCK (partially)	0x88
+		 *          YAS537_LCK (partially)	0x89
+		 * data[16] YAS537_OC  (partially)	0x9e
+		 */
+
+		for (i = 0; i < 3; i++) {
+			ret = regmap_write(yas5xx->map, YAS537_MTC + i,
+					   data[i]);
+			if (ret)
+				return ret;
+			ret = regmap_write(yas5xx->map, YAS537_OFFSET_X + i,
+					   data[i + 12]);
+			if (ret)
+				return ret;
+			yas5xx->hard_offsets[i] = data[i + 12];
+		}
+
+		/*
+		 * Visualization of partially taken data:
+		 *
+		 * data[3]       n 7 6 5 4 3 2 1 0
+		 * YAS537_MTC+3    x x x 1 0 0 0 0
+		 *
+		 * data[15]      n 7 6 5 4 3 2 1 0
+		 * YAS537_HCK      x x x x 0
+		 *
+		 * data[15]      n 7 6 5 4 3 2 1 0
+		 * YAS537_LCK              x x x x 0
+		 *
+		 * data[16]      n 7 6 5 4 3 2 1 0
+		 * YAS537_OC           x x x x x x
+		 */
+
+		ret = regmap_write(yas5xx->map, YAS537_MTC + 3,
+				   ((data[3] & GENMASK(7, 5)) | BIT(4)));
+		if (ret)
+			return ret;
+		ret = regmap_write(yas5xx->map, YAS537_HCK,
+				   (FIELD_GET(GENMASK(7, 4), data[15]) << 1));
+		if (ret)
+			return ret;
+		ret = regmap_write(yas5xx->map, YAS537_LCK,
+				   (FIELD_GET(GENMASK(3, 0), data[15]) << 1));
+		if (ret)
+			return ret;
+		ret = regmap_write(yas5xx->map, YAS537_OC,
+				   FIELD_GET(GENMASK(5, 0), data[16]));
+		if (ret)
+			return ret;
+
+		/*
+		 * For data extraction, build some blocks. Four 32-bit blocks
+		 * look appropriate.
+		 *
+		 *            n    7  6  5  4  3  2  1  0
+		 *  data[0]   0 [ Cx Cx Cx Cx Cx Cx Cx Cx ] bits 31 .. 24
+		 *  data[1]   1 [ Cx C1 C1 C1 C1 C1 C1 C1 ] bits 23 .. 16
+		 *  data[2]   2 [ C1 C1 C2 C2 C2 C2 C2 C2 ] bits 15 .. 8
+		 *  data[3]   3 [ C2 C2 C2                ] bits  7 .. 0
+		 *
+		 *            n    7  6  5  4  3  2  1  0
+		 *  data[3]   0 [          a2 a2 a2 a2 a2 ] bits 31 .. 24
+		 *  data[4]   1 [ a2 a2 a3 a3 a3 a3 a3 a3 ] bits 23 .. 16
+		 *  data[5]   2 [ a3 a4 a4 a4 a4 a4 a4 a4 ] bits 15 .. 8
+		 *  data[6]   3 [ a4                      ] bits  7 .. 0
+		 *
+		 *            n    7  6  5  4  3  2  1  0
+		 *  data[6]   0 [    a5 a5 a5 a5 a5 a5 a5 ] bits 31 .. 24
+		 *  data[7]   1 [ a5 a5 a6 a6 a6 a6 a6 a6 ] bits 23 .. 16
+		 *  data[8]   2 [ a6 a7 a7 a7 a7 a7 a7 a7 ] bits 15 .. 8
+		 *  data[9]   3 [ a7                      ] bits  7 .. 0
+		 *
+		 *            n    7  6  5  4  3  2  1  0
+		 *  data[9]   0 [    a8 a8 a8 a8 a8 a8 a8 ] bits 31 .. 24
+		 *  data[10]  1 [ a9 a9 a9 a9 a9 a9 a9 a9 ] bits 23 .. 16
+		 *  data[11]  2 [ a9  k  k  k  k  k  k  k ] bits 15 .. 8
+		 *  data[12]  3 [                         ] bits  7 .. 0
+		 */
+
+		/* Get data into these three blocks val1 to val3 */
+		val1 = get_unaligned_be32(&data[0]);
+		val2 = get_unaligned_be32(&data[3]);
+		val3 = get_unaligned_be32(&data[6]);
+		val4 = get_unaligned_be32(&data[9]);
+
+		/* Extract calibration coefficients and modify */
+		c->Cx  = FIELD_GET(GENMASK(31, 23), val1) - 256;
+		c->Cy1 = FIELD_GET(GENMASK(22, 14), val1) - 256;
+		c->Cy2 = FIELD_GET(GENMASK(13,  5), val1) - 256;
+		c->a2  = FIELD_GET(GENMASK(28, 22), val2) -  64;
+		c->a3  = FIELD_GET(GENMASK(21, 15), val2) -  64;
+		c->a4  = FIELD_GET(GENMASK(14,  7), val2) - 128;
+		c->a5  = FIELD_GET(GENMASK(30, 22), val3) - 112;
+		c->a6  = FIELD_GET(GENMASK(21, 15), val3) -  64;
+		c->a7  = FIELD_GET(GENMASK(14,  7), val3) - 128;
+		c->a8  = FIELD_GET(GENMASK(30, 24), val4) -  64;
+		c->a9  = FIELD_GET(GENMASK(23, 15), val4) - 112;
+		c->k   = FIELD_GET(GENMASK(14,  8), val4);
+
+		break;
+
+	default:
+		dev_err(yas5xx->dev, "unknown version of YAS537\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static void yas530_532_dump_calibration(struct yas5xx *yas5xx)
 {
 	struct yas5xx_calibration *c = &yas5xx->calibration;
 
@@ -748,20 +1164,68 @@ static void yas5xx_dump_calibration(struct yas5xx *yas5xx)
 	dev_dbg(yas5xx->dev, "dck = %d\n", c->dck);
 }
 
-static int yas5xx_set_offsets(struct yas5xx *yas5xx, s8 ox, s8 oy1, s8 oy2)
+static void yas537_dump_calibration(struct yas5xx *yas5xx)
+{
+	struct yas5xx_calibration *c = &yas5xx->calibration;
+	unsigned int val;
+	int i;
+
+	switch (yas5xx->version) {
+	case YAS537_VERSION_0:
+		for (i = 0x84; i < 0x9f; i++) {
+			if (i <= 0x86 || ((i >= 0x88) & (i <= 0x89)) ||
+			    i >= 0x93) {
+				regmap_read(yas5xx->map, i, &val);
+				dev_dbg(yas5xx->dev, "register 0x%02x: %u\n",
+					i, val);
+			}
+		}
+		break;
+	case YAS537_VERSION_1:
+		for (i = 0x84; i < 0x9f; i++) {
+			if (i <= 0x86 || ((i >= 0x88) & (i <= 0x89)) ||
+			    ((i >= 0x93) & (i <= 0x96)) || i == 0x9e) {
+				regmap_read(yas5xx->map, i, &val);
+				dev_dbg(yas5xx->dev, "register 0x%02x: %u\n",
+					i, val);
+			}
+		}
+		dev_dbg(yas5xx->dev, "hard_offset x: %d\n",
+			yas5xx->hard_offsets[0]);
+		dev_dbg(yas5xx->dev, "hard_offset y1: %d\n",
+			yas5xx->hard_offsets[1]);
+		dev_dbg(yas5xx->dev, "hard_offset y2: %d\n",
+			yas5xx->hard_offsets[2]);
+		dev_dbg(yas5xx->dev, "Cx = %d\n", c->Cx);
+		dev_dbg(yas5xx->dev, "Cy1 = %d\n", c->Cy1);
+		dev_dbg(yas5xx->dev, "Cy2 = %d\n", c->Cy2);
+		dev_dbg(yas5xx->dev, "a2 = %d\n", c->a2);
+		dev_dbg(yas5xx->dev, "a3 = %d\n", c->a3);
+		dev_dbg(yas5xx->dev, "a4 = %d\n", c->a4);
+		dev_dbg(yas5xx->dev, "a5 = %d\n", c->a5);
+		dev_dbg(yas5xx->dev, "a6 = %d\n", c->a6);
+		dev_dbg(yas5xx->dev, "a7 = %d\n", c->a7);
+		dev_dbg(yas5xx->dev, "a8 = %d\n", c->a8);
+		dev_dbg(yas5xx->dev, "a9 = %d\n", c->a9);
+		dev_dbg(yas5xx->dev, "k = %d\n", c->k);
+		break;
+	}
+}
+
+static int yas530_532_set_offsets(struct yas5xx *yas5xx, s8 ox, s8 oy1, s8 oy2)
 {
 	int ret;
 
-	ret = regmap_write(yas5xx->map, YAS5XX_OFFSET_X, ox);
+	ret = regmap_write(yas5xx->map, YAS530_532_OFFSET_X, ox);
 	if (ret)
 		return ret;
-	ret = regmap_write(yas5xx->map, YAS5XX_OFFSET_Y1, oy1);
+	ret = regmap_write(yas5xx->map, YAS530_532_OFFSET_Y1, oy1);
 	if (ret)
 		return ret;
-	return regmap_write(yas5xx->map, YAS5XX_OFFSET_Y2, oy2);
+	return regmap_write(yas5xx->map, YAS530_532_OFFSET_Y2, oy2);
 }
 
-static s8 yas5xx_adjust_offset(s8 old, int bit, u16 center, u16 measure)
+static s8 yas530_532_adjust_offset(s8 old, int bit, u16 center, u16 measure)
 {
 	if (measure > center)
 		return old + BIT(bit);
@@ -770,7 +1234,7 @@ static s8 yas5xx_adjust_offset(s8 old, int bit, u16 center, u16 measure)
 	return old;
 }
 
-static int yas5xx_meaure_offsets(struct yas5xx *yas5xx)
+static int yas530_532_measure_offsets(struct yas5xx *yas5xx)
 {
 	int ret;
 	u16 center;
@@ -779,7 +1243,7 @@ static int yas5xx_meaure_offsets(struct yas5xx *yas5xx)
 	int i;
 
 	/* Actuate the init coil and measure offsets */
-	ret = regmap_write(yas5xx->map, YAS5XX_ACTUATE_INIT_COIL, 0);
+	ret = regmap_write(yas5xx->map, YAS530_532_ACTUATE_INIT_COIL, 0);
 	if (ret)
 		return ret;
 
@@ -810,26 +1274,26 @@ static int yas5xx_meaure_offsets(struct yas5xx *yas5xx)
 	oy2 = 0;
 
 	for (i = 4; i >= 0; i--) {
-		ret = yas5xx_set_offsets(yas5xx, ox, oy1, oy2);
+		ret = yas530_532_set_offsets(yas5xx, ox, oy1, oy2);
 		if (ret)
 			return ret;
 
-		ret = yas5xx_measure(yas5xx, &t, &x, &y1, &y2);
+		ret = yas530_532_measure(yas5xx, &t, &x, &y1, &y2);
 		if (ret)
 			return ret;
 		dev_dbg(yas5xx->dev, "measurement %d: x=%d, y1=%d, y2=%d\n",
 			5-i, x, y1, y2);
 
-		ox = yas5xx_adjust_offset(ox, i, center, x);
-		oy1 = yas5xx_adjust_offset(oy1, i, center, y1);
-		oy2 = yas5xx_adjust_offset(oy2, i, center, y2);
+		ox = yas530_532_adjust_offset(ox, i, center, x);
+		oy1 = yas530_532_adjust_offset(oy1, i, center, y1);
+		oy2 = yas530_532_adjust_offset(oy2, i, center, y2);
 	}
 
 	/* Needed for calibration algorithm */
 	yas5xx->hard_offsets[0] = ox;
 	yas5xx->hard_offsets[1] = oy1;
 	yas5xx->hard_offsets[2] = oy2;
-	ret = yas5xx_set_offsets(yas5xx, ox, oy1, oy2);
+	ret = yas530_532_set_offsets(yas5xx, ox, oy1, oy2);
 	if (ret)
 		return ret;
 
@@ -838,27 +1302,64 @@ static int yas5xx_meaure_offsets(struct yas5xx *yas5xx)
 	return 0;
 }
 
-static int yas5xx_power_on(struct yas5xx *yas5xx)
+static int yas530_532_power_on(struct yas5xx *yas5xx)
 {
 	unsigned int val;
 	int ret;
 
 	/* Zero the test registers */
-	ret = regmap_write(yas5xx->map, YAS5XX_TEST1, 0);
+	ret = regmap_write(yas5xx->map, YAS530_532_TEST1, 0);
 	if (ret)
 		return ret;
-	ret = regmap_write(yas5xx->map, YAS5XX_TEST2, 0);
+	ret = regmap_write(yas5xx->map, YAS530_532_TEST2, 0);
 	if (ret)
 		return ret;
 
 	/* Set up for no interrupts, calibrated clock divider */
 	val = FIELD_PREP(YAS5XX_CONFIG_CCK_MASK, yas5xx->calibration.dck);
-	ret = regmap_write(yas5xx->map, YAS5XX_CONFIG, val);
+	ret = regmap_write(yas5xx->map, YAS530_532_CONFIG, val);
 	if (ret)
 		return ret;
 
 	/* Measure interval 0 (back-to-back?)  */
-	return regmap_write(yas5xx->map, YAS5XX_MEASURE_INTERVAL, 0);
+	return regmap_write(yas5xx->map, YAS530_532_MEASURE_INTERVAL, 0);
+}
+
+static int yas537_power_on(struct yas5xx *yas5xx)
+{
+	int ret;
+	u8 intrvl;
+
+	/* Write registers according to Android driver */
+	ret = regmap_write(yas5xx->map, YAS537_ADCCAL, GENMASK(1, 0));
+	if (ret)
+		return ret;
+	ret = regmap_write(yas5xx->map, YAS537_ADCCAL+1, GENMASK(7, 3));
+	if (ret)
+		return ret;
+	ret = regmap_write(yas5xx->map, YAS537_TRM, GENMASK(7, 0));
+	if (ret)
+		return ret;
+
+	/* The interval value is static in regular operation */
+	intrvl = (YAS537_DEFAULT_SENSOR_DELAY * 1000
+		 - YAS537_MEASURE_TIME_WORST) / 4100;
+	ret = regmap_write(yas5xx->map, YAS537_MEASURE_INTERVAL, intrvl);
+	if (ret)
+		return ret;
+
+	/* The average value is also static in regular operation */
+	ret = regmap_write(yas5xx->map, YAS537_AVR, YAS537_MAG_AVERAGE_32_MASK);
+	if (ret)
+		return ret;
+
+	/* Perform the "rcoil" part but skip the "last_after_rcoil" read */
+	ret = regmap_write(yas5xx->map, YAS537_CONFIG, BIT(3));
+	if (ret)
+		return ret;
+	usleep_range(YAS537_MAG_RCOIL_TIME, YAS537_MAG_RCOIL_TIME+100);
+
+	return 0;
 }
 
 static int yas5xx_probe(struct i2c_client *i2c,
@@ -919,35 +1420,55 @@ static int yas5xx_probe(struct i2c_client *i2c,
 
 	switch (yas5xx->devid) {
 	case YAS530_DEVICE_ID:
-		ret = yas530_get_calibration_data(yas5xx);
+	case YAS532_DEVICE_ID:
+
+		if (yas5xx->devid == YAS530_DEVICE_ID) {
+			ret = yas530_get_calibration_data(yas5xx);
+			if (ret)
+				goto assert_reset;
+			dev_info(dev, "detected YAS530 MS-3E %s",
+				 yas5xx->version ? "B" : "A");
+			strncpy(yas5xx->name, "yas530", sizeof(yas5xx->name));
+		} else {
+			ret = yas532_get_calibration_data(yas5xx);
+			if (ret)
+				goto assert_reset;
+			dev_info(dev, "detected YAS532/YAS533 MS-3R/F %s",
+				 yas5xx->version ? "AC" : "AB");
+			strncpy(yas5xx->name, "yas532", sizeof(yas5xx->name));
+		}
+
+		yas530_532_dump_calibration(yas5xx);
+		ret = yas530_532_power_on(yas5xx);
+		if (ret)
+			goto assert_reset;
+		ret = yas530_532_measure_offsets(yas5xx);
 		if (ret)
 			goto assert_reset;
-		dev_info(dev, "detected YAS530 MS-3E %s",
-			 yas5xx->version ? "B" : "A");
-		strncpy(yas5xx->name, "yas530", sizeof(yas5xx->name));
 		break;
-	case YAS532_DEVICE_ID:
-		ret = yas532_get_calibration_data(yas5xx);
+
+	case YAS537_DEVICE_ID:
+		ret = yas537_get_calibration_data(yas5xx);
+		if (ret)
+			goto assert_reset;
+		dev_info(dev, "detected YAS537 MS-3T");
+		/* As the version naming is unknown, provide it for debug only */
+		dev_dbg(yas5xx->dev, "YAS537 version: %s\n",
+			yas5xx->version ? "1" : "0");
+		strncpy(yas5xx->name, "yas537", sizeof(yas5xx->name));
+
+		yas537_dump_calibration(yas5xx);
+		ret = yas537_power_on(yas5xx);
 		if (ret)
 			goto assert_reset;
-		dev_info(dev, "detected YAS532/YAS533 MS-3R/F %s",
-			 yas5xx->version ? "AC" : "AB");
-		strncpy(yas5xx->name, "yas532", sizeof(yas5xx->name));
 		break;
+
 	default:
 		ret = -ENODEV;
 		dev_err(dev, "unhandled device ID %02x\n", yas5xx->devid);
 		goto assert_reset;
 	}
 
-	yas5xx_dump_calibration(yas5xx);
-	ret = yas5xx_power_on(yas5xx);
-	if (ret)
-		goto assert_reset;
-	ret = yas5xx_meaure_offsets(yas5xx);
-	if (ret)
-		goto assert_reset;
-
 	indio_dev->info = &yas5xx_info;
 	indio_dev->available_scan_masks = yas5xx_scan_masks;
 	indio_dev->modes = INDIO_DIRECT_MODE;
@@ -1043,7 +1564,15 @@ static int __maybe_unused yas5xx_runtime_resume(struct device *dev)
 	usleep_range(31000, 40000);
 	gpiod_set_value_cansleep(yas5xx->reset, 0);
 
-	ret = yas5xx_power_on(yas5xx);
+	switch (yas5xx->devid) {
+	case YAS530_DEVICE_ID:
+	case YAS532_DEVICE_ID:
+		ret = yas530_532_power_on(yas5xx);
+		break;
+	case YAS537_DEVICE_ID:
+		ret = yas537_power_on(yas5xx);
+		break;
+	}
 	if (ret) {
 		dev_err(dev, "cannot power on\n");
 		goto out_reset;
@@ -1069,6 +1598,7 @@ static const struct i2c_device_id yas5xx_id[] = {
 	{"yas530", },
 	{"yas532", },
 	{"yas533", },
+	{"yas537", },
 	{}
 };
 MODULE_DEVICE_TABLE(i2c, yas5xx_id);
@@ -1077,6 +1607,7 @@ static const struct of_device_id yas5xx_of_match[] = {
 	{ .compatible = "yamaha,yas530", },
 	{ .compatible = "yamaha,yas532", },
 	{ .compatible = "yamaha,yas533", },
+	{ .compatible = "yamaha,yas537", },
 	{}
 };
 MODULE_DEVICE_TABLE(of, yas5xx_of_match);
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/7] iio: magnetometer: yas530: Change range of data in volatile register
  2022-06-08 23:38   ` [PATCH 1/7] iio: magnetometer: yas530: Change range of data in volatile register Jakob Hauser
@ 2022-06-09  8:50     ` Linus Walleij
  0 siblings, 0 replies; 23+ messages in thread
From: Linus Walleij @ 2022-06-09  8:50 UTC (permalink / raw)
  To: Jakob Hauser
  Cc: Jonathan Cameron, Lars-Peter Clausen, linux-iio, phone-devel,
	~postmarketos/upstreaming

On Thu, Jun 9, 2022 at 1:38 AM Jakob Hauser <jahau@rocketmail.com> wrote:

> In function yas5xx_volatile_reg(), the range for measure data should end at
> "YAS5XX_MEASURE_DATA + 7" instead of "+ 8" as we count from 0 to 7 here.
>
> This change is of low importance as the "+ 8" register isn't called.
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Jakob Hauser <jahau@rocketmail.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/7] iio: magnetometer: yas530: Correct scaling of magnetic axes
  2022-06-08 23:38   ` [PATCH 2/7] iio: magnetometer: yas530: Correct scaling of magnetic axes Jakob Hauser
@ 2022-06-09  8:52     ` Linus Walleij
  0 siblings, 0 replies; 23+ messages in thread
From: Linus Walleij @ 2022-06-09  8:52 UTC (permalink / raw)
  To: Jakob Hauser
  Cc: Jonathan Cameron, Lars-Peter Clausen, linux-iio, phone-devel,
	~postmarketos/upstreaming

On Thu, Jun 9, 2022 at 1:38 AM Jakob Hauser <jahau@rocketmail.com> wrote:

> Looks like YAS530 raw values return picotesla and YAS532 nanotesla. Adapt
> comments and scaling.
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Jakob Hauser <jahau@rocketmail.com>

Ooops. In-kernel device trees only use YAS530 so the fix does not seem
urgent, but it's up to Jonathan to decide whether this should even go to stable.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/7] iio: magnetometer: yas530: Correct temperature handling
  2022-06-08 23:38   ` [PATCH 3/7] iio: magnetometer: yas530: Correct temperature handling Jakob Hauser
@ 2022-06-09  8:53     ` Linus Walleij
  0 siblings, 0 replies; 23+ messages in thread
From: Linus Walleij @ 2022-06-09  8:53 UTC (permalink / raw)
  To: Jakob Hauser
  Cc: Jonathan Cameron, Lars-Peter Clausen, linux-iio, phone-devel,
	~postmarketos/upstreaming

On Thu, Jun 9, 2022 at 1:38 AM Jakob Hauser <jahau@rocketmail.com> wrote:

> The raw temperature value is a number of counts from a certain starting
> point. The resolution of the temperature counts is different for the YAS
> variants.
>
> Temperature compensation for YAS532 version AC seems to be handled differently.
> It uses the deviation from 20 degree Celsius [1] whereas YAS530 and older
> versions of YAS532 apply solely the t value as a multiplier [2][3].
>
> In funtion yas5xx_read_raw(), add case IIO_CHAN_INFO_PROCESSED. Remove scale
> of temperature as this isn't applied.
>
> Additionally correct sign of temperature channel in iio_chan_spec. It's already
> defined that way in yas5xx_get_measure() function.
>
> [1] https://github.com/msm8916-mainline/android_kernel_qcom_msm8916/blob/GT-I9195I/drivers/iio/magnetometer/yas_mag_drv-yas532.c#L442
> [2] https://github.com/NovaFusion/android_kernel_samsung_golden/blob/cm-12.1/drivers/sensor/compass/yas_mag_driver-yas530.c#L881-L883
> [3] https://github.com/LineageOS/android_kernel_samsung_msm8930-common/blob/lineage-18.1/drivers/sensors/geomagnetic/yas_mag_driver-yas53x.c#L856-L858
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Jakob Hauser <jahau@rocketmail.com>

Thanks, my temperatures seem more accurate after this on YAS530.
Patch does not seem urgent as the temperature does not see much usage
outside of the device itself.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 4/7] iio: magnetometer: yas530: Change data type of hard_offsets to signed
  2022-06-08 23:38   ` [PATCH 4/7] iio: magnetometer: yas530: Change data type of hard_offsets to signed Jakob Hauser
@ 2022-06-09  8:54     ` Linus Walleij
  2022-06-10 14:22     ` Andy Shevchenko
  1 sibling, 0 replies; 23+ messages in thread
From: Linus Walleij @ 2022-06-09  8:54 UTC (permalink / raw)
  To: Jakob Hauser
  Cc: Jonathan Cameron, Lars-Peter Clausen, linux-iio, phone-devel,
	~postmarketos/upstreaming

On Thu, Jun 9, 2022 at 1:38 AM Jakob Hauser <jahau@rocketmail.com> wrote:

> The "hard_offsets" are currently unsigned u8 but they should be signed as they
> can get negative. They are signed in function yas5xx_meaure_offsets() and in the
> Yamaha drivers [1][2].
>
> [1] https://github.com/NovaFusion/android_kernel_samsung_golden/blob/cm-12.1/drivers/sensor/compass/yas.h#L156
> [2] https://github.com/msm8916-mainline/android_kernel_qcom_msm8916/blob/GT-I9195I/drivers/iio/magnetometer/yas_mag_drv-yas532.c#L91
>
> Fixes: de8860b1ed47 ("iio: magnetometer: Add driver for Yamaha YAS530")
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Jakob Hauser <jahau@rocketmail.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 5/7] iio: magnetometer: yas530: Change data type of calibration coefficients
  2022-06-08 23:38   ` [PATCH 5/7] iio: magnetometer: yas530: Change data type of calibration coefficients Jakob Hauser
@ 2022-06-09  8:55     ` Linus Walleij
  0 siblings, 0 replies; 23+ messages in thread
From: Linus Walleij @ 2022-06-09  8:55 UTC (permalink / raw)
  To: Jakob Hauser
  Cc: Jonathan Cameron, Lars-Peter Clausen, linux-iio, phone-devel,
	~postmarketos/upstreaming

On Thu, Jun 9, 2022 at 1:39 AM Jakob Hauser <jahau@rocketmail.com> wrote:

> This is a preparation for adding YAS537 variant.
>
> YAS537 uses other data types on the calibration coefficients [1] than YAS530 [2]
> and YAS532 [3].
>
> On YAS537, at least for a4 and a7 this could matter because 8-bit unsigned data
> from the register gets stored into a signed data type, therefore this should be
> 8-bit as well.
>
> For YAS530/532, on the other hand, it doesn't seem to matter. The size of a2-a9
> and k is smaller than 8-bit at extraction, also the applied math is low. And
> Cx/Cy1/Cy2, now being defined as signed 16-bit, are extracted as unsigned 8-bit
> and undergo only minor math.
>
> [1] https://github.com/msm8916-mainline/android_kernel_qcom_msm8916/blob/GT-I9195I/drivers/iio/magnetometer/yas_mag_drv-yas537.c#L76-L78
> [2] https://github.com/NovaFusion/android_kernel_samsung_golden/blob/cm-12.1/drivers/sensor/compass/yas_mag_driver-yas530.c#L526-L527
> [3] https://github.com/msm8916-mainline/android_kernel_qcom_msm8916/blob/GT-I9195I/drivers/iio/magnetometer/yas_mag_drv-yas532.c#L76-L77
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Jakob Hauser <jahau@rocketmail.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 6/7] iio: magnetometer: yas530: Remove redundant defaults on switch devid
  2022-06-08 23:38   ` [PATCH 6/7] iio: magnetometer: yas530: Remove redundant defaults on switch devid Jakob Hauser
@ 2022-06-09  8:55     ` Linus Walleij
  2022-06-10 14:24     ` Andy Shevchenko
  1 sibling, 0 replies; 23+ messages in thread
From: Linus Walleij @ 2022-06-09  8:55 UTC (permalink / raw)
  To: Jakob Hauser
  Cc: Jonathan Cameron, Lars-Peter Clausen, linux-iio, phone-devel,
	~postmarketos/upstreaming

On Thu, Jun 9, 2022 at 1:39 AM Jakob Hauser <jahau@rocketmail.com> wrote:

> This is a preparation for adding YAS537 variant.
>
> In function yas5xx_probe(), there is a switch statement checking for device
> IDs. If the ID is unknown, it exits with a device error. In later functions,
> it's not neccessary to check the validity of the device IDs again.
>
> When adding YAS537 in a later patch, several of such switch statements will be
> added. To make it more uniform, the redundant ones in YAS530/532 get herby
> removed. This is done in a separate patch for better history control.
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Jakob Hauser <jahau@rocketmail.com>

No big deal for me, this works fine.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 7/7] iio: magnetometer: yas530: Add YAS537 variant
  2022-06-08 23:38   ` [PATCH 7/7] iio: magnetometer: yas530: Add YAS537 variant Jakob Hauser
@ 2022-06-09  8:56     ` Linus Walleij
  2022-06-10 14:31     ` Andy Shevchenko
  1 sibling, 0 replies; 23+ messages in thread
From: Linus Walleij @ 2022-06-09  8:56 UTC (permalink / raw)
  To: Jakob Hauser
  Cc: Jonathan Cameron, Lars-Peter Clausen, linux-iio, phone-devel,
	~postmarketos/upstreaming

On Thu, Jun 9, 2022 at 1:39 AM Jakob Hauser <jahau@rocketmail.com> wrote:

> This adds support for the magnetometer Yamaha YAS537. The additions are based
> on comparison of Yamaha Android kernel drivers for YAS532 [1] and YAS537 [2].
>
> Functions used by YAS530 & YAS532 only were renamed from yas5xx to yas530_532.
> Registers were renamed accordingly.
>
> In the Yamaha YAS537 Android driver, there is an overflow/underflow control
> implemented. For regular usage, this seems not necessary. A similar overflow/
> underflow control of Yamaha YAS530/532 Android driver isn't integrated in the
> mainline driver. It is therefore skipped for YAS537 in mainline too.
>
> Also in the Yamaha YAS537 Android driver, at the end of the reset_yas537()
> function, a measurement is saved in "last_after_rcoil". Later on, this is
> compared to current measurements. If the difference gets too big, a new
> reset is intialized. The difference in measurements needs to be quite big,
> it's hard to say if this is necessary for regular operation. Therefore this
> isn't integrated in the mainline driver either.
>
> [1] https://github.com/msm8916-mainline/android_kernel_qcom_msm8916/blob/GT-I9195I/drivers/iio/magnetometer/yas_mag_drv-yas532.c
> [2] https://github.com/msm8916-mainline/android_kernel_qcom_msm8916/blob/GT-I9195I/drivers/iio/magnetometer/yas_mag_drv-yas537.c
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Jakob Hauser <jahau@rocketmail.com>

Nice work! I have reviewed the drafts before in private and can't see
any remaining issues with the final version so:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 4/7] iio: magnetometer: yas530: Change data type of hard_offsets to signed
  2022-06-08 23:38   ` [PATCH 4/7] iio: magnetometer: yas530: Change data type of hard_offsets to signed Jakob Hauser
  2022-06-09  8:54     ` Linus Walleij
@ 2022-06-10 14:22     ` Andy Shevchenko
  1 sibling, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2022-06-10 14:22 UTC (permalink / raw)
  To: Jakob Hauser
  Cc: Jonathan Cameron, Lars-Peter Clausen, Linus Walleij, linux-iio,
	phone-devel,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
	<devicetree@vger.kernel.org>,
	Hans de Goede <hdegoede@redhat.com>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,

On Thu, Jun 9, 2022 at 1:44 AM Jakob Hauser <jahau@rocketmail.com> wrote:
>
> The "hard_offsets" are currently unsigned u8 but they should be signed as they
> can get negative. They are signed in function yas5xx_meaure_offsets() and in the
> Yamaha drivers [1][2].
>
> [1] https://github.com/NovaFusion/android_kernel_samsung_golden/blob/cm-12.1/drivers/sensor/compass/yas.h#L156
> [2] https://github.com/msm8916-mainline/android_kernel_qcom_msm8916/blob/GT-I9195I/drivers/iio/magnetometer/yas_mag_drv-yas532.c#L91
>
> Fixes: de8860b1ed47 ("iio: magnetometer: Add driver for Yamaha YAS530")

The series should be organized like this:
1) definite fixes;
2) cleanups / refactoring;
3) new features / documentation.

It will give a maintainer easier times to handle it.

Code looks good to me.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Cc: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
> ---
>  drivers/iio/magnetometer/yamaha-yas530.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/magnetometer/yamaha-yas530.c b/drivers/iio/magnetometer/yamaha-yas530.c
> index 2e8d20b05217..9bfb3b573907 100644
> --- a/drivers/iio/magnetometer/yamaha-yas530.c
> +++ b/drivers/iio/magnetometer/yamaha-yas530.c
> @@ -133,7 +133,7 @@ struct yas5xx {
>         unsigned int version;
>         char name[16];
>         struct yas5xx_calibration calibration;
> -       u8 hard_offsets[3];
> +       s8 hard_offsets[3];
>         struct iio_mount_matrix orientation;
>         struct regmap *map;
>         struct regulator_bulk_data regs[2];
> --
> 2.35.1
>


-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 6/7] iio: magnetometer: yas530: Remove redundant defaults on switch devid
  2022-06-08 23:38   ` [PATCH 6/7] iio: magnetometer: yas530: Remove redundant defaults on switch devid Jakob Hauser
  2022-06-09  8:55     ` Linus Walleij
@ 2022-06-10 14:24     ` Andy Shevchenko
  1 sibling, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2022-06-10 14:24 UTC (permalink / raw)
  To: Jakob Hauser
  Cc: Jonathan Cameron, Lars-Peter Clausen, Linus Walleij, linux-iio,
	phone-devel,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
	<devicetree@vger.kernel.org>,
	Hans de Goede <hdegoede@redhat.com>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,

On Thu, Jun 9, 2022 at 1:44 AM Jakob Hauser <jahau@rocketmail.com> wrote:
>
> This is a preparation for adding YAS537 variant.
>
> In function yas5xx_probe(), there is a switch statement checking for device
> IDs. If the ID is unknown, it exits with a device error. In later functions,
> it's not neccessary to check the validity of the device IDs again.
>
> When adding YAS537 in a later patch, several of such switch statements will be
> added. To make it more uniform, the redundant ones in YAS530/532 get herby
> removed. This is done in a separate patch for better history control.

...

> -       default:

Please, leave it. Some static analyzers or weird compiler versions may
complain on this.

> -               dev_err(yas5xx->dev, "unknown data format\n");
> -               ret = -EINVAL;

> -               break;

So leave this.

>         }

Ditto for the rest.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 7/7] iio: magnetometer: yas530: Add YAS537 variant
  2022-06-08 23:38   ` [PATCH 7/7] iio: magnetometer: yas530: Add YAS537 variant Jakob Hauser
  2022-06-09  8:56     ` Linus Walleij
@ 2022-06-10 14:31     ` Andy Shevchenko
  2022-06-10 23:57       ` Jakob Hauser
  1 sibling, 1 reply; 23+ messages in thread
From: Andy Shevchenko @ 2022-06-10 14:31 UTC (permalink / raw)
  To: Jakob Hauser
  Cc: Jonathan Cameron, Lars-Peter Clausen, Linus Walleij, linux-iio,
	phone-devel,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
	<devicetree@vger.kernel.org>,
	Hans de Goede <hdegoede@redhat.com>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,

On Thu, Jun 9, 2022 at 1:44 AM Jakob Hauser <jahau@rocketmail.com> wrote:
>
> This adds support for the magnetometer Yamaha YAS537. The additions are based
> on comparison of Yamaha Android kernel drivers for YAS532 [1] and YAS537 [2].
>
> Functions used by YAS530 & YAS532 only were renamed from yas5xx to yas530_532.
> Registers were renamed accordingly.
>
> In the Yamaha YAS537 Android driver, there is an overflow/underflow control
> implemented. For regular usage, this seems not necessary. A similar overflow/
> underflow control of Yamaha YAS530/532 Android driver isn't integrated in the
> mainline driver. It is therefore skipped for YAS537 in mainline too.
>
> Also in the Yamaha YAS537 Android driver, at the end of the reset_yas537()
> function, a measurement is saved in "last_after_rcoil". Later on, this is
> compared to current measurements. If the difference gets too big, a new
> reset is intialized. The difference in measurements needs to be quite big,

initialized

> it's hard to say if this is necessary for regular operation. Therefore this
> isn't integrated in the mainline driver either.

I understand that Linus knows well this code and may review this, but
can you please split register renaming (at least, maybe something else
can be split as well as preparatory change) to the separate patch?

...

> +                               regmap_read(yas5xx->map, i, &val);
> +                               dev_dbg(yas5xx->dev, "register 0x%02x: %u\n",
> +                                       i, val);

Please, drop all these value reads/writes debug messages, they are
quite expensive (by resource consuming), noisy (may spam logs), and
most important duplicative. regmap API has tracepoints, use them!

Perhaps it would require an additional patch to clean this up, if
anything like this is present in the current code base..

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 7/7] iio: magnetometer: yas530: Add YAS537 variant
  2022-06-10 14:31     ` Andy Shevchenko
@ 2022-06-10 23:57       ` Jakob Hauser
  2022-06-11 10:56         ` Andy Shevchenko
  0 siblings, 1 reply; 23+ messages in thread
From: Jakob Hauser @ 2022-06-10 23:57 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Jonathan Cameron, Lars-Peter Clausen, Linus Walleij, linux-iio,
	phone-devel,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Hans de Goede, ~postmarketos/upstreaming

Hi Andy,

On 10.06.22 16:31, Andy Shevchenko wrote:
> I understand that Linus knows well this code and may review this, but
> can you please split register renaming (at least, maybe something else
> can be split as well as preparatory change) to the separate patch?

That makes sense, I'll move the renaming into a separate patch.

>> +                               regmap_read(yas5xx->map, i, &val);
>> +                               dev_dbg(yas5xx->dev, "register 0x%02x: %u\n",
>> +                                       i, val);
> 
> Please, drop all these value reads/writes debug messages, they are
> quite expensive (by resource consuming), noisy (may spam logs), and
> most important duplicative. regmap API has tracepoints, use them!
> 
> Perhaps it would require an additional patch to clean this up, if
> anything like this is present in the current code base..

Ok, I'll remove those direct regmap reads in yas537_dump_calibration().

However, I'd like to keep the others. The calibration data is dumped
before [1] and after [2] being processed by the driver. This is helpful
to check if it was processed correctly. Dumping the data is done only
once at probing.

In yas537_dump_calibration(), I'd also like to keep dumping the
"hard_offsets". Currently there is no linearization formula known for
YAS537. Providing the "hard_offsets" may help to find a way.

[1]
https://github.com/torvalds/linux/blob/v5.18/drivers/iio/magnetometer/yamaha-yas530.c#L592
[2]
https://github.com/torvalds/linux/blob/v5.18/drivers/iio/magnetometer/yamaha-yas530.c#L678-L699

Kind regards,
Jakob

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 7/7] iio: magnetometer: yas530: Add YAS537 variant
  2022-06-10 23:57       ` Jakob Hauser
@ 2022-06-11 10:56         ` Andy Shevchenko
  2022-06-11 14:00           ` Jakob Hauser
  0 siblings, 1 reply; 23+ messages in thread
From: Andy Shevchenko @ 2022-06-11 10:56 UTC (permalink / raw)
  To: Jakob Hauser
  Cc: Jonathan Cameron, Lars-Peter Clausen, Linus Walleij, linux-iio,
	phone-devel,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Hans de Goede,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
	<devicetree@vger.kernel.org>,
	Hans de Goede <hdegoede@redhat.com>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,

On Sat, Jun 11, 2022 at 1:57 AM Jakob Hauser <jahau@rocketmail.com> wrote:
> On 10.06.22 16:31, Andy Shevchenko wrote:

...

> >> +                               dev_dbg(yas5xx->dev, "register 0x%02x: %u\n",
> >> +                                       i, val);
> >
> > Please, drop all these value reads/writes debug messages, they are
> > quite expensive (by resource consuming), noisy (may spam logs), and
> > most important duplicative. regmap API has tracepoints, use them!
> >
> > Perhaps it would require an additional patch to clean this up, if
> > anything like this is present in the current code base..
>
> Ok, I'll remove those direct regmap reads in yas537_dump_calibration().

I'm not sure I understand what you are going to drop. I was talking
about debug messages, the regmap reads are fine. Or you are talking
about them as they are tightened together and one makes no sense
without the other?

> However, I'd like to keep the others. The calibration data is dumped
> before [1] and after [2] being processed by the driver. This is helpful
> to check if it was processed correctly. Dumping the data is done only
> once at probing.

Then it should be probably dev_info() in such cases.

> In yas537_dump_calibration(), I'd also like to keep dumping the
> "hard_offsets". Currently there is no linearization formula known for
> YAS537. Providing the "hard_offsets" may help to find a way.

I understand that, but per se this is not for production esp. taking
into account that regmap has a tracepoint mechanism.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 7/7] iio: magnetometer: yas530: Add YAS537 variant
  2022-06-11 10:56         ` Andy Shevchenko
@ 2022-06-11 14:00           ` Jakob Hauser
  2022-06-11 20:22             ` Andy Shevchenko
  0 siblings, 1 reply; 23+ messages in thread
From: Jakob Hauser @ 2022-06-11 14:00 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Jonathan Cameron, Lars-Peter Clausen, Linus Walleij, linux-iio,
	phone-devel,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Hans de Goede, ~postmarketos/upstreaming

Hi Andy,

(Side note: There is something wrong with the Cc list in your e-mail,
the list of addresses isn't handled correctly.)

On 11.06.22 12:56, Andy Shevchenko wrote:
> On Sat, Jun 11, 2022 at 1:57 AM Jakob Hauser <jahau@rocketmail.com> wrote:
>> Ok, I'll remove those direct regmap reads in yas537_dump_calibration().
> 
> I'm not sure I understand what you are going to drop. I was talking
> about debug messages, the regmap reads are fine. Or you are talking
> about them as they are tightened together and one makes no sense
> without the other?

Yes on your question. In the new function yas537_dump_calibration() I
added some regmap reads that are performed only because of debug
messages. I'll remove those regmap reads incl. the corresponding debug
messages. However, I'd like to keep the "other" debug messages.

I'll explain more detailed.

Generally speaking on YAS530/532:
At driver probe, calibration "raw" data is read from the calibration
register (currently YAS5XX_CAL). Within the driver, this data gets
processed into several calibration coefficients like e.g. c->a2. These
are later on applied on the measure data.

Calibration debug on YAS530/532:
After reading the calibration "raw" data from the register (for further
processing), we dump that "raw" data to debug. After having "calculated"
the calibration coefficients like c->a2 within the driver, we dump them
to debug too.

Generally speaking on YAS537 version 0:
There are two versions of YAS537. Version 0 reads calibration data from
the register (YAS537_CAL) and writes it directly back into some other
registers. The driver doesn't touch anything. This is done in new
function yas537_get_calibration_data() after "case YAS537_VERSION_0:".

Generally speaking on YAS537 version 1:
Version 1 of YAS537 is a mixture. Some of the data being read from the
calibration register (YAS537_CAL) is directly written back to some other
registers. But additionally, some calibration coefficients like c->a2
need to be "calculated". This happens in new function
yas537_get_calibration_data() after "case YAS537_VERSION_1:". The
calibration coefficients will be applied on the measure data later on.

Calibration debug on YAS537:
In the new function yas537_dump_calibration(), I implemented that
mixture also. Firstly, I added some regmap reads and debug dumps of the
registers where some of the calibration data was written into. Secondly,
for YAS537 version 1, I dumped the "calculated" calibration coefficients
like c->a2.

What I intend to change for patchset v2:
In the new function yas537_dump_calibration(), I'll remove the two "for
loops" (they contain the "unnecessary" regmap reads and debug dumps).
But I'd like to keep the debug dump of the "calculated" calibration
coefficients after the "for loops".

>> However, I'd like to keep the others. The calibration data is dumped
>> before [1] and after [2] being processed by the driver. This is helpful
>> to check if it was processed correctly. Dumping the data is done only
>> once at probing.
> 
> Then it should be probably dev_info() in such cases.

To my understanding, in this case it's rather debug. If at some point it
turns out that the driver doesn't work correctly, it allows a deeper
insight on the intermediate steps the driver is doing internally.

>> In yas537_dump_calibration(), I'd also like to keep dumping the
>> "hard_offsets". Currently there is no linearization formula known for
>> YAS537. Providing the "hard_offsets" may help to find a way.
> 
> I understand that, but per se this is not for production esp. taking
> into account that regmap has a tracepoint mechanism.

Well, ok, I'll drop the "hard_offsets" debug dump.

Kind regards,
Jakob

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 7/7] iio: magnetometer: yas530: Add YAS537 variant
  2022-06-11 14:00           ` Jakob Hauser
@ 2022-06-11 20:22             ` Andy Shevchenko
  2022-06-11 21:19               ` Jakob Hauser
  0 siblings, 1 reply; 23+ messages in thread
From: Andy Shevchenko @ 2022-06-11 20:22 UTC (permalink / raw)
  To: Jakob Hauser
  Cc: Jonathan Cameron, Lars-Peter Clausen, Linus Walleij, linux-iio,
	phone-devel,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Hans de Goede,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
	<devicetree@vger.kernel.org>,
	Hans de Goede <hdegoede@redhat.com>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,

On Sat, Jun 11, 2022 at 4:00 PM Jakob Hauser <jahau@rocketmail.com> wrote:
>
> Hi Andy,
>
> (Side note: There is something wrong with the Cc list in your e-mail,
> the list of addresses isn't handled correctly.)

The list is the same as in your mail. Dunno what exactly is the
problem you are referring to?

> On 11.06.22 12:56, Andy Shevchenko wrote:
> > On Sat, Jun 11, 2022 at 1:57 AM Jakob Hauser <jahau@rocketmail.com> wrote:
> >> Ok, I'll remove those direct regmap reads in yas537_dump_calibration().
> >
> > I'm not sure I understand what you are going to drop. I was talking
> > about debug messages, the regmap reads are fine. Or you are talking
> > about them as they are tightened together and one makes no sense
> > without the other?
>
> Yes on your question. In the new function yas537_dump_calibration() I
> added some regmap reads that are performed only because of debug
> messages. I'll remove those regmap reads incl. the corresponding debug
> messages. However, I'd like to keep the "other" debug messages.
>
> I'll explain more detailed.
>
> Generally speaking on YAS530/532:
> At driver probe, calibration "raw" data is read from the calibration
> register (currently YAS5XX_CAL). Within the driver, this data gets
> processed into several calibration coefficients like e.g. c->a2. These
> are later on applied on the measure data.
>
> Calibration debug on YAS530/532:
> After reading the calibration "raw" data from the register (for further
> processing), we dump that "raw" data to debug. After having "calculated"
> the calibration coefficients like c->a2 within the driver, we dump them
> to debug too.
>
> Generally speaking on YAS537 version 0:
> There are two versions of YAS537. Version 0 reads calibration data from
> the register (YAS537_CAL) and writes it directly back into some other
> registers. The driver doesn't touch anything. This is done in new
> function yas537_get_calibration_data() after "case YAS537_VERSION_0:".
>
> Generally speaking on YAS537 version 1:
> Version 1 of YAS537 is a mixture. Some of the data being read from the
> calibration register (YAS537_CAL) is directly written back to some other
> registers. But additionally, some calibration coefficients like c->a2
> need to be "calculated". This happens in new function
> yas537_get_calibration_data() after "case YAS537_VERSION_1:". The
> calibration coefficients will be applied on the measure data later on.
>
> Calibration debug on YAS537:
> In the new function yas537_dump_calibration(), I implemented that
> mixture also. Firstly, I added some regmap reads and debug dumps of the
> registers where some of the calibration data was written into. Secondly,
> for YAS537 version 1, I dumped the "calculated" calibration coefficients
> like c->a2.
>
> What I intend to change for patchset v2:
> In the new function yas537_dump_calibration(), I'll remove the two "for
> loops" (they contain the "unnecessary" regmap reads and debug dumps).
> But I'd like to keep the debug dump of the "calculated" calibration
> coefficients after the "for loops".

Sounds reasonable to me.

> >> However, I'd like to keep the others. The calibration data is dumped
> >> before [1] and after [2] being processed by the driver. This is helpful
> >> to check if it was processed correctly. Dumping the data is done only
> >> once at probing.
> >
> > Then it should be probably dev_info() in such cases.
>
> To my understanding, in this case it's rather debug. If at some point it
> turns out that the driver doesn't work correctly, it allows a deeper
> insight on the intermediate steps the driver is doing internally.

OK!

> >> In yas537_dump_calibration(), I'd also like to keep dumping the
> >> "hard_offsets". Currently there is no linearization formula known for
> >> YAS537. Providing the "hard_offsets" may help to find a way.
> >
> > I understand that, but per se this is not for production esp. taking
> > into account that regmap has a tracepoint mechanism.
>
> Well, ok, I'll drop the "hard_offsets" debug dump.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 7/7] iio: magnetometer: yas530: Add YAS537 variant
  2022-06-11 20:22             ` Andy Shevchenko
@ 2022-06-11 21:19               ` Jakob Hauser
  0 siblings, 0 replies; 23+ messages in thread
From: Jakob Hauser @ 2022-06-11 21:19 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Jonathan Cameron, Lars-Peter Clausen, Linus Walleij, linux-iio,
	phone-devel,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Hans de Goede, ~postmarketos/upstreaming

Hi Andy,

On 11.06.22 22:22, Andy Shevchenko wrote:
>
> On Sat, Jun 11, 2022 at 4:00 PM Jakob Hauser <jahau@rocketmail.com> wrote:
>>
>> (Side note: There is something wrong with the Cc list in your e-mail,
>> the list of addresses isn't handled correctly.)
> 
> The list is the same as in your mail. Dunno what exactly is the
> problem you are referring to?

It somehow got confused by the colon after "open list" and the tilde
before "postmarketos". Everything in between got interpreted as the
name/description of the postmarketos mailing list. Your first reply at
2022-06-10 14:31 didn't make it to the devicetree list and likely not to
Hans. Since then, these addresses show up twice in the thread, once for
real and once still interpreted as name/description of the postmarketos
mailing list.

The thread in the devicetree list:
https://lore.kernel.org/linux-devicetree/CAHp75VfV1E1WaLVDG3V9460AsVxahb9Epod-63T35eDV+h8Ubg@mail.gmail.com/T/#t

On the patchset, I'll send v2 probably at Sunday or Monday.

Kind regards,
Jakob

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2022-06-11 21:19 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <cover.1654727058.git.jahau.ref@rocketmail.com>
2022-06-08 23:36 ` [PATCH 0/7] Add support for magnetometer Yamaha YAS537 Jakob Hauser
2022-06-08 23:38   ` [PATCH 1/7] iio: magnetometer: yas530: Change range of data in volatile register Jakob Hauser
2022-06-09  8:50     ` Linus Walleij
2022-06-08 23:38   ` [PATCH 2/7] iio: magnetometer: yas530: Correct scaling of magnetic axes Jakob Hauser
2022-06-09  8:52     ` Linus Walleij
2022-06-08 23:38   ` [PATCH 3/7] iio: magnetometer: yas530: Correct temperature handling Jakob Hauser
2022-06-09  8:53     ` Linus Walleij
2022-06-08 23:38   ` [PATCH 4/7] iio: magnetometer: yas530: Change data type of hard_offsets to signed Jakob Hauser
2022-06-09  8:54     ` Linus Walleij
2022-06-10 14:22     ` Andy Shevchenko
2022-06-08 23:38   ` [PATCH 5/7] iio: magnetometer: yas530: Change data type of calibration coefficients Jakob Hauser
2022-06-09  8:55     ` Linus Walleij
2022-06-08 23:38   ` [PATCH 6/7] iio: magnetometer: yas530: Remove redundant defaults on switch devid Jakob Hauser
2022-06-09  8:55     ` Linus Walleij
2022-06-10 14:24     ` Andy Shevchenko
2022-06-08 23:38   ` [PATCH 7/7] iio: magnetometer: yas530: Add YAS537 variant Jakob Hauser
2022-06-09  8:56     ` Linus Walleij
2022-06-10 14:31     ` Andy Shevchenko
2022-06-10 23:57       ` Jakob Hauser
2022-06-11 10:56         ` Andy Shevchenko
2022-06-11 14:00           ` Jakob Hauser
2022-06-11 20:22             ` Andy Shevchenko
2022-06-11 21:19               ` Jakob Hauser

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.