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
    -----BEGIN PGP SIGNED MESSAGE-----
    Hash: SHA1

    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.

    | 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.

    |> + /* 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
    -----BEGIN PGP SIGNATURE-----
    Version: GnuPG v1.4.9 (GNU/Linux)
    Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

    iEYEARECAAYFAklLkHIACgkQOjLpvpq7dMoAtACeNxmM0m69xgbvT4YIPik2aHNP
    hSIAoIUUbXcibwPYqgasik4ALFEc0img
    =//X+
    -----END PGP SIGNATURE-----


    \
     
     \ /
      Last update: 2008-12-19 13:19    [W:0.024 / U:0.096 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site