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! ... > > 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. > Not all the registers are properly named in the original driver. E.g. > there is a register called "YAS537_REG_MTCR" [1] with no names for the > following registers. Further down, this and the following 11 registers > are written by just counting up the register number [2]. 12 (8-bit) registers, which may suggest 6 __be16, in any case looks like a mount matrix or so, with X,Y,Z values programmed. This is interesting as well... https://software-dl.ti.com/simplelink/esd/plugins/sail/1.10.00.06/exports/docs/sail/doxygen/html/structyas537__t.html (Btw, it also suggest that ADC calibration is 16-bit register) > It looks similar to the situation at register "YAS537_REG_ADCCALR", > where the numerically following register (0x92) doesn't have a name [3]. > It could be because of a 16-bit register, as you say, but it could also > be a naming thing. > > At the location in discussion, the original driver uses two single > writes [4]. I'd stick to that. > > [1] > https://github.com/msm8916-mainline/android_kernel_qcom_msm8916/blob/GT-I9195I/drivers/iio/magnetometer/yas_mag_drv-yas537.c#L38 > [2] > https://github.com/msm8916-mainline/android_kernel_qcom_msm8916/blob/GT-I9195I/drivers/iio/magnetometer/yas_mag_drv-yas537.c#L277-L279 > [3] > https://github.com/msm8916-mainline/android_kernel_qcom_msm8916/blob/GT-I9195I/drivers/iio/magnetometer/yas_mag_drv-yas537.c#L37 > [4] > https://github.com/msm8916-mainline/android_kernel_qcom_msm8916/blob/GT-I9195I/drivers/iio/magnetometer/yas_mag_drv-yas537.c#L345-L348 It's all the joy of reverse engineering... 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. ... > 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. -- With Best Regards, Andy Shevchenko
