From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.kernel.org ([198.145.29.99]:37922 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750817AbdG3NcC (ORCPT ); Sun, 30 Jul 2017 09:32:02 -0400 Date: Sun, 30 Jul 2017 14:31:58 +0100 From: Jonathan Cameron To: David Lechner Cc: Andy Shevchenko , Hartmut Knaack , Lars-Peter Clausen , linux-iio@vger.kernel.org Subject: Re: [PATCH v2 1/2] iio: adc: ti-ads7950: Allow to use on ACPI platforms Message-ID: <20170730143158.3b907e8f@kernel.org> In-Reply-To: <9d74cad7-be8e-9893-58bd-9fdd50298723@lechnology.com> References: <20170728222015.43574-1-andriy.shevchenko@linux.intel.com> <20170728222015.43574-2-andriy.shevchenko@linux.intel.com> <9d74cad7-be8e-9893-58bd-9fdd50298723@lechnology.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-iio-owner@vger.kernel.org List-Id: linux-iio@vger.kernel.org On Sat, 29 Jul 2017 20:27:13 -0500 David Lechner wrote: > On 07/28/2017 05:20 PM, Andy Shevchenko wrote: > > ACPI enabled platforms do not have a mean of regulators. Instead we use > > hard coded voltage value for reference pin. When value is 0 (default) we > > fall back to request a regulator. > > > > Signed-off-by: Andy Shevchenko > > --- > > drivers/iio/adc/ti-ads7950.c | 24 ++++++++++++++++++++---- > > 1 file changed, 20 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c > > index 16a06633332c..ab669af291b7 100644 > > --- a/drivers/iio/adc/ti-ads7950.c > > +++ b/drivers/iio/adc/ti-ads7950.c > > @@ -21,6 +21,7 @@ > > * GNU General Public License for more details. > > */ > > > > +#include > > #include > > #include > > #include > > @@ -37,6 +38,12 @@ > > #include > > #include > > > > +/* > > + * In case of ACPI, we use the 5000 mV as default for the reference pin. > > + * Device tree users encode that via the vref-supply regulator. > > + */ > > +#define TI_ADS7950_VA_MV_ACPI_DEFAULT 5000 > > Now that I've thought about it a bit, you don't need to call this the > ACPI default, just call it the default. Then it will be obvious that > this is used when you don't have a regulator. > > > + > > #define TI_ADS7950_CR_MANUAL BIT(12) > > #define TI_ADS7950_CR_WRITE BIT(11) > > #define TI_ADS7950_CR_CHAN(ch) ((ch) << 7) > > @@ -58,6 +65,7 @@ struct ti_ads7950_state { > > struct spi_message scan_single_msg; > > > > struct regulator *reg; > > + unsigned int vref_mv; > > > > unsigned int settings; > > > > @@ -305,11 +313,15 @@ static int ti_ads7950_get_range(struct ti_ads7950_state *st) > > { > > int vref; > > > > - vref = regulator_get_voltage(st->reg); > > - if (vref < 0) > > - return vref; > > + if (st->vref_mv) { > > + vref = st->vref_mv; > > + } else { > > + vref = regulator_get_voltage(st->reg); > > + if (vref < 0) > > + return vref; > > > > - vref /= 1000; > > + vref /= 1000; > > + } > > > > if (st->settings & TI_ADS7950_CR_RANGE_5V) > > vref *= 2; > > @@ -411,6 +423,10 @@ static int ti_ads7950_probe(struct spi_device *spi) > > spi_message_init_with_transfers(&st->scan_single_msg, > > st->scan_single_xfer, 3); > > > > + /* Use hard coded value for reference voltage in ACPI case */ > > + if (ACPI_COMPANION(&spi->dev)) > > + st->vref_mv = TI_ADS7950_VA_MV_ACPI_DEFAULT; > > Instead of checking or ACPI, you could just say "if we have a dummy > regulator, then use the default value". > Agreed. Sounds sensible to me. Hopefully in DT people will provide the right regulator, but chances are this won't always happen. Jonathan > > + > > st->reg = devm_regulator_get(&spi->dev, "vref"); > > if (IS_ERR(st->reg)) { > > dev_err(&spi->dev, "Failed get get regulator \"vref\"\n"); > > >