lkml.org 
[lkml]   [2008]   [Dec]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH V2 2/7] mfd: PCF50633 adc driver
Andy Green wrote:
> Somebody in the thread at some point said:
>
> Hi Johnathan -
>
> | This needs a bit more explanation. Particularly as the data
> | sheet describes that accsw as 'for rationmetric measurement'.
>
> What's going on here is that ACCSW can be driven from the PMU to bias a
> resistor divider to scale what's being measured... we use it to detect
> resistor to 0V on ID pin of USB on our charger.
>
> You can power accsw / that divider "by hand" which is what we're doing
> here, or you can have the ratiometric state machine change the mux /
> bias automatically between the two measurements it takes, which we're
> not doing.
That makes sense. Thanks for the explanation.
>
> | Also, seeing as I assume this is the only driver that can touch
> | these registers and you don't change them else where, why can't
> | they be in initial setup code rather than here? (probably a good
> | reason, but be nice to have it document here!)
>
> ADCC2 setting concerned with ratiometric disable could indeed be moved
> to init, it was done like this before I did the queuing stuff and we
> took ADC measurement in two different places in the code. But in fact
> for the best we should only enable accsw until the measurement completes
> and save a tiny bit of power.
Either way sounds fine to me as I'd be surprised if it is a significant
power
drain and anyway it would come at the cost of a couple more register writes
which, if the bus is slow, would be more of an issue.

>
> |> + /* kill ratiometric, but enable ACCSW biasing */
> |> + pcf50633_reg_write(pcf, PCF50633_REG_ADCC2, 0x00);
> |> + pcf50633_reg_write(pcf, PCF50633_REG_ADCC3, 0x01);
> |> +
> |> + /* start ADC conversion on selected channel */
> |> + pcf50633_reg_write(pcf, PCF50633_REG_ADCC1, channel | avg |
> | ...
> |> +
> |> +static void pcf50633_adc_irq(int irq, void *data)
> |> +{
> |> + struct pcf50633_adc *adc = data;
> |> + struct pcf50633 *pcf = adc->pcf;
> |> + struct pcf50633_adc_request *req;
> |> + int head;
> |> + mutex_lock(&adc->queue_mutex);
> |> + head = adc->queue_head;
> |> +
> |> + req = adc->queue[head];
> |> + if (WARN_ON(!req)) {
> |> + dev_err(pcf->dev, "pcf50633-adc irq: ADC queue empty!\n");
> |> + mutex_unlock(&adc->queue_mutex);
> |> + return;
> |> + }
> |> + adc->queue[head] = NULL;
> |
> | Weird formatting?
> |
> |> + adc->queue_head = (head + 1) &
> |> + (PCF50633_MAX_ADC_FIFO_DEPTH - 1);
> |> +
> |> + mutex_unlock(&adc->queue_mutex);
>
> This would save the power
>
> ~ pcf50633_reg_write(pcf, PCF50633_REG_ADCC3, 0x0);
>
> |> + req->callback(pcf, req->callback_param, adc_result(pcf));
> |> + kfree(req);
> |> +
> |> + trigger_next_adc_job_if_any(pcf);
> |> +}
> |> +
> | Rest looks good to me.
> |
> | Acked-by: Jonathan Cameron
> |
>
> -Andy



\
 
 \ /
  Last update: 2008-12-19 13:53    [from the cache]
©2003-2011 Jasper Spaans