[PATCH 0/2] coresight: Dismiss smatch warnings

[PATCH 0/2] coresight: Dismiss smatch warnings

[Leo Yan]

This series polishes the code to address warnings reported by the smatch
static checker.

Smatch reports a error "Function too hairy" for etm4x_sysreg_read() and
etm4x_sysreg_write(). This is a trade off to avoid large code blocks,
the implementation encapsulates logic using nested macros. But this is
not friendly to static checker. For now, the code will be kept as is.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
Leo Yan (2):
      coresight: stm: Remove redundant NULL checks
      coresight: perf: Use %px for printing pointers

 drivers/hwtracing/coresight/coresight-etm-perf.c | 4 ++--
 drivers/hwtracing/coresight/coresight-stm.c      | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)
---
base-commit: 408c97c4a5e0b634dcd15bf8b8808b382e888164
change-id: 20250611-arm_cs_fix_smatch_warning_v1-6c2b5e78e38a

Best regards,
-- 
Leo Yan <leo.yan@arm.com>

[PATCH 1/2] coresight: stm: Remove redundant NULL checks

[Leo Yan]

container_of() cannot return NULL, so the checks for NULL pointers are
unnecessary and can be safely removed.

As a result, this commit silences the following smatch warnings:

  coresight-stm.c:345 stm_generic_link() warn: can 'drvdata' even be NULL?
  coresight-stm.c:356 stm_generic_unlink() warn: can 'drvdata' even be NULL?
  coresight-stm.c:387 stm_generic_set_options() warn: can 'drvdata' even be NULL?
  coresight-stm.c:422 stm_generic_packet() warn: can 'drvdata' even be NULL?

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-stm.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
index e45c6c7204b4491e0f879bc7d5d445aa1d3118be..464b0c85c3f7d3519169d62a51e9f8c6281b5358 100644
--- a/drivers/hwtracing/coresight/coresight-stm.c
+++ b/drivers/hwtracing/coresight/coresight-stm.c
@@ -342,7 +342,7 @@ static int stm_generic_link(struct stm_data *stm_data,
 {
 	struct stm_drvdata *drvdata = container_of(stm_data,
 						   struct stm_drvdata, stm);
-	if (!drvdata || !drvdata->csdev)
+	if (!drvdata->csdev)
 		return -EINVAL;
 
 	return coresight_enable_sysfs(drvdata->csdev);
@@ -353,7 +353,7 @@ static void stm_generic_unlink(struct stm_data *stm_data,
 {
 	struct stm_drvdata *drvdata = container_of(stm_data,
 						   struct stm_drvdata, stm);
-	if (!drvdata || !drvdata->csdev)
+	if (!drvdata->csdev)
 		return;
 
 	coresight_disable_sysfs(drvdata->csdev);
@@ -384,7 +384,7 @@ static long stm_generic_set_options(struct stm_data *stm_data,
 {
 	struct stm_drvdata *drvdata = container_of(stm_data,
 						   struct stm_drvdata, stm);
-	if (!(drvdata && coresight_get_mode(drvdata->csdev)))
+	if (!coresight_get_mode(drvdata->csdev))
 		return -EINVAL;
 
 	if (channel >= drvdata->numsp)
@@ -419,7 +419,7 @@ static ssize_t notrace stm_generic_packet(struct stm_data *stm_data,
 						   struct stm_drvdata, stm);
 	unsigned int stm_flags;
 
-	if (!(drvdata && coresight_get_mode(drvdata->csdev)))
+	if (!coresight_get_mode(drvdata->csdev))
 		return -EACCES;
 
 	if (channel >= drvdata->numsp)

-- 
2.34.1

Re: [PATCH 1/2] coresight: stm: Remove redundant NULL checks

[Anshuman Khandual]

On 11/06/25 8:14 PM, Leo Yan wrote:
> container_of() cannot return NULL, so the checks for NULL pointers are
> unnecessary and can be safely removed.
> 
> As a result, this commit silences the following smatch warnings:
> 
>   coresight-stm.c:345 stm_generic_link() warn: can 'drvdata' even be NULL?
>   coresight-stm.c:356 stm_generic_unlink() warn: can 'drvdata' even be NULL?
>   coresight-stm.c:387 stm_generic_set_options() warn: can 'drvdata' even be NULL?
>   coresight-stm.c:422 stm_generic_packet() warn: can 'drvdata' even be NULL?
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-stm.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
> index e45c6c7204b4491e0f879bc7d5d445aa1d3118be..464b0c85c3f7d3519169d62a51e9f8c6281b5358 100644
> --- a/drivers/hwtracing/coresight/coresight-stm.c
> +++ b/drivers/hwtracing/coresight/coresight-stm.c
> @@ -342,7 +342,7 @@ static int stm_generic_link(struct stm_data *stm_data,
>  {
>  	struct stm_drvdata *drvdata = container_of(stm_data,
>  						   struct stm_drvdata, stm);
> -	if (!drvdata || !drvdata->csdev)
> +	if (!drvdata->csdev)
>  		return -EINVAL;
>  
>  	return coresight_enable_sysfs(drvdata->csdev);
> @@ -353,7 +353,7 @@ static void stm_generic_unlink(struct stm_data *stm_data,
>  {
>  	struct stm_drvdata *drvdata = container_of(stm_data,
>  						   struct stm_drvdata, stm);
> -	if (!drvdata || !drvdata->csdev)
> +	if (!drvdata->csdev)
>  		return;
>  
>  	coresight_disable_sysfs(drvdata->csdev);
> @@ -384,7 +384,7 @@ static long stm_generic_set_options(struct stm_data *stm_data,
>  {
>  	struct stm_drvdata *drvdata = container_of(stm_data,
>  						   struct stm_drvdata, stm);
> -	if (!(drvdata && coresight_get_mode(drvdata->csdev)))
> +	if (!coresight_get_mode(drvdata->csdev))
>  		return -EINVAL;
>  
>  	if (channel >= drvdata->numsp)
> @@ -419,7 +419,7 @@ static ssize_t notrace stm_generic_packet(struct stm_data *stm_data,
>  						   struct stm_drvdata, stm);
>  	unsigned int stm_flags;
>  
> -	if (!(drvdata && coresight_get_mode(drvdata->csdev)))
> +	if (!coresight_get_mode(drvdata->csdev))
>  		return -EACCES;
>  
>  	if (channel >= drvdata->numsp)
> 

Seems to be a sensible clean up.

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

Re: [PATCH 1/2] coresight: stm: Remove redundant NULL checks

[Mike Leach]

On Thu, 12 Jun 2025 at 06:19, Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
>
>
>
> On 11/06/25 8:14 PM, Leo Yan wrote:
> > container_of() cannot return NULL, so the checks for NULL pointers are
> > unnecessary and can be safely removed.
> >
> > As a result, this commit silences the following smatch warnings:
> >
> >   coresight-stm.c:345 stm_generic_link() warn: can 'drvdata' even be NULL?
> >   coresight-stm.c:356 stm_generic_unlink() warn: can 'drvdata' even be NULL?
> >   coresight-stm.c:387 stm_generic_set_options() warn: can 'drvdata' even be NULL?
> >   coresight-stm.c:422 stm_generic_packet() warn: can 'drvdata' even be NULL?
> >
> > Signed-off-by: Leo Yan <leo.yan@arm.com>
> > ---
> >  drivers/hwtracing/coresight/coresight-stm.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
> > index e45c6c7204b4491e0f879bc7d5d445aa1d3118be..464b0c85c3f7d3519169d62a51e9f8c6281b5358 100644
> > --- a/drivers/hwtracing/coresight/coresight-stm.c
> > +++ b/drivers/hwtracing/coresight/coresight-stm.c
> > @@ -342,7 +342,7 @@ static int stm_generic_link(struct stm_data *stm_data,
> >  {
> >       struct stm_drvdata *drvdata = container_of(stm_data,
> >                                                  struct stm_drvdata, stm);
> > -     if (!drvdata || !drvdata->csdev)
> > +     if (!drvdata->csdev)
> >               return -EINVAL;
> >
I'd agree that the container_of() cannot return NULL, but what happens
if stm_data is NULL?
Perhaps the NULL check was on the wrong parameter?

Mike

> >       return coresight_enable_sysfs(drvdata->csdev);
> > @@ -353,7 +353,7 @@ static void stm_generic_unlink(struct stm_data *stm_data,
> >  {
> >       struct stm_drvdata *drvdata = container_of(stm_data,
> >                                                  struct stm_drvdata, stm);
> > -     if (!drvdata || !drvdata->csdev)
> > +     if (!drvdata->csdev)
> >               return;
> >
> >       coresight_disable_sysfs(drvdata->csdev);
> > @@ -384,7 +384,7 @@ static long stm_generic_set_options(struct stm_data *stm_data,
> >  {
> >       struct stm_drvdata *drvdata = container_of(stm_data,
> >                                                  struct stm_drvdata, stm);
> > -     if (!(drvdata && coresight_get_mode(drvdata->csdev)))
> > +     if (!coresight_get_mode(drvdata->csdev))
> >               return -EINVAL;
> >
> >       if (channel >= drvdata->numsp)
> > @@ -419,7 +419,7 @@ static ssize_t notrace stm_generic_packet(struct stm_data *stm_data,
> >                                                  struct stm_drvdata, stm);
> >       unsigned int stm_flags;
> >
> > -     if (!(drvdata && coresight_get_mode(drvdata->csdev)))
> > +     if (!coresight_get_mode(drvdata->csdev))
> >               return -EACCES;
> >
> >       if (channel >= drvdata->numsp)
> >
>
> Seems to be a sensible clean up.
>
> Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
>


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

Re: [PATCH 1/2] coresight: stm: Remove redundant NULL checks

[Leo Yan]

On Thu, Jun 12, 2025 at 09:28:42AM +0100, Mike Leach wrote:
> On Thu, 12 Jun 2025 at 06:19, Anshuman Khandual
> <anshuman.khandual@arm.com> wrote:
> >
> >
> >
> > On 11/06/25 8:14 PM, Leo Yan wrote:
> > > container_of() cannot return NULL, so the checks for NULL pointers are
> > > unnecessary and can be safely removed.
> > >
> > > As a result, this commit silences the following smatch warnings:
> > >
> > >   coresight-stm.c:345 stm_generic_link() warn: can 'drvdata' even be NULL?
> > >   coresight-stm.c:356 stm_generic_unlink() warn: can 'drvdata' even be NULL?
> > >   coresight-stm.c:387 stm_generic_set_options() warn: can 'drvdata' even be NULL?
> > >   coresight-stm.c:422 stm_generic_packet() warn: can 'drvdata' even be NULL?
> > >
> > > Signed-off-by: Leo Yan <leo.yan@arm.com>
> > > ---
> > >  drivers/hwtracing/coresight/coresight-stm.c | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
> > > index e45c6c7204b4491e0f879bc7d5d445aa1d3118be..464b0c85c3f7d3519169d62a51e9f8c6281b5358 100644
> > > --- a/drivers/hwtracing/coresight/coresight-stm.c
> > > +++ b/drivers/hwtracing/coresight/coresight-stm.c
> > > @@ -342,7 +342,7 @@ static int stm_generic_link(struct stm_data *stm_data,
> > >  {
> > >       struct stm_drvdata *drvdata = container_of(stm_data,
> > >                                                  struct stm_drvdata, stm);
> > > -     if (!drvdata || !drvdata->csdev)
> > > +     if (!drvdata->csdev)
> > >               return -EINVAL;

> I'd agree that the container_of() cannot return NULL, but what happens
> if stm_data is NULL?
> Perhaps the NULL check was on the wrong parameter?

The pointer "stm_data" should be safe.  The data structure is allocated
in probe and released in the remove, the structure "stm_data" is safely
accessed during runtime.

If stm_data really has NULL pointer issue, we should already receive
alarms for kernel oops :)

Thanks,
Leo

[PATCH 2/2] coresight: perf: Use %px for printing pointers

[Leo Yan]

Use "%px" to print a pointer, which is better than casting the pointer
to unsigned long and printing it with the "%lx" specifier.

Note, the printing format will be updated as 64-bit value:

  # cat /sys/devices/cs_etm/sinks/trbe0
  0x000000003744496a

This commit dismisses the following smatch warnings:

  coresight-etm-perf.c:854 etm_perf_sink_name_show() warn: argument 4 to %lx specifier is cast from pointer
  coresight-etm-perf.c:946 etm_perf_cscfg_event_show() warn: argument 4 to %lx specifier is cast from pointer

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm-perf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
index f1551c08ecb20ebd7feab82dbaee3176e6bcc999..f677c08233ba1a28b277674662c6e6db904873dd 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.c
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
@@ -851,7 +851,7 @@ static ssize_t etm_perf_sink_name_show(struct device *dev,
 	struct dev_ext_attribute *ea;
 
 	ea = container_of(dattr, struct dev_ext_attribute, attr);
-	return scnprintf(buf, PAGE_SIZE, "0x%lx\n", (unsigned long)(ea->var));
+	return scnprintf(buf, PAGE_SIZE, "0x%px\n", ea->var);
 }
 
 static struct dev_ext_attribute *
@@ -943,7 +943,7 @@ static ssize_t etm_perf_cscfg_event_show(struct device *dev,
 	struct dev_ext_attribute *ea;
 
 	ea = container_of(dattr, struct dev_ext_attribute, attr);
-	return scnprintf(buf, PAGE_SIZE, "configid=0x%lx\n", (unsigned long)(ea->var));
+	return scnprintf(buf, PAGE_SIZE, "configid=0x%px\n", ea->var);
 }
 
 int etm_perf_add_symlink_cscfg(struct device *dev, struct cscfg_config_desc *config_desc)

-- 
2.34.1

Re: [PATCH 2/2] coresight: perf: Use %px for printing pointers

[Anshuman Khandual]

On 11/06/25 8:14 PM, Leo Yan wrote:
> Use "%px" to print a pointer, which is better than casting the pointer
> to unsigned long and printing it with the "%lx" specifier.
> 
> Note, the printing format will be updated as 64-bit value:
> 
>   # cat /sys/devices/cs_etm/sinks/trbe0
>   0x000000003744496a

But what was it before this patch is applied ? Just wondering if
there are sysfs changes that might be visible to the user space.

> 
> This commit dismisses the following smatch warnings:
> 
>   coresight-etm-perf.c:854 etm_perf_sink_name_show() warn: argument 4 to %lx specifier is cast from pointer
>   coresight-etm-perf.c:946 etm_perf_cscfg_event_show() warn: argument 4 to %lx specifier is cast from pointer
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-etm-perf.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
> index f1551c08ecb20ebd7feab82dbaee3176e6bcc999..f677c08233ba1a28b277674662c6e6db904873dd 100644
> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
> @@ -851,7 +851,7 @@ static ssize_t etm_perf_sink_name_show(struct device *dev,
>  	struct dev_ext_attribute *ea;
>  
>  	ea = container_of(dattr, struct dev_ext_attribute, attr);
> -	return scnprintf(buf, PAGE_SIZE, "0x%lx\n", (unsigned long)(ea->var));
> +	return scnprintf(buf, PAGE_SIZE, "0x%px\n", ea->var);
>  }
>  
>  static struct dev_ext_attribute *
> @@ -943,7 +943,7 @@ static ssize_t etm_perf_cscfg_event_show(struct device *dev,
>  	struct dev_ext_attribute *ea;
>  
>  	ea = container_of(dattr, struct dev_ext_attribute, attr);
> -	return scnprintf(buf, PAGE_SIZE, "configid=0x%lx\n", (unsigned long)(ea->var));
> +	return scnprintf(buf, PAGE_SIZE, "configid=0x%px\n", ea->var);
>  }
>  
>  int etm_perf_add_symlink_cscfg(struct device *dev, struct cscfg_config_desc *config_desc)
> 

Re: [PATCH 2/2] coresight: perf: Use %px for printing pointers

[Leo Yan]

On Thu, Jun 12, 2025 at 11:14:00AM +0530, Anshuman Khandual wrote:
> On 11/06/25 8:14 PM, Leo Yan wrote:
> > Use "%px" to print a pointer, which is better than casting the pointer
> > to unsigned long and printing it with the "%lx" specifier.
> > 
> > Note, the printing format will be updated as 64-bit value:
> > 
> >   # cat /sys/devices/cs_etm/sinks/trbe0
> >   0x000000003744496a
> 
> But what was it before this patch is applied ? Just wondering if
> there are sysfs changes that might be visible to the user space.

Good question. Before this patch, the printing does not pad with
leading zeros, so the output result is:

    # cat /sys/devices/cs_etm/sinks/trbe0
    0x3744496a

The perf tool in user space extracts the lower 32-bit value with the
specifier "%x". I verified that this would be fine that the tool can
read out the hash value properly.

    ret = perf_pmu__scan_file(pmu, path, "%x", &hash);

Thanks,
Leo


> > This commit dismisses the following smatch warnings:
> > 
> >   coresight-etm-perf.c:854 etm_perf_sink_name_show() warn: argument 4 to %lx specifier is cast from pointer
> >   coresight-etm-perf.c:946 etm_perf_cscfg_event_show() warn: argument 4 to %lx specifier is cast from pointer
> > 
> > Signed-off-by: Leo Yan <leo.yan@arm.com>
> > ---
> >  drivers/hwtracing/coresight/coresight-etm-perf.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
> > index f1551c08ecb20ebd7feab82dbaee3176e6bcc999..f677c08233ba1a28b277674662c6e6db904873dd 100644
> > --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
> > +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
> > @@ -851,7 +851,7 @@ static ssize_t etm_perf_sink_name_show(struct device *dev,
> >  	struct dev_ext_attribute *ea;
> >  
> >  	ea = container_of(dattr, struct dev_ext_attribute, attr);
> > -	return scnprintf(buf, PAGE_SIZE, "0x%lx\n", (unsigned long)(ea->var));
> > +	return scnprintf(buf, PAGE_SIZE, "0x%px\n", ea->var);
> >  }
> >  
> >  static struct dev_ext_attribute *
> > @@ -943,7 +943,7 @@ static ssize_t etm_perf_cscfg_event_show(struct device *dev,
> >  	struct dev_ext_attribute *ea;
> >  
> >  	ea = container_of(dattr, struct dev_ext_attribute, attr);
> > -	return scnprintf(buf, PAGE_SIZE, "configid=0x%lx\n", (unsigned long)(ea->var));
> > +	return scnprintf(buf, PAGE_SIZE, "configid=0x%px\n", ea->var);
> >  }
> >  
> >  int etm_perf_add_symlink_cscfg(struct device *dev, struct cscfg_config_desc *config_desc)
> > 

Re: [PATCH 0/2] coresight: Dismiss smatch warnings

[Suzuki K Poulose]

On Wed, 11 Jun 2025 15:44:00 +0100, Leo Yan wrote:
> This series polishes the code to address warnings reported by the smatch
> static checker.
> 
> Smatch reports a error "Function too hairy" for etm4x_sysreg_read() and
> etm4x_sysreg_write(). This is a trade off to avoid large code blocks,
> the implementation encapsulates logic using nested macros. But this is
> not friendly to static checker. For now, the code will be kept as is.
> 
> [...]

Applied, thanks!

[1/2] coresight: stm: Remove redundant NULL checks
      https://git.kernel.org/coresight/c/c33c8fc1
[2/2] coresight: perf: Use %px for printing pointers
      https://git.kernel.org/coresight/c/7b8584a4

Best regards,
-- 
Suzuki K Poulose <suzuki.poulose@arm.com>