Re: [PATCH v3 5/8] iio: magnetometer: yas530: Change data type of calibration coefficients

[Jakob Hauser <jahau@rocketmail.com>]

Hi Jonathan,

On 21.06.22 02:51, Jakob Hauser wrote:
>
> 
> On 18.06.22 16:56, Jonathan Cameron wrote:
>
>> On Sat, 18 Jun 2022 02:13:13 +0200
>> 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.
>>
>> Ok. If this is harmless to existing drivers fair enough, though my personal
>> inclination would have been to take the easier approach of making the
>> new variant sign extend on variable load (sign_extend_32() and similar)
>> just so we didn't need to check the older parts weren't affected.
> 
> I didn't know that operation :) Let's take this.

While working on patchset v4, I just realized that sign_extend32() can't
be used at the variable declaration but instead needs to be applied at
"variable load", as you wrote.

I wasn't aware of this until now. In that case, I'd  prefer to leave the
patch unchanged. Overall the resulting code looks simpler that way.
Applying sign_extend32() at all locations where we extract calibration
coefficients makes it more dizzy.

Kind regards,
Jakob