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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <200809260841.39433.eike-kernel@sf-tec.de>
Date:	Fri, 26 Sep 2008 08:41:31 +0200
From:	Rolf Eike Beer <eike-kernel@...tec.de>
To:	Jing Huang <huangj@...cade.com>
Cc:	James.Bottomley@...senpartnership.com,
	linux-kernel@...r.kernel.org, linux-scsi@...r.kernel.org,
	rvadivel@...cade.com, srayas@...cade.com, vravindr@...cade.com
Subject: Re: [PATCH 1/6] bfa: Brocade BFA FC SCSI driver submission (2nd try)

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

Download attachment "signature.asc " of type "application/pgp-signature" (198 bytes)