[PATCH 0/7] staging: kpc2000: various minor checkpatch fixes

[PATCH 0/7] staging: kpc2000: various minor checkpatch fixes

[Simon Sandström]

Here is a bunch of checkpatch fixes for core.c in staging/kpc2000.

Some of these were sent earlier but not applied. Now rebased on top of
staging-testing (incl. Jeremy's kpc2000 misc device removal commits).


- Simon

Simon Sandström (7):
  staging: kpc2000: simplify comparisons to NULL in core.c
  staging: kpc2000: remove unnecessary parentheses in core.c
  staging: kpc2000: remove unnecessary oom message in core.c
  staging: kpc2000: use __func__ in debug messages in core.c
  staging: kpc2000: remove unnecessary include in core.c
  staging: kpc2000: use sizeof(var) in kzalloc call
  staging: kpc2000: fix incorrect code comment in core.c

 drivers/staging/kpc2000/kpc2000/core.c | 36 ++++++++++++--------------
 1 file changed, 16 insertions(+), 20 deletions(-)

-- 
2.20.1

[PATCH 1/7] staging: kpc2000: simplify comparisons to NULL in core.c

[Simon Sandström]

Fixes checkpatch.pl warnings "Comparison to NULL could be written [...]"
and "Comparisons should place the constant on the right side of the
test".

Signed-off-by: Simon Sandström <simon@nikanor.nu>
---
 drivers/staging/kpc2000/kpc2000/core.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc2000/core.c b/drivers/staging/kpc2000/kpc2000/core.c
index 7f257c21e0cc..356a272c0b9c 100644
--- a/drivers/staging/kpc2000/kpc2000/core.c
+++ b/drivers/staging/kpc2000/kpc2000/core.c
@@ -319,7 +319,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
 	 * Step 1: Allocate a struct for the pcard
 	 */
 	pcard = kzalloc(sizeof(struct kp2000_device), GFP_KERNEL);
-	if (NULL == pcard) {
+	if (!pcard) {
 		dev_err(&pdev->dev,
 			"probe: failed to allocate private card data\n");
 		return -ENOMEM;
@@ -363,7 +363,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
 	reg_bar_phys_len = pci_resource_len(pcard->pdev, REG_BAR);
 
 	pcard->regs_bar_base = ioremap_nocache(reg_bar_phys_addr, PAGE_SIZE);
-	if (NULL == pcard->regs_bar_base) {
+	if (!pcard->regs_bar_base) {
 		dev_err(&pcard->pdev->dev,
 			"probe: REG_BAR could not remap memory to virtual space\n");
 		err = -ENODEV;
@@ -396,7 +396,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
 
 	pcard->dma_bar_base = ioremap_nocache(dma_bar_phys_addr,
 					      dma_bar_phys_len);
-	if (NULL == pcard->dma_bar_base) {
+	if (!pcard->dma_bar_base) {
 		dev_err(&pcard->pdev->dev,
 			"probe: DMA_BAR could not remap memory to virtual space\n");
 		err = -ENODEV;
@@ -546,7 +546,7 @@ static void kp2000_pcie_remove(struct pci_dev *pdev)
 
 	dev_dbg(&pdev->dev, "kp2000_pcie_remove(pdev=%p)\n", pdev);
 
-	if (pcard == NULL)
+	if (!pcard)
 		return;
 
 	mutex_lock(&pcard->sem);
@@ -555,12 +555,12 @@ static void kp2000_pcie_remove(struct pci_dev *pdev)
 	sysfs_remove_files(&(pdev->dev.kobj), kp_attr_list);
 	free_irq(pcard->pdev->irq, pcard);
 	pci_disable_msi(pcard->pdev);
-	if (pcard->dma_bar_base != NULL) {
+	if (pcard->dma_bar_base) {
 		iounmap(pcard->dma_bar_base);
 		pci_release_region(pdev, DMA_BAR);
 		pcard->dma_bar_base = NULL;
 	}
-	if (pcard->regs_bar_base != NULL) {
+	if (pcard->regs_bar_base) {
 		iounmap(pcard->regs_bar_base);
 		pci_release_region(pdev, REG_BAR);
 		pcard->regs_bar_base = NULL;
-- 
2.20.1

[PATCH 2/7] staging: kpc2000: remove unnecessary parentheses in core.c

[Simon Sandström]

Fixes checkpatch.pl check "Unnecessary parentheses around
pdev->dev.kobj".

Signed-off-by: Simon Sandström <simon@nikanor.nu>
---
 drivers/staging/kpc2000/kpc2000/core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc2000/core.c b/drivers/staging/kpc2000/kpc2000/core.c
index 356a272c0b9c..dc6940e6c320 100644
--- a/drivers/staging/kpc2000/kpc2000/core.c
+++ b/drivers/staging/kpc2000/kpc2000/core.c
@@ -491,7 +491,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
 	/*
 	 * Step 9: Setup sysfs attributes
 	 */
-	err = sysfs_create_files(&(pdev->dev.kobj), kp_attr_list);
+	err = sysfs_create_files(&pdev->dev.kobj, kp_attr_list);
 	if (err) {
 		dev_err(&pdev->dev, "Failed to add sysfs files: %d\n", err);
 		goto out9;
@@ -515,7 +515,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
 	return 0;
 
 out10:
-	sysfs_remove_files(&(pdev->dev.kobj), kp_attr_list);
+	sysfs_remove_files(&pdev->dev.kobj, kp_attr_list);
 out9:
 	free_irq(pcard->pdev->irq, pcard);
 out8b:
@@ -552,7 +552,7 @@ static void kp2000_pcie_remove(struct pci_dev *pdev)
 	mutex_lock(&pcard->sem);
 	kp2000_remove_cores(pcard);
 	mfd_remove_devices(PCARD_TO_DEV(pcard));
-	sysfs_remove_files(&(pdev->dev.kobj), kp_attr_list);
+	sysfs_remove_files(&pdev->dev.kobj, kp_attr_list);
 	free_irq(pcard->pdev->irq, pcard);
 	pci_disable_msi(pcard->pdev);
 	if (pcard->dma_bar_base) {
-- 
2.20.1

[PATCH 3/7] staging: kpc2000: remove unnecessary oom message in core.c

[Simon Sandström]

Fixes checkpatch.pl warning "Possible unnecessary 'out of memory'
message".

Signed-off-by: Simon Sandström <simon@nikanor.nu>
---
 drivers/staging/kpc2000/kpc2000/core.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc2000/core.c b/drivers/staging/kpc2000/kpc2000/core.c
index dc6940e6c320..a70665a202c3 100644
--- a/drivers/staging/kpc2000/kpc2000/core.c
+++ b/drivers/staging/kpc2000/kpc2000/core.c
@@ -319,11 +319,8 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
 	 * Step 1: Allocate a struct for the pcard
 	 */
 	pcard = kzalloc(sizeof(struct kp2000_device), GFP_KERNEL);
-	if (!pcard) {
-		dev_err(&pdev->dev,
-			"probe: failed to allocate private card data\n");
+	if (!pcard)
 		return -ENOMEM;
-	}
 	dev_dbg(&pdev->dev, "probe: allocated struct kp2000_device @ %p\n",
 		pcard);
 
-- 
2.20.1

[PATCH 4/7] staging: kpc2000: use __func__ in debug messages in core.c

[Simon Sandström]

Fixes checkpatch.pl warning "Prefer using '"%s...", __func__' to using
'<function name>', this function's name, in a string".

Signed-off-by: Simon Sandström <simon@nikanor.nu>
---
 drivers/staging/kpc2000/kpc2000/core.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc2000/core.c b/drivers/staging/kpc2000/kpc2000/core.c
index a70665a202c3..6d4fc1f37c9f 100644
--- a/drivers/staging/kpc2000/kpc2000/core.c
+++ b/drivers/staging/kpc2000/kpc2000/core.c
@@ -312,8 +312,8 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
 	unsigned long dma_bar_phys_len;
 	u16 regval;
 
-	dev_dbg(&pdev->dev, "kp2000_pcie_probe(pdev = [%p], id = [%p])\n",
-		pdev, id);
+	dev_dbg(&pdev->dev, "%s(pdev = [%p], id = [%p])\n",
+		__func__, pdev, id);
 
 	/*
 	 * Step 1: Allocate a struct for the pcard
@@ -481,7 +481,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
 			 pcard->name, pcard);
 	if (rv) {
 		dev_err(&pcard->pdev->dev,
-			"kp2000_pcie_probe: failed to request_irq: %d\n", rv);
+			"%s: failed to request_irq: %d\n", __func__, rv);
 		goto out8b;
 	}
 
@@ -507,7 +507,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
 	writel(KPC_DMA_CARD_IRQ_ENABLE | KPC_DMA_CARD_USER_INTERRUPT_MODE,
 	       pcard->dma_common_regs);
 
-	dev_dbg(&pcard->pdev->dev, "kp2000_pcie_probe() complete!\n");
+	dev_dbg(&pcard->pdev->dev, "%s() complete!\n", __func__);
 	mutex_unlock(&pcard->sem);
 	return 0;
 
@@ -541,7 +541,7 @@ static void kp2000_pcie_remove(struct pci_dev *pdev)
 {
 	struct kp2000_device *pcard = pci_get_drvdata(pdev);
 
-	dev_dbg(&pdev->dev, "kp2000_pcie_remove(pdev=%p)\n", pdev);
+	dev_dbg(&pdev->dev, "%s(pdev=%p)\n", __func__, pdev);
 
 	if (!pcard)
 		return;
-- 
2.20.1

Re: [PATCH 4/7] staging: kpc2000: use __func__ in debug messages in core.c

[Greg KH]

On Tue, Jun 04, 2019 at 12:29:13AM +0200, Simon Sandström wrote:
> Fixes checkpatch.pl warning "Prefer using '"%s...", __func__' to using
> '<function name>', this function's name, in a string".
> 
> Signed-off-by: Simon Sandström <simon@nikanor.nu>
> ---
>  drivers/staging/kpc2000/kpc2000/core.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/kpc2000/kpc2000/core.c b/drivers/staging/kpc2000/kpc2000/core.c
> index a70665a202c3..6d4fc1f37c9f 100644
> --- a/drivers/staging/kpc2000/kpc2000/core.c
> +++ b/drivers/staging/kpc2000/kpc2000/core.c
> @@ -312,8 +312,8 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
>  	unsigned long dma_bar_phys_len;
>  	u16 regval;
>  
> -	dev_dbg(&pdev->dev, "kp2000_pcie_probe(pdev = [%p], id = [%p])\n",
> -		pdev, id);
> +	dev_dbg(&pdev->dev, "%s(pdev = [%p], id = [%p])\n",
> +		__func__, pdev, id);

debugging lines that say "called this function!" can all be removed, as
we have ftrace in the kernel tree, we can use that instead.  I'll take
this, but feel free to clean them up as follow-on patches.

thanks,

greg k-h

Re: [PATCH 4/7] staging: kpc2000: use __func__ in debug messages in core.c

[Simon Sandström]

On 06/06, Greg KH wrote:
> On Tue, Jun 04, 2019 at 12:29:13AM +0200, Simon Sandström wrote:
> >  
> > -	dev_dbg(&pdev->dev, "kp2000_pcie_probe(pdev = [%p], id = [%p])\n",
> > -		pdev, id);
> > +	dev_dbg(&pdev->dev, "%s(pdev = [%p], id = [%p])\n",
> > +		__func__, pdev, id);
> 
> debugging lines that say "called this function!" can all be removed, as
> we have ftrace in the kernel tree, we can use that instead.  I'll take
> this, but feel free to clean them up as follow-on patches.
> 
> thanks,
> 
> greg k-h

Can they be removed even if they print function arguments or other
variables?

- Simon

Re: [PATCH 4/7] staging: kpc2000: use __func__ in debug messages in core.c

[Greg KH]

On Mon, Jun 10, 2019 at 09:20:09AM +0200, Simon Sandström wrote:
> On 06/06, Greg KH wrote:
> > On Tue, Jun 04, 2019 at 12:29:13AM +0200, Simon Sandström wrote:
> > >  
> > > -	dev_dbg(&pdev->dev, "kp2000_pcie_probe(pdev = [%p], id = [%p])\n",
> > > -		pdev, id);
> > > +	dev_dbg(&pdev->dev, "%s(pdev = [%p], id = [%p])\n",
> > > +		__func__, pdev, id);
> > 
> > debugging lines that say "called this function!" can all be removed, as
> > we have ftrace in the kernel tree, we can use that instead.  I'll take
> > this, but feel free to clean them up as follow-on patches.
> > 
> > thanks,
> > 
> > greg k-h
> 
> Can they be removed even if they print function arguments or other
> variables?

Of course!

[PATCH 5/7] staging: kpc2000: remove unnecessary include in core.c

[Simon Sandström]

Fixes checkpatch.pl warning "Use #include <linux/io.h> instead of
<asm/io.h>".

Signed-off-by: Simon Sandström <simon@nikanor.nu>
---
 drivers/staging/kpc2000/kpc2000/core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/kpc2000/kpc2000/core.c b/drivers/staging/kpc2000/kpc2000/core.c
index 6d4fc1f37c9f..3f17566a9d03 100644
--- a/drivers/staging/kpc2000/kpc2000/core.c
+++ b/drivers/staging/kpc2000/kpc2000/core.c
@@ -12,7 +12,6 @@
 #include <linux/cdev.h>
 #include <linux/rwsem.h>
 #include <linux/uaccess.h>
-#include <asm/io.h>
 #include <linux/io.h>
 #include <linux/mfd/core.h>
 #include <linux/platform_device.h>
-- 
2.20.1

[PATCH 6/7] staging: kpc2000: use sizeof(var) in kzalloc call

[Simon Sandström]

Fixes checkpatch.pl warning "Prefer kzalloc(sizeof(*pcard)...) over
kzalloc(sizeof(struct kp2000_device)...)".

Signed-off-by: Simon Sandström <simon@nikanor.nu>
---
 drivers/staging/kpc2000/kpc2000/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/kpc2000/kpc2000/core.c b/drivers/staging/kpc2000/kpc2000/core.c
index 3f17566a9d03..2d8d188624f7 100644
--- a/drivers/staging/kpc2000/kpc2000/core.c
+++ b/drivers/staging/kpc2000/kpc2000/core.c
@@ -317,7 +317,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
 	/*
 	 * Step 1: Allocate a struct for the pcard
 	 */
-	pcard = kzalloc(sizeof(struct kp2000_device), GFP_KERNEL);
+	pcard = kzalloc(sizeof(*pcard), GFP_KERNEL);
 	if (!pcard)
 		return -ENOMEM;
 	dev_dbg(&pdev->dev, "probe: allocated struct kp2000_device @ %p\n",
-- 
2.20.1

[PATCH 7/7] staging: kpc2000: fix incorrect code comment in core.c

[Simon Sandström]

Step 11 was removed from kp2000_pcie_probe in a previous commit but the
comment was not changed to reflect this, so do it now.

Signed-off-by: Simon Sandström <simon@nikanor.nu>
---
 drivers/staging/kpc2000/kpc2000/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/kpc2000/kpc2000/core.c b/drivers/staging/kpc2000/kpc2000/core.c
index 2d8d188624f7..cd3876f1ce17 100644
--- a/drivers/staging/kpc2000/kpc2000/core.c
+++ b/drivers/staging/kpc2000/kpc2000/core.c
@@ -501,7 +501,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
 		goto out10;
 
 	/*
-	 * Step 12: Enable IRQs in HW
+	 * Step 11: Enable IRQs in HW
 	 */
 	writel(KPC_DMA_CARD_IRQ_ENABLE | KPC_DMA_CARD_USER_INTERRUPT_MODE,
 	       pcard->dma_common_regs);
-- 
2.20.1

Re: [PATCH 7/7] staging: kpc2000: fix incorrect code comment in core.c

[Dan Carpenter]

On Tue, Jun 04, 2019 at 12:29:16AM +0200, Simon Sandström wrote:
> Step 11 was removed from kp2000_pcie_probe in a previous commit but the
> comment was not changed to reflect this, so do it now.
> 
> Signed-off-by: Simon Sandström <simon@nikanor.nu>
> ---
>  drivers/staging/kpc2000/kpc2000/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/kpc2000/kpc2000/core.c b/drivers/staging/kpc2000/kpc2000/core.c
> index 2d8d188624f7..cd3876f1ce17 100644
> --- a/drivers/staging/kpc2000/kpc2000/core.c
> +++ b/drivers/staging/kpc2000/kpc2000/core.c
> @@ -501,7 +501,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
>  		goto out10;
>  
>  	/*
> -	 * Step 12: Enable IRQs in HW
> +	 * Step 11: Enable IRQs in HW

I don't have a problem with this patch but for the future these numbers
don't add any value.  And the numbered out labels are sort of ugly.  The
label name should say what the label does just like a function name says
what the function does.  Really a lot of these comments in the probe
function are very obvious and don't add information (delete them).


   491          /*
   492           * Step 9: Setup sysfs attributes
   493           */
   494          err = sysfs_create_files(&(pdev->dev.kobj), kp_attr_list);

The comment is probably less informative than the code.

   495          if (err) {
   496                  dev_err(&pdev->dev, "Failed to add sysfs files: %d\n", err);
   497                  goto out9;

What does goto out9 do?

   498          }
   499  
   500          /*
   501           * Step 10: Probe cores
   502           */
   503          err = kp2000_probe_cores(pcard);
   504          if (err)
   505                  goto out10;

Hopefully, goto out10 deletes the sysfs files but we don't know because
the label doesn't give any clues away.  We have to search for it and
then come back.

   506  
   507          /*
   508           * Step 12: Enable IRQs in HW
   509           */
   510          writel(KPC_DMA_CARD_IRQ_ENABLE | KPC_DMA_CARD_USER_INTERRUPT_MODE,
   511                 pcard->dma_common_regs);
   512  
   513          dev_dbg(&pcard->pdev->dev, "kp2000_pcie_probe() complete!\n");
   514          mutex_unlock(&pcard->sem);
   515          return 0;
   516  
   517  out10:

err_remove_sysfs:

   518          sysfs_remove_files(&(pdev->dev.kobj), kp_attr_list);
   519  out9:

err_free_irq:

   520          free_irq(pcard->pdev->irq, pcard);
   521  out8b:

err_disable_msi:

   522          pci_disable_msi(pcard->pdev);
   523  out8a:
   524  out7:
   525  out6:

err_unmap_dma:

   526          iounmap(pcard->dma_bar_base);
   527          pci_release_region(pdev, DMA_BAR);
   528          pcard->dma_bar_base = NULL;
   529  out5:

err_unmap_regs:

   530          iounmap(pcard->regs_bar_base);
   531          pci_release_region(pdev, REG_BAR);
   532          pcard->regs_bar_base = NULL;

Something like that is way more useful because then you don't have to
scroll back and forth because new the label names are useful.

regards,
dan carpenter