[PATCH 5/5] iio: adc: cc10001: Power-up the ADC at probe time when used remotely

[PATCH 5/5] iio: adc: cc10001: Power-up the ADC at probe time when used remotely

[Ezequiel Garcia]

From: Naidu Tellapati <naidu.tellapati@imgtec.com>

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 <naidu.tellapati@imgtec.com>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
---
 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);
-- 
2.3.3

Re: [PATCH 5/5] iio: adc: cc10001: Power-up the ADC at probe time when used remotely

[Jonathan Cameron]

On 07/05/15 17:24, Ezequiel Garcia wrote:
> From: Naidu Tellapati <naidu.tellapati@imgtec.com>
> 
> 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 <naidu.tellapati@imgtec.com>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
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);
> 

Re: [PATCH 5/5] iio: adc: cc10001: Power-up the ADC at probe time when used remotely

[Ezequiel Garcia]

On 05/08/2015 10:41 AM, Jonathan Cameron wrote:
> On 07/05/15 17:24, Ezequiel Garcia wrote:
>> From: Naidu Tellapati <naidu.tellapati@imgtec.com>
>>
>> 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 <naidu.tellapati@imgtec.com>
>> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
> 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!
> 

Ping.

Thanks!
-- 
Ezequiel

Re: [PATCH 5/5] iio: adc: cc10001: Power-up the ADC at probe time when used remotely

[Jonathan Cameron]

On 26/05/15 14:27, Ezequiel Garcia wrote:
> 
> 
> On 05/08/2015 10:41 AM, Jonathan Cameron wrote:
>> On 07/05/15 17:24, Ezequiel Garcia wrote:
>>> From: Naidu Tellapati <naidu.tellapati@imgtec.com>
>>>
>>> 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 <naidu.tellapati@imgtec.com>
>>> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
>> 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!
>>
> 
> Ping.
They haven't made it to the togreg tree yet!  Will pick up when I next
go through a pull request...

Re: [PATCH 5/5] iio: adc: cc10001: Power-up the ADC at probe time when used remotely

[Jonathan Cameron]

On 07/06/15 17:47, Jonathan Cameron wrote:
> On 26/05/15 14:27, Ezequiel Garcia wrote:
>>
>>
>> On 05/08/2015 10:41 AM, Jonathan Cameron wrote:
>>> On 07/05/15 17:24, Ezequiel Garcia wrote:
>>>> From: Naidu Tellapati <naidu.tellapati@imgtec.com>
>>>>
>>>> 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 <naidu.tellapati@imgtec.com>
>>>> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
>>> 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!
>>>
>>
>> Ping.
> They haven't made it to the togreg tree yet!  Will pick up when I next
> go through a pull request...
Now they are there. Applied to the togreg branch of iio.git.
Timing was rather against us this time, so this won't hit until the next
merge cycle now.

Sorry about that, been a busy few months!

Jonathan