[PATCH 0/3] IIO get to warning free for W=1 C=1

[PATCH 0/3] IIO get to warning free for W=1 C=1

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Hi All,

Lee did most of the leg work for this last cycle, but there were a few
things left.  This set only gets us to warning free if we also have the
half dozen more significant fixes on the mailing list, remains of Lee's
early 2 patch sets and
[PATCH] kernel-doc: add support for ____cacheline_aligned attribute

I have a local version of the left over parts of Lee's series with the
____cacheline_aligned bits dropped but see no reason to waste review
bandwidth by posting that again.  Assuming the above patch is accepted
I'll apply those directly to the togreg branch and reply to the original
thread.

Note of course that this is warning free on my particular setup with
gcc x86_64 10.2 on other peoples particular configurations there
may still be warnings. If there are let me know or send a fix!

I have also deliberately not cleaned up the warnings in the drivers still
in staging as they make good tasks for people new to sending patches.

Jonathan Cameron (3):
  iio:imu:adis16400: Sort out missing kernel doc.
  iio: Add __printf() attributes to various allocation functions
  iio:core: Tidy up kernel-doc.

 drivers/iio/imu/adis16400.c          | 2 ++
 drivers/iio/industrialio-core.c      | 3 ++-
 drivers/iio/industrialio-trigger.c   | 2 +-
 include/linux/iio/iio.h              | 4 ++--
 include/linux/iio/trigger_consumer.h | 2 +-
 5 files changed, 8 insertions(+), 5 deletions(-)

-- 
2.28.0

[PATCH 1/3] iio:imu:adis16400: Sort out missing kernel doc.

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

I'd like to be enable W=1 for all IIO builds as it catches real issues as well
as more minor documentation issues such as this (also good to fix though!)

drivers/iio/imu/adis16400.c:183: warning: Function parameter or member 'avail_scan_mask' not described in 'adis16400_state'

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/iio/imu/adis16400.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iio/imu/adis16400.c b/drivers/iio/imu/adis16400.c
index 1ebe3e50d3e6..140cfc65ee8c 100644
--- a/drivers/iio/imu/adis16400.c
+++ b/drivers/iio/imu/adis16400.c
@@ -173,6 +173,8 @@ struct adis16400_chip_info {
  * @variant:	chip variant info
  * @filt_int:	integer part of requested filter frequency
  * @adis:	adis device
+ * @avail_scan_mask:	NULL terminated array of bitmaps of channels
+ *			that must be enabled together
  **/
 struct adis16400_state {
 	struct adis16400_chip_info	*variant;
-- 
2.28.0

[PATCH 2/3] iio: Add __printf() attributes to various allocation functions

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

A partial set of these was added to IIO a long time back.
This fills in some gaps in coverage highlighted by building
with W=1

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/industrialio-trigger.c   | 2 +-
 include/linux/iio/iio.h              | 4 ++--
 include/linux/iio/trigger_consumer.h | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
index 6f16357fd732..adfe023cad48 100644
--- a/drivers/iio/industrialio-trigger.c
+++ b/drivers/iio/industrialio-trigger.c
@@ -516,7 +516,7 @@ static void iio_trig_subirqunmask(struct irq_data *d)
 	trig->subirqs[d->irq - trig->subirq_base].enabled = true;
 }
 
-static struct iio_trigger *viio_trigger_alloc(const char *fmt, va_list vargs)
+__printf(1, 0) static struct iio_trigger *viio_trigger_alloc(const char *fmt, va_list vargs)
 {
 	struct iio_trigger *trig;
 	int i;
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index e2df67a3b9ab..a02a3efad794 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -691,8 +691,8 @@ static inline void *iio_priv(const struct iio_dev *indio_dev)
 
 void iio_device_free(struct iio_dev *indio_dev);
 struct iio_dev *devm_iio_device_alloc(struct device *parent, int sizeof_priv);
-struct iio_trigger *devm_iio_trigger_alloc(struct device *dev,
-						const char *fmt, ...);
+__printf(2, 3) struct iio_trigger *devm_iio_trigger_alloc(struct device *dev,
+							  const char *fmt, ...);
 /**
  * iio_buffer_enabled() - helper function to test if the buffer is enabled
  * @indio_dev:		IIO device structure for device
diff --git a/include/linux/iio/trigger_consumer.h b/include/linux/iio/trigger_consumer.h
index 3aa2f132dd67..2c05dfad88d7 100644
--- a/include/linux/iio/trigger_consumer.h
+++ b/include/linux/iio/trigger_consumer.h
@@ -38,7 +38,7 @@ struct iio_poll_func {
 };
 
 
-struct iio_poll_func
+__printf(5, 6) struct iio_poll_func
 *iio_alloc_pollfunc(irqreturn_t (*h)(int irq, void *p),
 		    irqreturn_t (*thread)(int irq, void *p),
 		    int type,
-- 
2.28.0

[PATCH 3/3] iio:core: Tidy up kernel-doc.

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

One comment isn't kernel-doc at all, but starts with /** and another
is simply missing a parameter that was introduced recently.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/industrialio-core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 8ddc6dfc8ed4..6e388293c828 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -170,7 +170,7 @@ static const char * const iio_chan_info_postfix[] = {
 };
 
 #if defined(CONFIG_DEBUG_FS)
-/**
+/*
  * There's also a CONFIG_DEBUG_FS guard in include/linux/iio/iio.h for
  * iio_get_debugfs_dentry() to make it inline if CONFIG_DEBUG_FS is undefined
  */
@@ -1525,6 +1525,7 @@ struct device_type iio_device_type = {
 
 /**
  * iio_device_alloc() - allocate an iio_dev from a driver
+ * @parent:		Parent device.
  * @sizeof_priv:	Space to allocate for private structure.
  **/
 struct iio_dev *iio_device_alloc(struct device *parent, int sizeof_priv)
-- 
2.28.0

Re: [PATCH 1/3] iio:imu:adis16400: Sort out missing kernel doc.

[Alexandru Ardelean]

On Sun, Sep 13, 2020 at 4:24 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> I'd like to be enable W=1 for all IIO builds as it catches real issues as well
> as more minor documentation issues such as this (also good to fix though!)
>
> drivers/iio/imu/adis16400.c:183: warning: Function parameter or member 'avail_scan_mask' not described in 'adis16400_state'
>

Acked-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
>  drivers/iio/imu/adis16400.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/iio/imu/adis16400.c b/drivers/iio/imu/adis16400.c
> index 1ebe3e50d3e6..140cfc65ee8c 100644
> --- a/drivers/iio/imu/adis16400.c
> +++ b/drivers/iio/imu/adis16400.c
> @@ -173,6 +173,8 @@ struct adis16400_chip_info {
>   * @variant:   chip variant info
>   * @filt_int:  integer part of requested filter frequency
>   * @adis:      adis device
> + * @avail_scan_mask:   NULL terminated array of bitmaps of channels
> + *                     that must be enabled together
>   **/
>  struct adis16400_state {
>         struct adis16400_chip_info      *variant;
> --
> 2.28.0
>

Re: [PATCH 1/3] iio:imu:adis16400: Sort out missing kernel doc.

[Jonathan Cameron]

On Mon, 14 Sep 2020 08:52:34 +0300
Alexandru Ardelean <ardeleanalex@gmail.com> wrote:

> On Sun, Sep 13, 2020 at 4:24 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > I'd like to be enable W=1 for all IIO builds as it catches real issues as well
> > as more minor documentation issues such as this (also good to fix though!)
> >
> > drivers/iio/imu/adis16400.c:183: warning: Function parameter or member 'avail_scan_mask' not described in 'adis16400_state'
> >  
> 
> Acked-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> 
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Cc: Alexandru Ardelean <alexandru.ardelean@analog.com>
Applied to the togreg branch of iio.git
Thanks,

Jonathan

> > ---
> >  drivers/iio/imu/adis16400.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/iio/imu/adis16400.c b/drivers/iio/imu/adis16400.c
> > index 1ebe3e50d3e6..140cfc65ee8c 100644
> > --- a/drivers/iio/imu/adis16400.c
> > +++ b/drivers/iio/imu/adis16400.c
> > @@ -173,6 +173,8 @@ struct adis16400_chip_info {
> >   * @variant:   chip variant info
> >   * @filt_int:  integer part of requested filter frequency
> >   * @adis:      adis device
> > + * @avail_scan_mask:   NULL terminated array of bitmaps of channels
> > + *                     that must be enabled together
> >   **/
> >  struct adis16400_state {
> >         struct adis16400_chip_info      *variant;
> > --
> > 2.28.0
> >  

Re: [PATCH 3/3] iio:core: Tidy up kernel-doc.

[Alexandru Ardelean]

On Sun, Sep 13, 2020 at 4:24 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> One comment isn't kernel-doc at all, but starts with /** and another
> is simply missing a parameter that was introduced recently.
>

Apologies for missing this earlier.

Acked-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  drivers/iio/industrialio-core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 8ddc6dfc8ed4..6e388293c828 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -170,7 +170,7 @@ static const char * const iio_chan_info_postfix[] = {
>  };
>
>  #if defined(CONFIG_DEBUG_FS)
> -/**
> +/*
>   * There's also a CONFIG_DEBUG_FS guard in include/linux/iio/iio.h for
>   * iio_get_debugfs_dentry() to make it inline if CONFIG_DEBUG_FS is undefined
>   */
> @@ -1525,6 +1525,7 @@ struct device_type iio_device_type = {
>
>  /**
>   * iio_device_alloc() - allocate an iio_dev from a driver
> + * @parent:            Parent device.
>   * @sizeof_priv:       Space to allocate for private structure.
>   **/
>  struct iio_dev *iio_device_alloc(struct device *parent, int sizeof_priv)
> --
> 2.28.0
>

Re: [PATCH 2/3] iio: Add __printf() attributes to various allocation functions

[Alexandru Ardelean]

On Sun, Sep 13, 2020 at 4:24 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> A partial set of these was added to IIO a long time back.
> This fills in some gaps in coverage highlighted by building
> with W=1
>

Stylistically, it looks like plenty of these attributes are specified
on a separate line before the function definition.
Example:

__printf(1, 0)
static struct iio_trigger *viio_trigger_alloc(const char *fmt, va_list vargs)

It doesn't make much difference to me how it's placed.

Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  drivers/iio/industrialio-trigger.c   | 2 +-
>  include/linux/iio/iio.h              | 4 ++--
>  include/linux/iio/trigger_consumer.h | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
> index 6f16357fd732..adfe023cad48 100644
> --- a/drivers/iio/industrialio-trigger.c
> +++ b/drivers/iio/industrialio-trigger.c
> @@ -516,7 +516,7 @@ static void iio_trig_subirqunmask(struct irq_data *d)
>         trig->subirqs[d->irq - trig->subirq_base].enabled = true;
>  }
>
> -static struct iio_trigger *viio_trigger_alloc(const char *fmt, va_list vargs)
> +__printf(1, 0) static struct iio_trigger *viio_trigger_alloc(const char *fmt, va_list vargs)
>  {
>         struct iio_trigger *trig;
>         int i;
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index e2df67a3b9ab..a02a3efad794 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -691,8 +691,8 @@ static inline void *iio_priv(const struct iio_dev *indio_dev)
>
>  void iio_device_free(struct iio_dev *indio_dev);
>  struct iio_dev *devm_iio_device_alloc(struct device *parent, int sizeof_priv);
> -struct iio_trigger *devm_iio_trigger_alloc(struct device *dev,
> -                                               const char *fmt, ...);
> +__printf(2, 3) struct iio_trigger *devm_iio_trigger_alloc(struct device *dev,
> +                                                         const char *fmt, ...);
>  /**
>   * iio_buffer_enabled() - helper function to test if the buffer is enabled
>   * @indio_dev:         IIO device structure for device
> diff --git a/include/linux/iio/trigger_consumer.h b/include/linux/iio/trigger_consumer.h
> index 3aa2f132dd67..2c05dfad88d7 100644
> --- a/include/linux/iio/trigger_consumer.h
> +++ b/include/linux/iio/trigger_consumer.h
> @@ -38,7 +38,7 @@ struct iio_poll_func {
>  };
>
>
> -struct iio_poll_func
> +__printf(5, 6) struct iio_poll_func
>  *iio_alloc_pollfunc(irqreturn_t (*h)(int irq, void *p),
>                     irqreturn_t (*thread)(int irq, void *p),
>                     int type,
> --
> 2.28.0
>

Re: [PATCH 3/3] iio:core: Tidy up kernel-doc.

[Jonathan Cameron]

On Fri, 18 Sep 2020 15:48:02 +0300
Alexandru Ardelean <ardeleanalex@gmail.com> wrote:

> On Sun, Sep 13, 2020 at 4:24 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > One comment isn't kernel-doc at all, but starts with /** and another
> > is simply missing a parameter that was introduced recently.
> >  
> 
> Apologies for missing this earlier.
No problem.  This one is you being nice rather than an ADI driver :)
> 
> Acked-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

Thanks.  Applied with yours and Andy's tag (from the request
for review email).

Jonathan

> 
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---
> >  drivers/iio/industrialio-core.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> > index 8ddc6dfc8ed4..6e388293c828 100644
> > --- a/drivers/iio/industrialio-core.c
> > +++ b/drivers/iio/industrialio-core.c
> > @@ -170,7 +170,7 @@ static const char * const iio_chan_info_postfix[] = {
> >  };
> >
> >  #if defined(CONFIG_DEBUG_FS)
> > -/**
> > +/*
> >   * There's also a CONFIG_DEBUG_FS guard in include/linux/iio/iio.h for
> >   * iio_get_debugfs_dentry() to make it inline if CONFIG_DEBUG_FS is undefined
> >   */
> > @@ -1525,6 +1525,7 @@ struct device_type iio_device_type = {
> >
> >  /**
> >   * iio_device_alloc() - allocate an iio_dev from a driver
> > + * @parent:            Parent device.
> >   * @sizeof_priv:       Space to allocate for private structure.
> >   **/
> >  struct iio_dev *iio_device_alloc(struct device *parent, int sizeof_priv)
> > --
> > 2.28.0
> >  

Re: [PATCH 2/3] iio: Add __printf() attributes to various allocation functions

[Jonathan Cameron]

On Fri, 18 Sep 2020 16:25:05 +0300
Alexandru Ardelean <ardeleanalex@gmail.com> wrote:

> On Sun, Sep 13, 2020 at 4:24 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > A partial set of these was added to IIO a long time back.
> > This fills in some gaps in coverage highlighted by building
> > with W=1
> >  
> 
> Stylistically, it looks like plenty of these attributes are specified
> on a separate line before the function definition.
> Example:
> 
> __printf(1, 0)
> static struct iio_trigger *viio_trigger_alloc(const char *fmt, va_list vargs)
> 
> It doesn't make much difference to me how it's placed.
> 
> Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Thanks. I've tweaked it wrt to both the feedback you gave and that
which Andy gave in the request to review thread.

So on this example we end up with
static printf(..)
struct

Thanks!

Jonathan

> 
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---
> >  drivers/iio/industrialio-trigger.c   | 2 +-
> >  include/linux/iio/iio.h              | 4 ++--
> >  include/linux/iio/trigger_consumer.h | 2 +-
> >  3 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
> > index 6f16357fd732..adfe023cad48 100644
> > --- a/drivers/iio/industrialio-trigger.c
> > +++ b/drivers/iio/industrialio-trigger.c
> > @@ -516,7 +516,7 @@ static void iio_trig_subirqunmask(struct irq_data *d)
> >         trig->subirqs[d->irq - trig->subirq_base].enabled = true;
> >  }
> >
> > -static struct iio_trigger *viio_trigger_alloc(const char *fmt, va_list vargs)
> > +__printf(1, 0) static struct iio_trigger *viio_trigger_alloc(const char *fmt, va_list vargs)
> >  {
> >         struct iio_trigger *trig;
> >         int i;
> > diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> > index e2df67a3b9ab..a02a3efad794 100644
> > --- a/include/linux/iio/iio.h
> > +++ b/include/linux/iio/iio.h
> > @@ -691,8 +691,8 @@ static inline void *iio_priv(const struct iio_dev *indio_dev)
> >
> >  void iio_device_free(struct iio_dev *indio_dev);
> >  struct iio_dev *devm_iio_device_alloc(struct device *parent, int sizeof_priv);
> > -struct iio_trigger *devm_iio_trigger_alloc(struct device *dev,
> > -                                               const char *fmt, ...);
> > +__printf(2, 3) struct iio_trigger *devm_iio_trigger_alloc(struct device *dev,
> > +                                                         const char *fmt, ...);
> >  /**
> >   * iio_buffer_enabled() - helper function to test if the buffer is enabled
> >   * @indio_dev:         IIO device structure for device
> > diff --git a/include/linux/iio/trigger_consumer.h b/include/linux/iio/trigger_consumer.h
> > index 3aa2f132dd67..2c05dfad88d7 100644
> > --- a/include/linux/iio/trigger_consumer.h
> > +++ b/include/linux/iio/trigger_consumer.h
> > @@ -38,7 +38,7 @@ struct iio_poll_func {
> >  };
> >
> >
> > -struct iio_poll_func
> > +__printf(5, 6) struct iio_poll_func
> >  *iio_alloc_pollfunc(irqreturn_t (*h)(int irq, void *p),
> >                     irqreturn_t (*thread)(int irq, void *p),
> >                     int type,
> > --
> > 2.28.0
> >