* [PATCH 0/5] Adding scale support to the lpc32xx ADC driver
@ 2019-02-08 16:09 Gregory CLEMENT
2019-02-08 16:09 ` [PATCH 1/5] dt-bindings: iio: adc: move lpc32xx-adc out of staging Gregory CLEMENT
` (4 more replies)
0 siblings, 5 replies; 18+ messages in thread
From: Gregory CLEMENT @ 2019-02-08 16:09 UTC (permalink / raw)
To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler
Cc: devicetree, Gregory CLEMENT, Rob Herring, linux-arm-kernel,
Thomas Petazzoni
Hello,
This series adds the support of the scale feature to the lpc32xx ADC
driver. In order to use it we need to have the voltage reference as a
device tree property, however in order to be backward compatible, this
property is optional and do not prevent to use the driver if missing.
I updated the binding documentation accordingly.
While I was on this driver, I made also some clean-up.
Gregory
Gregory CLEMENT (5):
dt-bindings: iio: adc: move lpc32xx-adc out of staging
dt-bindings: iio: adc: lpc32xx-adc: Document vref-supply
iio:adc:lpc32xx use SPDX-License-Identifier
iio:adc:lpc32xx Cleanup headers
iio:adc:lpc32xx Add scale feature
.../{staging => }/iio/adc/lpc32xx-adc.txt | 5 ++
drivers/iio/adc/lpc32xx_adc.c | 57 +++++++++----------
2 files changed, 33 insertions(+), 29 deletions(-)
rename Documentation/devicetree/bindings/{staging => }/iio/adc/lpc32xx-adc.txt (67%)
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 1/5] dt-bindings: iio: adc: move lpc32xx-adc out of staging 2019-02-08 16:09 [PATCH 0/5] Adding scale support to the lpc32xx ADC driver Gregory CLEMENT @ 2019-02-08 16:09 ` Gregory CLEMENT 2019-02-09 17:05 ` Jonathan Cameron 2019-02-08 16:09 ` [PATCH 2/5] dt-bindings: iio: adc: lpc32xx-adc: Document vref-supply Gregory CLEMENT ` (3 subsequent siblings) 4 siblings, 1 reply; 18+ messages in thread From: Gregory CLEMENT @ 2019-02-08 16:09 UTC (permalink / raw) To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler Cc: devicetree, Gregory CLEMENT, Rob Herring, linux-arm-kernel, Thomas Petazzoni The drivers has been moved out since 0097e20e7771 ("staging:iio:adc:lpc32xx Move out of staging.") in v4.10, so let's align the binding documentation. Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> --- .../devicetree/bindings/{staging => }/iio/adc/lpc32xx-adc.txt | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename Documentation/devicetree/bindings/{staging => }/iio/adc/lpc32xx-adc.txt (100%) diff --git a/Documentation/devicetree/bindings/staging/iio/adc/lpc32xx-adc.txt b/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt similarity index 100% rename from Documentation/devicetree/bindings/staging/iio/adc/lpc32xx-adc.txt rename to Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt -- 2.20.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] dt-bindings: iio: adc: move lpc32xx-adc out of staging 2019-02-08 16:09 ` [PATCH 1/5] dt-bindings: iio: adc: move lpc32xx-adc out of staging Gregory CLEMENT @ 2019-02-09 17:05 ` Jonathan Cameron 0 siblings, 0 replies; 18+ messages in thread From: Jonathan Cameron @ 2019-02-09 17:05 UTC (permalink / raw) To: Gregory CLEMENT Cc: devicetree, Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio, Rob Herring, Thomas Petazzoni, Hartmut Knaack, linux-arm-kernel On Fri, 8 Feb 2019 17:09:40 +0100 Gregory CLEMENT <gregory.clement@bootlin.com> wrote: > The drivers has been moved out since > 0097e20e7771 ("staging:iio:adc:lpc32xx Move out of staging.") in v4.10, > so let's align the binding documentation. > > Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> Oops. My fault. You forgot the iio list on these so +cc Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to play with it. Thanks, Jonathan > --- > .../devicetree/bindings/{staging => }/iio/adc/lpc32xx-adc.txt | 0 > 1 file changed, 0 insertions(+), 0 deletions(-) > rename Documentation/devicetree/bindings/{staging => }/iio/adc/lpc32xx-adc.txt (100%) > > diff --git a/Documentation/devicetree/bindings/staging/iio/adc/lpc32xx-adc.txt b/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt > similarity index 100% > rename from Documentation/devicetree/bindings/staging/iio/adc/lpc32xx-adc.txt > rename to Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/5] dt-bindings: iio: adc: lpc32xx-adc: Document vref-supply 2019-02-08 16:09 [PATCH 0/5] Adding scale support to the lpc32xx ADC driver Gregory CLEMENT 2019-02-08 16:09 ` [PATCH 1/5] dt-bindings: iio: adc: move lpc32xx-adc out of staging Gregory CLEMENT @ 2019-02-08 16:09 ` Gregory CLEMENT 2019-02-09 17:09 ` Jonathan Cameron ` (2 more replies) 2019-02-08 16:09 ` [PATCH 3/5] iio:adc:lpc32xx use SPDX-License-Identifier Gregory CLEMENT ` (2 subsequent siblings) 4 siblings, 3 replies; 18+ messages in thread From: Gregory CLEMENT @ 2019-02-08 16:09 UTC (permalink / raw) To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler Cc: devicetree, Gregory CLEMENT, Rob Herring, linux-arm-kernel, Thomas Petazzoni As most of the other ADC the lpc32xx one use a vref-supply property: document it. Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> --- Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt b/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt index b3629d3a9adf..3a1bc669bd51 100644 --- a/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt +++ b/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt @@ -6,6 +6,10 @@ Required properties: region. - interrupts: The ADC interrupt +Optional: + - vref-supply: The regulator supply ADC reference voltage, optional + for legacy reason, but highly encouraging to us in new device tree + Example: adc@40048000 { @@ -13,4 +17,5 @@ Example: reg = <0x40048000 0x1000>; interrupt-parent = <&mic>; interrupts = <39 0>; + vref-supply = <&vcc>; }; -- 2.20.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/5] dt-bindings: iio: adc: lpc32xx-adc: Document vref-supply 2019-02-08 16:09 ` [PATCH 2/5] dt-bindings: iio: adc: lpc32xx-adc: Document vref-supply Gregory CLEMENT @ 2019-02-09 17:09 ` Jonathan Cameron 2019-02-09 18:07 ` Vladimir Zapolskiy 2019-02-25 23:32 ` Rob Herring 2 siblings, 0 replies; 18+ messages in thread From: Jonathan Cameron @ 2019-02-09 17:09 UTC (permalink / raw) To: Gregory CLEMENT Cc: devicetree, Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio, Rob Herring, Thomas Petazzoni, Hartmut Knaack, linux-arm-kernel On Fri, 8 Feb 2019 17:09:41 +0100 Gregory CLEMENT <gregory.clement@bootlin.com> wrote: > As most of the other ADC the lpc32xx one use a vref-supply property: > document it. > > Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> Hmm. This is indeed an oddity as you document. Normally we would insist on it, but we can't because of legacy and as it is actually queries, we can't even use the fact a stub regulator will be provided to get around it. I'll have some comments on the patch implementing it anyway, but good to let this sit for a while given it's slightly unusual nature. Thanks, Jonathan > --- > Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt b/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt > index b3629d3a9adf..3a1bc669bd51 100644 > --- a/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt > +++ b/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt > @@ -6,6 +6,10 @@ Required properties: > region. > - interrupts: The ADC interrupt > > +Optional: > + - vref-supply: The regulator supply ADC reference voltage, optional > + for legacy reason, but highly encouraging to us in new device tree > + > Example: > > adc@40048000 { > @@ -13,4 +17,5 @@ Example: > reg = <0x40048000 0x1000>; > interrupt-parent = <&mic>; > interrupts = <39 0>; > + vref-supply = <&vcc>; > }; _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/5] dt-bindings: iio: adc: lpc32xx-adc: Document vref-supply 2019-02-08 16:09 ` [PATCH 2/5] dt-bindings: iio: adc: lpc32xx-adc: Document vref-supply Gregory CLEMENT 2019-02-09 17:09 ` Jonathan Cameron @ 2019-02-09 18:07 ` Vladimir Zapolskiy 2019-02-15 16:07 ` Gregory CLEMENT 2019-02-25 23:32 ` Rob Herring 2 siblings, 1 reply; 18+ messages in thread From: Vladimir Zapolskiy @ 2019-02-09 18:07 UTC (permalink / raw) To: Gregory CLEMENT, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler Cc: devicetree, Rob Herring, linux-arm-kernel, Thomas Petazzoni Hi Gregory, On 02/08/2019 06:09 PM, Gregory CLEMENT wrote: > As most of the other ADC the lpc32xx one use a vref-supply property: > document it. > > Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> > --- > Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt b/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt > index b3629d3a9adf..3a1bc669bd51 100644 > --- a/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt > +++ b/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt > @@ -6,6 +6,10 @@ Required properties: > region. > - interrupts: The ADC interrupt > > +Optional: > + - vref-supply: The regulator supply ADC reference voltage, optional > + for legacy reason, but highly encouraging to us in new device tree > + here vref-supply voltage is 3.3v only, I think that might be a reason why the property is omitted. My concern is that the documentation shall not contain sections with description of properties found in the past for 'legacy reasons', if you like to support the backward compatibility in the driver, please update the documentation to the expected state and add a stub to care about legacy DTBs in the driver only. FWIW I personally don't mind about breaking the backward compatibility for NXP LPC32xx, it might be a concern when CONFIG_ARM_ATAG_DTB_COMPAT=y is removed from the lpc32xx_defconfig though. > Example: > > adc@40048000 { > @@ -13,4 +17,5 @@ Example: > reg = <0x40048000 0x1000>; > interrupt-parent = <&mic>; > interrupts = <39 0>; > + vref-supply = <&vcc>; > }; > I kindly ask you to include me to To: list, if you send another changes for NXP LPC32xx platform. Thank you. -- Best wishes, Vladimir _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/5] dt-bindings: iio: adc: lpc32xx-adc: Document vref-supply 2019-02-09 18:07 ` Vladimir Zapolskiy @ 2019-02-15 16:07 ` Gregory CLEMENT 2019-02-20 12:11 ` Sylvain Lemieux 0 siblings, 1 reply; 18+ messages in thread From: Gregory CLEMENT @ 2019-02-15 16:07 UTC (permalink / raw) To: Vladimir Zapolskiy, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler Cc: devicetree, Rob Herring, Thomas Petazzoni, linux-arm-kernel Hi Vladimir, On sam., févr. 09 2019, Vladimir Zapolskiy <vz@mleia.com> wrote: > Hi Gregory, > > On 02/08/2019 06:09 PM, Gregory CLEMENT wrote: >> As most of the other ADC the lpc32xx one use a vref-supply property: >> document it. >> >> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> >> --- >> Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt b/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt >> index b3629d3a9adf..3a1bc669bd51 100644 >> --- a/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt >> +++ b/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt >> @@ -6,6 +6,10 @@ Required properties: >> region. >> - interrupts: The ADC interrupt >> >> +Optional: >> + - vref-supply: The regulator supply ADC reference voltage, optional >> + for legacy reason, but highly encouraging to us in new device tree >> + > > here vref-supply voltage is 3.3v only, I think that might be a reason why > the property is omitted. VDD_AD is an external pin so in theroy it would be possible to use a different voltage, but indeed the datasheet describe this pin as the 3.3V supply for ADC. > > My concern is that the documentation shall not contain sections with > description of properties found in the past for 'legacy reasons', if you > like to support the backward compatibility in the driver, please update > the documentation to the expected state and add a stub to care about > legacy DTBs in the driver only. I saw a lot of reference about legacy properties in the binding, and I found it quite usefull when I read old device trees. > > FWIW I personally don't mind about breaking the backward compatibility > for NXP LPC32xx, it might be a concern when CONFIG_ARM_ATAG_DTB_COMPAT=y > is removed from the lpc32xx_defconfig though. > >> Example: >> >> adc@40048000 { >> @@ -13,4 +17,5 @@ Example: >> reg = <0x40048000 0x1000>; >> interrupt-parent = <&mic>; >> interrupts = <39 0>; >> + vref-supply = <&vcc>; >> }; >> > > I kindly ask you to include me to To: list, if you send another changes for > NXP LPC32xx platform. Yes sure! I wrongly only use the first lines provided by ./scripts/get_maintainer.pl, I will include you in the next version. Thanks, Gregory > > Thank you. > > -- > Best wishes, > Vladimir > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- Gregory Clement, Bootlin Embedded Linux and Kernel engineering http://bootlin.com _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/5] dt-bindings: iio: adc: lpc32xx-adc: Document vref-supply 2019-02-15 16:07 ` Gregory CLEMENT @ 2019-02-20 12:11 ` Sylvain Lemieux 0 siblings, 0 replies; 18+ messages in thread From: Sylvain Lemieux @ 2019-02-20 12:11 UTC (permalink / raw) To: Gregory CLEMENT Cc: devicetree, Lars-Peter Clausen, Thomas Petazzoni, Vladimir Zapolskiy, Rob Herring, moderated list:ARM PORT, Peter Meerwald-Stadler, Hartmut Knaack, Jonathan Cameron Hi Gregory, On Fri, Feb 15, 2019 at 11:07 AM Gregory CLEMENT <gregory.clement@bootlin.com> wrote: > > Hi Vladimir, > > On sam., févr. 09 2019, Vladimir Zapolskiy <vz@mleia.com> wrote: > > > Hi Gregory, > > > > On 02/08/2019 06:09 PM, Gregory CLEMENT wrote: > >> As most of the other ADC the lpc32xx one use a vref-supply property: > >> document it. > >> > >> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> > >> --- > >> Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt | 5 +++++ > >> 1 file changed, 5 insertions(+) > >> [...] > >> > > > > I kindly ask you to include me to To: list, if you send another changes for > > NXP LPC32xx platform. > > Yes sure! I wrongly only use the first lines provided by > ./scripts/get_maintainer.pl, I will include you in the next version. > Please also include myself in the next LPC32xx related patches. Regards, Sylvain > > Thanks, > > Gregory > > > > > > > Thank you. > > > > -- > > Best wishes, > > Vladimir > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > -- > Gregory Clement, Bootlin > Embedded Linux and Kernel engineering > http://bootlin.com > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/5] dt-bindings: iio: adc: lpc32xx-adc: Document vref-supply 2019-02-08 16:09 ` [PATCH 2/5] dt-bindings: iio: adc: lpc32xx-adc: Document vref-supply Gregory CLEMENT 2019-02-09 17:09 ` Jonathan Cameron 2019-02-09 18:07 ` Vladimir Zapolskiy @ 2019-02-25 23:32 ` Rob Herring 2 siblings, 0 replies; 18+ messages in thread From: Rob Herring @ 2019-02-25 23:32 UTC (permalink / raw) To: Gregory CLEMENT Cc: devicetree, Lars-Peter Clausen, Peter Meerwald-Stadler, Gregory CLEMENT, linux-arm-kernel, Thomas Petazzoni, Hartmut Knaack, Jonathan Cameron On Fri, 8 Feb 2019 17:09:41 +0100, Gregory CLEMENT wrote: > As most of the other ADC the lpc32xx one use a vref-supply property: > document it. > > Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> > --- > Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt | 5 +++++ > 1 file changed, 5 insertions(+) > Reviewed-by: Rob Herring <robh@kernel.org> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/5] iio:adc:lpc32xx use SPDX-License-Identifier 2019-02-08 16:09 [PATCH 0/5] Adding scale support to the lpc32xx ADC driver Gregory CLEMENT 2019-02-08 16:09 ` [PATCH 1/5] dt-bindings: iio: adc: move lpc32xx-adc out of staging Gregory CLEMENT 2019-02-08 16:09 ` [PATCH 2/5] dt-bindings: iio: adc: lpc32xx-adc: Document vref-supply Gregory CLEMENT @ 2019-02-08 16:09 ` Gregory CLEMENT 2019-02-09 17:10 ` Jonathan Cameron 2019-02-08 16:09 ` [PATCH 4/5] iio:adc:lpc32xx Cleanup headers Gregory CLEMENT 2019-02-08 16:09 ` [PATCH 5/5] iio:adc:lpc32xx Add scale feature Gregory CLEMENT 4 siblings, 1 reply; 18+ messages in thread From: Gregory CLEMENT @ 2019-02-08 16:09 UTC (permalink / raw) To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler Cc: devicetree, Gregory CLEMENT, Rob Herring, linux-arm-kernel, Thomas Petazzoni Convert the driver to SPDX license description which allow removing several lines in the file. Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> --- drivers/iio/adc/lpc32xx_adc.c | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/drivers/iio/adc/lpc32xx_adc.c b/drivers/iio/adc/lpc32xx_adc.c index 20b36690fa4f..e361c1532a75 100644 --- a/drivers/iio/adc/lpc32xx_adc.c +++ b/drivers/iio/adc/lpc32xx_adc.c @@ -1,23 +1,10 @@ +// SPDX-License-Identifier: GPL-2.0+ /* * lpc32xx_adc.c - Support for ADC in LPC32XX * * 3-channel, 10-bit ADC * * Copyright (C) 2011, 2012 Roland Stigge <stigge@antcom.de> - * - * 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. */ #include <linux/module.h> -- 2.20.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 3/5] iio:adc:lpc32xx use SPDX-License-Identifier 2019-02-08 16:09 ` [PATCH 3/5] iio:adc:lpc32xx use SPDX-License-Identifier Gregory CLEMENT @ 2019-02-09 17:10 ` Jonathan Cameron 0 siblings, 0 replies; 18+ messages in thread From: Jonathan Cameron @ 2019-02-09 17:10 UTC (permalink / raw) To: Gregory CLEMENT Cc: devicetree, Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio, Rob Herring, Thomas Petazzoni, Hartmut Knaack, linux-arm-kernel On Fri, 8 Feb 2019 17:09:42 +0100 Gregory CLEMENT <gregory.clement@bootlin.com> wrote: > Convert the driver to SPDX license description which allow removing > several lines in the file. > > Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> Applied to the togreg branch of iio.git and pushed out as testing. Thanks, Jonathan > --- > drivers/iio/adc/lpc32xx_adc.c | 15 +-------------- > 1 file changed, 1 insertion(+), 14 deletions(-) > > diff --git a/drivers/iio/adc/lpc32xx_adc.c b/drivers/iio/adc/lpc32xx_adc.c > index 20b36690fa4f..e361c1532a75 100644 > --- a/drivers/iio/adc/lpc32xx_adc.c > +++ b/drivers/iio/adc/lpc32xx_adc.c > @@ -1,23 +1,10 @@ > +// SPDX-License-Identifier: GPL-2.0+ > /* > * lpc32xx_adc.c - Support for ADC in LPC32XX > * > * 3-channel, 10-bit ADC > * > * Copyright (C) 2011, 2012 Roland Stigge <stigge@antcom.de> > - * > - * 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. > */ > > #include <linux/module.h> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 4/5] iio:adc:lpc32xx Cleanup headers 2019-02-08 16:09 [PATCH 0/5] Adding scale support to the lpc32xx ADC driver Gregory CLEMENT ` (2 preceding siblings ...) 2019-02-08 16:09 ` [PATCH 3/5] iio:adc:lpc32xx use SPDX-License-Identifier Gregory CLEMENT @ 2019-02-08 16:09 ` Gregory CLEMENT 2019-02-09 17:16 ` Jonathan Cameron 2019-02-08 16:09 ` [PATCH 5/5] iio:adc:lpc32xx Add scale feature Gregory CLEMENT 4 siblings, 1 reply; 18+ messages in thread From: Gregory CLEMENT @ 2019-02-08 16:09 UTC (permalink / raw) To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler Cc: devicetree, Gregory CLEMENT, Rob Herring, linux-arm-kernel, Thomas Petazzoni A few headers was useless: remove them, and also sort them in alphabetic order. Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> --- drivers/iio/adc/lpc32xx_adc.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/drivers/iio/adc/lpc32xx_adc.c b/drivers/iio/adc/lpc32xx_adc.c index e361c1532a75..f391c1e10136 100644 --- a/drivers/iio/adc/lpc32xx_adc.c +++ b/drivers/iio/adc/lpc32xx_adc.c @@ -7,20 +7,13 @@ * Copyright (C) 2011, 2012 Roland Stigge <stigge@antcom.de> */ -#include <linux/module.h> -#include <linux/platform_device.h> -#include <linux/interrupt.h> -#include <linux/device.h> -#include <linux/kernel.h> -#include <linux/slab.h> -#include <linux/io.h> #include <linux/clk.h> -#include <linux/err.h> #include <linux/completion.h> -#include <linux/of.h> - #include <linux/iio/iio.h> -#include <linux/iio/sysfs.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/module.h> +#include <linux/platform_device.h> /* * LPC32XX registers definitions -- 2.20.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 4/5] iio:adc:lpc32xx Cleanup headers 2019-02-08 16:09 ` [PATCH 4/5] iio:adc:lpc32xx Cleanup headers Gregory CLEMENT @ 2019-02-09 17:16 ` Jonathan Cameron 2019-02-15 15:42 ` Gregory CLEMENT 0 siblings, 1 reply; 18+ messages in thread From: Jonathan Cameron @ 2019-02-09 17:16 UTC (permalink / raw) To: Gregory CLEMENT Cc: devicetree, Lars-Peter Clausen, Peter Meerwald-Stadler, Rob Herring, Thomas Petazzoni, Hartmut Knaack, linux-arm-kernel On Fri, 8 Feb 2019 17:09:43 +0100 Gregory CLEMENT <gregory.clement@bootlin.com> wrote: > A few headers was useless: remove them, and also sort them in alphabetic > order. > > Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> Hmm. Given the headers in question are mostly only useless (I think) in the sense they are always included by something else, I'm not sure this patch is worth the churn. It's also tricky to see which ones were actually removed given the combination with sorting. I think it's just kernel.h, device.h, err.h all of which are used in various ways and often directly included. The other one is the iio/sysfs.h file. That one we could do to eventually kill off entirely, so happy to see that one alone go. Jonathan > --- > drivers/iio/adc/lpc32xx_adc.c | 15 ++++----------- > 1 file changed, 4 insertions(+), 11 deletions(-) > > diff --git a/drivers/iio/adc/lpc32xx_adc.c b/drivers/iio/adc/lpc32xx_adc.c > index e361c1532a75..f391c1e10136 100644 > --- a/drivers/iio/adc/lpc32xx_adc.c > +++ b/drivers/iio/adc/lpc32xx_adc.c > @@ -7,20 +7,13 @@ > * Copyright (C) 2011, 2012 Roland Stigge <stigge@antcom.de> > */ > > -#include <linux/module.h> > -#include <linux/platform_device.h> > -#include <linux/interrupt.h> > -#include <linux/device.h> > -#include <linux/kernel.h> > -#include <linux/slab.h> > -#include <linux/io.h> > #include <linux/clk.h> > -#include <linux/err.h> > #include <linux/completion.h> > -#include <linux/of.h> > - > #include <linux/iio/iio.h> > -#include <linux/iio/sysfs.h> > +#include <linux/interrupt.h> > +#include <linux/io.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > > /* > * LPC32XX registers definitions _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/5] iio:adc:lpc32xx Cleanup headers 2019-02-09 17:16 ` Jonathan Cameron @ 2019-02-15 15:42 ` Gregory CLEMENT 2019-02-18 14:07 ` Jonathan Cameron 0 siblings, 1 reply; 18+ messages in thread From: Gregory CLEMENT @ 2019-02-15 15:42 UTC (permalink / raw) To: Jonathan Cameron Cc: devicetree, Lars-Peter Clausen, Peter Meerwald-Stadler, Rob Herring, Thomas Petazzoni, Hartmut Knaack, linux-arm-kernel Hi Jonathan, On sam., févr. 09 2019, Jonathan Cameron <jic23@kernel.org> wrote: > On Fri, 8 Feb 2019 17:09:43 +0100 > Gregory CLEMENT <gregory.clement@bootlin.com> wrote: > >> A few headers was useless: remove them, and also sort them in alphabetic >> order. >> >> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> > Hmm. Given the headers in question are mostly only useless (I think) > in the sense they are always included by something else, I'm > not sure this patch is worth the churn. > > It's also tricky to see which ones were actually removed > given the combination with sorting. > I think it's just kernel.h, device.h, err.h all of which > are used in various ways and often directly included. Actually, as I needed to add a header, I wanted to sort the list to put it in the right place and then some of the header looked superfluous that why I remove some of them. For example there was no reason to use slab.h or of.h. > > The other one is the iio/sysfs.h file. That one we could > do to eventually kill off entirely, so happy to see that > one alone go. I am nit sure to know what do you want with this patch. Gregory > > Jonathan > >> --- >> drivers/iio/adc/lpc32xx_adc.c | 15 ++++----------- >> 1 file changed, 4 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/iio/adc/lpc32xx_adc.c b/drivers/iio/adc/lpc32xx_adc.c >> index e361c1532a75..f391c1e10136 100644 >> --- a/drivers/iio/adc/lpc32xx_adc.c >> +++ b/drivers/iio/adc/lpc32xx_adc.c >> @@ -7,20 +7,13 @@ >> * Copyright (C) 2011, 2012 Roland Stigge <stigge@antcom.de> >> */ >> >> -#include <linux/module.h> >> -#include <linux/platform_device.h> >> -#include <linux/interrupt.h> >> -#include <linux/device.h> >> -#include <linux/kernel.h> >> -#include <linux/slab.h> >> -#include <linux/io.h> >> #include <linux/clk.h> >> -#include <linux/err.h> >> #include <linux/completion.h> >> -#include <linux/of.h> >> - >> #include <linux/iio/iio.h> >> -#include <linux/iio/sysfs.h> >> +#include <linux/interrupt.h> >> +#include <linux/io.h> >> +#include <linux/module.h> >> +#include <linux/platform_device.h> >> >> /* >> * LPC32XX registers definitions > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- Gregory Clement, Bootlin Embedded Linux and Kernel engineering http://bootlin.com _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/5] iio:adc:lpc32xx Cleanup headers 2019-02-15 15:42 ` Gregory CLEMENT @ 2019-02-18 14:07 ` Jonathan Cameron 0 siblings, 0 replies; 18+ messages in thread From: Jonathan Cameron @ 2019-02-18 14:07 UTC (permalink / raw) To: Gregory CLEMENT Cc: devicetree, Lars-Peter Clausen, Peter Meerwald-Stadler, Rob Herring, linux-arm-kernel, Thomas Petazzoni, Hartmut Knaack, Jonathan Cameron On Fri, 15 Feb 2019 16:42:55 +0100 Gregory CLEMENT <gregory.clement@bootlin.com> wrote: > Hi Jonathan, > > On sam., févr. 09 2019, Jonathan Cameron <jic23@kernel.org> wrote: > > > On Fri, 8 Feb 2019 17:09:43 +0100 > > Gregory CLEMENT <gregory.clement@bootlin.com> wrote: > > > >> A few headers was useless: remove them, and also sort them in alphabetic > >> order. > >> > >> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> > > Hmm. Given the headers in question are mostly only useless (I think) > > in the sense they are always included by something else, I'm > > not sure this patch is worth the churn. > > > > It's also tricky to see which ones were actually removed > > given the combination with sorting. > > I think it's just kernel.h, device.h, err.h all of which > > are used in various ways and often directly included. > > Actually, as I needed to add a header, I wanted to sort the list to put > it in the right place and then some of the header looked superfluous > that why I remove some of them. For example there was no reason to use > slab.h or of.h. Agreed on those two. I missed them probably because of the re organization being combined with them. No specific allocations except via interfaces from elsewhere and no direct use of the devicetree stuff, so fine to drop them. > > > > > The other one is the iio/sysfs.h file. That one we could > > do to eventually kill off entirely, so happy to see that > > one alone go. > > I am nit sure to know what do you want with this patch. Break it in two. Drop the unwanted ones first, then reorganize. That way the diff is easy to read. Clearly I missed some changes on my first read as it stands! Jonathan > > Gregory > > > > > > Jonathan > > > >> --- > >> drivers/iio/adc/lpc32xx_adc.c | 15 ++++----------- > >> 1 file changed, 4 insertions(+), 11 deletions(-) > >> > >> diff --git a/drivers/iio/adc/lpc32xx_adc.c b/drivers/iio/adc/lpc32xx_adc.c > >> index e361c1532a75..f391c1e10136 100644 > >> --- a/drivers/iio/adc/lpc32xx_adc.c > >> +++ b/drivers/iio/adc/lpc32xx_adc.c > >> @@ -7,20 +7,13 @@ > >> * Copyright (C) 2011, 2012 Roland Stigge <stigge@antcom.de> > >> */ > >> > >> -#include <linux/module.h> > >> -#include <linux/platform_device.h> > >> -#include <linux/interrupt.h> > >> -#include <linux/device.h> > >> -#include <linux/kernel.h> > >> -#include <linux/slab.h> > >> -#include <linux/io.h> > >> #include <linux/clk.h> > >> -#include <linux/err.h> > >> #include <linux/completion.h> > >> -#include <linux/of.h> > >> - > >> #include <linux/iio/iio.h> > >> -#include <linux/iio/sysfs.h> > >> +#include <linux/interrupt.h> > >> +#include <linux/io.h> > >> +#include <linux/module.h> > >> +#include <linux/platform_device.h> > >> > >> /* > >> * LPC32XX registers definitions > > > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 5/5] iio:adc:lpc32xx Add scale feature 2019-02-08 16:09 [PATCH 0/5] Adding scale support to the lpc32xx ADC driver Gregory CLEMENT ` (3 preceding siblings ...) 2019-02-08 16:09 ` [PATCH 4/5] iio:adc:lpc32xx Cleanup headers Gregory CLEMENT @ 2019-02-08 16:09 ` Gregory CLEMENT 2019-02-09 17:23 ` Jonathan Cameron 4 siblings, 1 reply; 18+ messages in thread From: Gregory CLEMENT @ 2019-02-08 16:09 UTC (permalink / raw) To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler Cc: devicetree, Gregory CLEMENT, Rob Herring, linux-arm-kernel, Thomas Petazzoni Until now this driver only exposed the raw value of the channels. With this patch, the scale value is also exposed. It depends of a regulator supply, and unlike most of the other driver, do not having this regulator won't prevent to use the driver. The reason for it is to allow to continue to use this driver with an old device tree. If there is no regulator supply then the scale won't be exposed. Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> --- drivers/iio/adc/lpc32xx_adc.c | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/drivers/iio/adc/lpc32xx_adc.c b/drivers/iio/adc/lpc32xx_adc.c index f391c1e10136..e36ca307f065 100644 --- a/drivers/iio/adc/lpc32xx_adc.c +++ b/drivers/iio/adc/lpc32xx_adc.c @@ -14,6 +14,7 @@ #include <linux/io.h> #include <linux/module.h> #include <linux/platform_device.h> +#include <linux/regulator/consumer.h> /* * LPC32XX registers definitions @@ -45,6 +46,7 @@ struct lpc32xx_adc_state { void __iomem *adc_base; struct clk *clk; struct completion completion; + struct regulator *vref; u32 value; }; @@ -57,7 +59,9 @@ static int lpc32xx_read_raw(struct iio_dev *indio_dev, { struct lpc32xx_adc_state *st = iio_priv(indio_dev); int ret; - if (mask == IIO_CHAN_INFO_RAW) { + + switch (mask) { + case IIO_CHAN_INFO_RAW: mutex_lock(&indio_dev->mlock); ret = clk_prepare_enable(st->clk); if (ret) { @@ -77,6 +81,12 @@ static int lpc32xx_read_raw(struct iio_dev *indio_dev, mutex_unlock(&indio_dev->mlock); return IIO_VAL_INT; + + case IIO_CHAN_INFO_SCALE: + *val = regulator_get_voltage(st->vref) / 1000; + *val2 = chan->scan_type.realbits; + + return IIO_VAL_FRACTIONAL_LOG2; } return -EINVAL; @@ -93,9 +103,10 @@ static const struct iio_info lpc32xx_adc_iio_info = { .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ .address = LPC32XXAD_IN * _index, \ .scan_index = _index, \ + .scan_type.realbits = 10 \ } -static const struct iio_chan_spec lpc32xx_adc_iio_channels[] = { +static struct iio_chan_spec lpc32xx_adc_iio_channels[] = { LPC32XX_ADC_CHANNEL(0), LPC32XX_ADC_CHANNEL(1), LPC32XX_ADC_CHANNEL(2), @@ -119,7 +130,7 @@ static int lpc32xx_adc_probe(struct platform_device *pdev) struct resource *res; int retval = -ENODEV; struct iio_dev *iodev = NULL; - int irq; + int irq, i; res = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!res) { @@ -159,6 +170,15 @@ static int lpc32xx_adc_probe(struct platform_device *pdev) return retval; } + st->vref = devm_regulator_get(&pdev->dev, "vref"); + if (IS_ERR(st->vref)) + dev_err(&pdev->dev, + "Missing vref regulator: No scaling available\n"); + else + for (i = 0; i < ARRAY_SIZE(lpc32xx_adc_iio_channels); i++) + lpc32xx_adc_iio_channels[i].info_mask_shared_by_type = + BIT(IIO_CHAN_INFO_SCALE); + platform_set_drvdata(pdev, iodev); init_completion(&st->completion); @@ -175,7 +195,6 @@ static int lpc32xx_adc_probe(struct platform_device *pdev) return retval; dev_info(&pdev->dev, "LPC32XX ADC driver loaded, IRQ %d\n", irq); - return 0; } -- 2.20.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 5/5] iio:adc:lpc32xx Add scale feature 2019-02-08 16:09 ` [PATCH 5/5] iio:adc:lpc32xx Add scale feature Gregory CLEMENT @ 2019-02-09 17:23 ` Jonathan Cameron 2019-02-15 15:35 ` Gregory CLEMENT 0 siblings, 1 reply; 18+ messages in thread From: Jonathan Cameron @ 2019-02-09 17:23 UTC (permalink / raw) To: Gregory CLEMENT Cc: devicetree, Lars-Peter Clausen, Peter Meerwald-Stadler, Rob Herring, Thomas Petazzoni, Hartmut Knaack, linux-arm-kernel On Fri, 8 Feb 2019 17:09:44 +0100 Gregory CLEMENT <gregory.clement@bootlin.com> wrote: > Until now this driver only exposed the raw value of the channels. With > this patch, the scale value is also exposed. > > It depends of a regulator supply, and unlike most of the other driver, do > not having this regulator won't prevent to use the driver. The reason for > it is to allow to continue to use this driver with an old device tree. If > there is no regulator supply then the scale won't be exposed. > > Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> Hi Gregory, A few minor comments inline. I'll admit to being surprised to see any patches at all for this driver given how long it has been since we had any known users. We nearly dropped it as unused years ago IIRC! Good thing we didn't it seems. Thanks, Jonathan > --- > drivers/iio/adc/lpc32xx_adc.c | 27 +++++++++++++++++++++++---- > 1 file changed, 23 insertions(+), 4 deletions(-) > > diff --git a/drivers/iio/adc/lpc32xx_adc.c b/drivers/iio/adc/lpc32xx_adc.c > index f391c1e10136..e36ca307f065 100644 > --- a/drivers/iio/adc/lpc32xx_adc.c > +++ b/drivers/iio/adc/lpc32xx_adc.c > @@ -14,6 +14,7 @@ > #include <linux/io.h> > #include <linux/module.h> > #include <linux/platform_device.h> > +#include <linux/regulator/consumer.h> > > /* > * LPC32XX registers definitions > @@ -45,6 +46,7 @@ struct lpc32xx_adc_state { > void __iomem *adc_base; > struct clk *clk; > struct completion completion; > + struct regulator *vref; > > u32 value; > }; > @@ -57,7 +59,9 @@ static int lpc32xx_read_raw(struct iio_dev *indio_dev, > { > struct lpc32xx_adc_state *st = iio_priv(indio_dev); > int ret; > - if (mask == IIO_CHAN_INFO_RAW) { > + > + switch (mask) { > + case IIO_CHAN_INFO_RAW: > mutex_lock(&indio_dev->mlock); > ret = clk_prepare_enable(st->clk); > if (ret) { > @@ -77,6 +81,12 @@ static int lpc32xx_read_raw(struct iio_dev *indio_dev, > mutex_unlock(&indio_dev->mlock); > > return IIO_VAL_INT; > + > + case IIO_CHAN_INFO_SCALE: > + *val = regulator_get_voltage(st->vref) / 1000; > + *val2 = chan->scan_type.realbits; > + > + return IIO_VAL_FRACTIONAL_LOG2; Please add a default, otherwise we are going to get some compiler warnings that are irritating as it will think we 'forgot' to handle the other cases. > } > > return -EINVAL; > @@ -93,9 +103,10 @@ static const struct iio_info lpc32xx_adc_iio_info = { > .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ > .address = LPC32XXAD_IN * _index, \ > .scan_index = _index, \ > + .scan_type.realbits = 10 \ Given scan_type is only used in the core in buffered modes that this driver doesn't support this is a little 'unusual'. Also, only one value, so why bother rather than just putting it in the one place it is used? > } > > -static const struct iio_chan_spec lpc32xx_adc_iio_channels[] = { > +static struct iio_chan_spec lpc32xx_adc_iio_channels[] = { Please provide two const versions and pick between them rather than modifying the one. Clearly we might 'know' there is only ever one such ADC on these devices but it's not obvious to a reviewer who isn't familiar with the hardware, making this look like a bug. > LPC32XX_ADC_CHANNEL(0), > LPC32XX_ADC_CHANNEL(1), > LPC32XX_ADC_CHANNEL(2), > @@ -119,7 +130,7 @@ static int lpc32xx_adc_probe(struct platform_device *pdev) > struct resource *res; > int retval = -ENODEV; > struct iio_dev *iodev = NULL; > - int irq; > + int irq, i; > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > if (!res) { > @@ -159,6 +170,15 @@ static int lpc32xx_adc_probe(struct platform_device *pdev) > return retval; > } > > + st->vref = devm_regulator_get(&pdev->dev, "vref"); > + if (IS_ERR(st->vref)) > + dev_err(&pdev->dev, > + "Missing vref regulator: No scaling available\n"); > + else > + for (i = 0; i < ARRAY_SIZE(lpc32xx_adc_iio_channels); i++) > + lpc32xx_adc_iio_channels[i].info_mask_shared_by_type = > + BIT(IIO_CHAN_INFO_SCALE); > + > platform_set_drvdata(pdev, iodev); > > init_completion(&st->completion); > @@ -175,7 +195,6 @@ static int lpc32xx_adc_probe(struct platform_device *pdev) > return retval; > > dev_info(&pdev->dev, "LPC32XX ADC driver loaded, IRQ %d\n", irq); > - Unrelated (and in my view) bad whitespace change. > return 0; > } > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/5] iio:adc:lpc32xx Add scale feature 2019-02-09 17:23 ` Jonathan Cameron @ 2019-02-15 15:35 ` Gregory CLEMENT 0 siblings, 0 replies; 18+ messages in thread From: Gregory CLEMENT @ 2019-02-15 15:35 UTC (permalink / raw) To: Jonathan Cameron Cc: devicetree, Lars-Peter Clausen, Peter Meerwald-Stadler, Rob Herring, Thomas Petazzoni, Hartmut Knaack, linux-arm-kernel Hi Jonathan, On sam., févr. 09 2019, Jonathan Cameron <jic23@kernel.org> wrote: > On Fri, 8 Feb 2019 17:09:44 +0100 > Gregory CLEMENT <gregory.clement@bootlin.com> wrote: > >> Until now this driver only exposed the raw value of the channels. With >> this patch, the scale value is also exposed. >> >> It depends of a regulator supply, and unlike most of the other driver, do >> not having this regulator won't prevent to use the driver. The reason for >> it is to allow to continue to use this driver with an old device tree. If >> there is no regulator supply then the scale won't be exposed. >> >> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com> > Hi Gregory, > > A few minor comments inline. > > I'll admit to being surprised to see any patches at all for this driver given > how long it has been since we had any known users. We nearly dropped it as > unused years ago IIRC! Good thing we didn't it seems. > > Thanks, > > Jonathan > >> --- >> drivers/iio/adc/lpc32xx_adc.c | 27 +++++++++++++++++++++++---- >> 1 file changed, 23 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/iio/adc/lpc32xx_adc.c b/drivers/iio/adc/lpc32xx_adc.c >> index f391c1e10136..e36ca307f065 100644 >> --- a/drivers/iio/adc/lpc32xx_adc.c >> +++ b/drivers/iio/adc/lpc32xx_adc.c >> @@ -14,6 +14,7 @@ >> #include <linux/io.h> >> #include <linux/module.h> >> #include <linux/platform_device.h> >> +#include <linux/regulator/consumer.h> >> >> /* >> * LPC32XX registers definitions >> @@ -45,6 +46,7 @@ struct lpc32xx_adc_state { >> void __iomem *adc_base; >> struct clk *clk; >> struct completion completion; >> + struct regulator *vref; >> >> u32 value; >> }; >> @@ -57,7 +59,9 @@ static int lpc32xx_read_raw(struct iio_dev *indio_dev, >> { >> struct lpc32xx_adc_state *st = iio_priv(indio_dev); >> int ret; >> - if (mask == IIO_CHAN_INFO_RAW) { >> + >> + switch (mask) { >> + case IIO_CHAN_INFO_RAW: >> mutex_lock(&indio_dev->mlock); >> ret = clk_prepare_enable(st->clk); >> if (ret) { >> @@ -77,6 +81,12 @@ static int lpc32xx_read_raw(struct iio_dev *indio_dev, >> mutex_unlock(&indio_dev->mlock); >> >> return IIO_VAL_INT; >> + >> + case IIO_CHAN_INFO_SCALE: >> + *val = regulator_get_voltage(st->vref) / 1000; >> + *val2 = chan->scan_type.realbits; >> + >> + return IIO_VAL_FRACTIONAL_LOG2; > > Please add a default, otherwise we are going to get some compiler > warnings that are irritating as it will think we 'forgot' to handle > the other cases. Sure, I will do it. > >> } >> >> return -EINVAL; >> @@ -93,9 +103,10 @@ static const struct iio_info lpc32xx_adc_iio_info = { >> .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ >> .address = LPC32XXAD_IN * _index, \ >> .scan_index = _index, \ >> + .scan_type.realbits = 10 \ > > Given scan_type is only used in the core in buffered modes > that this driver doesn't support this is a little 'unusual'. I found it in other drivers and thought it was expected to store the bit resolution here. > > Also, only one value, so why bother rather than just putting it > in the one place it is used? OK I will just use it in the lpc32xx_read_raw function then. > >> } >> >> -static const struct iio_chan_spec lpc32xx_adc_iio_channels[] = { >> +static struct iio_chan_spec lpc32xx_adc_iio_channels[] = { > Please provide two const versions and pick between them rather than > modifying the one. > > Clearly we might 'know' there is only ever one such ADC on these devices > but it's not obvious to a reviewer who isn't familiar with the hardware, > making this look like a bug. OK > >> LPC32XX_ADC_CHANNEL(0), >> LPC32XX_ADC_CHANNEL(1), >> LPC32XX_ADC_CHANNEL(2), >> @@ -119,7 +130,7 @@ static int lpc32xx_adc_probe(struct platform_device *pdev) >> struct resource *res; >> int retval = -ENODEV; >> struct iio_dev *iodev = NULL; >> - int irq; >> + int irq, i; >> >> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> if (!res) { >> @@ -159,6 +170,15 @@ static int lpc32xx_adc_probe(struct platform_device *pdev) >> return retval; >> } >> >> + st->vref = devm_regulator_get(&pdev->dev, "vref"); >> + if (IS_ERR(st->vref)) >> + dev_err(&pdev->dev, >> + "Missing vref regulator: No scaling available\n"); >> + else >> + for (i = 0; i < ARRAY_SIZE(lpc32xx_adc_iio_channels); i++) >> + lpc32xx_adc_iio_channels[i].info_mask_shared_by_type = >> + BIT(IIO_CHAN_INFO_SCALE); >> + >> platform_set_drvdata(pdev, iodev); >> >> init_completion(&st->completion); >> @@ -175,7 +195,6 @@ static int lpc32xx_adc_probe(struct platform_device *pdev) >> return retval; >> >> dev_info(&pdev->dev, "LPC32XX ADC driver loaded, IRQ %d\n", irq); >> - > Unrelated (and in my view) bad whitespace change. Indeed, it was a mistake. Thanks, Gregory >> return 0; >> } >> > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- Gregory Clement, Bootlin Embedded Linux and Kernel engineering http://bootlin.com _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2019-02-25 23:32 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-02-08 16:09 [PATCH 0/5] Adding scale support to the lpc32xx ADC driver Gregory CLEMENT 2019-02-08 16:09 ` [PATCH 1/5] dt-bindings: iio: adc: move lpc32xx-adc out of staging Gregory CLEMENT 2019-02-09 17:05 ` Jonathan Cameron 2019-02-08 16:09 ` [PATCH 2/5] dt-bindings: iio: adc: lpc32xx-adc: Document vref-supply Gregory CLEMENT 2019-02-09 17:09 ` Jonathan Cameron 2019-02-09 18:07 ` Vladimir Zapolskiy 2019-02-15 16:07 ` Gregory CLEMENT 2019-02-20 12:11 ` Sylvain Lemieux 2019-02-25 23:32 ` Rob Herring 2019-02-08 16:09 ` [PATCH 3/5] iio:adc:lpc32xx use SPDX-License-Identifier Gregory CLEMENT 2019-02-09 17:10 ` Jonathan Cameron 2019-02-08 16:09 ` [PATCH 4/5] iio:adc:lpc32xx Cleanup headers Gregory CLEMENT 2019-02-09 17:16 ` Jonathan Cameron 2019-02-15 15:42 ` Gregory CLEMENT 2019-02-18 14:07 ` Jonathan Cameron 2019-02-08 16:09 ` [PATCH 5/5] iio:adc:lpc32xx Add scale feature Gregory CLEMENT 2019-02-09 17:23 ` Jonathan Cameron 2019-02-15 15:35 ` Gregory CLEMENT
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).