lkml.org 
[lkml]   [2022]   [Jan]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v3 3/7] iio: adc: qcom-spmi-rradc: introduce round robin adc
From


On 09/01/2022 17:29, Jonathan Cameron wrote:
> On Thu, 6 Jan 2022 17:31:27 +0000
> Caleb Connolly <caleb.connolly@linaro.org> wrote:
>
>> The Round Robin ADC is responsible for reading data about the rate of
>> charge from the USB or DC in jacks, it can also read the battery
>> ID (resistence) and some temperatures. It is found on the PMI8998 and
>> PM660 Qualcomm PMICs.
>>
>> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> Hi Calib,
Hi Jonathan,

I've spent some time on this and mostly reworked things, thanks a lot for
your feedback, it's been quite interesting to learn about IIO. :)

Quite a few of the channels fit well into the (adc_code + offset) * scale format,
however the one you commented on "rradc_post_process_chg_temp()" doesn't seem to
fit, it requires multiple steps of applying offsets and scale and I haven't been
able to re-arrange it to work sensibly.

I noticed the calibbias properties which seems like something I should expose
for "rradc_get_fab_coeff()"?

Could you point me in the right direction here? For reference my WIP tree can be
found here: https://github.com/aospm/linux/commits/upstreaming/spmi-rradc

I also tried switching to labels, but I found that when I drop the extend_name
property the driver fails to probe because multiple channels end up with the same
name in sysfs (e.g. "in_temp_raw"). I've read through the docs and looked at a few
other drivers but I wasn't able to find out what I'm missing for this to work.

I've snipped to the relevant bits below.

Kind regards,
Caleb
>
> Various things inline but biggest is probably that in IIO we prefer
> if possible to make application of offsets and scales a job for the caller,
> either userspace or in kernel callers. This allows them to maintain precision
> better if they need to further transform the data.
>
> Jonathan
>
>> ---
>> drivers/iio/adc/Kconfig | 13 +
>> drivers/iio/adc/Makefile | 1 +
>> drivers/iio/adc/qcom-spmi-rradc.c | 1070 +++++++++++++++++++++++++++++
>> 3 files changed, 1084 insertions(+)
>> create mode 100644 drivers/iio/adc/qcom-spmi-rradc.c
>>

[snip]

>> +static int rradc_post_process_chg_temp(struct rradc_chip *chip, u16 adc_code,
>> + int *result_millidegc)
>> +{
>> + int64_t uv, offset, slope;
>> + int ret;
>> +
>> + ret = rradc_get_fab_coeff(chip, &offset, &slope);
>> + if (ret < 0) {
>> + dev_err(chip->dev, "Unable to get fab id coefficients\n");
>> + return -EINVAL;
>> + }
>> +
>> + uv = ((int64_t)adc_code * RR_ADC_TEMP_FS_VOLTAGE_NUM);
>> + uv = div64_s64(uv,
>> + (RR_ADC_TEMP_FS_VOLTAGE_DEN * RR_ADC_CHAN_MAX_VALUE));
>> + uv = offset - uv;
>> + uv = div64_s64((uv * MILLI), slope);
>> + uv += RR_ADC_CHG_TEMP_OFFSET_MILLI_DEGC;
>> + *result_millidegc = (int)uv;
>
> Marginally harder than the one below, but this is still looking like it can
> be well expressed as an offset + scale. Thus making the tedious maths
> userspaces or callers problem. I'm working backwards hence won't comment on
> similar before this point. Key is to transform whatever maths you have into
>
> (adc_code + offset) * scale then expose offset and scale as well as the
> raw value. The right maths will get done for in kernel users and
> userspace can do it nicely with floating point.
>
>> +
>> + return 0;
>> +}

[snip]

>> +static const struct iio_chan_spec rradc_iio_chans[RR_ADC_CHAN_MAX] = {
>> + {
>> + .extend_name = "batt_id",
>
> We recently introduced channel labels to try and avoid the need for
> extend_name. The problem with extend_name is that generic software then
> has trouble parsing the resulting sysfs files as they can have very
> freeform naming. Moving it to label makes that much easier. Note that
> there is code to give a default label of extend_name to work around
> this problem for older drivers.
>
>> + .type = IIO_RESISTANCE,
>> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
>> + .address = RR_ADC_BATT_ID,
>> + },

>
> Thanks,
>
> Jonathan

--
Kind Regards,
Caleb (they/them)

\
 
 \ /
  Last update: 2022-01-19 18:43    [W:0.066 / U:0.068 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site