Hi Andy, On 29.07.22 19:24, Andy Shevchenko wrote: > On Fri, Jul 29, 2022 at 1:13 AM Jakob Hauser <jahau@xxxxxxxxxxxxxx> wrote: >> On 27.07.22 19:46, Andy Shevchenko wrote: > > .. > >> /* >> * Write registers according to Android driver, the exact meaning >> * is unknown > > With a period at the end :-) > >> */ > >> This reminded me of another location where I first had a comment >> "Writing SRST register, the exact meaning is unknown". There you >> criticized the part "the exact meaning is unknown", so I changed it to >> simply "Writing SRST register". > > Yeah, but that is different, SRST seems like easy to deduce to "soft > reset" (taking into account where it's programmed in the run flow). > >> Accordingly, I would choose the following comment here: >> >> /* Writing ADCCAL and TRM registers */ > > Fine with me! OK, I'll apply the comment "Writing ADCCAL and TRM registers". > > .. > >>> You seem to program the 16-bit register with a single value, I don't >>> think it's a good idea to split a such. When it's a bulk write and >>> value defined with __be16 / __le16 it makes much more clear what >>> hardware is and what it expects. >> >> We don't know for sure whether it is a 16-bit register or an incomplete >> register naming. > > By the values you write into it seems to be a __be16 calibration > register. The value to write is 0x3f8 which might ring a bell to you > if you know what other values related to ADC. Sigh, ok, I'll apply bulk write. How to do it correctly? I guess: __be16 buf = cpu_to_be16(GENMASK(9, 3)); ret = regmap_bulk_write(yas5xx->map, YAS537_ADCCAL, &buf, 2); if (ret) return ret; The whole block would then look like: /* Writing ADCCAL and TRM registers */ __be16 buf = cpu_to_be16(GENMASK(9, 3)); ret = regmap_bulk_write(yas5xx->map, YAS537_ADCCAL, &buf, 2); if (ret) return ret; ret = regmap_write(yas5xx->map, YAS537_TRM, GENMASK(7, 0)); if (ret) return ret; ... > To the 4100 denominator: > https://github.com/XPerience-AOSP-Lollipop/android_kernel_wingtech_msm8916/blob/xpe-11.1/drivers/input/misc/yas_mag_drv-yas537.c#L235, > seems you can find a lot by browsing someone's code and perhaps a Git > history. I've seen that comment before but I don't understand its meaning. >> Still I didn't get your comment. Is your intention to change the "50 >> milliseconds * 1000" to "50000 microseconds" in the define? >> >> It would look like ... >> >> #define YAS537_DEFAULT_SENSOR_DELAY_US 50000 >> >> ... though I would prefer to keep current define, as it is implemented >> now and stated above: >> >> #define YAS537_DEFAULT_SENSOR_DELAY_MS 50 > > No, just to show in the actual calculation that you convert MS to US > using MILLI. Sorry, I still don't get what you want me to do. What do you mean by "using MILLI", can you elaborate? Kind regards, Jakob