On 12/20/2012 10:46 AM, Maxime Ripard wrote:
> Hi,
>
> Le 19/12/2012 19:37, Jean-Christophe PLAGNIOL-VILLARD a écrit :
>> The sleep mode will allow to put the add in sleep between conversion.
>>
>> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@xxxxxxxxxxxx>
>> Cc: linux-iio@xxxxxxxxxxxxxxx
>> Cc: Nicolas Ferre <nicolas.ferre@xxxxxxxxx>
>> Cc: Ludovic Desroches <ludovic.desroches@xxxxxxxxx>
>> ---
>> Documentation/devicetree/bindings/arm/atmel-adc.txt | 1 +
>> drivers/iio/adc/at91_adc.c | 19 ++++++++++---------
>> 2 files changed, 11 insertions(+), 9 deletions(-)
I have no particular problem with this, but would like a brief note on what
one looses when sleep mode is enabled? How much slower is the conversion if
we first have to come out of sleep mode? Basically I want a justification
of why we don't always enable this. Is this hardware specific or
should it simply be controllable from userspace?
>>
>> diff --git a/Documentation/devicetree/bindings/arm/atmel-adc.txt b/Documentation/devicetree/bindings/arm/atmel-adc.txt
>> index fd2d69e..efb6f02 100644
>> --- a/Documentation/devicetree/bindings/arm/atmel-adc.txt
>> +++ b/Documentation/devicetree/bindings/arm/atmel-adc.txt
>> @@ -25,6 +25,7 @@ Optional properties:
>> - atmel,adc-use-res: String corresponding to an identifier from
>> atmel,adc-res-names property. If not specified, the highest
>> resolution will be used.
>> + - atmel,atmel,adc-sleep-mode: Boolean to enable of sleep mode when no conversion
>>
>> Optional trigger Nodes:
>> - Required properties:
>> diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
>> index f175a86..c563488 100644
>> --- a/drivers/iio/adc/at91_adc.c
>> +++ b/drivers/iio/adc/at91_adc.c
>> @@ -52,6 +52,7 @@ struct at91_adc_state {
>> void __iomem *reg_base;
>> struct at91_adc_reg_desc *registers;
>> u8 startup_time;
>> + bool sleep_mode;
>> struct iio_trigger **trig;
>> struct at91_adc_trigger *trigger_list;
>> u32 trigger_number;
>> @@ -455,6 +456,8 @@ static int at91_adc_probe_dt(struct at91_adc_state *st,
>> }
>> st->num_channels = prop;
>>
>> + st->sleep_mode = of_property_read_bool(node, "atmel,adc-sleep-mode");
>> +
>> if (of_property_read_u32(node, "atmel,adc-startup-time", &prop)) {
>> dev_err(&idev->dev, "Missing adc-startup-time property in the DT.\n");
>> ret = -EINVAL;
>> @@ -580,6 +583,7 @@ static int __devinit at91_adc_probe(struct platform_device *pdev)
>> struct iio_dev *idev;
>> struct at91_adc_state *st;
>> struct resource *res;
>> + u32 reg;
>>
>> idev = iio_device_alloc(sizeof(struct at91_adc_state));
>> if (idev == NULL) {
>> @@ -687,16 +691,13 @@ static int __devinit at91_adc_probe(struct platform_device *pdev)
>> */
>> ticks = round_up((st->startup_time * adc_clk /
>> 1000000) - 1, 8) / 8;
>> -
>> + reg = AT91_ADC_PRESCAL_(prsc) & AT91_ADC_PRESCAL;
>> + reg |= AT91_ADC_STARTUP_(ticks) & AT91_ADC_STARTUP;
>> if (st->low_res)
>> - at91_adc_writel(st, AT91_ADC_MR,
>> - AT91_ADC_LOWRES |
>> - (AT91_ADC_PRESCAL_(prsc) & AT91_ADC_PRESCAL) |
>> - (AT91_ADC_STARTUP_(ticks) & AT91_ADC_STARTUP));
>> - else
>> - at91_adc_writel(st, AT91_ADC_MR,
>> - (AT91_ADC_PRESCAL_(prsc) & AT91_ADC_PRESCAL) |
>> - (AT91_ADC_STARTUP_(ticks) & AT91_ADC_STARTUP));
>> + reg |= AT91_ADC_LOWRES;
>> + if (st->sleep_mode)
>> + reg |= AT91_ADC_SLEEP;
>> + at91_adc_writel(st, AT91_ADC_MR, reg);
>
> I'm fine with the code in itself, but since this also refactors what has
> been added in the previous patch, maybe you can add it in the first patch.
>
> Apart from that, you can add my Acked-by.
>
> Maxime
>
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
