[PATCH v2 0/2] Minor sparse and style fixes

[PATCH v2 0/2] Minor sparse and style fixes

[Stephen Boyd]

I got a report that kcalloc() didn't exist in coresight-cti-platform.c
on arm builds and that looked like we didn't include very many headers
to get prototypes of functions like kcalloc(), etc. The first patch fixes
this problem by including the headers and then the rest of these
patches fix minor sparse and style issues that I saw while looking
through the coresight directory.

Pathes based on linux-next.

Cc: Suzuki K Poulose <suzuki.poulose@arm.com> 
Cc: Mike Leach <mike.leach@linaro.org>
Cc: Douglas Anderson <dianders@chromium.org>

Changes from v1:
 * Dropped patches that were applied
 * Added more includes to cti.c and priv.h

Stephen Boyd (2):
  coresight: Include required headers in C files
  coresight: Avoid casting void pointers

 .../hwtracing/coresight/coresight-cti-platform.c    |  8 +++++++-
 drivers/hwtracing/coresight/coresight-cti-sysfs.c   |  7 +++++++
 drivers/hwtracing/coresight/coresight-cti.c         | 13 +++++++++++++
 drivers/hwtracing/coresight/coresight-cti.h         |  8 +++++++-
 drivers/hwtracing/coresight/coresight-priv.h        |  9 ++++++---
 5 files changed, 40 insertions(+), 5 deletions(-)


base-commit: 62eb0c79662ccb3c09c3724d0d4df2501cb8277c
-- 
Sent by a computer, using git, on the internet

[PATCH v2 1/2] coresight: Include required headers in C files

[Stephen Boyd]

We should include headers that C files use in the C files that use them
and avoid relying on implicit includes as much as possible. This helps
avoid compiler errors in the future about missing declarations when
header files change includes in the future.

Cc: Douglas Anderson <dianders@chromium.org>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Mike Leach <mike.leach@linaro.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 .../hwtracing/coresight/coresight-cti-platform.c    |  8 +++++++-
 drivers/hwtracing/coresight/coresight-cti-sysfs.c   |  7 +++++++
 drivers/hwtracing/coresight/coresight-cti.c         | 13 +++++++++++++
 drivers/hwtracing/coresight/coresight-cti.h         |  8 +++++++-
 4 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-cti-platform.c b/drivers/hwtracing/coresight/coresight-cti-platform.c
index c6c0c9b4827e..ab3bd4ed0910 100644
--- a/drivers/hwtracing/coresight/coresight-cti-platform.c
+++ b/drivers/hwtracing/coresight/coresight-cti-platform.c
@@ -2,11 +2,17 @@
 /*
  * Copyright (c) 2019, The Linaro Limited. All rights reserved.
  */
+#include <linux/coresight.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/property.h>
+#include <linux/slab.h>
 
 #include <dt-bindings/arm/coresight-cti-dt.h>
-#include <linux/of.h>
 
 #include "coresight-cti.h"
+#include "coresight-priv.h"
 
 /* Number of CTI signals in the v8 architecturally defined connection */
 #define NR_V8PE_IN_SIGS		2
diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
index aeea39cbd161..77e14e770806 100644
--- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
@@ -4,7 +4,14 @@
  * Author: Mike Leach <mike.leach@linaro.org>
  */
 
+#include <linux/atomic.h>
 #include <linux/coresight.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/sysfs.h>
 
 #include "coresight-cti.h"
 
diff --git a/drivers/hwtracing/coresight/coresight-cti.c b/drivers/hwtracing/coresight/coresight-cti.c
index 7fc1fc8d7738..be61c1705916 100644
--- a/drivers/hwtracing/coresight/coresight-cti.c
+++ b/drivers/hwtracing/coresight/coresight-cti.c
@@ -4,7 +4,20 @@
  * Author: Mike Leach <mike.leach@linaro.org>
  */
 
+#include <linux/amba/bus.h>
+#include <linux/atomic.h>
+#include <linux/bits.h>
+#include <linux/coresight.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/pm_runtime.h>
 #include <linux/property.h>
+#include <linux/spinlock.h>
+
+#include "coresight-priv.h"
 #include "coresight-cti.h"
 
 /**
diff --git a/drivers/hwtracing/coresight/coresight-cti.h b/drivers/hwtracing/coresight/coresight-cti.h
index 004df3ab9dd0..acf7b545e6b9 100644
--- a/drivers/hwtracing/coresight/coresight-cti.h
+++ b/drivers/hwtracing/coresight/coresight-cti.h
@@ -7,8 +7,14 @@
 #ifndef _CORESIGHT_CORESIGHT_CTI_H
 #define _CORESIGHT_CORESIGHT_CTI_H
 
-#include <asm/local.h>
+#include <linux/coresight.h>
+#include <linux/device.h>
+#include <linux/fwnode.h>
+#include <linux/list.h>
 #include <linux/spinlock.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
+
 #include "coresight-priv.h"
 
 /*
-- 
Sent by a computer, using git, on the internet

[PATCH v2 2/2] coresight: Avoid casting void pointers

[Stephen Boyd]

We don't need to cast void pointers, such as the amba_id data. Assign to
a local variable to make the code prettier and also return NULL instead
of 0 to make sparse happy.

Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Mike Leach <mike.leach@linaro.org>
Reviewed-by: Joe Perches <joe@perches.com>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---

Thanks Joe for finding my thinko!

 drivers/hwtracing/coresight/coresight-priv.h | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
index 5a36f0f50899..36c943ae94d5 100644
--- a/drivers/hwtracing/coresight/coresight-priv.h
+++ b/drivers/hwtracing/coresight/coresight-priv.h
@@ -215,9 +215,12 @@ cti_remove_assoc_from_csdev(struct coresight_device *csdev) {}
 /* extract the data value from a UCI structure given amba_id pointer. */
 static inline void *coresight_get_uci_data(const struct amba_id *id)
 {
-	if (id->data)
-		return ((struct amba_cs_uci_id *)(id->data))->data;
-	return 0;
+	struct amba_cs_uci_id *uci_id = id->data;
+
+	if (!uci_id)
+		return NULL;
+
+	return uci_id->data;
 }
 
 void coresight_release_platform_data(struct coresight_device *csdev,
-- 
Sent by a computer, using git, on the internet

Re: [PATCH v2 2/2] coresight: Avoid casting void pointers

[Mike Leach]

Reviewed-by mike.leach@linaro.org

On Tue, 28 Apr 2020 at 19:10, Stephen Boyd <swboyd@chromium.org> wrote:
>
> We don't need to cast void pointers, such as the amba_id data. Assign to
> a local variable to make the code prettier and also return NULL instead
> of 0 to make sparse happy.
>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Mike Leach <mike.leach@linaro.org>
> Reviewed-by: Joe Perches <joe@perches.com>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>
> Thanks Joe for finding my thinko!
>
>  drivers/hwtracing/coresight/coresight-priv.h | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> index 5a36f0f50899..36c943ae94d5 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -215,9 +215,12 @@ cti_remove_assoc_from_csdev(struct coresight_device *csdev) {}
>  /* extract the data value from a UCI structure given amba_id pointer. */
>  static inline void *coresight_get_uci_data(const struct amba_id *id)
>  {
> -       if (id->data)
> -               return ((struct amba_cs_uci_id *)(id->data))->data;
> -       return 0;
> +       struct amba_cs_uci_id *uci_id = id->data;
> +
> +       if (!uci_id)
> +               return NULL;
> +
> +       return uci_id->data;
>  }
>
>  void coresight_release_platform_data(struct coresight_device *csdev,
> --
> Sent by a computer, using git, on the internet
>


-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

Re: [PATCH v2 1/2] coresight: Include required headers in C files

[Mathieu Poirier]

Hi Stephen,

On Tue, Apr 28, 2020 at 11:10:09AM -0700, Stephen Boyd wrote:
> We should include headers that C files use in the C files that use them
> and avoid relying on implicit includes as much as possible. This helps
> avoid compiler errors in the future about missing declarations when
> header files change includes in the future.
> 
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Mike Leach <mike.leach@linaro.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>  .../hwtracing/coresight/coresight-cti-platform.c    |  8 +++++++-
>  drivers/hwtracing/coresight/coresight-cti-sysfs.c   |  7 +++++++
>  drivers/hwtracing/coresight/coresight-cti.c         | 13 +++++++++++++
>  drivers/hwtracing/coresight/coresight-cti.h         |  8 +++++++-
>  4 files changed, 34 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-cti-platform.c b/drivers/hwtracing/coresight/coresight-cti-platform.c
> index c6c0c9b4827e..ab3bd4ed0910 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-platform.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-platform.c
> @@ -2,11 +2,17 @@
>  /*
>   * Copyright (c) 2019, The Linaro Limited. All rights reserved.
>   */
> +#include <linux/coresight.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/of.h>
> +#include <linux/property.h>
> +#include <linux/slab.h>
>  
>  #include <dt-bindings/arm/coresight-cti-dt.h>
> -#include <linux/of.h>
>  
>  #include "coresight-cti.h"
> +#include "coresight-priv.h"
>  
>  /* Number of CTI signals in the v8 architecturally defined connection */
>  #define NR_V8PE_IN_SIGS		2
> diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> index aeea39cbd161..77e14e770806 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> @@ -4,7 +4,14 @@
>   * Author: Mike Leach <mike.leach@linaro.org>
>   */
>  
> +#include <linux/atomic.h>
>  #include <linux/coresight.h>
> +#include <linux/device.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +#include <linux/sysfs.h>

What is io.h and slab.h used for in coresight-cti-sysfs.c ?

>  
>  #include "coresight-cti.h"
>  
> diff --git a/drivers/hwtracing/coresight/coresight-cti.c b/drivers/hwtracing/coresight/coresight-cti.c
> index 7fc1fc8d7738..be61c1705916 100644
> --- a/drivers/hwtracing/coresight/coresight-cti.c
> +++ b/drivers/hwtracing/coresight/coresight-cti.c
> @@ -4,7 +4,20 @@
>   * Author: Mike Leach <mike.leach@linaro.org>
>   */
>  
> +#include <linux/amba/bus.h>
> +#include <linux/atomic.h>
> +#include <linux/bits.h>
> +#include <linux/coresight.h>
> +#include <linux/device.h>
> +#include <linux/io.h>

Same comment as above.

No need to send another version if these are mistakes - just let me know and
I'll do the adjustment.

Thanks,
Mathieu

> +#include <linux/kernel.h>
> +#include <linux/list.h>
> +#include <linux/mutex.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/property.h>
> +#include <linux/spinlock.h>
> +
> +#include "coresight-priv.h"
>  #include "coresight-cti.h"
>  
>  /**
> diff --git a/drivers/hwtracing/coresight/coresight-cti.h b/drivers/hwtracing/coresight/coresight-cti.h
> index 004df3ab9dd0..acf7b545e6b9 100644
> --- a/drivers/hwtracing/coresight/coresight-cti.h
> +++ b/drivers/hwtracing/coresight/coresight-cti.h
> @@ -7,8 +7,14 @@
>  #ifndef _CORESIGHT_CORESIGHT_CTI_H
>  #define _CORESIGHT_CORESIGHT_CTI_H
>  
> -#include <asm/local.h>
> +#include <linux/coresight.h>
> +#include <linux/device.h>
> +#include <linux/fwnode.h>
> +#include <linux/list.h>
>  #include <linux/spinlock.h>
> +#include <linux/sysfs.h>
> +#include <linux/types.h>
> +
>  #include "coresight-priv.h"
>  
>  /*
> -- 
> Sent by a computer, using git, on the internet
> 

Re: [PATCH v2 1/2] coresight: Include required headers in C files

[Stephen Boyd]

Quoting Mathieu Poirier (2020-04-29 11:08:18)
> Hi Stephen,
> 
> On Tue, Apr 28, 2020 at 11:10:09AM -0700, Stephen Boyd wrote:
> > diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> > index aeea39cbd161..77e14e770806 100644
> > --- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> > +++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> > @@ -4,7 +4,14 @@
> >   * Author: Mike Leach <mike.leach@linaro.org>
> >   */
> >  
> > +#include <linux/atomic.h>
> >  #include <linux/coresight.h>
> > +#include <linux/device.h>
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/slab.h>
> > +#include <linux/spinlock.h>
> > +#include <linux/sysfs.h>
> 
> What is io.h and slab.h used for in coresight-cti-sysfs.c ?

io.h is for readl_relaxed() usage in this file. I added slab for the
devm_kcalloc() but it doesn't look necessary given that device.h is
where that is defined, not slab.h. Thanks for catching that!

> 
> >  
> >  #include "coresight-cti.h"
> >  
> > diff --git a/drivers/hwtracing/coresight/coresight-cti.c b/drivers/hwtracing/coresight/coresight-cti.c
> > index 7fc1fc8d7738..be61c1705916 100644
> > --- a/drivers/hwtracing/coresight/coresight-cti.c
> > +++ b/drivers/hwtracing/coresight/coresight-cti.c
> > @@ -4,7 +4,20 @@
> >   * Author: Mike Leach <mike.leach@linaro.org>
> >   */
> >  
> > +#include <linux/amba/bus.h>
> > +#include <linux/atomic.h>
> > +#include <linux/bits.h>
> > +#include <linux/coresight.h>
> > +#include <linux/device.h>
> > +#include <linux/io.h>
> 
> Same comment as above.
> 
> No need to send another version if these are mistakes - just let me know and
> I'll do the adjustment.
> 

Same here, io.h is for the readl_relaxed() and writel_relaxed() calls.

So please remove slab.h from the two files (but not the other one) when
applying. Thanks.

Re: [PATCH v2 1/2] coresight: Include required headers in C files

[Mathieu Poirier]

On Wed, 29 Apr 2020 at 12:31, Stephen Boyd <swboyd@chromium.org> wrote:
>
> Quoting Mathieu Poirier (2020-04-29 11:08:18)
> > Hi Stephen,
> >
> > On Tue, Apr 28, 2020 at 11:10:09AM -0700, Stephen Boyd wrote:
> > > diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> > > index aeea39cbd161..77e14e770806 100644
> > > --- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> > > +++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> > > @@ -4,7 +4,14 @@
> > >   * Author: Mike Leach <mike.leach@linaro.org>
> > >   */
> > >
> > > +#include <linux/atomic.h>
> > >  #include <linux/coresight.h>
> > > +#include <linux/device.h>
> > > +#include <linux/io.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/slab.h>
> > > +#include <linux/spinlock.h>
> > > +#include <linux/sysfs.h>
> >
> > What is io.h and slab.h used for in coresight-cti-sysfs.c ?
>
> io.h is for readl_relaxed() usage in this file. I added slab for the
> devm_kcalloc() but it doesn't look necessary given that device.h is
> where that is defined, not slab.h. Thanks for catching that!
>
> >
> > >
> > >  #include "coresight-cti.h"
> > >
> > > diff --git a/drivers/hwtracing/coresight/coresight-cti.c b/drivers/hwtracing/coresight/coresight-cti.c
> > > index 7fc1fc8d7738..be61c1705916 100644
> > > --- a/drivers/hwtracing/coresight/coresight-cti.c
> > > +++ b/drivers/hwtracing/coresight/coresight-cti.c
> > > @@ -4,7 +4,20 @@
> > >   * Author: Mike Leach <mike.leach@linaro.org>
> > >   */
> > >
> > > +#include <linux/amba/bus.h>
> > > +#include <linux/atomic.h>
> > > +#include <linux/bits.h>
> > > +#include <linux/coresight.h>
> > > +#include <linux/device.h>
> > > +#include <linux/io.h>
> >
> > Same comment as above.
> >
> > No need to send another version if these are mistakes - just let me know and
> > I'll do the adjustment.
> >
>
> Same here, io.h is for the readl_relaxed() and writel_relaxed() calls.

I just noticed the "asm/io.h" at the top of linux/io.h - ok for that one.

>
> So please remove slab.h from the two files (but not the other one) when
> applying. Thanks.

You got it.

Re: [PATCH v2 1/2] coresight: Include required headers in C files

[Stephen Boyd]

Quoting Mathieu Poirier (2020-04-29 12:24:42)
> 
> >
> > So please remove slab.h from the two files (but not the other one) when
> > applying. Thanks.
> 
> You got it.

I looked in next but coresight-cti-platform.c is missing slab.h even
though I included it in my patch. There's a bare kcalloc() call in that
file, so slab.h is required.

Re: [PATCH v2 1/2] coresight: Include required headers in C files

[Mathieu Poirier]

On Sun, May 03, 2020 at 11:04:37AM -0700, Stephen Boyd wrote:
> Quoting Mathieu Poirier (2020-04-29 12:24:42)
> > 
> > >
> > > So please remove slab.h from the two files (but not the other one) when
> > > applying. Thanks.
> > 
> > You got it.
> 
> I looked in next but coresight-cti-platform.c is missing slab.h even
> though I included it in my patch. There's a bare kcalloc() call in that
> file, so slab.h is required.

I know what happened.  The above comment mentions removing slab.h in two and
leaving the "other" one in place.  But looking at the original file only
coresight-cti-platform.c and coresight-cti-sysfs.c had an slab.h.

I have made the correction.