Re: [PATCH v5 14/14] iio: magnetometer: yas530: Add YAS537 variant

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

 



Hi Andy,

On 08.08.22 13:47, Andy Shevchenko wrote:
> On Mon, Aug 8, 2022 at 1:12 AM Jakob Hauser <jahau@xxxxxxxxxxxxxx> wrote:
>>
>> This adds support for the magnetometer Yamaha YAS537. The additions are based
> 
> Add support

OK

>> on comparison of Yamaha Android kernel drivers for YAS532 [1] and YAS537 [2].
>>
>> In the Yamaha YAS537 Android driver, there is an overflow/underflow control
>> implemented. For regular usage, this seems not necessary. A similar overflow/
>> underflow control of Yamaha YAS530/532 Android driver isn't integrated in the
>> mainline driver. It is therefore skipped for YAS537 in mainline too.
> 
> the mainline

OK

>> Also in the Yamaha YAS537 Android driver, at the end of the reset_yas537()
>> function, a measurement is saved in "last_after_rcoil". Later on, this is
>> compared to current measurements. If the difference gets too big, a new
>> reset is initialized. The difference in measurements needs to be quite big,
>> it's hard to say if this is necessary for regular operation. Therefore this
>> isn't integrated in the mainline driver either.
> 
> ..
> 
>>         help
>>           Say Y here to add support for the Yamaha YAS530 series of
>> -         3-Axis Magnetometers. Right now YAS530, YAS532 and YAS533 are
>> -         fully supported.
>> +         3-Axis Magnetometers. YAS530, YAS532, YAS533 and YAS537 are
>> +         supported.
> 
> So, after this change the rest become partially supported?
> 
> Perhaps you want to leave the original and add a new sentence like:
> 
>   "The YAS537 is partially supported."
> 
> ?

All four variants are fully supported. I changed the sentence according
to Jonathan's suggestion in v3:
https://lore.kernel.org/linux-iio/cover.1655509425.git.jahau@xxxxxxxxxxxxxx/T/#m6ba6b576cdf6f86f550f7598fcf7c408ac5f2d46

> 
> ..
> 
>> - * For YAS532/533, this value is known from the Android driver. For YAS530,
> 
> It seems this comma is unneeded in the original comment.

I'll remove the comma here and in patch 12, where it was introduced.

>> - * it was approximately measured.
>> + * For YAS532/533, this value is known from the Android driver. For YAS530
>> + * and YAS537, it was approximately measured.
> 
> P.S. Do you see now how your series and the end result become better?

The driver improves. Though we kind of get lost in details, I have the
impression we could go on like this forever.

Kind regards,
Jakob



[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