Re: [PATCH v4 10/10] iio: magnetometer: yas530: Add YAS537 variant

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux