RE: [PATCH v8 01/20] dlb: add skeleton for DLB driver

2021-01-08 Thread Chen, Mike Ximing



> -Original Message-
> From: Greg KH 
> Sent: Thursday, January 7, 2021 2:36 PM
> To: Chen, Mike Ximing 
> Cc: linux-kernel@vger.kernel.org; a...@arndb.de; Williams, Dan J
> ; pierre-louis.boss...@linux.intel.com; Gage Eads
> 
> Subject: Re: [PATCH v8 01/20] dlb: add skeleton for DLB driver
> 
> On Mon, Jan 04, 2021 at 08:58:20PM -0600, Mike Ximing Chen wrote:
> > +static int dlb_probe(struct pci_dev *pdev,
> > +const struct pci_device_id *pdev_id)
> > +{
> > +   struct dlb *dlb;
> > +   int ret;
> > +
> > +   dlb = devm_kzalloc(>dev, sizeof(*dlb), GFP_KERNEL);
> > +   if (!dlb)
> > +   return -ENOMEM;
> > +
> > +   pci_set_drvdata(pdev, dlb);
> > +
> > +   dlb->pdev = pdev;
> > +
> > +   spin_lock(_ids_lock);
> > +   dlb->id = idr_alloc(_ids,
> > +   (void *)dlb,
> > +   0,
> > +   DLB_MAX_NUM_DEVICES - 1,
> > +   GFP_KERNEL);
> > +   spin_unlock(_ids_lock);
> > +
> > +   if (dlb->id < 0) {
> > +   dev_err(>dev, "probe: device ID allocation failed\n");
> > +
> > +   ret = dlb->id;
> > +   goto alloc_id_fail;
> > +   }
> > +
> > +   ret = pcim_enable_device(pdev);
> > +   if (ret != 0) {
> > +   dev_err(>dev, "pcim_enable_device() returned %d\n", ret);
> > +
> > +   goto pci_enable_device_fail;
> > +   }
> > +
> > +   ret = pcim_iomap_regions(pdev,
> > +(1U << DLB_CSR_BAR) | (1U << DLB_FUNC_BAR),
> > +"dlb");
> > +   if (ret != 0) {
> > +   dev_err(>dev,
> > +   "pcim_iomap_regions(): returned %d\n", ret);
> > +
> > +   goto pci_enable_device_fail;
> > +   }
> > +
> > +   pci_set_master(pdev);
> > +
> > +   if (pci_enable_pcie_error_reporting(pdev))
> > +   dev_info(>dev, "[%s()] Failed to enable AER\n", __func__);
> 
> Shouldn't that be dev_err() and you fail here?
> 
Some of our earlier devices/platforms do not support AER.  
pci_enable_pcie_error_reporting() fails, 
everything else works fine. Will change to dev_err() when the old HWs are 
phased out.

> And no need for __func__ please, the driver name and device is listed,
> that's all that is necessary.

Will remove __func__. 
Thanks

> 
> thanks,
> 
> greg k-h


Re: [PATCH v8 01/20] dlb: add skeleton for DLB driver

2021-01-07 Thread Greg KH
On Mon, Jan 04, 2021 at 08:58:20PM -0600, Mike Ximing Chen wrote:
> +static int dlb_probe(struct pci_dev *pdev,
> +  const struct pci_device_id *pdev_id)
> +{
> + struct dlb *dlb;
> + int ret;
> +
> + dlb = devm_kzalloc(>dev, sizeof(*dlb), GFP_KERNEL);
> + if (!dlb)
> + return -ENOMEM;
> +
> + pci_set_drvdata(pdev, dlb);
> +
> + dlb->pdev = pdev;
> +
> + spin_lock(_ids_lock);
> + dlb->id = idr_alloc(_ids,
> + (void *)dlb,
> + 0,
> + DLB_MAX_NUM_DEVICES - 1,
> + GFP_KERNEL);
> + spin_unlock(_ids_lock);
> +
> + if (dlb->id < 0) {
> + dev_err(>dev, "probe: device ID allocation failed\n");
> +
> + ret = dlb->id;
> + goto alloc_id_fail;
> + }
> +
> + ret = pcim_enable_device(pdev);
> + if (ret != 0) {
> + dev_err(>dev, "pcim_enable_device() returned %d\n", ret);
> +
> + goto pci_enable_device_fail;
> + }
> +
> + ret = pcim_iomap_regions(pdev,
> +  (1U << DLB_CSR_BAR) | (1U << DLB_FUNC_BAR),
> +  "dlb");
> + if (ret != 0) {
> + dev_err(>dev,
> + "pcim_iomap_regions(): returned %d\n", ret);
> +
> + goto pci_enable_device_fail;
> + }
> +
> + pci_set_master(pdev);
> +
> + if (pci_enable_pcie_error_reporting(pdev))
> + dev_info(>dev, "[%s()] Failed to enable AER\n", __func__);

Shouldn't that be dev_err() and you fail here?

And no need for __func__ please, the driver name and device is listed,
that's all that is necessary.

thanks,

greg k-h