Re: [PATCH v3 4/5] iio: mxs-lradc: add scale_available file to channels

[Alexandre Belloni]

Hi Jonathan, Lars

I would really like to see that included in 3.12. While I agree there is
room for improvement, I don't think there are any comments that need an
immediate action before inclusion apart from the decision that has to be
taken for the device tree.

Can you confirm whether you'll take this series as is ?

Regards,

On 26/07/2013 18:13, Jonathan Cameron wrote:
>
> Alexandre Belloni <alexandre.belloni@xxxxxxxxxxxxxxxxxx> wrote:
>> On 23/07/2013 15:25, Hector Palacios wrote:
>>> Dear Lars,
>>>
>>> On 07/23/2013 10:46 AM, Lars-Peter Clausen wrote:
>>>> On 07/22/2013 04:04 PM, Hector Palacios wrote:
>>>> [...]
>>>>> +static ssize_t mxs_lradc_show_scale_available_ch(struct device
>> *dev,
>>>>> +        struct device_attribute *attr,
>>>>> +        char *buf,
>>>>> +        int ch)
>>>>> +{
>>>>> +    struct iio_dev *iio = dev_to_iio_dev(dev);
>>>>> +    struct mxs_lradc *lradc = iio_priv(iio);
>>>>> +    int i, len = 0;
>>>>> +
>>>>> +    for (i = 0; i < ARRAY_SIZE(lradc->scale_avail[ch]); i++)
>>>>> +        len += sprintf(buf + len, "%d.%09u ",
>>>>> +                   lradc->scale_avail[ch][i].integer,
>>>>> +                   lradc->scale_avail[ch][i].nano);
>>>>> +
>>>>> +    len += sprintf(buf + len, "\n");
>>>>> +
>>>>> +    return len;
>>>>> +}
>>>>> +
>>>>> +static ssize_t mxs_lradc_show_scale_available(struct device *dev,
>>>>> +        struct device_attribute *attr,
>>>>> +        char *buf)
>>>>> +{
>>>>> +    struct iio_dev_attr *iio_attr = to_iio_dev_attr(attr);
>>>>> +
>>>>> +    return mxs_lradc_show_scale_available_ch(dev, attr, buf,
>>>>> +                         iio_attr->address);
>>>>> +}
>>>>> +
>>>>> +#define SHOW_SCALE_AVAILABLE_ATTR(ch)                    \
>>>>> +static IIO_DEVICE_ATTR(in_voltage##ch##_scale_available, S_IRUGO, 
>>  \
>>>>> +               mxs_lradc_show_scale_available, NULL, ch)
>>>>> +
>>>>> +SHOW_SCALE_AVAILABLE_ATTR(0);
>>>>> +SHOW_SCALE_AVAILABLE_ATTR(1);
>>>>> +SHOW_SCALE_AVAILABLE_ATTR(2);
>>>>> +SHOW_SCALE_AVAILABLE_ATTR(3);
>>>>> +SHOW_SCALE_AVAILABLE_ATTR(4);
>>>>> +SHOW_SCALE_AVAILABLE_ATTR(5);
>>>>> +SHOW_SCALE_AVAILABLE_ATTR(6);
>>>>> +SHOW_SCALE_AVAILABLE_ATTR(7);
>>>>> +SHOW_SCALE_AVAILABLE_ATTR(8);
>>>>> +SHOW_SCALE_AVAILABLE_ATTR(9);
>>>>> +SHOW_SCALE_AVAILABLE_ATTR(10);
>>>>> +SHOW_SCALE_AVAILABLE_ATTR(11);
>>>>> +SHOW_SCALE_AVAILABLE_ATTR(12);
>>>>> +SHOW_SCALE_AVAILABLE_ATTR(13);
>>>>> +SHOW_SCALE_AVAILABLE_ATTR(14);
>>>>> +SHOW_SCALE_AVAILABLE_ATTR(15);
>>>>> +
>>>>> +static struct attribute *mxs_lradc_attributes[] = {
>>>>> +    &iio_dev_attr_in_voltage0_scale_available.dev_attr.attr,
>>>>> +    &iio_dev_attr_in_voltage1_scale_available.dev_attr.attr,
>>>>> +    &iio_dev_attr_in_voltage2_scale_available.dev_attr.attr,
>>>>> +    &iio_dev_attr_in_voltage3_scale_available.dev_attr.attr,
>>>>> +    &iio_dev_attr_in_voltage4_scale_available.dev_attr.attr,
>>>>> +    &iio_dev_attr_in_voltage5_scale_available.dev_attr.attr,
>>>>> +    &iio_dev_attr_in_voltage6_scale_available.dev_attr.attr,
>>>>> +    &iio_dev_attr_in_voltage7_scale_available.dev_attr.attr,
>>>>> +    &iio_dev_attr_in_voltage8_scale_available.dev_attr.attr,
>>>>> +    &iio_dev_attr_in_voltage9_scale_available.dev_attr.attr,
>>>>> +    &iio_dev_attr_in_voltage10_scale_available.dev_attr.attr,
>>>>> +    &iio_dev_attr_in_voltage11_scale_available.dev_attr.attr,
>>>>> +    &iio_dev_attr_in_voltage12_scale_available.dev_attr.attr,
>>>>> +    &iio_dev_attr_in_voltage13_scale_available.dev_attr.attr,
>>>>> +    &iio_dev_attr_in_voltage14_scale_available.dev_attr.attr,
>>>>> +    &iio_dev_attr_in_voltage15_scale_available.dev_attr.attr,
>>>>> +    NULL
>>>>> +};
>>>> This should really be using the iio_chan_spec_ext_info
>>>> infrastructure. Bonus
>>>> points for factoring out the common code used to calculate and
>>>> display the
>>>> scales.
>>> I perfectly understand. Sadly, I don't currently have the time and
>>> expertise to try to work this out the proper way. It already took
>> much
>>> longer than expected to have this driver toggle a divider flag.
>>>
>>> I'd appreciate if anyone wishes to complete this job.
>>> @Alexander, please feel free to submit your other temp patch without
>>> waiting for this one.
>>>
>> Maybe, we can get the patch set as is and do further clean up later.
>> Anyway, that driver is still in staging, right ?
>>
>> As said, I'm willing to propose something for the scale calculation.
> My thoughts exactly. Long term plans include generic handling of available attributes for the whole of IIO.
>
> It will take me a little while to catch up my backlog though!
>
> Jonathan
>
>
>


-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/