Skip to contentSkip to conversation

Remove the mistakes detected by the check tools.

PATCH RESEND In review Eric Long 2 participants 6 messages Jan 20, 2016 – Jan 22, 2016

Subsystems (2)
lore
Share & tools
lore

Eric Long

author
#0 lore raw

cover Remove the mistakes detected by the check tools.

Both of the two patches optimize the code and remove the mistakes and warnings detected by the check tools.

Eric Long (2):
coresight: tmc/etm4x: Remove the mistakes detected by sparse tool
coresight: Remove the mistakes detected by smatch tool

 drivers/hwtracing/coresight/coresight-etm4x.c |    2 +-
 drivers/hwtracing/coresight/coresight-tmc.c   |    6 +++---
 drivers/hwtracing/coresight/of_coresight.c    |    2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)
Signature · 1 line
1.7.9.5
author
#1 lore raw

PATCH RESEND 1/2 coresight: tmc/etm4x: Remove the mistakes detected by sparse tool

The dma_alloc_coherent return an "void *" not an "void __iomen *". It uses the wrong parameters when calls dma_free coherent function, and "DEVICE_ATTR_RO" should be defined as static.

The sparse tool output logs as the following:
coresight-etm4x.c:2224:1: warning: symbol 'dev_attr_trcoslsr' was not declared. Should it be static? coresight-etm4x.c:2225:1: warning: symbol 'dev_attr_trcpdcr' was not declared. Should it be static? coresight-etm4x.c:2226:1: warning: symbol 'dev_attr_trcpdsr' was not declared. Should it be static? coresight-tmc.c:199:23: warning: incorrect type in argument 1 (different address spaces)
coresight-tmc.c:199:23: expected void *<noident>
coresight-tmc.c:199:23: got void [noderef] <asn:2>*vaddr
coresight-tmc.c:336:30: warning: incorrect type in assignment (different address spaces)
coresight-tmc.c:336:30: expected char *buf
coresight-tmc.c:336:30: got void [noderef] <asn:2>*
coresight-tmc.c:769:50: warning: incorrect type in argument 4 (different base types)
coresight-tmc.c:769:50: expected unsigned long long
[unsigned] [usertype] dma_handle
coresight-tmc.c:769:50: got restricted gfp_t

---
 drivers/hwtracing/coresight/coresight-etm4x.c |    2 +-
 drivers/hwtracing/coresight/coresight-tmc.c   |    6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)
drivers/hwtracing/coresight/coresight-etm4x.c +1 -1
index a670764..1ec6798 100644
@@ -2219,7 +2219,7 @@ static ssize_t name##_show(struct device *_dev,				\
 	return scnprintf(buf, PAGE_SIZE, "0x%x\n",			\
 			 readl_relaxed(drvdata->base + offset));	\
 }									\
-DEVICE_ATTR_RO(name)
+static DEVICE_ATTR_RO(name)
 
 coresight_simple_func(trcoslsr, TRCOSLSR);
 coresight_simple_func(trcpdcr, TRCPDCR);
drivers/hwtracing/coresight/coresight-tmc.c +3 -3
index a57c7ec..5b052d1 100644
@@ -124,7 +124,7 @@ struct tmc_drvdata {
 	bool			reading;
 	char			*buf;
 	dma_addr_t		paddr;
-	void __iomem		*vaddr;
+	void			*vaddr;
 	u32			size;
 	bool			enable;
 	enum tmc_config_type	config_type;
@@ -766,7 +766,7 @@ err_misc_register:
 err_devm_kzalloc:
 	if (drvdata->config_type == TMC_CONFIG_TYPE_ETR)
 		dma_free_coherent(dev, drvdata->size,
-				&drvdata->paddr, GFP_KERNEL);
+				drvdata->vaddr, drvdata->paddr);
 	return ret;
 }
 
@@ -778,7 +778,7 @@ static int tmc_remove(struct amba_device *adev)
 	coresight_unregister(drvdata->csdev);
 	if (drvdata->config_type == TMC_CONFIG_TYPE_ETR)
 		dma_free_coherent(drvdata->dev, drvdata->size,
-				  &drvdata->paddr, GFP_KERNEL);
+				  drvdata->vaddr, drvdata->paddr);
 
 	return 0;
 }
Signature · 1 line
1.7.9.5
author
#2 lore raw

PATCH RESEND 2/2 coresight: Remove the mistakes detected by smatch tool

In reply to Eric Long

There is an unnecessary space at the front of the code.

The smatch tool output logs as the following:
of_coresight.c:89 of_coresight_alloc_memory() warn:
inconsistent indenting

---
 drivers/hwtracing/coresight/of_coresight.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/hwtracing/coresight/of_coresight.c +1 -1
index b097361..3cc57c1 100644
@@ -86,7 +86,7 @@ static int of_coresight_alloc_memory(struct device *dev,
 		return -ENOMEM;
 
 	/* Children connected to this component via @outports */
-	 pdata->child_names = devm_kzalloc(dev, pdata->nr_outport *
+	pdata->child_names = devm_kzalloc(dev, pdata->nr_outport *
 					  sizeof(*pdata->child_names),
 					  GFP_KERNEL);
 	if (!pdata->child_names)
Signature · 1 line
1.7.9.5
#3 lore raw
In reply to Eric Long
Quoted 12 lines · Eric Long

Both of the two patches optimize the code and remove the mistakes and warnings detected by the check tools.

Eric Long (2):
coresight: tmc/etm4x: Remove the mistakes detected by sparse tool
coresight: Remove the mistakes detected by smatch tool

 drivers/hwtracing/coresight/coresight-etm4x.c |    2 +-
 drivers/hwtracing/coresight/coresight-tmc.c   |    6 +++---
 drivers/hwtracing/coresight/of_coresight.c    |    2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

Please run checkpath.pl on your patches. Also if the changes are of a different nature, ex. indentation problem and wrong argument to a function, please provide two separate patches.

Thanks,
Mathieu

Quoted 3 lines

--
1.7.9.5

author
#4 lore raw
Quoted 5 lines · Mathieu Poirier

Please run checkpath.pl on your patches. Also if the changes are of a different nature, ex. indentation problem and wrong argument to a function, please provide two separate patches.

Hi Mathieu,

I had already ran checkpath.pl, and there was no error and warning. All the changes are either indentation problems or wrong argument, except one static defined problem. So I think there is no need to separate the patch.
By the way, the patches I had send were created base on the check tools, one for smatch and another for sparse. In case I need to separate the patches, do I need to follow the this principle. Thanks.

Best Regards.
Eric

#5 lore raw
Quoted 10 lines · Eric Long
Quoted 5 lines · Mathieu Poirier

Please run checkpath.pl on your patches. Also if the changes are of a different nature, ex. indentation problem and wrong argument to a function, please provide two separate patches.

Hi Mathieu,

I had already ran checkpath.pl, and there was no error and warning.

Using a 4.4 kernel I got the following chackpatch warnings:

mpoirier at t430:~/work/linaro/coresight/kernel-maint$ ./scripts/checkpatch.pl 0001-coresight-Remove-the-mistakes-detected-by-sparse-too.patch 0002-coresight-Remove-the-mistakes-detected-by-smatch-too.patch
---------------------------------------------------------------
0001-coresight-Remove-the-mistakes-detected-by-sparse-too.patch
---------------------------------------------------------------
WARNING: A patch subject line should describe the change not the tool that found it
#4:
Subject: [PATCH 1/2] coresight: Remove the mistakes detected by sparse tool

total: 0 errors, 1 warnings, 32 lines checked

0001-coresight-Remove-the-mistakes-detected-by-sparse-too.patch has style problems, please review.
---------------------------------------------------------------
0002-coresight-Remove-the-mistakes-detected-by-smatch-too.patch
---------------------------------------------------------------
WARNING: A patch subject line should describe the change not the tool that found it
#4:
Subject: [PATCH 2/2] coresight: Remove the mistakes detected by smatch tool

total: 0 errors, 1 warnings, 8 lines checked

0002-coresight-Remove-the-mistakes-detected-by-smatch-too.patch has style problems, please review.

NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
mpoirier at t430:~/work/linaro/coresight/kernel-maint$ ls

Quoted 3 lines

All the changes are either indentation problems or wrong argument, except one static defined problem. So I think there is no need to separate the patch.

After fixing the above checkpatch warning bunching changes by tools won't make sense anymore.

Quoted 3 lines

By the way, the patches I had send were created base on the check tools, one for smatch and another for sparse. In case I need to separate the patches, do I need to follow the this principle. Thanks.

It's usually one semantic change per patch - the tool that highlighted the problem is secondary.

Quoted 3 lines

Best Regards.
Eric