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

[Rolf Eike Beer]

Jing Huang wrote:
> From: Jing Huang <huangj@xxxxxxxxxxx>
>
> 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@xxxxxxxxxxx>

> +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: signature.asc
Description: This is a digitally signed message part.