Re: [PATCH 1/6] bfa: Brocade BFA FC SCSI driver submission (2nd try)

Rolf Eike Beer
2008-09-26T06:41:58Z

[-- Attachment #1: Type: text/plain, Size: 2206 bytes --]

Jing Huang wrote:
> From: Jing Huang <huangj@brocade.com>
>
> This patch contains code that interfaces to upper layer linux kernel,
> such as PCI, SCSI mid-layer and sysfs etc. It is created using 2.6.27-rc7
> kernel.
>
> Signed-off-by: Jing Huang <huangj@brocade.com>

> +int
> +bfad_pci_init(struct pci_dev *pdev, struct bfad_s *bfad)
> +{
> + unsigned long bar0_len;
> + int rc = -ENODEV;
> +
> + if (pci_enable_device(pdev)) {
> + BFA_PRINTF(BFA_ERR, "pci_enable_device fail %p\n", pdev);
> + goto out;
> + }

You should use pcim_enable_device(). This would help you by simplifying your
error and release code as it keeps track of freeing a bunch of resources. See
Documentation/driver-model/devres.txt.

> +
> + if (pci_request_regions(pdev, BFAD_DRIVER_NAME))
> + goto out_disable_device;
> +
> + pci_set_master(pdev);
> +
> +
> + if (pci_set_dma_mask(pdev, DMA_64BIT_MASK) != 0)
> + if (pci_set_dma_mask(pdev, DMA_32BIT_MASK) != 0) {
> + BFA_PRINTF(BFA_ERR, "pci_set_dma_mask fail %p\n", pdev);
> + goto out_release_region;
> + }
> +#ifdef SUPPORT_PCI_AER
> + /*
> + * Enable PCIE Advanced Error Recovery (AER) if the kernel version
> + * supports.
> + */
> + BFAD_ENABLE_PCIE_AER(pdev);
> +#endif
> +
> + bfad->pci_bar0_map = pci_resource_start(pdev, 0);
> + bar0_len = pci_resource_len(pdev, 0);
> + bfad->pci_bar0_kva = ioremap(bfad->pci_bar0_map, bar0_len);

bfad->pci_bar0_kva = pcim_iomap(pdev, 0, 0);

> +
> + if (bfad->pci_bar0_kva == NULL) {
> + BFA_DEV_PRINTF(bfad, BFA_ERR, "Fail to map bar0\n");
> + goto out_release_region;
> + }
> +
> + bfad->hal_pcidev.pci_slot = PCI_SLOT(pdev->devfn);
> + bfad->hal_pcidev.pci_func = PCI_FUNC(pdev->devfn);
> + bfad->hal_pcidev.pci_bar_kva = bfad->pci_bar0_kva;
> + bfad->hal_pcidev.device_id = pdev->device;
> + bfad->pci_name = pci_name(pdev);
> +
> + bfad->pci_attr.vendor_id = pdev->vendor;
> + bfad->pci_attr.device_id = pdev->device;
> + bfad->pci_attr.ssid = pdev->subsystem_device;
> + bfad->pci_attr.ssvid = pdev->subsystem_vendor;
> + bfad->pci_attr.pcifn = PCI_FUNC(pdev->devfn);

Why duplicate all this information?

Greetings,

Eike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]