RE: [PATCH 2/2] iio:magnetometer: Support for ST magnetic sensors

[LI Qingwu]

Hi Andy,

Thanks a lot for your input, I just extend the existing driver in patch V2 for the sersor.



Best Regards

**********************************************
Li Qingwu (Terry)
Senior Embedded Software Engineer 
Leica Geosystems(Shanghai)Co.,Limited
(Tel): +86 21 61061036
(FAX): +86 21 61061008
(Mobile): +86 187 0185 9600
E-mail: Qing-wu.Li@xxxxxxxxxxxxxxxxxxxxxxx
Http: www.leica-geosystems.com.cn
**********************************************

-----Original Message-----
From: Andy Shevchenko <andy.shevchenko@xxxxxxxxx> 
Sent: Tuesday, March 9, 2021 7:01 PM
To: Jonathan Cameron <jic23@xxxxxxxxxx>
Cc: LI Qingwu <qing-wu.li@xxxxxxxxxxxxxxxxxxxxxxx>; Lars-Peter Clausen <lars@xxxxxxxxxx>; Peter Meerwald <pmeerw@xxxxxxxxxx>; Rob Herring <robh+dt@xxxxxxxxxx>; Denis Ciocca <denis.ciocca@xxxxxx>; linux-iio <linux-iio@xxxxxxxxxxxxxxx>; devicetree <devicetree@xxxxxxxxxxxxxxx>; Linux Kernel Mailing List <linux-kernel@xxxxxxxxxxxxxxx>; TERTYCHNYI Grygorii <grygorii.tertychnyi@xxxxxxxxxxxxxxxxxxxx>; ZHIZHIKIN Andrey <andrey.zhizhikin@xxxxxxxxxxxxxxxxxxxx>; Lorenzo Bianconi <lorenzo.bianconi@xxxxxxxxxx>
Subject: Re: [PATCH 2/2] iio:magnetometer: Support for ST magnetic sensors

This email is not from Hexagon’s Office 365 instance. Please be careful while clicking links, opening attachments, or replying to this email.


On Sat, Mar 6, 2021 at 6:44 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
> On Fri,  5 Mar 2021 07:05:36 +0000
> LI Qingwu <Qing-wu.Li@xxxxxxxxxxxxxxxxxxxxxxx> wrote:
>
> > Add support for STMicroelectronics digital magnetic sensors, 
> > LSM303AH,LSM303AGR,LIS2MDL,ISM303DAC,IIS2MDC.
> >
> > The patch tested with IIS2MDC instrument.
> >
> > Signed-off-by: LI Qingwu <Qing-wu.Li@xxxxxxxxxxxxxxxxxxxxxxx>
>
> Hi,
>
> Given that at least two parts in here is supported by the existing 
> driver in iio/magnetometers/st_magn_*.c (lsm303agr) can you confirm 
> that it doesn't make sense to simply extend that driver to support the 
> other parts?  This is particularly true when the WHO AM I register 
> reads 0x40 for all these parts.
>
> I've done a fairly superficial review whilst here, but please check 
> you can't just add the relevant entries to the existing driver.

I even hadn't looked into the code because this one needs a very good justification why it's a new driver rather than extension of the existing one.

--
With Best Regards,
Andy Shevchenko