This is a demo usage of new "label" attribute for channel.
Signed-off-by: Cristian Pop <[email protected]>
---
Changes in V5:
Create a separate patch file for this commit
drivers/iio/adc/ad7768-1.c | 49 ++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
index 0d132708c429..5ca9f9febb5a 100644
--- a/drivers/iio/adc/ad7768-1.c
+++ b/drivers/iio/adc/ad7768-1.c
@@ -161,6 +161,7 @@ struct ad7768_state {
struct completion completion;
struct iio_trigger *trig;
struct gpio_desc *gpio_sync_in;
+ const char **labels;
/*
* DMA (thus cache coherency maintenance) requires the
* transfer buffers to live in their own cache lines.
@@ -407,6 +408,14 @@ static int ad7768_write_raw(struct iio_dev *indio_dev,
}
}
+static int ad7768_read_label(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan, char *label)
+{
+ struct ad7768_state *st = iio_priv(indio_dev);
+
+ return sprintf(label, "%s\n", st->labels[chan->channel]);
+}
+
static struct attribute *ad7768_attributes[] = {
&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
NULL
@@ -420,6 +429,7 @@ static const struct iio_info ad7768_info = {
.attrs = &ad7768_group,
.read_raw = &ad7768_read_raw,
.write_raw = &ad7768_write_raw,
+ .read_label = ad7768_read_label,
.debugfs_reg_access = &ad7768_reg_access,
};
@@ -538,6 +548,41 @@ static void ad7768_clk_disable(void *data)
clk_disable_unprepare(st->mclk);
}
+static int ad7768_set_channel_label(struct iio_dev *indio_dev,
+ int num_channels)
+{
+ struct ad7768_state *st = iio_priv(indio_dev);
+ struct device *device = indio_dev->dev.parent;
+ struct fwnode_handle *fwnode;
+ struct fwnode_handle *child;
+ const char *label;
+ int crt_ch = 0;
+
+ st->labels = devm_kcalloc(indio_dev->dev.parent,
+ num_channels,
+ sizeof(**st->labels),
+ GFP_KERNEL);
+
+ if (!st->labels)
+ return -ENOMEM;
+
+ fwnode = dev_fwnode(device);
+ fwnode_for_each_child_node(fwnode, child) {
+ if (fwnode_property_read_u32(child, "reg", &crt_ch))
+ continue;
+
+ if (crt_ch >= num_channels)
+ continue;
+
+ if (fwnode_property_read_string(child, "label", &label))
+ continue;
+
+ st->labels[crt_ch] = label;
+ }
+
+ return 0;
+}
+
static int ad7768_probe(struct spi_device *spi)
{
struct ad7768_state *st;
@@ -611,6 +656,10 @@ static int ad7768_probe(struct spi_device *spi)
init_completion(&st->completion);
+ ret = ad7768_set_channel_label(indio_dev, ARRAY_SIZE(ad7768_channels));
+ if (ret)
+ return ret;
+
ret = devm_request_irq(&spi->dev, spi->irq,
&ad7768_interrupt,
IRQF_TRIGGER_RISING | IRQF_ONESHOT,
--
2.17.1
Hi Cristian,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on iio/togreg]
[also build test ERROR on linux/master linus/master v5.9-rc5 next-20200918]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Cristian-Pop/iio-adc-ad7768-1-Add-channel-label-example/20200918-190340
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: x86_64-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
# save the attached .config to linux build tree
make W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
>> drivers/iio/adc/ad7768-1.c:432:3: error: 'const struct iio_info' has no member named 'read_label'; did you mean 'read_raw'?
432 | .read_label = ad7768_read_label,
| ^~~~~~~~~~
| read_raw
>> drivers/iio/adc/ad7768-1.c:432:16: error: initialization of 'int (*)(struct iio_dev *, const struct iio_chan_spec *, long int)' from incompatible pointer type 'int (*)(struct iio_dev *, const struct iio_chan_spec *, char *)' [-Werror=incompatible-pointer-types]
432 | .read_label = ad7768_read_label,
| ^~~~~~~~~~~~~~~~~
drivers/iio/adc/ad7768-1.c:432:16: note: (near initialization for 'ad7768_info.write_raw_get_fmt')
cc1: some warnings being treated as errors
# https://github.com/0day-ci/linux/commit/6144f5a8d7fa8961b6d39e36092c14301ad823c1
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Cristian-Pop/iio-adc-ad7768-1-Add-channel-label-example/20200918-190340
git checkout 6144f5a8d7fa8961b6d39e36092c14301ad823c1
vim +432 drivers/iio/adc/ad7768-1.c
427
428 static const struct iio_info ad7768_info = {
429 .attrs = &ad7768_group,
430 .read_raw = &ad7768_read_raw,
431 .write_raw = &ad7768_write_raw,
> 432 .read_label = ad7768_read_label,
433 .debugfs_reg_access = &ad7768_reg_access,
434 };
435
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
On Fri, 18 Sep 2020 20:55:10 +0800
kernel test robot <[email protected]> wrote:
> Hi Cristian,
>
> Thank you for the patch! Yet something to improve:
For anyone tracking this. The error is down to Cristian working around
an email issue by sending the series as individual patches.
Not ideal and confuses autobuilding but something odd was going on
and this got it out.
Thanks,
Jonathan
>
> [auto build test ERROR on iio/togreg]
> [also build test ERROR on linux/master linus/master v5.9-rc5 next-20200918]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/0day-ci/linux/commits/Cristian-Pop/iio-adc-ad7768-1-Add-channel-label-example/20200918-190340
> base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
> config: x86_64-allyesconfig (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
> reproduce (this is a W=1 build):
> # save the attached .config to linux build tree
> make W=1 ARCH=x86_64
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <[email protected]>
>
> All errors (new ones prefixed by >>):
>
> >> drivers/iio/adc/ad7768-1.c:432:3: error: 'const struct iio_info' has no member named 'read_label'; did you mean 'read_raw'?
> 432 | .read_label = ad7768_read_label,
> | ^~~~~~~~~~
> | read_raw
> >> drivers/iio/adc/ad7768-1.c:432:16: error: initialization of 'int (*)(struct iio_dev *, const struct iio_chan_spec *, long int)' from incompatible pointer type 'int (*)(struct iio_dev *, const struct iio_chan_spec *, char *)' [-Werror=incompatible-pointer-types]
> 432 | .read_label = ad7768_read_label,
> | ^~~~~~~~~~~~~~~~~
> drivers/iio/adc/ad7768-1.c:432:16: note: (near initialization for 'ad7768_info.write_raw_get_fmt')
> cc1: some warnings being treated as errors
>
> # https://github.com/0day-ci/linux/commit/6144f5a8d7fa8961b6d39e36092c14301ad823c1
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review Cristian-Pop/iio-adc-ad7768-1-Add-channel-label-example/20200918-190340
> git checkout 6144f5a8d7fa8961b6d39e36092c14301ad823c1
> vim +432 drivers/iio/adc/ad7768-1.c
>
> 427
> 428 static const struct iio_info ad7768_info = {
> 429 .attrs = &ad7768_group,
> 430 .read_raw = &ad7768_read_raw,
> 431 .write_raw = &ad7768_write_raw,
> > 432 .read_label = ad7768_read_label,
> 433 .debugfs_reg_access = &ad7768_reg_access,
> 434 };
> 435
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/[email protected]
On Fri, 18 Sep 2020 12:33:56 +0300
Cristian Pop <[email protected]> wrote:
> This is a demo usage of new "label" attribute for channel.
While I can see this is a demo in someways, I assume we also have
a valid usecase for doing this beyond as a demo!
So perhaps rephrase that intro.
>
> Signed-off-by: Cristian Pop <[email protected]>
> ---
> Changes in V5:
> Create a separate patch file for this commit
> drivers/iio/adc/ad7768-1.c | 49 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
>
> diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
> index 0d132708c429..5ca9f9febb5a 100644
> --- a/drivers/iio/adc/ad7768-1.c
> +++ b/drivers/iio/adc/ad7768-1.c
> @@ -161,6 +161,7 @@ struct ad7768_state {
> struct completion completion;
> struct iio_trigger *trig;
> struct gpio_desc *gpio_sync_in;
> + const char **labels;
> /*
> * DMA (thus cache coherency maintenance) requires the
> * transfer buffers to live in their own cache lines.
> @@ -407,6 +408,14 @@ static int ad7768_write_raw(struct iio_dev *indio_dev,
> }
> }
>
> +static int ad7768_read_label(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan, char *label)
> +{
> + struct ad7768_state *st = iio_priv(indio_dev);
> +
> + return sprintf(label, "%s\n", st->labels[chan->channel]);
> +}
> +
> static struct attribute *ad7768_attributes[] = {
> &iio_dev_attr_sampling_frequency_available.dev_attr.attr,
> NULL
> @@ -420,6 +429,7 @@ static const struct iio_info ad7768_info = {
> .attrs = &ad7768_group,
> .read_raw = &ad7768_read_raw,
> .write_raw = &ad7768_write_raw,
> + .read_label = ad7768_read_label,
> .debugfs_reg_access = &ad7768_reg_access,
> };
>
> @@ -538,6 +548,41 @@ static void ad7768_clk_disable(void *data)
> clk_disable_unprepare(st->mclk);
> }
>
> +static int ad7768_set_channel_label(struct iio_dev *indio_dev,
> + int num_channels)
> +{
> + struct ad7768_state *st = iio_priv(indio_dev);
> + struct device *device = indio_dev->dev.parent;
> + struct fwnode_handle *fwnode;
> + struct fwnode_handle *child;
> + const char *label;
> + int crt_ch = 0;
> +
> + st->labels = devm_kcalloc(indio_dev->dev.parent,
> + num_channels,
> + sizeof(**st->labels),
I think that ends up being the size of a single character which isn't
the intent. I assume aim is a suitable sized array of pointers to
strings which we will fill in later?
If so we can probably just use a static sized array as the maximum
number of channels is well constrained. In this particular driver
that is 1 I think!
> + GFP_KERNEL);
> +
> + if (!st->labels)
> + return -ENOMEM;
> +
> + fwnode = dev_fwnode(device);
> + fwnode_for_each_child_node(fwnode, child) {
> + if (fwnode_property_read_u32(child, "reg", &crt_ch))
> + continue;
> +
> + if (crt_ch >= num_channels)
> + continue;
> +
> + if (fwnode_property_read_string(child, "label", &label))
> + continue;
> +
> + st->labels[crt_ch] = label;
> + }
> +
> + return 0;
> +}
> +
> static int ad7768_probe(struct spi_device *spi)
> {
> struct ad7768_state *st;
> @@ -611,6 +656,10 @@ static int ad7768_probe(struct spi_device *spi)
>
> init_completion(&st->completion);
>
> + ret = ad7768_set_channel_label(indio_dev, ARRAY_SIZE(ad7768_channels));
> + if (ret)
> + return ret;
> +
> ret = devm_request_irq(&spi->dev, spi->irq,
> &ad7768_interrupt,
> IRQF_TRIGGER_RISING | IRQF_ONESHOT,
On Sat, 19 Sep 2020 16:14:40 +0100
Jonathan Cameron <[email protected]> wrote:
> On Fri, 18 Sep 2020 12:33:56 +0300
> Cristian Pop <[email protected]> wrote:
>
> > This is a demo usage of new "label" attribute for channel.
> While I can see this is a demo in someways, I assume we also have
> a valid usecase for doing this beyond as a demo!
>
> So perhaps rephrase that intro.
I also forgot to say that you also need to update the dt-bindings
for this driver. I've just committed a patch converting them to yaml
so work on top of that.
Thanks
Jonathan
>
> >
> > Signed-off-by: Cristian Pop <[email protected]>
> > ---
> > Changes in V5:
> > Create a separate patch file for this commit
> > drivers/iio/adc/ad7768-1.c | 49 ++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 49 insertions(+)
> >
> > diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
> > index 0d132708c429..5ca9f9febb5a 100644
> > --- a/drivers/iio/adc/ad7768-1.c
> > +++ b/drivers/iio/adc/ad7768-1.c
> > @@ -161,6 +161,7 @@ struct ad7768_state {
> > struct completion completion;
> > struct iio_trigger *trig;
> > struct gpio_desc *gpio_sync_in;
> > + const char **labels;
> > /*
> > * DMA (thus cache coherency maintenance) requires the
> > * transfer buffers to live in their own cache lines.
> > @@ -407,6 +408,14 @@ static int ad7768_write_raw(struct iio_dev *indio_dev,
> > }
> > }
> >
> > +static int ad7768_read_label(struct iio_dev *indio_dev,
> > + const struct iio_chan_spec *chan, char *label)
> > +{
> > + struct ad7768_state *st = iio_priv(indio_dev);
> > +
> > + return sprintf(label, "%s\n", st->labels[chan->channel]);
> > +}
> > +
> > static struct attribute *ad7768_attributes[] = {
> > &iio_dev_attr_sampling_frequency_available.dev_attr.attr,
> > NULL
> > @@ -420,6 +429,7 @@ static const struct iio_info ad7768_info = {
> > .attrs = &ad7768_group,
> > .read_raw = &ad7768_read_raw,
> > .write_raw = &ad7768_write_raw,
> > + .read_label = ad7768_read_label,
> > .debugfs_reg_access = &ad7768_reg_access,
> > };
> >
> > @@ -538,6 +548,41 @@ static void ad7768_clk_disable(void *data)
> > clk_disable_unprepare(st->mclk);
> > }
> >
> > +static int ad7768_set_channel_label(struct iio_dev *indio_dev,
> > + int num_channels)
> > +{
> > + struct ad7768_state *st = iio_priv(indio_dev);
> > + struct device *device = indio_dev->dev.parent;
> > + struct fwnode_handle *fwnode;
> > + struct fwnode_handle *child;
> > + const char *label;
> > + int crt_ch = 0;
> > +
> > + st->labels = devm_kcalloc(indio_dev->dev.parent,
> > + num_channels,
> > + sizeof(**st->labels),
>
> I think that ends up being the size of a single character which isn't
> the intent. I assume aim is a suitable sized array of pointers to
> strings which we will fill in later?
>
> If so we can probably just use a static sized array as the maximum
> number of channels is well constrained. In this particular driver
> that is 1 I think!
>
> > + GFP_KERNEL);
> > +
> > + if (!st->labels)
> > + return -ENOMEM;
> > +
> > + fwnode = dev_fwnode(device);
> > + fwnode_for_each_child_node(fwnode, child) {
> > + if (fwnode_property_read_u32(child, "reg", &crt_ch))
> > + continue;
> > +
> > + if (crt_ch >= num_channels)
> > + continue;
> > +
> > + if (fwnode_property_read_string(child, "label", &label))
> > + continue;
> > +
> > + st->labels[crt_ch] = label;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static int ad7768_probe(struct spi_device *spi)
> > {
> > struct ad7768_state *st;
> > @@ -611,6 +656,10 @@ static int ad7768_probe(struct spi_device *spi)
> >
> > init_completion(&st->completion);
> >
> > + ret = ad7768_set_channel_label(indio_dev, ARRAY_SIZE(ad7768_channels));
> > + if (ret)
> > + return ret;
> > +
> > ret = devm_request_irq(&spi->dev, spi->irq,
> > &ad7768_interrupt,
> > IRQF_TRIGGER_RISING | IRQF_ONESHOT,
>