Add support for magnetometer Yamaha YAS537

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 <[email protected]>

Signed-off-by Jakob Hauser <[email protected]>

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

Cc Linus Walleij <[email protected]>

Signed-off-by Jakob Hauser <[email protected]>

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 <[email protected]>

Signed-off-by Jakob Hauser <[email protected]>

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 <[email protected]>

Signed-off-by Jakob Hauser <[email protected]>

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 <[email protected]>

Signed-off-by Jakob Hauser <[email protected]>

---
 drivers/iio/magnetometer/yamaha-yas530.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/iio/magnetometer/yamaha-yas530.c +1 -2

index 2e8d20b05217..9bfb3b573907 100644
@@ -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

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 <[email protected]>

Signed-off-by Jakob Hauser <[email protected]>

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 <[email protected]>

Signed-off-by Jakob Hauser <[email protected]>

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:

        }

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 <[email protected]>

Signed-off-by Jakob Hauser <[email protected]>

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.

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?

drivers/iio/magnetometer/yamaha-yas530.c static void yas5xx_dump_calibration(struct yas5xx *yas5xx) · line 1180

Quoted 3 lines

drivers/iio/magnetometer/yamaha-yas530.c +3 -0

+                               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..

On 10.06.22 16:31, Andy Shevchenko wrote:

Quoted 9 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+                               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.

Quoted 1 line · Jakob Hauser

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?

Quoted 4 lines

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.

Quoted 3 lines

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.

Hi Andy,

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

Quoted 7 lines · Jakob Hauser

Quoted 1 line

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".

Quoted 6 lines

Quoted 4 lines

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.

Quoted 6 lines

Quoted 3 lines

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.

Quoted 3 lines · Jakob Hauser

(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?