netdev - Re: [PATCH iwl-next v4 02/15] idpf: add module register and probe functionality

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]

Date: Tue, 9 May 2023 10:11:36 +0800
From: Yunsheng Lin <linyunsheng@...wei.com>
To: Emil Tantilov <emil.s.tantilov@...el.com>,
	<intel-wired-lan@...ts.osuosl.org>
CC: <shannon.nelson@....com>, <simon.horman@...igine.com>, <leon@...nel.org>,
	<decot@...gle.com>, <willemb@...gle.com>, Phani Burra
	<phani.r.burra@...el.com>, <jesse.brandeburg@...el.com>,
	<anthony.l.nguyen@...el.com>, <davem@...emloft.net>, <edumazet@...gle.com>,
	<kuba@...nel.org>, <pabeni@...hat.com>, <netdev@...r.kernel.org>, Alan Brady
	<alan.brady@...el.com>, Madhu Chittim <madhu.chittim@...el.com>, Shailendra
 Bhatnagar <shailendra.bhatnagar@...el.com>, Pavan Kumar Linga
	<pavan.kumar.linga@...el.com>
Subject: Re: [PATCH iwl-next v4 02/15] idpf: add module register and probe
 functionality

On 2023/5/9 3:43, Emil Tantilov wrote:
> +
> +/**
> + * idpf_cfg_hw - Initialize HW struct
> + * @adapter: adapter to setup hw struct for
> + *
> + * Returns 0 on success, negative on failure
> + */
> +static int idpf_cfg_hw(struct idpf_adapter *adapter)
> +{
> +	struct pci_dev *pdev = adapter->pdev;
> +	struct idpf_hw *hw = &adapter->hw;
> +
> +	hw->hw_addr = pcim_iomap_table(pdev)[0];
> +	if (!hw->hw_addr) {
> +		pci_err(pdev, "failed to allocate PCI iomap table\n");
> +

Nit: unnecessary blank line here.

> +		return -ENOMEM;
> +	}
> +
> +	hw->back = adapter;
> +
> +	return 0;
> +}
> +
> +/**
> + * idpf_probe - Device initialization routine
> + * @pdev: PCI device information struct
> + * @ent: entry in idpf_pci_tbl
> + *
> + * Returns 0 on success, negative on failure
> + */
> +static int idpf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct idpf_adapter *adapter;
> +	int err;
> +
> +	adapter = kzalloc(sizeof(*adapter), GFP_KERNEL);
> +	if (!adapter)
> +		return -ENOMEM;

Nit: add a blank line here.

> +	adapter->pdev = pdev;
> +
> +	err = pcim_enable_device(pdev);
> +	if (err)
> +		goto err_free;
> +
> +	err = pcim_iomap_regions(pdev, BIT(0), pci_name(pdev));
> +	if (err) {
> +		pci_err(pdev, "pcim_iomap_regions failed %pe\n", ERR_PTR(err));
> +
> +		goto err_free;
> +	}
> +
> +	/* set up for high or low dma */
> +	err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
> +	if (err) {
> +		pci_err(pdev, "DMA configuration failed: %pe\n", ERR_PTR(err));
> +
> +		goto err_free;
> +	}
> +
> +	pci_enable_pcie_error_reporting(pdev);

It seems pci_enable_pcie_error_reporting() has a return value, is there a reason
not to check the return value and handle it?

> +	pci_set_master(pdev);
> +	pci_set_drvdata(pdev, adapter);
> +
> +	/* setup msglvl */
> +	adapter->msg_enable = netif_msg_init(-1, IDPF_AVAIL_NETIF_M);
> +
> +	err = idpf_cfg_hw(adapter);
> +	if (err) {
> +		dev_err(dev, "Failed to configure HW structure for adapter: %d\n",
> +			err);
> +		goto err_cfg_hw;
> +	}
> +
> +	return 0;
> +
> +err_cfg_hw:

Is pci_clear_master() needed here?

> +	pci_disable_pcie_error_reporting(pdev);
> +err_free:
> +	kfree(adapter);
> +	return err;
> +}