| Message ID | 20190208160944.13281-1-gregory.clement@bootlin.com |
|---|---|
| Headers | show |
| Series | Adding scale support to the lpc32xx ADC driver | expand |
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>; > };
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>
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
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; > } >
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
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
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
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
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 >
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
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>
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%)