All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexandre Belloni <alexandre.belloni@free-electrons.com>
To: Hector Palacios <hector.palacios@digi.com>
Cc: Lars-Peter Clausen <lars@metafoo.de>,
	"linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"devicetree-discuss@lists.ozlabs.org" 
	<devicetree-discuss@lists.ozlabs.org>,
	"jic23@kernel.org" <jic23@kernel.org>,
	"fabio.estevam@freescale.com" <fabio.estevam@freescale.com>,
	"marex@denx.de" <marex@denx.de>
Subject: Re: [PATCH v3 4/5] iio: mxs-lradc: add scale_available file to channels
Date: Fri, 26 Jul 2013 15:17:12 +0200	[thread overview]
Message-ID: <51F276D8.9090906@free-electrons.com> (raw)
In-Reply-To: <51EE8445.6070603@digi.com>

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.



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


WARNING: multiple messages have this Message-ID (diff)
From: Alexandre Belloni <alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
To: Hector Palacios <hector.palacios-i7dp0qKlBMg@public.gmane.org>
Cc: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>,
	"linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org"
	<devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org>,
	"jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org"
	<jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	"fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org"
	<fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>,
	"marex-ynQEQJNshbs@public.gmane.org"
	<marex-ynQEQJNshbs@public.gmane.org>
Subject: Re: [PATCH v3 4/5] iio: mxs-lradc: add scale_available file to channels
Date: Fri, 26 Jul 2013 15:17:12 +0200	[thread overview]
Message-ID: <51F276D8.9090906@free-electrons.com> (raw)
In-Reply-To: <51EE8445.6070603-i7dp0qKlBMg@public.gmane.org>

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.



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

  reply	other threads:[~2013-07-26 13:17 UTC|newest]

Thread overview: 76+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-22 14:03 [PATCH v3 0/5] iio: mxs-lradc: add support to optional divider_by_two Hector Palacios
2013-07-22 14:03 ` Hector Palacios
2013-07-22 14:03 ` [PATCH v3 1/5] iio: mxs-lradc: change the realbits to 12 Hector Palacios
2013-07-22 14:03   ` Hector Palacios
2013-08-13 21:24   ` Jonathan Cameron
2013-08-13 21:24     ` Jonathan Cameron
2013-07-22 14:04 ` [PATCH v3 2/5] ARM: dts: add reference voltage property for MXS LRADC Hector Palacios
2013-07-22 14:04   ` Hector Palacios
2013-07-22 18:34   ` Lars-Peter Clausen
2013-07-22 18:34     ` Lars-Peter Clausen
2013-07-22 22:06     ` Marek Vasut
2013-07-22 22:06       ` Marek Vasut
2013-07-26  9:23       ` Alexandre Belloni
2013-07-26  9:23         ` Alexandre Belloni
2013-08-13 21:23   ` Jonathan Cameron
2013-08-13 21:23     ` Jonathan Cameron
2013-08-13 21:23     ` Jonathan Cameron
2013-08-14 14:44     ` Pawel Moll
2013-08-14 14:44       ` Pawel Moll
2013-08-14 14:44       ` Pawel Moll
2013-08-21 22:13       ` Alexandre Belloni
2013-08-21 22:13         ` Alexandre Belloni
2013-08-21 22:13         ` Alexandre Belloni
2013-08-22  6:17         ` Jonathan Cameron
2013-08-22  6:17           ` Jonathan Cameron
2013-08-22  6:17           ` Jonathan Cameron
2013-08-22 16:51           ` Pawel Moll
2013-08-22 16:51             ` Pawel Moll
2013-08-23 23:00             ` Jonathan Cameron
2013-09-23 12:47               ` Alexandre Belloni
2013-09-23 12:47                 ` Alexandre Belloni
2013-09-23 13:39                 ` Hector Palacios
2013-09-23 13:39                   ` Hector Palacios
2013-08-22  8:05         ` Hector Palacios
2013-08-22  8:05           ` Hector Palacios
2013-08-22  8:05           ` Hector Palacios
2013-08-22 16:50           ` Pawel Moll
2013-08-22 16:50             ` Pawel Moll
2013-08-22 16:50             ` Pawel Moll
2013-08-22 16:41         ` Pawel Moll
2013-08-22 16:41           ` Pawel Moll
2013-08-22 16:41           ` Pawel Moll
2013-08-22 17:00           ` Lars-Peter Clausen
2013-08-22 17:00             ` Lars-Peter Clausen
2013-08-22 17:00             ` Lars-Peter Clausen
2013-07-22 14:04 ` [PATCH v3 3/5] iio: mxs-lradc: add scale attribute to channels Hector Palacios
2013-07-22 14:04   ` Hector Palacios
2013-07-22 14:04 ` [PATCH v3 4/5] iio: mxs-lradc: add scale_available file " Hector Palacios
2013-07-22 14:04   ` Hector Palacios
2013-07-22 22:36   ` Marek Vasut
2013-07-23  7:00     ` Hector Palacios
2013-07-23  7:00       ` Hector Palacios
2013-07-23  7:00       ` Hector Palacios
2013-07-23  8:46   ` Lars-Peter Clausen
2013-07-23  8:46     ` Lars-Peter Clausen
2013-07-23 13:25     ` Hector Palacios
2013-07-23 13:25       ` Hector Palacios
2013-07-23 13:25       ` Hector Palacios
2013-07-26 13:17       ` Alexandre Belloni [this message]
2013-07-26 13:17         ` Alexandre Belloni
2013-07-26 13:17         ` Alexandre Belloni
2013-07-26 16:13         ` Jonathan Cameron
2013-07-26 16:13           ` Jonathan Cameron
2013-07-26 16:13           ` Jonathan Cameron
2013-08-07  7:50           ` Alexandre Belloni
2013-08-07  7:50             ` Alexandre Belloni
2013-08-07  7:50             ` Alexandre Belloni
2013-08-13 21:26             ` Jonathan Cameron
2013-08-13 21:26               ` Jonathan Cameron
2013-08-13 21:26               ` Jonathan Cameron
2013-07-22 14:04 ` [PATCH v3 5/5] iio: mxs-lradc: add write_raw function to modify scale Hector Palacios
2013-07-22 14:04   ` Hector Palacios
2013-07-22 22:37   ` Marek Vasut
2013-07-22 22:37     ` Marek Vasut
2013-12-06 16:28   ` Harald Geyer
2013-12-06 16:32     ` Alexandre Belloni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=51F276D8.9090906@free-electrons.com \
    --to=alexandre.belloni@free-electrons.com \
    --cc=devicetree-discuss@lists.ozlabs.org \
    --cc=fabio.estevam@freescale.com \
    --cc=hector.palacios@digi.com \
    --cc=jic23@kernel.org \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marex@denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.