Re: [PATCH 2/2] staging: kpc2000: remove unnecessary comments in kp2000_pcie_probe

2019-06-12 Thread Simon Sandström
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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 2/2] staging: kpc2000: remove unnecessary comments in kp2000_pcie_probe

2019-06-12 Thread Greg KH
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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 2/2] staging: kpc2000: remove unnecessary comments in kp2000_pcie_probe

2019-06-12 Thread Dan Carpenter
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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 2/2] staging: kpc2000: remove unnecessary comments in kp2000_pcie_probe

2019-06-10 Thread 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 
---
 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(>dev, "probe: allocated struct kp2000_device @ %p\n",
pcard);
 
-   /*
-* Step 2: Initialize trivial pcard elements
-*/
err = ida_simple_get(_num_ida, 1, INT_MAX, GFP_KERNEL);
if (err < 0) {
dev_err(>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(>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(0x,
   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, );
regval |= (PCI_COMMAND_IO | PCI_COMMAND_MEMORY);
@@ -466,9 +449,6 @@ static int kp2000_pcie_probe(struct pci_dev *pdev,
dev_dbg(>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(>dev.kobj, kp_attr_list);
if (err) {
dev_err(>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

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel