* [PATCH 0/2] staging: kpc2000: minor fixes in kp2000_pcie_probe
@ 2019-06-10 20:05 Simon Sandström
2019-06-10 20:05 ` [PATCH 1/2] staging: kpc2000: improve label names " Simon Sandström
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Simon Sandström @ 2019-06-10 20:05 UTC (permalink / raw)
To: gregkh; +Cc: dan.carpenter, jeremy, devel, linux-kernel, Simon Sandström
These two patches fixes issues pointed out by Dan in a previous
staging/kpc2000 patch thread: many comments in kp2000_pcie_probe just
repeats the code and the current label names doesn't add any information
and makes it hard to follow the code.
Rename all labels and remove the comments that just repeats the code.
Simon Sandström (2):
staging: kpc2000: improve label names in kp2000_pcie_probe
staging: kpc2000: remove unnecessary comments in kp2000_pcie_probe
drivers/staging/kpc2000/kpc2000/core.c | 80 ++++++++------------------
1 file changed, 25 insertions(+), 55 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] staging: kpc2000: improve label names in kp2000_pcie_probe
2019-06-10 20:05 [PATCH 0/2] staging: kpc2000: minor fixes in kp2000_pcie_probe Simon Sandström
@ 2019-06-10 20:05 ` Simon Sandström
2019-06-12 7:37 ` Dan Carpenter
2019-06-10 20:05 ` [PATCH 2/2] staging: kpc2000: remove unnecessary comments " Simon Sandström
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Simon Sandström @ 2019-06-10 20:05 UTC (permalink / raw)
To: gregkh; +Cc: dan.carpenter, jeremy, devel, linux-kernel, Simon Sandström
Use self-explanatory label names instead of the generic numbered ones,
to make it easier to follow and understand the code.
Signed-off-by: Simon Sandström <simon@nikanor.nu>
---
drivers/staging/kpc2000/kpc2000/core.c | 42 ++++++++++++--------------
1 file changed, 20 insertions(+), 22 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc2000/core.c b/drivers/staging/kpc2000/kpc2000/core.c
index 9b9b29ac90c5..ee6b9be7127d 100644
--- a/drivers/staging/kpc2000/kpc2000/core.c
+++ b/drivers/staging/kpc2000/kpc2000/core.c
@@ -327,7 +327,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
if (err < 0) {
dev_err(&pdev->dev, "probe: failed to get card number (%d)\n",
err);
- goto out2;
+ goto err_free_pcard;
}
pcard->card_num = err;
scnprintf(pcard->name, 16, "kpcard%u", pcard->card_num);
@@ -346,7 +346,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
dev_err(&pcard->pdev->dev,
"probe: failed to enable PCIE2000 PCIe device (%d)\n",
err);
- goto out3;
+ goto err_remove_ida;
}
/*
@@ -360,7 +360,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
dev_err(&pcard->pdev->dev,
"probe: REG_BAR could not remap memory to virtual space\n");
err = -ENODEV;
- goto out4;
+ goto err_disable_device;
}
dev_dbg(&pcard->pdev->dev,
"probe: REG_BAR virt hardware address start [%p]\n",
@@ -373,7 +373,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
"probe: failed to acquire PCI region (%d)\n",
err);
err = -ENODEV;
- goto out4;
+ goto err_disable_device;
}
pcard->regs_base_resource.start = reg_bar_phys_addr;
@@ -393,7 +393,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
dev_err(&pcard->pdev->dev,
"probe: DMA_BAR could not remap memory to virtual space\n");
err = -ENODEV;
- goto out5;
+ goto err_unmap_regs;
}
dev_dbg(&pcard->pdev->dev,
"probe: DMA_BAR virt hardware address start [%p]\n",
@@ -407,7 +407,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
dev_err(&pcard->pdev->dev,
"probe: failed to acquire PCI region (%d)\n", err);
err = -ENODEV;
- goto out5;
+ goto err_unmap_regs;
}
pcard->dma_base_resource.start = dma_bar_phys_addr;
@@ -421,7 +421,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
pcard->sysinfo_regs_base = pcard->regs_bar_base;
err = read_system_regs(pcard);
if (err)
- goto out6;
+ goto err_unmap_dma;
// Disable all "user" interrupts because they're not used yet.
writeq(0xFFFFFFFFFFFFFFFF,
@@ -461,7 +461,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
if (err) {
dev_err(&pcard->pdev->dev,
"CANNOT use DMA mask %0llx\n", DMA_BIT_MASK(64));
- goto out7;
+ goto err_unmap_dma;
}
dev_dbg(&pcard->pdev->dev,
"Using DMA mask %0llx\n", dma_get_mask(PCARD_TO_DEV(pcard)));
@@ -471,14 +471,14 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
*/
err = pci_enable_msi(pcard->pdev);
if (err < 0)
- goto out8a;
+ goto err_unmap_dma;
rv = request_irq(pcard->pdev->irq, kp2000_irq_handler, IRQF_SHARED,
pcard->name, pcard);
if (rv) {
dev_err(&pcard->pdev->dev,
"%s: failed to request_irq: %d\n", __func__, rv);
- goto out8b;
+ goto err_disable_msi;
}
/*
@@ -487,7 +487,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
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;
+ goto err_free_irq;
}
/*
@@ -495,7 +495,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
*/
err = kp2000_probe_cores(pcard);
if (err)
- goto out10;
+ goto err_remove_sysfs;
/*
* Step 11: Enable IRQs in HW
@@ -506,28 +506,26 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
mutex_unlock(&pcard->sem);
return 0;
-out10:
+err_remove_sysfs:
sysfs_remove_files(&pdev->dev.kobj, kp_attr_list);
-out9:
+err_free_irq:
free_irq(pcard->pdev->irq, pcard);
-out8b:
+err_disable_msi:
pci_disable_msi(pcard->pdev);
-out8a:
-out7:
-out6:
+err_unmap_dma:
iounmap(pcard->dma_bar_base);
pci_release_region(pdev, DMA_BAR);
pcard->dma_bar_base = NULL;
-out5:
+err_unmap_regs:
iounmap(pcard->regs_bar_base);
pci_release_region(pdev, REG_BAR);
pcard->regs_bar_base = NULL;
-out4:
+err_disable_device:
pci_disable_device(pcard->pdev);
-out3:
+err_remove_ida:
mutex_unlock(&pcard->sem);
ida_simple_remove(&card_num_ida, pcard->card_num);
-out2:
+err_free_pcard:
kfree(pcard);
return err;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] staging: kpc2000: remove unnecessary comments in kp2000_pcie_probe
2019-06-10 20:05 [PATCH 0/2] staging: kpc2000: minor fixes in kp2000_pcie_probe Simon Sandström
2019-06-10 20:05 ` [PATCH 1/2] staging: kpc2000: improve label names " Simon Sandström
@ 2019-06-10 20:05 ` Simon Sandström
2019-06-12 7:39 ` Dan Carpenter
2019-06-12 13:58 ` [PATCH v2 0/2] staging: kpc2000: minor fixes " Simon Sandström
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Simon Sandström @ 2019-06-10 20:05 UTC (permalink / raw)
To: gregkh; +Cc: dan.carpenter, jeremy, devel, linux-kernel, Simon Sandström
Much of the code comments in kp2000_pcie_probe just repeats the code and
does not add any additional information. Delete them and make sure that
comments still left in the function all use the same style.
Signed-off-by: Simon Sandström <simon@nikanor.nu>
---
drivers/staging/kpc2000/kpc2000/core.c | 38 ++++----------------------
1 file changed, 5 insertions(+), 33 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc2000/core.c b/drivers/staging/kpc2000/kpc2000/core.c
index ee6b9be7127d..7ec54b672c20 100644
--- a/drivers/staging/kpc2000/kpc2000/core.c
+++ b/drivers/staging/kpc2000/kpc2000/core.c
@@ -311,18 +311,12 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
unsigned long dma_bar_phys_len;
u16 regval;
- /*
- * Step 1: Allocate a struct for the pcard
- */
pcard = kzalloc(sizeof(*pcard), GFP_KERNEL);
if (!pcard)
return -ENOMEM;
dev_dbg(&pdev->dev, "probe: allocated struct kp2000_device @ %p\n",
pcard);
- /*
- * Step 2: Initialize trivial pcard elements
- */
err = ida_simple_get(&card_num_ida, 1, INT_MAX, GFP_KERNEL);
if (err < 0) {
dev_err(&pdev->dev, "probe: failed to get card number (%d)\n",
@@ -338,9 +332,6 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
pcard->pdev = pdev;
pci_set_drvdata(pdev, pcard);
- /*
- * Step 3: Enable PCI device
- */
err = pci_enable_device(pcard->pdev);
if (err) {
dev_err(&pcard->pdev->dev,
@@ -349,9 +340,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
goto err_remove_ida;
}
- /*
- * Step 4: Setup the Register BAR
- */
+ // Setup the Register BAR
reg_bar_phys_addr = pci_resource_start(pcard->pdev, REG_BAR);
reg_bar_phys_len = pci_resource_len(pcard->pdev, REG_BAR);
@@ -381,9 +370,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
reg_bar_phys_len - 1;
pcard->regs_base_resource.flags = IORESOURCE_MEM;
- /*
- * Step 5: Setup the DMA BAR
- */
+ // Setup the DMA BAR
dma_bar_phys_addr = pci_resource_start(pcard->pdev, DMA_BAR);
dma_bar_phys_len = pci_resource_len(pcard->pdev, DMA_BAR);
@@ -415,9 +402,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
dma_bar_phys_len - 1;
pcard->dma_base_resource.flags = IORESOURCE_MEM;
- /*
- * Step 6: System Regs
- */
+ // Read System Regs
pcard->sysinfo_regs_base = pcard->regs_bar_base;
err = read_system_regs(pcard);
if (err)
@@ -427,11 +412,9 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
writeq(0xFFFFFFFFFFFFFFFF,
pcard->sysinfo_regs_base + REG_INTERRUPT_MASK);
- /*
- * Step 7: Configure PCI thingies
- */
// let the card master PCIe
pci_set_master(pcard->pdev);
+
// enable IO and mem if not already done
pci_read_config_word(pcard->pdev, PCI_COMMAND, ®val);
regval |= (PCI_COMMAND_IO | PCI_COMMAND_MEMORY);
@@ -466,9 +449,6 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
dev_dbg(&pcard->pdev->dev,
"Using DMA mask %0llx\n", dma_get_mask(PCARD_TO_DEV(pcard)));
- /*
- * Step 8: Configure IRQs
- */
err = pci_enable_msi(pcard->pdev);
if (err < 0)
goto err_unmap_dma;
@@ -481,25 +461,17 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
goto err_disable_msi;
}
- /*
- * Step 9: Setup sysfs attributes
- */
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 err_free_irq;
}
- /*
- * Step 10: Probe cores
- */
err = kp2000_probe_cores(pcard);
if (err)
goto err_remove_sysfs;
- /*
- * Step 11: Enable IRQs in HW
- */
+ // Enable IRQs in HW
writel(KPC_DMA_CARD_IRQ_ENABLE | KPC_DMA_CARD_USER_INTERRUPT_MODE,
pcard->dma_common_regs);
--
2.20.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] staging: kpc2000: improve label names in kp2000_pcie_probe
2019-06-10 20:05 ` [PATCH 1/2] staging: kpc2000: improve label names " Simon Sandström
@ 2019-06-12 7:37 ` Dan Carpenter
0 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2019-06-12 7:37 UTC (permalink / raw)
To: Simon Sandström; +Cc: gregkh, devel, linux-kernel
Thanks!
Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
Not related to your patch (IOW ignore if you want to) the error handling
is slightly more complicated than required:
drivers/staging/kpc2000/kpc2000/core.c
356 * Step 4: Setup the Register BAR
357 */
358 reg_bar_phys_addr = pci_resource_start(pcard->pdev, REG_BAR);
359 reg_bar_phys_len = pci_resource_len(pcard->pdev, REG_BAR);
360
361 pcard->regs_bar_base = ioremap_nocache(reg_bar_phys_addr, PAGE_SIZE);
362 if (!pcard->regs_bar_base) {
363 dev_err(&pcard->pdev->dev,
364 "probe: REG_BAR could not remap memory to virtual space\n");
365 err = -ENODEV;
366 goto err_disable_device;
367 }
368 dev_dbg(&pcard->pdev->dev,
369 "probe: REG_BAR virt hardware address start [%p]\n",
370 pcard->regs_bar_base);
371
372 err = pci_request_region(pcard->pdev, REG_BAR, KP_DRIVER_NAME_KP2000);
373 if (err) {
374 iounmap(pcard->regs_bar_base);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
We could move this to the bottom of the function. We would need to
re-order it slightly to free things in the mirror of how they are
allocated. (Always just free the most recent allocation).
375 dev_err(&pcard->pdev->dev,
376 "probe: failed to acquire PCI region (%d)\n",
377 err);
378 err = -ENODEV;
379 goto err_disable_device;
380 }
381
382 pcard->regs_base_resource.start = reg_bar_phys_addr;
383 pcard->regs_base_resource.end = reg_bar_phys_addr +
384 reg_bar_phys_len - 1;
385 pcard->regs_base_resource.flags = IORESOURCE_MEM;
386
387 /*
388 * Step 5: Setup the DMA BAR
389 */
390 dma_bar_phys_addr = pci_resource_start(pcard->pdev, DMA_BAR);
391 dma_bar_phys_len = pci_resource_len(pcard->pdev, DMA_BAR);
392
393 pcard->dma_bar_base = ioremap_nocache(dma_bar_phys_addr,
394 dma_bar_phys_len);
395 if (!pcard->dma_bar_base) {
396 dev_err(&pcard->pdev->dev,
397 "probe: DMA_BAR could not remap memory to virtual space\n");
398 err = -ENODEV;
399 goto err_unmap_regs;
400 }
401 dev_dbg(&pcard->pdev->dev,
402 "probe: DMA_BAR virt hardware address start [%p]\n",
403 pcard->dma_bar_base);
404
405 pcard->dma_common_regs = pcard->dma_bar_base + KPC_DMA_COMMON_OFFSET;
406
407 err = pci_request_region(pcard->pdev, DMA_BAR, "kp2000_pcie");
408 if (err) {
409 iounmap(pcard->dma_bar_base);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Same.
410 dev_err(&pcard->pdev->dev,
411 "probe: failed to acquire PCI region (%d)\n", err);
412 err = -ENODEV;
413 goto err_unmap_regs;
414 }
415
416 pcard->dma_base_resource.start = dma_bar_phys_addr;
[ snip ]
509 dev_dbg(&pcard->pdev->dev, "%s() complete!\n", __func__);
510 mutex_unlock(&pcard->sem);
511 return 0;
512
513 err_remove_sysfs:
514 sysfs_remove_files(&pdev->dev.kobj, kp_attr_list);
515 err_free_irq:
516 free_irq(pcard->pdev->irq, pcard);
517 err_disable_msi:
518 pci_disable_msi(pcard->pdev);
519 err_unmap_dma:
520 iounmap(pcard->dma_bar_base);
521 pci_release_region(pdev, DMA_BAR);
522 pcard->dma_bar_base = NULL;
523 err_unmap_regs:
524 iounmap(pcard->regs_bar_base);
525 pci_release_region(pdev, REG_BAR);
526 pcard->regs_bar_base = NULL;
err_release_dma:
pci_release_region(pdev, DMA_BAR);
err_unmap_dma:
iounmap(pcard->dma_bar_base);
err_release_reg:
pci_release_region(pdev, REG_BAR);
err_unmap_reg:
iounmap(pcard->regs_bar_base);
I moved swapped the pci_release_region() and the iounmap() order. There
is no need to set "pcard->regs_bar_base = NULL;" so just remove that.
527 err_disable_device:
528 pci_disable_device(pcard->pdev);
529 err_remove_ida:
530 mutex_unlock(&pcard->sem);
531 ida_simple_remove(&card_num_ida, pcard->card_num);
532 err_free_pcard:
533 kfree(pcard);
534 return err;
535 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] staging: kpc2000: remove unnecessary comments in kp2000_pcie_probe
2019-06-10 20:05 ` [PATCH 2/2] staging: kpc2000: remove unnecessary comments " Simon Sandström
@ 2019-06-12 7:39 ` Dan Carpenter
2019-06-12 7:46 ` Greg KH
0 siblings, 1 reply; 10+ messages in thread
From: Dan Carpenter @ 2019-06-12 7:39 UTC (permalink / raw)
To: Simon Sandström; +Cc: gregkh, devel, linux-kernel
On Mon, Jun 10, 2019 at 10:05:35PM +0200, Simon Sandström wrote:
> @@ -349,9 +340,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
> goto err_remove_ida;
> }
>
> - /*
> - * Step 4: Setup the Register BAR
> - */
> + // Setup the Register BAR
Greg, are we moving the C++ style comments? Linus is fine with them. I
don't like them but whatever...
> reg_bar_phys_addr = pci_resource_start(pcard->pdev, REG_BAR);
> reg_bar_phys_len = pci_resource_len(pcard->pdev, REG_BAR);
>
regards,
dan carpenter
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] staging: kpc2000: remove unnecessary comments in kp2000_pcie_probe
2019-06-12 7:39 ` Dan Carpenter
@ 2019-06-12 7:46 ` Greg KH
2019-06-12 10:07 ` Simon Sandström
0 siblings, 1 reply; 10+ messages in thread
From: Greg KH @ 2019-06-12 7:46 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Simon Sandström, devel, linux-kernel
On Wed, Jun 12, 2019 at 10:39:36AM +0300, Dan Carpenter wrote:
> On Mon, Jun 10, 2019 at 10:05:35PM +0200, Simon Sandström wrote:
> > @@ -349,9 +340,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
> > goto err_remove_ida;
> > }
> >
> > - /*
> > - * Step 4: Setup the Register BAR
> > - */
> > + // Setup the Register BAR
>
> Greg, are we moving the C++ style comments? Linus is fine with them. I
> don't like them but whatever...
I don't like them either. I'm only "ok" with them on the very first
line of the file. Linus chose // to make it "stand out" from the normal
flow of the file, which is fine for an SPDX line. So putting these in
here like this is not ok to me.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] staging: kpc2000: remove unnecessary comments in kp2000_pcie_probe
2019-06-12 7:46 ` Greg KH
@ 2019-06-12 10:07 ` Simon Sandström
0 siblings, 0 replies; 10+ messages in thread
From: Simon Sandström @ 2019-06-12 10:07 UTC (permalink / raw)
To: Greg KH; +Cc: Dan Carpenter, devel, linux-kernel
On 12/06, Greg KH wrote:
> On Wed, Jun 12, 2019 at 10:39:36AM +0300, Dan Carpenter wrote:
> > On Mon, Jun 10, 2019 at 10:05:35PM +0200, Simon Sandström wrote:
> > > @@ -349,9 +340,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
> > > goto err_remove_ida;
> > > }
> > >
> > > - /*
> > > - * Step 4: Setup the Register BAR
> > > - */
> > > + // Setup the Register BAR
> >
> > Greg, are we moving the C++ style comments? Linus is fine with them. I
> > don't like them but whatever...
>
> I don't like them either. I'm only "ok" with them on the very first
> line of the file. Linus chose // to make it "stand out" from the normal
> flow of the file, which is fine for an SPDX line. So putting these in
> here like this is not ok to me.
>
> thanks,
>
> greg k-h
I changed them to C++ style so that they would match the other comments
in the file, which are C++ style, but I guess that it should have been
done the other way around with the C++ style changed to C style.
Good to know. I'll change them back and send a v2.
- Simon
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 0/2] staging: kpc2000: minor fixes in kp2000_pcie_probe
2019-06-10 20:05 [PATCH 0/2] staging: kpc2000: minor fixes in kp2000_pcie_probe Simon Sandström
2019-06-10 20:05 ` [PATCH 1/2] staging: kpc2000: improve label names " Simon Sandström
2019-06-10 20:05 ` [PATCH 2/2] staging: kpc2000: remove unnecessary comments " Simon Sandström
@ 2019-06-12 13:58 ` Simon Sandström
2019-06-12 13:58 ` [PATCH v2 1/2] staging: kpc2000: improve label names " Simon Sandström
2019-06-12 13:58 ` [PATCH v2 2/2] staging: kpc2000: remove unnecessary comments " Simon Sandström
4 siblings, 0 replies; 10+ messages in thread
From: Simon Sandström @ 2019-06-12 13:58 UTC (permalink / raw)
To: gregkh; +Cc: simon, dan.carpenter, devel, linux-kernel
These two patches fixes issues pointed out by Dan in a previous
staging/kpc2000 patch thread: many comments in kp2000_pcie_probe just
repeats the code and the current label names doesn't add any information
and makes it hard to follow the code.
Rename all labels and remove the comments that just repeats the code.
Version 2:
- Don't convert C style comments to C++ style
Simon Sandström (2):
staging: kpc2000: improve label names in kp2000_pcie_probe
staging: kpc2000: remove unnecessary comments in kp2000_pcie_probe
drivers/staging/kpc2000/kpc2000/core.c | 80 ++++++++------------------
1 file changed, 25 insertions(+), 55 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/2] staging: kpc2000: improve label names in kp2000_pcie_probe
2019-06-10 20:05 [PATCH 0/2] staging: kpc2000: minor fixes in kp2000_pcie_probe Simon Sandström
` (2 preceding siblings ...)
2019-06-12 13:58 ` [PATCH v2 0/2] staging: kpc2000: minor fixes " Simon Sandström
@ 2019-06-12 13:58 ` Simon Sandström
2019-06-12 13:58 ` [PATCH v2 2/2] staging: kpc2000: remove unnecessary comments " Simon Sandström
4 siblings, 0 replies; 10+ messages in thread
From: Simon Sandström @ 2019-06-12 13:58 UTC (permalink / raw)
To: gregkh; +Cc: simon, dan.carpenter, devel, linux-kernel
Use self-explanatory label names instead of the generic numbered ones,
to make it easier to follow and understand the code.
Signed-off-by: Simon Sandström <simon@nikanor.nu>
---
drivers/staging/kpc2000/kpc2000/core.c | 42 ++++++++++++--------------
1 file changed, 20 insertions(+), 22 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc2000/core.c b/drivers/staging/kpc2000/kpc2000/core.c
index 9b9b29ac90c5..ee6b9be7127d 100644
--- a/drivers/staging/kpc2000/kpc2000/core.c
+++ b/drivers/staging/kpc2000/kpc2000/core.c
@@ -327,7 +327,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
if (err < 0) {
dev_err(&pdev->dev, "probe: failed to get card number (%d)\n",
err);
- goto out2;
+ goto err_free_pcard;
}
pcard->card_num = err;
scnprintf(pcard->name, 16, "kpcard%u", pcard->card_num);
@@ -346,7 +346,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
dev_err(&pcard->pdev->dev,
"probe: failed to enable PCIE2000 PCIe device (%d)\n",
err);
- goto out3;
+ goto err_remove_ida;
}
/*
@@ -360,7 +360,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
dev_err(&pcard->pdev->dev,
"probe: REG_BAR could not remap memory to virtual space\n");
err = -ENODEV;
- goto out4;
+ goto err_disable_device;
}
dev_dbg(&pcard->pdev->dev,
"probe: REG_BAR virt hardware address start [%p]\n",
@@ -373,7 +373,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
"probe: failed to acquire PCI region (%d)\n",
err);
err = -ENODEV;
- goto out4;
+ goto err_disable_device;
}
pcard->regs_base_resource.start = reg_bar_phys_addr;
@@ -393,7 +393,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
dev_err(&pcard->pdev->dev,
"probe: DMA_BAR could not remap memory to virtual space\n");
err = -ENODEV;
- goto out5;
+ goto err_unmap_regs;
}
dev_dbg(&pcard->pdev->dev,
"probe: DMA_BAR virt hardware address start [%p]\n",
@@ -407,7 +407,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
dev_err(&pcard->pdev->dev,
"probe: failed to acquire PCI region (%d)\n", err);
err = -ENODEV;
- goto out5;
+ goto err_unmap_regs;
}
pcard->dma_base_resource.start = dma_bar_phys_addr;
@@ -421,7 +421,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
pcard->sysinfo_regs_base = pcard->regs_bar_base;
err = read_system_regs(pcard);
if (err)
- goto out6;
+ goto err_unmap_dma;
// Disable all "user" interrupts because they're not used yet.
writeq(0xFFFFFFFFFFFFFFFF,
@@ -461,7 +461,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
if (err) {
dev_err(&pcard->pdev->dev,
"CANNOT use DMA mask %0llx\n", DMA_BIT_MASK(64));
- goto out7;
+ goto err_unmap_dma;
}
dev_dbg(&pcard->pdev->dev,
"Using DMA mask %0llx\n", dma_get_mask(PCARD_TO_DEV(pcard)));
@@ -471,14 +471,14 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
*/
err = pci_enable_msi(pcard->pdev);
if (err < 0)
- goto out8a;
+ goto err_unmap_dma;
rv = request_irq(pcard->pdev->irq, kp2000_irq_handler, IRQF_SHARED,
pcard->name, pcard);
if (rv) {
dev_err(&pcard->pdev->dev,
"%s: failed to request_irq: %d\n", __func__, rv);
- goto out8b;
+ goto err_disable_msi;
}
/*
@@ -487,7 +487,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
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;
+ goto err_free_irq;
}
/*
@@ -495,7 +495,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
*/
err = kp2000_probe_cores(pcard);
if (err)
- goto out10;
+ goto err_remove_sysfs;
/*
* Step 11: Enable IRQs in HW
@@ -506,28 +506,26 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
mutex_unlock(&pcard->sem);
return 0;
-out10:
+err_remove_sysfs:
sysfs_remove_files(&pdev->dev.kobj, kp_attr_list);
-out9:
+err_free_irq:
free_irq(pcard->pdev->irq, pcard);
-out8b:
+err_disable_msi:
pci_disable_msi(pcard->pdev);
-out8a:
-out7:
-out6:
+err_unmap_dma:
iounmap(pcard->dma_bar_base);
pci_release_region(pdev, DMA_BAR);
pcard->dma_bar_base = NULL;
-out5:
+err_unmap_regs:
iounmap(pcard->regs_bar_base);
pci_release_region(pdev, REG_BAR);
pcard->regs_bar_base = NULL;
-out4:
+err_disable_device:
pci_disable_device(pcard->pdev);
-out3:
+err_remove_ida:
mutex_unlock(&pcard->sem);
ida_simple_remove(&card_num_ida, pcard->card_num);
-out2:
+err_free_pcard:
kfree(pcard);
return err;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/2] staging: kpc2000: remove unnecessary comments in kp2000_pcie_probe
2019-06-10 20:05 [PATCH 0/2] staging: kpc2000: minor fixes in kp2000_pcie_probe Simon Sandström
` (3 preceding siblings ...)
2019-06-12 13:58 ` [PATCH v2 1/2] staging: kpc2000: improve label names " Simon Sandström
@ 2019-06-12 13:58 ` Simon Sandström
4 siblings, 0 replies; 10+ messages in thread
From: Simon Sandström @ 2019-06-12 13:58 UTC (permalink / raw)
To: gregkh; +Cc: simon, dan.carpenter, devel, linux-kernel
Much of the code comments in kp2000_pcie_probe just repeats the code and
does not add any additional information. Delete them and make sure that
comments still left in the function all use the same style.
Signed-off-by: Simon Sandström <simon@nikanor.nu>
---
drivers/staging/kpc2000/kpc2000/core.c | 38 ++++----------------------
1 file changed, 5 insertions(+), 33 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc2000/core.c b/drivers/staging/kpc2000/kpc2000/core.c
index ee6b9be7127d..6a5999e8ff4e 100644
--- a/drivers/staging/kpc2000/kpc2000/core.c
+++ b/drivers/staging/kpc2000/kpc2000/core.c
@@ -311,18 +311,12 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
unsigned long dma_bar_phys_len;
u16 regval;
- /*
- * Step 1: Allocate a struct for the pcard
- */
pcard = kzalloc(sizeof(*pcard), GFP_KERNEL);
if (!pcard)
return -ENOMEM;
dev_dbg(&pdev->dev, "probe: allocated struct kp2000_device @ %p\n",
pcard);
- /*
- * Step 2: Initialize trivial pcard elements
- */
err = ida_simple_get(&card_num_ida, 1, INT_MAX, GFP_KERNEL);
if (err < 0) {
dev_err(&pdev->dev, "probe: failed to get card number (%d)\n",
@@ -338,9 +332,6 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
pcard->pdev = pdev;
pci_set_drvdata(pdev, pcard);
- /*
- * Step 3: Enable PCI device
- */
err = pci_enable_device(pcard->pdev);
if (err) {
dev_err(&pcard->pdev->dev,
@@ -349,9 +340,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
goto err_remove_ida;
}
- /*
- * Step 4: Setup the Register BAR
- */
+ /* Setup the Register BAR */
reg_bar_phys_addr = pci_resource_start(pcard->pdev, REG_BAR);
reg_bar_phys_len = pci_resource_len(pcard->pdev, REG_BAR);
@@ -381,9 +370,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
reg_bar_phys_len - 1;
pcard->regs_base_resource.flags = IORESOURCE_MEM;
- /*
- * Step 5: Setup the DMA BAR
- */
+ /* Setup the DMA BAR */
dma_bar_phys_addr = pci_resource_start(pcard->pdev, DMA_BAR);
dma_bar_phys_len = pci_resource_len(pcard->pdev, DMA_BAR);
@@ -415,9 +402,7 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
dma_bar_phys_len - 1;
pcard->dma_base_resource.flags = IORESOURCE_MEM;
- /*
- * Step 6: System Regs
- */
+ /* Read System Regs */
pcard->sysinfo_regs_base = pcard->regs_bar_base;
err = read_system_regs(pcard);
if (err)
@@ -427,11 +412,9 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
writeq(0xFFFFFFFFFFFFFFFF,
pcard->sysinfo_regs_base + REG_INTERRUPT_MASK);
- /*
- * Step 7: Configure PCI thingies
- */
// let the card master PCIe
pci_set_master(pcard->pdev);
+
// enable IO and mem if not already done
pci_read_config_word(pcard->pdev, PCI_COMMAND, ®val);
regval |= (PCI_COMMAND_IO | PCI_COMMAND_MEMORY);
@@ -466,9 +449,6 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
dev_dbg(&pcard->pdev->dev,
"Using DMA mask %0llx\n", dma_get_mask(PCARD_TO_DEV(pcard)));
- /*
- * Step 8: Configure IRQs
- */
err = pci_enable_msi(pcard->pdev);
if (err < 0)
goto err_unmap_dma;
@@ -481,25 +461,17 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
goto err_disable_msi;
}
- /*
- * Step 9: Setup sysfs attributes
- */
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 err_free_irq;
}
- /*
- * Step 10: Probe cores
- */
err = kp2000_probe_cores(pcard);
if (err)
goto err_remove_sysfs;
- /*
- * Step 11: Enable IRQs in HW
- */
+ /* Enable IRQs in HW */
writel(KPC_DMA_CARD_IRQ_ENABLE | KPC_DMA_CARD_USER_INTERRUPT_MODE,
pcard->dma_common_regs);
--
2.20.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-06-12 13:58 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-10 20:05 [PATCH 0/2] staging: kpc2000: minor fixes in kp2000_pcie_probe Simon Sandström
2019-06-10 20:05 ` [PATCH 1/2] staging: kpc2000: improve label names " Simon Sandström
2019-06-12 7:37 ` Dan Carpenter
2019-06-10 20:05 ` [PATCH 2/2] staging: kpc2000: remove unnecessary comments " Simon Sandström
2019-06-12 7:39 ` Dan Carpenter
2019-06-12 7:46 ` Greg KH
2019-06-12 10:07 ` Simon Sandström
2019-06-12 13:58 ` [PATCH v2 0/2] staging: kpc2000: minor fixes " Simon Sandström
2019-06-12 13:58 ` [PATCH v2 1/2] staging: kpc2000: improve label names " Simon Sandström
2019-06-12 13:58 ` [PATCH v2 2/2] staging: kpc2000: remove unnecessary comments " Simon Sandström
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).