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@xxxxxxxxxxxxxx> 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
