From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from saturn.retrosnub.co.uk ([178.18.118.26]:52513 "EHLO saturn.retrosnub.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753590AbbEHSaR (ORCPT ); Fri, 8 May 2015 14:30:17 -0400 Message-ID: <554CBD24.3060205@kernel.org> Date: Fri, 08 May 2015 09:41:56 -0400 From: Jonathan Cameron MIME-Version: 1.0 To: Ezequiel Garcia , linux-iio@vger.kernel.org, Lars-Peter Clausen CC: Naidu Tellapati , James Hartley , phani.movva@imgtec.com Subject: Re: [PATCH 5/5] iio: adc: cc10001: Power-up the ADC at probe time when used remotely References: <1431033842-1149-1-git-send-email-ezequiel.garcia@imgtec.com> In-Reply-To: <1431033842-1149-1-git-send-email-ezequiel.garcia@imgtec.com> Content-Type: text/plain; charset=windows-1252 Sender: linux-iio-owner@vger.kernel.org List-Id: linux-iio@vger.kernel.org On 07/05/15 17:24, Ezequiel Garcia wrote: > From: Naidu Tellapati > > The ADC is typically shared with remote CPUs not running Linux. > However, there is only one register to power-up/power-down. Remote CPUs > aren't able to power-up the ADC, and rely in Linux doing it instead. > > This commit uses the adc-reserved-channels devicetree property to > distinguish shared usage. In this case, the ADC is powered up at > probe time. > > If the ADC is used only by the CPU running Linux, power-up/down > at runtime, only when neeeded. > > Signed-off-by: Naidu Tellapati > Signed-off-by: Ezequiel Garcia Seems sensible to me, but as you observe it's not a fix as such, so will hold this until the other patches have propagated through to the togreg tree. Feel free to remind me if I seem to have forgotten! Jonathan > --- > drivers/iio/adc/cc10001_adc.c | 26 +++++++++++++++++++++----- > 1 file changed, 21 insertions(+), 5 deletions(-) > > diff --git a/drivers/iio/adc/cc10001_adc.c b/drivers/iio/adc/cc10001_adc.c > index 10c734d..7d1b2ae 100644 > --- a/drivers/iio/adc/cc10001_adc.c > +++ b/drivers/iio/adc/cc10001_adc.c > @@ -62,6 +62,7 @@ struct cc10001_adc_device { > struct regulator *reg; > u16 *buf; > > + bool shared; > struct mutex lock; > unsigned int start_delay_ns; > unsigned int eoc_delay_ns; > @@ -155,7 +156,8 @@ static irqreturn_t cc10001_adc_trigger_h(int irq, void *p) > > mutex_lock(&adc_dev->lock); > > - cc10001_adc_power_up(adc_dev); > + if (!adc_dev->shared) > + cc10001_adc_power_up(adc_dev); > > /* Calculate delay step for eoc and sampled data */ > delay_ns = adc_dev->eoc_delay_ns / CC10001_MAX_POLL_COUNT; > @@ -179,7 +181,8 @@ static irqreturn_t cc10001_adc_trigger_h(int irq, void *p) > } > > done: > - cc10001_adc_power_down(adc_dev); > + if (!adc_dev->shared) > + cc10001_adc_power_down(adc_dev); > > mutex_unlock(&adc_dev->lock); > > @@ -198,7 +201,8 @@ static u16 cc10001_adc_read_raw_voltage(struct iio_dev *indio_dev, > unsigned int delay_ns; > u16 val; > > - cc10001_adc_power_up(adc_dev); > + if (!adc_dev->shared) > + cc10001_adc_power_up(adc_dev); > > /* Calculate delay step for eoc and sampled data */ > delay_ns = adc_dev->eoc_delay_ns / CC10001_MAX_POLL_COUNT; > @@ -207,7 +211,8 @@ static u16 cc10001_adc_read_raw_voltage(struct iio_dev *indio_dev, > > val = cc10001_adc_poll_done(indio_dev, chan->channel, delay_ns); > > - cc10001_adc_power_down(adc_dev); > + if (!adc_dev->shared) > + cc10001_adc_power_down(adc_dev); > > return val; > } > @@ -324,8 +329,10 @@ static int cc10001_adc_probe(struct platform_device *pdev) > adc_dev = iio_priv(indio_dev); > > channel_map = GENMASK(CC10001_ADC_NUM_CHANNELS - 1, 0); > - if (!of_property_read_u32(node, "adc-reserved-channels", &ret)) > + if (!of_property_read_u32(node, "adc-reserved-channels", &ret)) { > + adc_dev->shared = true; > channel_map &= ~ret; > + } > > adc_dev->reg = devm_regulator_get(&pdev->dev, "vref"); > if (IS_ERR(adc_dev->reg)) > @@ -370,6 +377,14 @@ static int cc10001_adc_probe(struct platform_device *pdev) > adc_dev->eoc_delay_ns = NSEC_PER_SEC / adc_clk_rate; > adc_dev->start_delay_ns = adc_dev->eoc_delay_ns * CC10001_WAIT_CYCLES; > > + /* > + * There is only one register to power-up/power-down the AUX ADC. > + * If the ADC is shared among multiple CPUs, always power it up here. > + * If the ADC is used only by the MIPS, power-up/power-down at runtime. > + */ > + if (adc_dev->shared) > + cc10001_adc_power_up(adc_dev); > + > /* Setup the ADC channels available on the device */ > ret = cc10001_adc_channel_init(indio_dev, channel_map); > if (ret < 0) > @@ -404,6 +419,7 @@ static int cc10001_adc_remove(struct platform_device *pdev) > struct iio_dev *indio_dev = platform_get_drvdata(pdev); > struct cc10001_adc_device *adc_dev = iio_priv(indio_dev); > > + cc10001_adc_power_down(adc_dev); > iio_device_unregister(indio_dev); > iio_triggered_buffer_cleanup(indio_dev); > clk_disable_unprepare(adc_dev->adc_clk); >