Re: [PATCH v2 1/4] iio: accel: st_accel_spi: add OF capability to st_accel_spi

[Lorenzo Bianconi <lorenzo.bianconi83-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

>
>
> On 25 June 2017 18:20:33 CEST, Lorenzo Bianconi <lorenzo.bianconi83-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> On Sun, 25 Jun 2017 17:24:10 +0200
>>> Lorenzo Bianconi <lorenzo.bianconi83-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>
>>>> Add device tree support for LIS3DH, LSM330D, LSM330DL, LSM330DLC,
>>>> LIS331DLH, LSM330, LSM303AGR, LIS2DH12, LIS3L02DQ, LNG2DM accel
>>sensors.
>>>>
>>>> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
>>>> ---
>>>>  drivers/iio/accel/st_accel_spi.c | 67
>>+++++++++++++++++++++++++++++++++++-----
>>>>  1 file changed, 59 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/drivers/iio/accel/st_accel_spi.c
>>b/drivers/iio/accel/st_accel_spi.c
>>>> index d13742edee0e..a8b61daa83d7 100644
>>>> --- a/drivers/iio/accel/st_accel_spi.c
>>>> +++ b/drivers/iio/accel/st_accel_spi.c
>>>> @@ -18,6 +18,63 @@
>>>>  #include <linux/iio/common/st_sensors_spi.h>
>>>>  #include "st_accel.h"
>>>>
>>>> +#ifdef CONFIG_OF
>>>> +/*
>>>> + * For new single-chip sensors use <device_name> as compatible
>>string.
>>>> + * For old single-chip devices keep <device_name>-accel to maintain
>>>> + * compatibility
>>>> + */
>>>> +static const struct of_device_id st_accel_of_match[] = {
>>>> +     {
>>>> +             .compatible = "st,lis302dl-spi"
>>> Leaving this without some explanation of why it doesn't have
>>> .data set requires a comment at least. It will result
>>> in the name never being set - or ending up as
>>> the default spi->modalias anyway.. Presumably not what is
>>> desired?
>>>
>>> If I have missed something about why this works, please
>>> add a comment here in v3 to explain it.
>>
>>Right, it will not work, but it needs a device (lis3lv02dl) that is
>>not currently support in st_accel_spi.
>>So I preferred to fix OF capability for currently supported devices
>>and add missing ones (LIS3LV02DL, LIS331DL and H3LIS331DL)
>>in a subsequent patch. What do you think? Is it better to squash the
>>two patches together?
> Up to you, but if separate squash this element for now.
>>
>>Regards,
>>Lorenzo

Ok, I figured out I can fix support for lis302dl without adding new devices.
I will send a v3 with that fix.
Regards,
Lorenzo

>>
>>>> +     },
>>>> +     {
>>>> +             .compatible = "st,lis3dh-accel",
>>>> +             .data = LIS3DH_ACCEL_DEV_NAME,
>>>> +     },
>>>> +     {
>>>> +             .compatible = "st,lsm330d-accel",
>>>> +             .data = LSM330D_ACCEL_DEV_NAME,
>>>> +     },
>>>> +     {
>>>> +             .compatible = "st,lsm330dl-accel",
>>>> +             .data = LSM330DL_ACCEL_DEV_NAME,
>>>> +     },
>>>> +     {
>>>> +             .compatible = "st,lsm330dlc-accel",
>>>> +             .data = LSM330DLC_ACCEL_DEV_NAME,
>>>> +     },
>>>> +     {
>>>> +             .compatible = "st,lis331dlh-accel",
>>>> +             .data = LIS331DLH_ACCEL_DEV_NAME,
>>>> +     },
>>>> +     {
>>>> +             .compatible = "st,lsm330-accel",
>>>> +             .data = LSM330_ACCEL_DEV_NAME,
>>>> +     },
>>>> +     {
>>>> +             .compatible = "st,lsm303agr-accel",
>>>> +             .data = LSM303AGR_ACCEL_DEV_NAME,
>>>> +     },
>>>> +     {
>>>> +             .compatible = "st,lis2dh12-accel",
>>>> +             .data = LIS2DH12_ACCEL_DEV_NAME,
>>>> +     },
>>>> +     {
>>>> +             .compatible = "st,lis3l02dq",
>>>> +             .data = LIS3L02DQ_ACCEL_DEV_NAME,
>>>> +     },
>>>> +     {
>>>> +             .compatible = "st,lng2dm-accel",
>>>> +             .data = LNG2DM_ACCEL_DEV_NAME,
>>>> +     },
>>>> +     {}
>>>> +};
>>>> +MODULE_DEVICE_TABLE(of, st_accel_of_match);
>>>> +#else
>>>> +#define st_accel_of_match    NULL
>>>> +#endif
>>>> +
>>>>  static int st_accel_spi_probe(struct spi_device *spi)
>>>>  {
>>>>       struct iio_dev *indio_dev;
>>>> @@ -30,6 +87,8 @@ static int st_accel_spi_probe(struct spi_device
>>*spi)
>>>>
>>>>       adata = iio_priv(indio_dev);
>>>>
>>>> +     st_sensors_of_name_probe(&spi->dev, st_accel_of_match,
>>>> +                              spi->modalias,
>>sizeof(spi->modalias));
>>>>       st_sensors_spi_configure(indio_dev, spi, adata);
>>>>
>>>>       err = st_accel_common_probe(indio_dev);
>>>> @@ -61,14 +120,6 @@ static const struct spi_device_id
>>st_accel_id_table[] = {
>>>>  };
>>>>  MODULE_DEVICE_TABLE(spi, st_accel_id_table);
>>>>
>>>> -#ifdef CONFIG_OF
>>>> -static const struct of_device_id st_accel_of_match[] = {
>>>> -     { .compatible = "st,lis302dl-spi" },
>>>> -     {}
>>>> -};
>>>> -MODULE_DEVICE_TABLE(of, st_accel_of_match);
>>>> -#endif
>>>> -
>>>>  static struct spi_driver st_accel_driver = {
>>>>       .driver = {
>>>>               .name = "st-accel-spi",
>>>
>
> --
> Sent from my Android device with K-9 Mail. Please excuse my brevity.



-- 
UNIX is Sexy: who | grep -i blonde | talk; cd ~; wine; talk; touch;
unzip; touch; strip; gasp; finger; gasp; mount; fsck; more; yes; gasp;
umount; make clean; sleep