| Message ID | 20200918093356.93670-1-cristian.pop@analog.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v5] iio: adc: ad7768-1: Add channel label example | expand |
On Fri, 18 Sep 2020 20:55:10 +0800 kernel test robot <lkp@intel.com> 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 <lkp@intel.com> > > 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/kbuild-all@lists.01.org
On Fri, 18 Sep 2020 12:33:56 +0300 Cristian Pop <cristian.pop@analog.com> 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 <cristian.pop@analog.com> > --- > 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 <jic23@kernel.org> wrote: > On Fri, 18 Sep 2020 12:33:56 +0300 > Cristian Pop <cristian.pop@analog.com> 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 <cristian.pop@analog.com> > > --- > > 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, >
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,
This is a demo usage of new "label" attribute for channel. Signed-off-by: Cristian Pop <cristian.pop@analog.com> --- Changes in V5: Create a separate patch file for this commit drivers/iio/adc/ad7768-1.c | 49 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+)