Re: [PATCH] staging:iio:adc:ad7280a: add of_match_table entry

[Matheus Tavares Bernardino]

On Fri, Jul 26, 2019 at 2:30 AM Ardelean, Alexandru
 wrote:
>
> On Fri, 2019-07-26 at 01:38 +0530, Kartik Kulkarni wrote:
> > Add the of_device_id struct and the respective
> > of_match_device entry to complete device tree support.
> >
>
> This would be a [V2] I suppose.
>
> This change also does the rename of the driver name in a single go.
> Since it's a trivial change, it's fine from my side.

I think there was a small confusion when we sent the patches. Sorry
for that. Originally, Kartik made the rename in its own patch. Would
it be better if we resend the two patches separately?

Thanks,
Matheus

> Reviewed-by: Alexandru Ardelean 
>
>
> > Signed-off-by: Kartik Kulkarni 
> > Reviewed-by: Matheus Tavares 
> > ---
> >  drivers/staging/iio/adc/ad7280a.c | 9 -
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/iio/adc/ad7280a.c 
> > b/drivers/staging/iio/adc/ad7280a.c
> > index 19a5f244dcae..ded0ba093a28 100644
> > --- a/drivers/staging/iio/adc/ad7280a.c
> > +++ b/drivers/staging/iio/adc/ad7280a.c
> > @@ -1027,9 +1027,16 @@ static const struct spi_device_id ad7280_id[] = {
> >  };
> >  MODULE_DEVICE_TABLE(spi, ad7280_id);
> >
> > +static const struct of_device_id ad7280_of_match[] = {
> > + { .compatible = "adi,ad7280a", },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(of, ad7280_of_match);
> > +
> >  static struct spi_driver ad7280_driver = {
> >   .driver = {
> > - .name   = "ad7280",
> > + .name   = "ad7280a",
> > + .of_match_table = ad7280_of_match,
> >   },
> >   .probe  = ad7280_probe,
> >   .id_table   = ad7280_id,
>
> --
> You received this message because you are subscribed to the Google Groups 
> "Kernel USP" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to kernel-usp+unsubscr...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/kernel-usp/0e273486f1c4fb6249896225837cdf2da0fd2415.camel%40analog.com.

Re: [PATCH] iio: adc: Replace license text w/ SPDX identifier

[Matheus Tavares Bernardino]

On Thu, Nov 29, 2018 at 5:37 AM Krzysztof Kozlowski  wrote:
>
> On Wed, 28 Nov 2018 at 18:53, Matheus Tavares  
> wrote:
> >
> > From: Lucas Santos 
> >
> > This patch removes all license boilerplate texts from the .c and .h
> > files at drivers/iio/adc/ and, instead, adds the proper SPDX license
> > identifiers.
> >
> > Signed-off-by: Lucas Santos 
> > Signed-off-by: Matheus Tavares 
> > ---
> >  drivers/iio/adc/ad7266.c |  3 +--
> >  drivers/iio/adc/ad7291.c |  3 +--
> >  drivers/iio/adc/ad7298.c |  3 +--
> >  drivers/iio/adc/ad7766.c |  3 +--
> >  drivers/iio/adc/ad7791.c |  3 +--
> >  drivers/iio/adc/ad7793.c |  3 +--
> >  drivers/iio/adc/ad7887.c |  3 +--
> >  drivers/iio/adc/ad7923.c |  3 +--
> >  drivers/iio/adc/ad799x.c |  5 +
> >  drivers/iio/adc/ad_sigma_delta.c |  3 +--
> >  drivers/iio/adc/aspeed_adc.c |  6 +-
> >  drivers/iio/adc/at91-sama5d2_adc.c   | 10 +-
> >  drivers/iio/adc/at91_adc.c   |  3 +--
> >  drivers/iio/adc/axp20x_adc.c |  5 +
> >  drivers/iio/adc/axp288_adc.c | 13 +
> >  drivers/iio/adc/bcm_iproc_adc.c  | 13 +
> >  drivers/iio/adc/berlin2-adc.c|  5 +
> >  drivers/iio/adc/cc10001_adc.c|  6 +-
> >  drivers/iio/adc/cpcap-adc.c  | 10 +-
> >  drivers/iio/adc/da9150-gpadc.c   |  6 +-
> >  drivers/iio/adc/dln2-adc.c   |  5 +
> >  drivers/iio/adc/ep93xx_adc.c |  5 +
> >  drivers/iio/adc/exynos_adc.c | 11 +--
> >  drivers/iio/adc/fsl-imx25-gcq.c  |  5 +
> >  drivers/iio/adc/hi8435.c |  6 +-
> >  drivers/iio/adc/hx711.c  |  6 +-
> >  drivers/iio/adc/imx7d_adc.c  |  6 +-
> >  drivers/iio/adc/ina2xx-adc.c |  7 +--
> >  drivers/iio/adc/lp8788_adc.c |  5 +
> >  drivers/iio/adc/lpc18xx_adc.c|  5 +
> >  drivers/iio/adc/lpc32xx_adc.c| 15 +--
> >  drivers/iio/adc/ltc2471.c|  3 +--
> >  drivers/iio/adc/ltc2485.c|  5 +
> >  drivers/iio/adc/ltc2497.c|  3 +--
> >  drivers/iio/adc/max1027.c|  5 +
> >  drivers/iio/adc/max1118.c|  5 +
> >  drivers/iio/adc/max1363.c|  5 +
> >  drivers/iio/adc/mcp320x.c|  5 +
> >  drivers/iio/adc/mcp3422.c|  6 +-
> >  drivers/iio/adc/men_z188_adc.c   |  5 +
> >  drivers/iio/adc/meson_saradc.c   |  5 +
> >  drivers/iio/adc/mt6577_auxadc.c  | 10 +-
> >  drivers/iio/adc/mxs-lradc-adc.c  | 11 +--
> >  drivers/iio/adc/nau7802.c|  3 +--
> >  drivers/iio/adc/palmas_gpadc.c   |  5 +
> >  drivers/iio/adc/qcom-pm8xxx-xoadc.c  |  1 +
> >  drivers/iio/adc/qcom-spmi-iadc.c | 10 +-
> >  drivers/iio/adc/qcom-spmi-vadc.c | 10 +-
> >  drivers/iio/adc/rockchip_saradc.c| 11 +--
> >  drivers/iio/adc/spear_adc.c  |  3 +--
> >  drivers/iio/adc/stx104.c | 10 +-
> >  drivers/iio/adc/sun4i-gpadc-iio.c|  5 +
> >  drivers/iio/adc/ti-adc081c.c |  5 +
> >  drivers/iio/adc/ti-adc0832.c |  5 +
> >  drivers/iio/adc/ti-adc084s021.c  |  5 +
> >  drivers/iio/adc/ti-adc108s102.c  | 10 +-
> >  drivers/iio/adc/ti-adc12138.c|  5 +
> >  drivers/iio/adc/ti-ads1015.c |  5 +
> >  drivers/iio/adc/ti-ads8688.c |  5 +
> >  drivers/iio/adc/ti-tlc4541.c |  5 +
> >  drivers/iio/adc/ti_am335x_adc.c  | 10 +-
> >  drivers/iio/adc/twl4030-madc.c   | 16 +---
> >  drivers/iio/adc/twl6030-gpadc.c  | 16 +---
> >  drivers/iio/adc/vf610_adc.c  | 15 +--
> >  drivers/iio/adc/viperboard_adc.c |  7 +--
> >  drivers/iio/adc/xilinx-xadc-core.c   |  3 +--
> >  drivers/iio/adc/xilinx-xadc-events.c |  3 +--
> >  drivers/iio/adc/xilinx-xadc.h|  3 +--
> >  68 files changed, 68 insertions(+), 361 deletions(-)
> >
>
> (...)
>
> > diff --git a/drivers/iio/adc/ep93xx_adc.c b/drivers/iio/adc/ep93xx_adc.c
> > index 5036c392cb20..8f614c84b643 100644
> > --- a/drivers/iio/adc/ep93xx_adc.c
> > +++ b/drivers/iio/adc/ep93xx_adc.c
> > @@ -1,12 +1,9 @@
> > +// SPDX-License-Identifier: GPL-2.0
> >  /*
> >   * Driver for ADC module on the Cirrus Logic EP93xx series of SoCs
> >   *
> >   * Copyright (C) 2015 Alexander Sverdlin
> >   *
> > - * This program is free software; you can redistribute it and/or modify
> > - * it under the terms of the GNU General Public License version 2 as
> > - * published by the Free Software Foundation.
> > - *
> >   * The driver uses polling to get the conversion status. According to 
> > EP93xx
> >   * datasheets, reading ADCResult register starts the conversion, but user 
> > is also
> >   * responsible for ensuring that delay 

Re: [PATCH] iio: adc: Replace license text w/ SPDX identifier

[Matheus Tavares Bernardino]

On Thu, Nov 29, 2018 at 5:37 AM Krzysztof Kozlowski  wrote:
>
> On Wed, 28 Nov 2018 at 18:53, Matheus Tavares  
> wrote:
> >
> > From: Lucas Santos 
> >
> > This patch removes all license boilerplate texts from the .c and .h
> > files at drivers/iio/adc/ and, instead, adds the proper SPDX license
> > identifiers.
> >
> > Signed-off-by: Lucas Santos 
> > Signed-off-by: Matheus Tavares 
> > ---
> >  drivers/iio/adc/ad7266.c |  3 +--
> >  drivers/iio/adc/ad7291.c |  3 +--
> >  drivers/iio/adc/ad7298.c |  3 +--
> >  drivers/iio/adc/ad7766.c |  3 +--
> >  drivers/iio/adc/ad7791.c |  3 +--
> >  drivers/iio/adc/ad7793.c |  3 +--
> >  drivers/iio/adc/ad7887.c |  3 +--
> >  drivers/iio/adc/ad7923.c |  3 +--
> >  drivers/iio/adc/ad799x.c |  5 +
> >  drivers/iio/adc/ad_sigma_delta.c |  3 +--
> >  drivers/iio/adc/aspeed_adc.c |  6 +-
> >  drivers/iio/adc/at91-sama5d2_adc.c   | 10 +-
> >  drivers/iio/adc/at91_adc.c   |  3 +--
> >  drivers/iio/adc/axp20x_adc.c |  5 +
> >  drivers/iio/adc/axp288_adc.c | 13 +
> >  drivers/iio/adc/bcm_iproc_adc.c  | 13 +
> >  drivers/iio/adc/berlin2-adc.c|  5 +
> >  drivers/iio/adc/cc10001_adc.c|  6 +-
> >  drivers/iio/adc/cpcap-adc.c  | 10 +-
> >  drivers/iio/adc/da9150-gpadc.c   |  6 +-
> >  drivers/iio/adc/dln2-adc.c   |  5 +
> >  drivers/iio/adc/ep93xx_adc.c |  5 +
> >  drivers/iio/adc/exynos_adc.c | 11 +--
> >  drivers/iio/adc/fsl-imx25-gcq.c  |  5 +
> >  drivers/iio/adc/hi8435.c |  6 +-
> >  drivers/iio/adc/hx711.c  |  6 +-
> >  drivers/iio/adc/imx7d_adc.c  |  6 +-
> >  drivers/iio/adc/ina2xx-adc.c |  7 +--
> >  drivers/iio/adc/lp8788_adc.c |  5 +
> >  drivers/iio/adc/lpc18xx_adc.c|  5 +
> >  drivers/iio/adc/lpc32xx_adc.c| 15 +--
> >  drivers/iio/adc/ltc2471.c|  3 +--
> >  drivers/iio/adc/ltc2485.c|  5 +
> >  drivers/iio/adc/ltc2497.c|  3 +--
> >  drivers/iio/adc/max1027.c|  5 +
> >  drivers/iio/adc/max1118.c|  5 +
> >  drivers/iio/adc/max1363.c|  5 +
> >  drivers/iio/adc/mcp320x.c|  5 +
> >  drivers/iio/adc/mcp3422.c|  6 +-
> >  drivers/iio/adc/men_z188_adc.c   |  5 +
> >  drivers/iio/adc/meson_saradc.c   |  5 +
> >  drivers/iio/adc/mt6577_auxadc.c  | 10 +-
> >  drivers/iio/adc/mxs-lradc-adc.c  | 11 +--
> >  drivers/iio/adc/nau7802.c|  3 +--
> >  drivers/iio/adc/palmas_gpadc.c   |  5 +
> >  drivers/iio/adc/qcom-pm8xxx-xoadc.c  |  1 +
> >  drivers/iio/adc/qcom-spmi-iadc.c | 10 +-
> >  drivers/iio/adc/qcom-spmi-vadc.c | 10 +-
> >  drivers/iio/adc/rockchip_saradc.c| 11 +--
> >  drivers/iio/adc/spear_adc.c  |  3 +--
> >  drivers/iio/adc/stx104.c | 10 +-
> >  drivers/iio/adc/sun4i-gpadc-iio.c|  5 +
> >  drivers/iio/adc/ti-adc081c.c |  5 +
> >  drivers/iio/adc/ti-adc0832.c |  5 +
> >  drivers/iio/adc/ti-adc084s021.c  |  5 +
> >  drivers/iio/adc/ti-adc108s102.c  | 10 +-
> >  drivers/iio/adc/ti-adc12138.c|  5 +
> >  drivers/iio/adc/ti-ads1015.c |  5 +
> >  drivers/iio/adc/ti-ads8688.c |  5 +
> >  drivers/iio/adc/ti-tlc4541.c |  5 +
> >  drivers/iio/adc/ti_am335x_adc.c  | 10 +-
> >  drivers/iio/adc/twl4030-madc.c   | 16 +---
> >  drivers/iio/adc/twl6030-gpadc.c  | 16 +---
> >  drivers/iio/adc/vf610_adc.c  | 15 +--
> >  drivers/iio/adc/viperboard_adc.c |  7 +--
> >  drivers/iio/adc/xilinx-xadc-core.c   |  3 +--
> >  drivers/iio/adc/xilinx-xadc-events.c |  3 +--
> >  drivers/iio/adc/xilinx-xadc.h|  3 +--
> >  68 files changed, 68 insertions(+), 361 deletions(-)
> >
>
> (...)
>
> > diff --git a/drivers/iio/adc/ep93xx_adc.c b/drivers/iio/adc/ep93xx_adc.c
> > index 5036c392cb20..8f614c84b643 100644
> > --- a/drivers/iio/adc/ep93xx_adc.c
> > +++ b/drivers/iio/adc/ep93xx_adc.c
> > @@ -1,12 +1,9 @@
> > +// SPDX-License-Identifier: GPL-2.0
> >  /*
> >   * Driver for ADC module on the Cirrus Logic EP93xx series of SoCs
> >   *
> >   * Copyright (C) 2015 Alexander Sverdlin
> >   *
> > - * This program is free software; you can redistribute it and/or modify
> > - * it under the terms of the GNU General Public License version 2 as
> > - * published by the Free Software Foundation.
> > - *
> >   * The driver uses polling to get the conversion status. According to 
> > EP93xx
> >   * datasheets, reading ADCResult register starts the conversion, but user 
> > is also
> >   * responsible for ensuring that delay 

Re: [PATCH] iio: adc: Replace license text w/ SPDX identifier

[Matheus Tavares Bernardino]

Thanks for the review! I will fix those places.

Matheus

On Wed, Nov 28, 2018 at 5:29 PM Alexandre Belloni
 wrote:
>
> Hello,
>
> On 28/11/2018 15:53:12-0200, Matheus Tavares wrote:
> > diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
> > index f10443f92e4c..9f9c186e2135 100644
> > --- a/drivers/iio/adc/exynos_adc.c
> > +++ b/drivers/iio/adc/exynos_adc.c
> > @@ -1,3 +1,4 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> >  /*
> >   *  exynos_adc.c - Support for ADC in EXYNOS SoCs
> >   *
> > @@ -5,16 +6,6 @@
> >   *
> >   *  Copyright (C) 2013 Naveen Krishna Chatradhi 
> >   *
> > - *  This program is free software; you can redistribute it and/or modify
> > - *  it under the terms of the GNU General Public License as published by
> > - *  the Free Software Foundation; either version 2 of the License, or
> > - *  (at your option) any later version.
> > - *
> > - *  This program is distributed in the hope that it will be useful,
> > - *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> > - *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > - *  GNU General Public License for more details.
> > - *
> >   *  You should have received a copy of the GNU General Public License
> >   *  along with this program; if not, write to the Free Software
> >   *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
>
> You forgot to remove some of the boilerplate here.
>
> > diff --git a/drivers/iio/adc/hx711.c b/drivers/iio/adc/hx711.c
> > index 36b59d8957fb..894d49843854 100644
> > --- a/drivers/iio/adc/hx711.c
> > +++ b/drivers/iio/adc/hx711.c
> > @@ -1,13 +1,9 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> >  /*
> >   * HX711: analog to digital converter for weight sensor module
> >   *
> >   * Copyright (c) 2016 Andreas Klinger 
> >   *
> > - * This program is free software; you can redistribute it and/or modify
> > - * it under the terms of the GNU General Public License as published by
> > - * the Free Software Foundation; either version 2 of the License, or
> > - * (at your option) any later version.
> > - *
> >   * This program is distributed in the hope that it will be useful,
> >   * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >   * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>
> and here
>
> > diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> > index 729becb2d3d9..f5299c9bdbde 100644
> > --- a/drivers/iio/adc/meson_saradc.c
> > +++ b/drivers/iio/adc/meson_saradc.c
> > @@ -1,12 +1,9 @@
> > +// SPDX-License-Identifier: GPL-2.0
> >  /*
> >   * Amlogic Meson Successive Approximation Register (SAR) A/D Converter
> >   *
> >   * Copyright (C) 2017 Martin Blumenstingl 
> > 
> >   *
> > - * This program is free software; you can redistribute it and/or modify
> > - * it under the terms of the GNU General Public License version 2 as
> > - * published by the Free Software Foundation.
> > - *
> >   * You should have received a copy of the GNU General Public License
> >   * along with this program. If not, see .
>
> and here.
>
> >   */
>
> --
> Alexandre Belloni, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>
> --
> You received this message because you are subscribed to the Google Groups 
> "Kernel USP" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to kernel-usp+unsubscr...@googlegroups.com.
> To post to this group, send email to kernel-...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/kernel-usp/20181128192901.GG8952%40piout.net.
> For more options, visit https://groups.google.com/d/optout.

Re: [PATCH] iio: adc: Replace license text w/ SPDX identifier

[Matheus Tavares Bernardino]

Thanks for the review! I will fix those places.

Matheus

On Wed, Nov 28, 2018 at 5:29 PM Alexandre Belloni
 wrote:
>
> Hello,
>
> On 28/11/2018 15:53:12-0200, Matheus Tavares wrote:
> > diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
> > index f10443f92e4c..9f9c186e2135 100644
> > --- a/drivers/iio/adc/exynos_adc.c
> > +++ b/drivers/iio/adc/exynos_adc.c
> > @@ -1,3 +1,4 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> >  /*
> >   *  exynos_adc.c - Support for ADC in EXYNOS SoCs
> >   *
> > @@ -5,16 +6,6 @@
> >   *
> >   *  Copyright (C) 2013 Naveen Krishna Chatradhi 
> >   *
> > - *  This program is free software; you can redistribute it and/or modify
> > - *  it under the terms of the GNU General Public License as published by
> > - *  the Free Software Foundation; either version 2 of the License, or
> > - *  (at your option) any later version.
> > - *
> > - *  This program is distributed in the hope that it will be useful,
> > - *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> > - *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > - *  GNU General Public License for more details.
> > - *
> >   *  You should have received a copy of the GNU General Public License
> >   *  along with this program; if not, write to the Free Software
> >   *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
>
> You forgot to remove some of the boilerplate here.
>
> > diff --git a/drivers/iio/adc/hx711.c b/drivers/iio/adc/hx711.c
> > index 36b59d8957fb..894d49843854 100644
> > --- a/drivers/iio/adc/hx711.c
> > +++ b/drivers/iio/adc/hx711.c
> > @@ -1,13 +1,9 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> >  /*
> >   * HX711: analog to digital converter for weight sensor module
> >   *
> >   * Copyright (c) 2016 Andreas Klinger 
> >   *
> > - * This program is free software; you can redistribute it and/or modify
> > - * it under the terms of the GNU General Public License as published by
> > - * the Free Software Foundation; either version 2 of the License, or
> > - * (at your option) any later version.
> > - *
> >   * This program is distributed in the hope that it will be useful,
> >   * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >   * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>
> and here
>
> > diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> > index 729becb2d3d9..f5299c9bdbde 100644
> > --- a/drivers/iio/adc/meson_saradc.c
> > +++ b/drivers/iio/adc/meson_saradc.c
> > @@ -1,12 +1,9 @@
> > +// SPDX-License-Identifier: GPL-2.0
> >  /*
> >   * Amlogic Meson Successive Approximation Register (SAR) A/D Converter
> >   *
> >   * Copyright (C) 2017 Martin Blumenstingl 
> > 
> >   *
> > - * This program is free software; you can redistribute it and/or modify
> > - * it under the terms of the GNU General Public License version 2 as
> > - * published by the Free Software Foundation.
> > - *
> >   * You should have received a copy of the GNU General Public License
> >   * along with this program. If not, see .
>
> and here.
>
> >   */
>
> --
> Alexandre Belloni, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>
> --
> You received this message because you are subscribed to the Google Groups 
> "Kernel USP" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to kernel-usp+unsubscr...@googlegroups.com.
> To post to this group, send email to kernel-...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/kernel-usp/20181128192901.GG8952%40piout.net.
> For more options, visit https://groups.google.com/d/optout.

Re: [PATCH] iio: adc: Replace license text w/ SPDX identifier

[Matheus Tavares Bernardino]

On Wed, Nov 28, 2018 at 4:54 PM Scott Branden
 wrote:
>>
>>
>> On 2018-11-28 9:53 a.m., Matheus Tavares wrote:
>>
>> From: Lucas Santos 
>>
>> This patch removes all license boilerplate texts from the .c and .h
>>
> I know other people don't follow the patch guidelines,
>
> but more info on proper commit message format in link.
>
> (Don't say "this patch" or "this commit" or "I changed") - use imperative 
> present tense.
>
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
>

Ok, thanks for the review. I'll correct that in v2.

Matheus

Re: [PATCH] iio: adc: Replace license text w/ SPDX identifier

[Matheus Tavares Bernardino]

On Wed, Nov 28, 2018 at 4:54 PM Scott Branden
 wrote:
>>
>>
>> On 2018-11-28 9:53 a.m., Matheus Tavares wrote:
>>
>> From: Lucas Santos 
>>
>> This patch removes all license boilerplate texts from the .c and .h
>>
> I know other people don't follow the patch guidelines,
>
> but more info on proper commit message format in link.
>
> (Don't say "this patch" or "this commit" or "I changed") - use imperative 
> present tense.
>
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
>

Ok, thanks for the review. I'll correct that in v2.

Matheus

Re: [PATCH v2 1/7] staging:iio:ad2s90: Add device tree support

[Matheus Tavares Bernardino]

On Mon, Nov 19, 2018 at 6:09 AM Ardelean, Alexandru
 wrote:
>
> On Sun, 2018-11-18 at 02:25 -0200, Matheus Tavares wrote:
> > This patch adds device tree support to ad2s90 with standard
> > device tree id table.
> >
>
> Hey,
>
> Comment inline
>
> > Signed-off-by: Matheus Tavares 
> > ---
> > Changes in v2:
> >  - none
> >
> >  drivers/staging/iio/resolver/ad2s90.c | 7 +++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/staging/iio/resolver/ad2s90.c
> > b/drivers/staging/iio/resolver/ad2s90.c
> > index 3e257ac46f7a..6ffbac66b837 100644
> > --- a/drivers/staging/iio/resolver/ad2s90.c
> > +++ b/drivers/staging/iio/resolver/ad2s90.c
> > @@ -107,6 +107,12 @@ static int ad2s90_probe(struct spi_device *spi)
> >   return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
> >  }
> >
> > +static const struct of_device_id ad2s90_of_match[] = {
> > + { .compatible = "adi,ad2s90", },
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(of, ad2s90_of_match);
> > +
> >  static const struct spi_device_id ad2s90_id[] = {
> >   { "ad2s90" },
> >   {}
> > @@ -116,6 +122,7 @@ MODULE_DEVICE_TABLE(spi, ad2s90_id);
> >  static struct spi_driver ad2s90_driver = {
> >   .driver = {
> >   .name = "ad2s90",
> > + .of_match_table = of_match_ptr(ad2s90_of_match),
>
> I think you need to remove the of_match_ptr().
> There was a comment from Jonathan on another thread about this.
> See:
>https://patchwork.kernel.org/patch/10682963/
>

Hm, got it, thanks!

I don't understand much about ACPI yet, and I had understood the
"of_match_ptr" as a guard. Could someone point me in which cases it
should be used? Or is it obsolete?

Matheus

> So,
> +   .of_match_table = of_match_ptr(ad2s90_of_match),
>
> becomes
> > + .of_match_table = ad2s90_of_match,
>
> >   },
> >   .probe = ad2s90_probe,
> >   .id_table = ad2s90_id,
>
> --
> You received this message because you are subscribed to the Google Groups 
> "Kernel USP" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to kernel-usp+unsubscr...@googlegroups.com.
> To post to this group, send email to kernel-...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/kernel-usp/f250fa3a01b51d59979e7a2e3e42cc34d02aa52e.camel%40analog.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [PATCH v2 1/7] staging:iio:ad2s90: Add device tree support

[Matheus Tavares Bernardino]

On Mon, Nov 19, 2018 at 6:09 AM Ardelean, Alexandru
 wrote:
>
> On Sun, 2018-11-18 at 02:25 -0200, Matheus Tavares wrote:
> > This patch adds device tree support to ad2s90 with standard
> > device tree id table.
> >
>
> Hey,
>
> Comment inline
>
> > Signed-off-by: Matheus Tavares 
> > ---
> > Changes in v2:
> >  - none
> >
> >  drivers/staging/iio/resolver/ad2s90.c | 7 +++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/staging/iio/resolver/ad2s90.c
> > b/drivers/staging/iio/resolver/ad2s90.c
> > index 3e257ac46f7a..6ffbac66b837 100644
> > --- a/drivers/staging/iio/resolver/ad2s90.c
> > +++ b/drivers/staging/iio/resolver/ad2s90.c
> > @@ -107,6 +107,12 @@ static int ad2s90_probe(struct spi_device *spi)
> >   return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
> >  }
> >
> > +static const struct of_device_id ad2s90_of_match[] = {
> > + { .compatible = "adi,ad2s90", },
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(of, ad2s90_of_match);
> > +
> >  static const struct spi_device_id ad2s90_id[] = {
> >   { "ad2s90" },
> >   {}
> > @@ -116,6 +122,7 @@ MODULE_DEVICE_TABLE(spi, ad2s90_id);
> >  static struct spi_driver ad2s90_driver = {
> >   .driver = {
> >   .name = "ad2s90",
> > + .of_match_table = of_match_ptr(ad2s90_of_match),
>
> I think you need to remove the of_match_ptr().
> There was a comment from Jonathan on another thread about this.
> See:
>https://patchwork.kernel.org/patch/10682963/
>

Hm, got it, thanks!

I don't understand much about ACPI yet, and I had understood the
"of_match_ptr" as a guard. Could someone point me in which cases it
should be used? Or is it obsolete?

Matheus

> So,
> +   .of_match_table = of_match_ptr(ad2s90_of_match),
>
> becomes
> > + .of_match_table = ad2s90_of_match,
>
> >   },
> >   .probe = ad2s90_probe,
> >   .id_table = ad2s90_id,
>
> --
> You received this message because you are subscribed to the Google Groups 
> "Kernel USP" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to kernel-usp+unsubscr...@googlegroups.com.
> To post to this group, send email to kernel-...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/kernel-usp/f250fa3a01b51d59979e7a2e3e42cc34d02aa52e.camel%40analog.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [PATCH v2 4/7] dt-bindings:iio:resolver: Add docs for ad2s90

[Matheus Tavares Bernardino]

On Mon, Nov 19, 2018 at 6:22 AM Ardelean, Alexandru
 wrote:
>
> On Sun, 2018-11-18 at 02:25 -0200, Matheus Tavares wrote:
> > This patch adds the device tree binding documentation for the ad2s90
> > resolver-to-digital converter.
> >
>
> One minor comment inline.
>
> > Signed-off-by: Matheus Tavares 
> > ---
> > Changes in v2:
> >  - Rewritten 'spi-cpol and spi-cpha' item to say that the device can
> >  work in either mode (0,0) or (1,1) and explain how they should be
> >  specified in DT.
> >
> >  .../bindings/iio/resolver/ad2s90.txt  | 28 +++
> >  1 file changed, 28 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
> >
> > diff --git a/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
> > b/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
> > new file mode 100644
> > index ..594417539938
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
> > @@ -0,0 +1,28 @@
> > +Analog Devices AD2S90 Resolver-to-Digital Converter
> > +
> > +https://www.analog.com/en/products/ad2s90.html
> > +
> > +Required properties:
> > +  - compatible: should be "adi,ad2s90"
> > +  - reg: SPI chip select number for the device
> > +  - spi-max-frequency: set maximum clock frequency, must be 83
> > +  - spi-cpol and spi-cpha:
> > +Either SPI mode (0,0) or (1,1) must be used, so specify none or
> > both of
> > +spi-cpha, spi-cpol.
> For SPI properties it's a good idea to also reference the document for SPI
> bindings.
> Something like:
> See for more details:
>  Documentation/devicetree/bindings/spi/spi-bus.txt
>

Thanks, Alex! I'll add that for v3.

Also, can you confirm AD2S90 works in both spi mode 0 and 3? It's not
explicitly stated in the datasheet, but that's what it seemed to me
and some colleagues.

Thanks,
Matheus

> > +
> > +Note about max frequency:
> > +Chip's max frequency, as specified in its datasheet, is 2Mhz. But a
> > 600ns
> > +delay is expected between the application of a logic LO to CS and
> > the
> > +application of SCLK, as also specified. And since the delay is not
> > +implemented in the spi code, to satisfy it, SCLK's period should be
> > at most
> > +2 * 600ns, so the max frequency should be 1 / (2 * 6e-7), which
> > gives
> > +roughly 83Hz.
> > +
> > +Example:
> > +resolver@0 {
> > + compatible = "adi,ad2s90";
> > + reg = <0>;
> > + spi-max-frequency = <83>;
> > + spi-cpol;
> > + spi-cpha;
> > +};
>
> --
> You received this message because you are subscribed to the Google Groups 
> "Kernel USP" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to kernel-usp+unsubscr...@googlegroups.com.
> To post to this group, send email to kernel-...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/kernel-usp/563614e00314ba92b9513645a82fde06504a42d5.camel%40analog.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [PATCH v2 4/7] dt-bindings:iio:resolver: Add docs for ad2s90

[Matheus Tavares Bernardino]

On Mon, Nov 19, 2018 at 6:22 AM Ardelean, Alexandru
 wrote:
>
> On Sun, 2018-11-18 at 02:25 -0200, Matheus Tavares wrote:
> > This patch adds the device tree binding documentation for the ad2s90
> > resolver-to-digital converter.
> >
>
> One minor comment inline.
>
> > Signed-off-by: Matheus Tavares 
> > ---
> > Changes in v2:
> >  - Rewritten 'spi-cpol and spi-cpha' item to say that the device can
> >  work in either mode (0,0) or (1,1) and explain how they should be
> >  specified in DT.
> >
> >  .../bindings/iio/resolver/ad2s90.txt  | 28 +++
> >  1 file changed, 28 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
> >
> > diff --git a/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
> > b/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
> > new file mode 100644
> > index ..594417539938
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
> > @@ -0,0 +1,28 @@
> > +Analog Devices AD2S90 Resolver-to-Digital Converter
> > +
> > +https://www.analog.com/en/products/ad2s90.html
> > +
> > +Required properties:
> > +  - compatible: should be "adi,ad2s90"
> > +  - reg: SPI chip select number for the device
> > +  - spi-max-frequency: set maximum clock frequency, must be 83
> > +  - spi-cpol and spi-cpha:
> > +Either SPI mode (0,0) or (1,1) must be used, so specify none or
> > both of
> > +spi-cpha, spi-cpol.
> For SPI properties it's a good idea to also reference the document for SPI
> bindings.
> Something like:
> See for more details:
>  Documentation/devicetree/bindings/spi/spi-bus.txt
>

Thanks, Alex! I'll add that for v3.

Also, can you confirm AD2S90 works in both spi mode 0 and 3? It's not
explicitly stated in the datasheet, but that's what it seemed to me
and some colleagues.

Thanks,
Matheus

> > +
> > +Note about max frequency:
> > +Chip's max frequency, as specified in its datasheet, is 2Mhz. But a
> > 600ns
> > +delay is expected between the application of a logic LO to CS and
> > the
> > +application of SCLK, as also specified. And since the delay is not
> > +implemented in the spi code, to satisfy it, SCLK's period should be
> > at most
> > +2 * 600ns, so the max frequency should be 1 / (2 * 6e-7), which
> > gives
> > +roughly 83Hz.
> > +
> > +Example:
> > +resolver@0 {
> > + compatible = "adi,ad2s90";
> > + reg = <0>;
> > + spi-max-frequency = <83>;
> > + spi-cpol;
> > + spi-cpha;
> > +};
>
> --
> You received this message because you are subscribed to the Google Groups 
> "Kernel USP" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to kernel-usp+unsubscr...@googlegroups.com.
> To post to this group, send email to kernel-...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/kernel-usp/563614e00314ba92b9513645a82fde06504a42d5.camel%40analog.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 4/6] dt-bindings:iio:resolver: Add docs for ad2s90

[Matheus Tavares Bernardino]

On Sun, Nov 11, 2018 at 9:48 AM Jonathan Cameron  wrote:
>
> On Fri,  9 Nov 2018 20:00:42 -0200
> Matheus Tavares  wrote:
>
> > This patch adds the device tree binding documentation for the ad2s90
> > resolver-to-digital converter.
> >
> > Signed-off-by: Matheus Tavares 
> > ---
> >  .../bindings/iio/resolver/ad2s90.txt  | 26 +++
> >  1 file changed, 26 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
> >
> > diff --git a/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt 
> > b/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
> > new file mode 100644
> > index ..b42cc7752ffd
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
> > @@ -0,0 +1,26 @@
> > +Analog Devices AD2S90 Resolver-to-Digital Converter
> > +
> > +https://www.analog.com/en/products/ad2s90.html
> > +
> > +Required properties:
> > +  - compatible : should be "adi,ad2s90"
> > +  - reg : SPI chip select number for the device
> > +  - spi-max-frequency : set maximum clock frequency, must be 83
> > +  - spi-cpol and spi-cpha : must be defined to enable SPI mode 3
>
> As the part only works in mode 3, my gut feeling is that this belongs
> in the driver, not here.  Rob, what do you think?
>

For this patch, I assumed the part only worked in mode 3 based on the
driver's code that set this at probe. But today I carefully looked for
it at the datasheet and now I'm unsure. It is never said, explicitly,
which SPI mode ad2s90 works with. But looking at the diagram that
shows the expected pins signals at each communication moment, it seems
to me that this chip can either work in mode 0 (CPOL=0, CPHA=0) or
mode 3 (CPOL=1, CPHA=1). Could someone help me to confirm this? And if
that is the case, them the SPI mode setting should be left in DT, as
adc/mcp320x and dac/ti-dac082s085 do, right?

Also, when I thought that ad2s90 only worked in mode 3, I wrote this
patch based on the dt-binding docs for the adxl345 accelerometer,
which only works in mode 3 but lets this setting to DT not in the
driver. Do you think, perhaps, it is wrong in adxl345, them?

Thanks,
Matheus.

> > +
> > +Note about max frequency:
> > +Chip's max frequency, as specified in its datasheet, is 2Mhz. But a 
> > 600ns
> > +delay is expected between the application of a logic LO to CS and the
> > +application of SCLK, as also specified. And since the delay is not
> > +implemented in the spi code, to satisfy it, SCLK's period should be at 
> > most
> > +2 * 600ns, so the max frequency should be 1 / (2 * 6e-7), which gives
> > +roughly 83Hz.
> > +
> > +Example:
> > +resolver@0 {
> > + compatible = "adi,ad2s90";
> > + reg = <0>;
> > + spi-max-frequency = <83>;
> > + spi-cpol;
> > + spi-cpha;
> > +};
>

Re: [PATCH 4/6] dt-bindings:iio:resolver: Add docs for ad2s90

[Matheus Tavares Bernardino]

On Sun, Nov 11, 2018 at 9:48 AM Jonathan Cameron  wrote:
>
> On Fri,  9 Nov 2018 20:00:42 -0200
> Matheus Tavares  wrote:
>
> > This patch adds the device tree binding documentation for the ad2s90
> > resolver-to-digital converter.
> >
> > Signed-off-by: Matheus Tavares 
> > ---
> >  .../bindings/iio/resolver/ad2s90.txt  | 26 +++
> >  1 file changed, 26 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
> >
> > diff --git a/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt 
> > b/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
> > new file mode 100644
> > index ..b42cc7752ffd
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
> > @@ -0,0 +1,26 @@
> > +Analog Devices AD2S90 Resolver-to-Digital Converter
> > +
> > +https://www.analog.com/en/products/ad2s90.html
> > +
> > +Required properties:
> > +  - compatible : should be "adi,ad2s90"
> > +  - reg : SPI chip select number for the device
> > +  - spi-max-frequency : set maximum clock frequency, must be 83
> > +  - spi-cpol and spi-cpha : must be defined to enable SPI mode 3
>
> As the part only works in mode 3, my gut feeling is that this belongs
> in the driver, not here.  Rob, what do you think?
>

For this patch, I assumed the part only worked in mode 3 based on the
driver's code that set this at probe. But today I carefully looked for
it at the datasheet and now I'm unsure. It is never said, explicitly,
which SPI mode ad2s90 works with. But looking at the diagram that
shows the expected pins signals at each communication moment, it seems
to me that this chip can either work in mode 0 (CPOL=0, CPHA=0) or
mode 3 (CPOL=1, CPHA=1). Could someone help me to confirm this? And if
that is the case, them the SPI mode setting should be left in DT, as
adc/mcp320x and dac/ti-dac082s085 do, right?

Also, when I thought that ad2s90 only worked in mode 3, I wrote this
patch based on the dt-binding docs for the adxl345 accelerometer,
which only works in mode 3 but lets this setting to DT not in the
driver. Do you think, perhaps, it is wrong in adxl345, them?

Thanks,
Matheus.

> > +
> > +Note about max frequency:
> > +Chip's max frequency, as specified in its datasheet, is 2Mhz. But a 
> > 600ns
> > +delay is expected between the application of a logic LO to CS and the
> > +application of SCLK, as also specified. And since the delay is not
> > +implemented in the spi code, to satisfy it, SCLK's period should be at 
> > most
> > +2 * 600ns, so the max frequency should be 1 / (2 * 6e-7), which gives
> > +roughly 83Hz.
> > +
> > +Example:
> > +resolver@0 {
> > + compatible = "adi,ad2s90";
> > + reg = <0>;
> > + spi-max-frequency = <83>;
> > + spi-cpol;
> > + spi-cpha;
> > +};
>

Re: [PATCH 2/6] staging:iio:ad2s90: Remove spi setup that should be done via dt

[Matheus Tavares Bernardino]

On Sun, Nov 11, 2018 at 9:42 AM Jonathan Cameron  wrote:
>
> On Fri,  9 Nov 2018 20:00:40 -0200
> Matheus Tavares  wrote:
>
> > The ad2s90 driver currently sets some spi settings (max_speed_hz and
> > mode) at ad2s90_probe. This should, instead, be handled via device tree.
> > This patch removes these configurations from the probe function.
> >
> > Note: The way in which the mentioned spi settings need to be specified
> > on the ad2s90's node of a device tree will be documented in the future
> > patch "dt-bindings:iio:resolver: Add docs for ad2s90".
> >
> > Signed-off-by: Matheus Tavares 
> I'd actually like to get Rob and Mark's views on this one.  Previously
> I would just have applied it without thinking on the basis these can
> be easily specified from devicetree.
>
> Recent discussions with Rob have suggested that the settings in devicetree
> should perhaps be concerned with specifying constraints about the device
> that are not visible to the driver.  The driver itself should apply
> the device constraints, but there are others such as wiring that
> might reduce the maximum frequency for example...
>
> So should a driver be clamping an over specified value from DT for
> example?  Or given that is optional in DT, should it be making sure
> that a controller max frequency isn't too high for the sensor?
>

First of all, thanks for the review and comments.

By what you've said here and in the reviews for patches 3 and 4 of
this patch-set, it seems to me that the most reasonable thing would be
to keep the SPI mode 3 settings at the driver but the max frequency
setting at DT and check if it exceeds the maximum at the driver (as
patch 3 does). This makes sense to me, based on what you've said,
because mode 3 is a device constraint visible to the driver (as it
won't change) but max frequency is not (because of things such as
wiring, as you said).

What do you think, Jonathan, Rob, and Mark?

Matheus

> It seems to be unusual to do this, but to my mind it would make
> sense and might be worth pushing out into more drivers.
>
> Jonathan
> > ---
> >  drivers/staging/iio/resolver/ad2s90.c | 11 ---
> >  1 file changed, 11 deletions(-)
> >
> > diff --git a/drivers/staging/iio/resolver/ad2s90.c 
> > b/drivers/staging/iio/resolver/ad2s90.c
> > index ff32ca76ca00..95c118c48400 100644
> > --- a/drivers/staging/iio/resolver/ad2s90.c
> > +++ b/drivers/staging/iio/resolver/ad2s90.c
> > @@ -77,7 +77,6 @@ static int ad2s90_probe(struct spi_device *spi)
> >  {
> >   struct iio_dev *indio_dev;
> >   struct ad2s90_state *st;
> > - int ret;
> >
> >   indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
> >   if (!indio_dev)
> > @@ -94,16 +93,6 @@ static int ad2s90_probe(struct spi_device *spi)
> >   indio_dev->num_channels = 1;
> >   indio_dev->name = spi_get_device_id(spi)->name;
> >
> > - /* need 600ns between CS and the first falling edge of SCLK */
> > - spi->max_speed_hz = 83;
> > - spi->mode = SPI_MODE_3;
> > - ret = spi_setup(spi);
> > -
> > - if (ret < 0) {
> > - dev_err(>dev, "spi_setup failed!\n");
> > - return ret;
> > - }
> > -
> >   return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
> >  }
> >
>

Re: [PATCH 2/6] staging:iio:ad2s90: Remove spi setup that should be done via dt

[Matheus Tavares Bernardino]

On Sun, Nov 11, 2018 at 9:42 AM Jonathan Cameron  wrote:
>
> On Fri,  9 Nov 2018 20:00:40 -0200
> Matheus Tavares  wrote:
>
> > The ad2s90 driver currently sets some spi settings (max_speed_hz and
> > mode) at ad2s90_probe. This should, instead, be handled via device tree.
> > This patch removes these configurations from the probe function.
> >
> > Note: The way in which the mentioned spi settings need to be specified
> > on the ad2s90's node of a device tree will be documented in the future
> > patch "dt-bindings:iio:resolver: Add docs for ad2s90".
> >
> > Signed-off-by: Matheus Tavares 
> I'd actually like to get Rob and Mark's views on this one.  Previously
> I would just have applied it without thinking on the basis these can
> be easily specified from devicetree.
>
> Recent discussions with Rob have suggested that the settings in devicetree
> should perhaps be concerned with specifying constraints about the device
> that are not visible to the driver.  The driver itself should apply
> the device constraints, but there are others such as wiring that
> might reduce the maximum frequency for example...
>
> So should a driver be clamping an over specified value from DT for
> example?  Or given that is optional in DT, should it be making sure
> that a controller max frequency isn't too high for the sensor?
>

First of all, thanks for the review and comments.

By what you've said here and in the reviews for patches 3 and 4 of
this patch-set, it seems to me that the most reasonable thing would be
to keep the SPI mode 3 settings at the driver but the max frequency
setting at DT and check if it exceeds the maximum at the driver (as
patch 3 does). This makes sense to me, based on what you've said,
because mode 3 is a device constraint visible to the driver (as it
won't change) but max frequency is not (because of things such as
wiring, as you said).

What do you think, Jonathan, Rob, and Mark?

Matheus

> It seems to be unusual to do this, but to my mind it would make
> sense and might be worth pushing out into more drivers.
>
> Jonathan
> > ---
> >  drivers/staging/iio/resolver/ad2s90.c | 11 ---
> >  1 file changed, 11 deletions(-)
> >
> > diff --git a/drivers/staging/iio/resolver/ad2s90.c 
> > b/drivers/staging/iio/resolver/ad2s90.c
> > index ff32ca76ca00..95c118c48400 100644
> > --- a/drivers/staging/iio/resolver/ad2s90.c
> > +++ b/drivers/staging/iio/resolver/ad2s90.c
> > @@ -77,7 +77,6 @@ static int ad2s90_probe(struct spi_device *spi)
> >  {
> >   struct iio_dev *indio_dev;
> >   struct ad2s90_state *st;
> > - int ret;
> >
> >   indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
> >   if (!indio_dev)
> > @@ -94,16 +93,6 @@ static int ad2s90_probe(struct spi_device *spi)
> >   indio_dev->num_channels = 1;
> >   indio_dev->name = spi_get_device_id(spi)->name;
> >
> > - /* need 600ns between CS and the first falling edge of SCLK */
> > - spi->max_speed_hz = 83;
> > - spi->mode = SPI_MODE_3;
> > - ret = spi_setup(spi);
> > -
> > - if (ret < 0) {
> > - dev_err(>dev, "spi_setup failed!\n");
> > - return ret;
> > - }
> > -
> >   return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
> >  }
> >
>

Re: [PATCH 5/6] staging:iio:ad2s90: Add SPDX license identifier

[Matheus Tavares Bernardino]

On Sat, Nov 10, 2018 at 11:23 AM Fabio Estevam  wrote:>
> Hi Matheus,
>
> On Fri, Nov 9, 2018 at 10:27 PM Matheus Tavares Bernardino
>  wrote:
>
> > Got it, thanks for the explanation! I'll correct this in v2.
>
> One more suggestion: in v2 you could also consider to remove the legal
> text that says GPL v2, as you are adding the SPDX tag.

Okay, I'll do it! Thanks again for the review and suggestions!

Matheus

Re: [PATCH 5/6] staging:iio:ad2s90: Add SPDX license identifier

[Matheus Tavares Bernardino]

On Sat, Nov 10, 2018 at 11:23 AM Fabio Estevam  wrote:>
> Hi Matheus,
>
> On Fri, Nov 9, 2018 at 10:27 PM Matheus Tavares Bernardino
>  wrote:
>
> > Got it, thanks for the explanation! I'll correct this in v2.
>
> One more suggestion: in v2 you could also consider to remove the legal
> text that says GPL v2, as you are adding the SPDX tag.

Okay, I'll do it! Thanks again for the review and suggestions!

Matheus

Re: [PATCH 5/6] staging:iio:ad2s90: Add SPDX license identifier

[Matheus Tavares Bernardino]

On Fri, Nov 9, 2018 at 10:20 PM Greg Kroah-Hartman
 wrote:
>
> On Fri, Nov 09, 2018 at 09:19:52PM -0200, Matheus Tavares Bernardino wrote:
> > On Fri, Nov 9, 2018 at 8:13 PM Fabio Estevam  wrote:
> > >
> > > Hi Matheus,
> > >
> >
> > Hi, Fabio
> >
> > > On Fri, Nov 9, 2018 at 8:01 PM Matheus Tavares
> > >  wrote:
> > > >
> > > > This patch adds the SPDX GPL-2.0-only license identifier to ad2s90.c,
> > > > which solves the checkpatch.pl warning:
> > > > "WARNING: Missing or malformed SPDX-License-Identifier tag in line 1".
> > > >
> > > > Signed-off-by: Matheus Tavares 
> > > > ---
> > > >  drivers/staging/iio/resolver/ad2s90.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/drivers/staging/iio/resolver/ad2s90.c 
> > > > b/drivers/staging/iio/resolver/ad2s90.c
> > > > index 949ff55ac6b0..f439da721df8 100644
> > > > --- a/drivers/staging/iio/resolver/ad2s90.c
> > > > +++ b/drivers/staging/iio/resolver/ad2s90.c
> > > > @@ -1,3 +1,4 @@
> > > > +// SPDX-License-Identifier: GPL-2.0-only
> > >
> > > This should be:
> > > // SPDX-License-Identifier: GPL-2.0
> >
> > Hm, but it seems that the identifier "GPL-2.0" is deprecated, look:
> > https://spdx.org/licenses/GPL-2.0.html. It has been updated to
> > "GPL-2.0-only" in license list v3
> > (https://spdx.org/licenses/GPL-2.0-only.html). Is there some other
> > reason to use the deprecated "GPL-2.0" that I'm not aware of?
>
> Yes, please read the in-kernel documentation for all of this at:
> Documentation/process/license-rules.rst
>
> Long story short, we started the adding of these tags to the kernel
> before the crazyness of the "-only" markings for GPL in spdx.  Let's
> keep it this way for now, if we ever get the whole kernel finished, then
> we can revisit the markings and maybe do a wholesale conversion, if it's
> really needed.
>

Got it, thanks for the explanation! I'll correct this in v2.

Thanks,
Matheus

> thanks,
>
> greg k-h

Re: [PATCH 5/6] staging:iio:ad2s90: Add SPDX license identifier

[Matheus Tavares Bernardino]

On Fri, Nov 9, 2018 at 10:20 PM Greg Kroah-Hartman
 wrote:
>
> On Fri, Nov 09, 2018 at 09:19:52PM -0200, Matheus Tavares Bernardino wrote:
> > On Fri, Nov 9, 2018 at 8:13 PM Fabio Estevam  wrote:
> > >
> > > Hi Matheus,
> > >
> >
> > Hi, Fabio
> >
> > > On Fri, Nov 9, 2018 at 8:01 PM Matheus Tavares
> > >  wrote:
> > > >
> > > > This patch adds the SPDX GPL-2.0-only license identifier to ad2s90.c,
> > > > which solves the checkpatch.pl warning:
> > > > "WARNING: Missing or malformed SPDX-License-Identifier tag in line 1".
> > > >
> > > > Signed-off-by: Matheus Tavares 
> > > > ---
> > > >  drivers/staging/iio/resolver/ad2s90.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/drivers/staging/iio/resolver/ad2s90.c 
> > > > b/drivers/staging/iio/resolver/ad2s90.c
> > > > index 949ff55ac6b0..f439da721df8 100644
> > > > --- a/drivers/staging/iio/resolver/ad2s90.c
> > > > +++ b/drivers/staging/iio/resolver/ad2s90.c
> > > > @@ -1,3 +1,4 @@
> > > > +// SPDX-License-Identifier: GPL-2.0-only
> > >
> > > This should be:
> > > // SPDX-License-Identifier: GPL-2.0
> >
> > Hm, but it seems that the identifier "GPL-2.0" is deprecated, look:
> > https://spdx.org/licenses/GPL-2.0.html. It has been updated to
> > "GPL-2.0-only" in license list v3
> > (https://spdx.org/licenses/GPL-2.0-only.html). Is there some other
> > reason to use the deprecated "GPL-2.0" that I'm not aware of?
>
> Yes, please read the in-kernel documentation for all of this at:
> Documentation/process/license-rules.rst
>
> Long story short, we started the adding of these tags to the kernel
> before the crazyness of the "-only" markings for GPL in spdx.  Let's
> keep it this way for now, if we ever get the whole kernel finished, then
> we can revisit the markings and maybe do a wholesale conversion, if it's
> really needed.
>

Got it, thanks for the explanation! I'll correct this in v2.

Thanks,
Matheus

> thanks,
>
> greg k-h

Re: [PATCH 5/6] staging:iio:ad2s90: Add SPDX license identifier

[Matheus Tavares Bernardino]

On Fri, Nov 9, 2018 at 8:13 PM Fabio Estevam  wrote:
>
> Hi Matheus,
>

Hi, Fabio

> On Fri, Nov 9, 2018 at 8:01 PM Matheus Tavares
>  wrote:
> >
> > This patch adds the SPDX GPL-2.0-only license identifier to ad2s90.c,
> > which solves the checkpatch.pl warning:
> > "WARNING: Missing or malformed SPDX-License-Identifier tag in line 1".
> >
> > Signed-off-by: Matheus Tavares 
> > ---
> >  drivers/staging/iio/resolver/ad2s90.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/staging/iio/resolver/ad2s90.c 
> > b/drivers/staging/iio/resolver/ad2s90.c
> > index 949ff55ac6b0..f439da721df8 100644
> > --- a/drivers/staging/iio/resolver/ad2s90.c
> > +++ b/drivers/staging/iio/resolver/ad2s90.c
> > @@ -1,3 +1,4 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
>
> This should be:
> // SPDX-License-Identifier: GPL-2.0

Hm, but it seems that the identifier "GPL-2.0" is deprecated, look:
https://spdx.org/licenses/GPL-2.0.html. It has been updated to
"GPL-2.0-only" in license list v3
(https://spdx.org/licenses/GPL-2.0-only.html). Is there some other
reason to use the deprecated "GPL-2.0" that I'm not aware of?

Thanks,
Matheus

Re: [PATCH 5/6] staging:iio:ad2s90: Add SPDX license identifier

[Matheus Tavares Bernardino]

On Fri, Nov 9, 2018 at 8:13 PM Fabio Estevam  wrote:
>
> Hi Matheus,
>

Hi, Fabio

> On Fri, Nov 9, 2018 at 8:01 PM Matheus Tavares
>  wrote:
> >
> > This patch adds the SPDX GPL-2.0-only license identifier to ad2s90.c,
> > which solves the checkpatch.pl warning:
> > "WARNING: Missing or malformed SPDX-License-Identifier tag in line 1".
> >
> > Signed-off-by: Matheus Tavares 
> > ---
> >  drivers/staging/iio/resolver/ad2s90.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/staging/iio/resolver/ad2s90.c 
> > b/drivers/staging/iio/resolver/ad2s90.c
> > index 949ff55ac6b0..f439da721df8 100644
> > --- a/drivers/staging/iio/resolver/ad2s90.c
> > +++ b/drivers/staging/iio/resolver/ad2s90.c
> > @@ -1,3 +1,4 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
>
> This should be:
> // SPDX-License-Identifier: GPL-2.0

Hm, but it seems that the identifier "GPL-2.0" is deprecated, look:
https://spdx.org/licenses/GPL-2.0.html. It has been updated to
"GPL-2.0-only" in license list v3
(https://spdx.org/licenses/GPL-2.0-only.html). Is there some other
reason to use the deprecated "GPL-2.0" that I'm not aware of?

Thanks,
Matheus

Re: [PATCH v2 5/6] staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and read_raw

[Matheus Tavares Bernardino]

On Sun, Oct 28, 2018 at 1:50 PM Jonathan Cameron  wrote:
>
> On Fri, 26 Oct 2018 23:00:04 -0300
> Matheus Tavares  wrote:
>
> > This patch adds the IIO_CHAN_INFO_SCALE mask to ad2s90_chan and
> > implements the relative read behavior at ad2s90_read_raw.
> >
> > Signed-off-by: Victor Colombo 
> > Signed-off-by: Matheus Tavares 
>
> Hi,
>
> A suggestion inline.  This is a common case that we have infrastructure
> to simplify.  + I think your scale factor is very slightly wrong.
>
> Jonathan
>
> > ---
> >  drivers/staging/iio/resolver/ad2s90.c | 32 ++-
> >  1 file changed, 22 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/staging/iio/resolver/ad2s90.c 
> > b/drivers/staging/iio/resolver/ad2s90.c
> > index b4a6a89c11b0..52b656875ed1 100644
> > --- a/drivers/staging/iio/resolver/ad2s90.c
> > +++ b/drivers/staging/iio/resolver/ad2s90.c
> > @@ -34,19 +34,31 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
> >   int ret;
> >   struct ad2s90_state *st = iio_priv(indio_dev);
> >
> > - mutex_lock(>lock);
> > + switch (m) {
> > + case IIO_CHAN_INFO_SCALE:
> > + /* 2 * Pi / (2^12 - 1) ~= 0.001534355 */
> > + *val = 0;
> > + *val2 = 1534355;
> Definitely 2^12 - 1?  That's a bit unusual if true as it would imply
> that 2^12 - 1 and 0 were the same value.
>
> Imagine a smaller version with on 2^2 bits so 0, 1, 2, 3
> Values of each are
>
> 0, M_PI/2, M_PI, 3*M_PI/2
>
> So the multiplier is 2*M_PI/(2^2) not 2*M_PI/(2^2 - 1)
> 1/2 vs 2/3 * M_PI

Oh, that makes a lot of sense! We used 2^12 - 1 here based on driver
drivers/iio/resolver/ad2s1200.c, whose resolution is also 12 bits, as
the ad2s90.c. Do you think this section is, perhaps, wrong on
ad2s1200.c too, or maybe there is some difference between these two
drivers that I didn't catch regarding the resolution?

Matheus.

> Now this is a very common case so we have the return type
> IIO_VAL_FRACTIONAL_LOG2 to give a more obvious and potentially
> more accurate representation.
>
> > + return IIO_VAL_INT_PLUS_NANO;
> > + case IIO_CHAN_INFO_RAW:
> > + mutex_lock(>lock);
> > +
> > + ret = spi_read(st->sdev, st->rx, 2);
> > + if (ret < 0) {
> > + mutex_unlock(>lock);
> > + return ret;
> > + }
> > +
> > + *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
> >
> > - ret = spi_read(st->sdev, st->rx, 2);
> > - if (ret < 0) {
> >   mutex_unlock(>lock);
> > - return ret;
> > - }
> >
> > - *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
> > -
> > - mutex_unlock(>lock);
> > + return IIO_VAL_INT;
> > + default:
> > + break;
> > + }
> >
> > - return IIO_VAL_INT;
> > + return -EINVAL;
> >  }
> >
> >  static const struct iio_info ad2s90_info = {
> > @@ -57,7 +69,7 @@ static const struct iio_chan_spec ad2s90_chan = {
> >   .type = IIO_ANGL,
> >   .indexed = 1,
> >   .channel = 0,
> > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | 
> > BIT(IIO_CHAN_INFO_SCALE),
> >  };
> >
> >  static int ad2s90_probe(struct spi_device *spi)
>

Re: [PATCH v2 5/6] staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and read_raw

[Matheus Tavares Bernardino]

On Sun, Oct 28, 2018 at 1:50 PM Jonathan Cameron  wrote:
>
> On Fri, 26 Oct 2018 23:00:04 -0300
> Matheus Tavares  wrote:
>
> > This patch adds the IIO_CHAN_INFO_SCALE mask to ad2s90_chan and
> > implements the relative read behavior at ad2s90_read_raw.
> >
> > Signed-off-by: Victor Colombo 
> > Signed-off-by: Matheus Tavares 
>
> Hi,
>
> A suggestion inline.  This is a common case that we have infrastructure
> to simplify.  + I think your scale factor is very slightly wrong.
>
> Jonathan
>
> > ---
> >  drivers/staging/iio/resolver/ad2s90.c | 32 ++-
> >  1 file changed, 22 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/staging/iio/resolver/ad2s90.c 
> > b/drivers/staging/iio/resolver/ad2s90.c
> > index b4a6a89c11b0..52b656875ed1 100644
> > --- a/drivers/staging/iio/resolver/ad2s90.c
> > +++ b/drivers/staging/iio/resolver/ad2s90.c
> > @@ -34,19 +34,31 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
> >   int ret;
> >   struct ad2s90_state *st = iio_priv(indio_dev);
> >
> > - mutex_lock(>lock);
> > + switch (m) {
> > + case IIO_CHAN_INFO_SCALE:
> > + /* 2 * Pi / (2^12 - 1) ~= 0.001534355 */
> > + *val = 0;
> > + *val2 = 1534355;
> Definitely 2^12 - 1?  That's a bit unusual if true as it would imply
> that 2^12 - 1 and 0 were the same value.
>
> Imagine a smaller version with on 2^2 bits so 0, 1, 2, 3
> Values of each are
>
> 0, M_PI/2, M_PI, 3*M_PI/2
>
> So the multiplier is 2*M_PI/(2^2) not 2*M_PI/(2^2 - 1)
> 1/2 vs 2/3 * M_PI

Oh, that makes a lot of sense! We used 2^12 - 1 here based on driver
drivers/iio/resolver/ad2s1200.c, whose resolution is also 12 bits, as
the ad2s90.c. Do you think this section is, perhaps, wrong on
ad2s1200.c too, or maybe there is some difference between these two
drivers that I didn't catch regarding the resolution?

Matheus.

> Now this is a very common case so we have the return type
> IIO_VAL_FRACTIONAL_LOG2 to give a more obvious and potentially
> more accurate representation.
>
> > + return IIO_VAL_INT_PLUS_NANO;
> > + case IIO_CHAN_INFO_RAW:
> > + mutex_lock(>lock);
> > +
> > + ret = spi_read(st->sdev, st->rx, 2);
> > + if (ret < 0) {
> > + mutex_unlock(>lock);
> > + return ret;
> > + }
> > +
> > + *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
> >
> > - ret = spi_read(st->sdev, st->rx, 2);
> > - if (ret < 0) {
> >   mutex_unlock(>lock);
> > - return ret;
> > - }
> >
> > - *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
> > -
> > - mutex_unlock(>lock);
> > + return IIO_VAL_INT;
> > + default:
> > + break;
> > + }
> >
> > - return IIO_VAL_INT;
> > + return -EINVAL;
> >  }
> >
> >  static const struct iio_info ad2s90_info = {
> > @@ -57,7 +69,7 @@ static const struct iio_chan_spec ad2s90_chan = {
> >   .type = IIO_ANGL,
> >   .indexed = 1,
> >   .channel = 0,
> > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | 
> > BIT(IIO_CHAN_INFO_SCALE),
> >  };
> >
> >  static int ad2s90_probe(struct spi_device *spi)
>

Re: [PATCH v2 0/6] staging:iio:ad2s90: Add scale info and improve error handling

[Matheus Tavares Bernardino]

On Sun, Oct 28, 2018 at 1:52 PM Jonathan Cameron  wrote:
>
> On Fri, 26 Oct 2018 22:59:59 -0300
> Matheus Tavares  wrote:
>
> > This patch set adds scale info to ad2s90's single channel, improve
> > error handling in it's functions and fix a possible race condition
> > issue.
> >
> > The goal with this patch set is to address the points discussed in the
> > mailing list in an effort to move ad2s90.c out of staging.
> Thanks,
>
> A good series in general.  A few suggested improvements.
> If I haven't commented on a patch, usually it means I'm happy with it
> and will pick it up with the rest of the series.
>
> Jonathan
>

Thanks for the review, Jonathan. We will address the necessary changes in v3!

Matheus

> >
> > Changes in v2:
> >  - Added my S-o-B in patch 5.
> >
> > Matheus Tavares (5):
> >   staging:iio:ad2s90: Make read_raw return spi_read's error code
> >   staging:iio:ad2s90: Make probe handle spi_setup failure
> >   staging:iio:ad2s90: Remove always overwritten assignment
> >   staging:iio:ad2s90: Move device registration to the end of probe
> >   staging:iio:ad2s90: Check channel type at read_raw
> >
> > Victor Colombo (1):
> >   staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and
> > read_raw
> >
> >  drivers/staging/iio/resolver/ad2s90.c | 55 ++-
> >  1 file changed, 37 insertions(+), 18 deletions(-)
> >
>

Re: [PATCH v2 0/6] staging:iio:ad2s90: Add scale info and improve error handling

[Matheus Tavares Bernardino]

On Sun, Oct 28, 2018 at 1:52 PM Jonathan Cameron  wrote:
>
> On Fri, 26 Oct 2018 22:59:59 -0300
> Matheus Tavares  wrote:
>
> > This patch set adds scale info to ad2s90's single channel, improve
> > error handling in it's functions and fix a possible race condition
> > issue.
> >
> > The goal with this patch set is to address the points discussed in the
> > mailing list in an effort to move ad2s90.c out of staging.
> Thanks,
>
> A good series in general.  A few suggested improvements.
> If I haven't commented on a patch, usually it means I'm happy with it
> and will pick it up with the rest of the series.
>
> Jonathan
>

Thanks for the review, Jonathan. We will address the necessary changes in v3!

Matheus

> >
> > Changes in v2:
> >  - Added my S-o-B in patch 5.
> >
> > Matheus Tavares (5):
> >   staging:iio:ad2s90: Make read_raw return spi_read's error code
> >   staging:iio:ad2s90: Make probe handle spi_setup failure
> >   staging:iio:ad2s90: Remove always overwritten assignment
> >   staging:iio:ad2s90: Move device registration to the end of probe
> >   staging:iio:ad2s90: Check channel type at read_raw
> >
> > Victor Colombo (1):
> >   staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and
> > read_raw
> >
> >  drivers/staging/iio/resolver/ad2s90.c | 55 ++-
> >  1 file changed, 37 insertions(+), 18 deletions(-)
> >
>

Re: [PATCH 5/6] staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and read_raw

[Matheus Tavares Bernardino]

On Fri, Oct 26, 2018 at 7:04 AM Dan Carpenter  wrote:
>
> On Thu, Oct 25, 2018 at 09:45:11PM -0300, Matheus Tavares wrote:
> > From: Victor Colombo 
> >
> > This patch adds the IIO_CHAN_INFO_SCALE mask to ad2s90_chan and
> > implements the relative read behavior at ad2s90_read_raw.
> >
> > Signed-off-by: Victor Colombo 
> > ---
>
> You should be adding your S-o-B here as well because the patch is
> passing through your hands.

Thanks for the review! I'll be sending a v2 with my S-o-B there.

> regards,
> dan carpenter
>
> --
> You received this message because you are subscribed to the Google Groups 
> "Kernel USP" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to kernel-usp+unsubscr...@googlegroups.com.
> To post to this group, send email to kernel-...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/kernel-usp/20181026100422.lvz2avowd6ddix54%40mwanda.
> For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 5/6] staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and read_raw

[Matheus Tavares Bernardino]

On Fri, Oct 26, 2018 at 7:04 AM Dan Carpenter  wrote:
>
> On Thu, Oct 25, 2018 at 09:45:11PM -0300, Matheus Tavares wrote:
> > From: Victor Colombo 
> >
> > This patch adds the IIO_CHAN_INFO_SCALE mask to ad2s90_chan and
> > implements the relative read behavior at ad2s90_read_raw.
> >
> > Signed-off-by: Victor Colombo 
> > ---
>
> You should be adding your S-o-B here as well because the patch is
> passing through your hands.

Thanks for the review! I'll be sending a v2 with my S-o-B there.

> regards,
> dan carpenter
>
> --
> You received this message because you are subscribed to the Google Groups 
> "Kernel USP" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to kernel-usp+unsubscr...@googlegroups.com.
> To post to this group, send email to kernel-...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/kernel-usp/20181026100422.lvz2avowd6ddix54%40mwanda.
> For more options, visit https://groups.google.com/d/optout.

[PATCH] staging: iio: ad2s1210: fix 'assignment operator' style checks

[Matheus Tavares Bernardino]

This patch fixes all "Assignment operator '=' should be on the previous
line" checks found in ad2s1210.c by checkpatch.pl.

Signed-off-by: Matheus Tavares 
---
 drivers/staging/iio/resolver/ad2s1210.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1210.c
b/drivers/staging/iio/resolver/ad2s1210.c
index ac13b99bd9cb..d4b1c2c010f2 100644
--- a/drivers/staging/iio/resolver/ad2s1210.c
+++ b/drivers/staging/iio/resolver/ad2s1210.c
@@ -301,8 +301,8 @@ static ssize_t ad2s1210_store_control(struct device *dev,
 "ad2s1210: write control register fail\n");
 goto error_ret;
 }
-st->resolution
-= ad2s1210_resolution_value[data & AD2S1210_SET_RESOLUTION];
+st->resolution =
+ad2s1210_resolution_value[data & AD2S1210_SET_RESOLUTION];
 if (st->pdata->gpioin) {
 data = ad2s1210_read_resolution_pin(st);
 if (data != st->resolution)
@@ -363,8 +363,8 @@ static ssize_t ad2s1210_store_resolution(struct device *dev,
 dev_err(dev, "ad2s1210: setting resolution fail\n");
 goto error_ret;
 }
-st->resolution
-= ad2s1210_resolution_value[data & AD2S1210_SET_RESOLUTION];
+st->resolution =
+ad2s1210_resolution_value[data & AD2S1210_SET_RESOLUTION];
 if (st->pdata->gpioin) {
 data = ad2s1210_read_resolution_pin(st);
 if (data != st->resolution)
-- 
2.18.0

[PATCH] staging: iio: ad2s1210: fix 'assignment operator' style checks

[Matheus Tavares Bernardino]

This patch fixes all "Assignment operator '=' should be on the previous
line" checks found in ad2s1210.c by checkpatch.pl.

Signed-off-by: Matheus Tavares 
---
 drivers/staging/iio/resolver/ad2s1210.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1210.c
b/drivers/staging/iio/resolver/ad2s1210.c
index ac13b99bd9cb..d4b1c2c010f2 100644
--- a/drivers/staging/iio/resolver/ad2s1210.c
+++ b/drivers/staging/iio/resolver/ad2s1210.c
@@ -301,8 +301,8 @@ static ssize_t ad2s1210_store_control(struct device *dev,
 "ad2s1210: write control register fail\n");
 goto error_ret;
 }
-st->resolution
-= ad2s1210_resolution_value[data & AD2S1210_SET_RESOLUTION];
+st->resolution =
+ad2s1210_resolution_value[data & AD2S1210_SET_RESOLUTION];
 if (st->pdata->gpioin) {
 data = ad2s1210_read_resolution_pin(st);
 if (data != st->resolution)
@@ -363,8 +363,8 @@ static ssize_t ad2s1210_store_resolution(struct device *dev,
 dev_err(dev, "ad2s1210: setting resolution fail\n");
 goto error_ret;
 }
-st->resolution
-= ad2s1210_resolution_value[data & AD2S1210_SET_RESOLUTION];
+st->resolution =
+ad2s1210_resolution_value[data & AD2S1210_SET_RESOLUTION];
 if (st->pdata->gpioin) {
 data = ad2s1210_read_resolution_pin(st);
 if (data != st->resolution)
-- 
2.18.0