Re: [PATCH v2 2/7] dmaengine: ae4dma: Add AMD ae4dma controller driver

[Bjorn Helgaas <helgaas@kernel.org>]

On Mon, Jun 17, 2024 at 03:33:54PM +0530, Basavaraj Natikar wrote:
> Add support for AMD AE4DMA controller. It performs high-bandwidth
> memory to memory and IO copy operation. Device commands are managed
> via a circular queue of 'descriptors', each of which specifies source
> and destination addresses for copying a single buffer of data.

> +static void ae4_free_irqs(struct ae4_device *ae4)
> +{
> +	struct ae4_msix *ae4_msix;
> +	struct pci_dev *pdev;
> +	struct pt_device *pt;
> +	struct device *dev;
> +	int i;
> +
> +	if (ae4) {

I don't think this test is necessary.  I don't think it's possible to
get here with ae4==0.

> +		pt = &ae4->pt;
> +		dev = pt->dev;
> +		pdev = to_pci_dev(dev);
> +
> +		ae4_msix = ae4->ae4_msix;
> +		if (ae4_msix && ae4_msix->msix_count)
> +			pci_disable_msix(pdev);
> +		else if (pdev->irq)
> +			pci_disable_msi(pdev);
> +
> +		for (i = 0; i < MAX_AE4_HW_QUEUES; i++)
> +			ae4->ae4_irq[i] = 0;

Clearing ae4_irq[] also doesn't seem necessary, since this is only
used in .remove(), and ae4 should never be used again.  If this path
becomes used in some future path that depends on ae4_irq[] being
cleared, perhaps the clearing could be moved to that patch.

> +	}
> +}
> +
> +static void ae4_deinit(struct ae4_device *ae4)
> +{
> +	ae4_free_irqs(ae4);
> +}

> +static void ae4_pci_remove(struct pci_dev *pdev)
> +{
> +	struct ae4_device *ae4 = dev_get_drvdata(&pdev->dev);
> +
> +	ae4_destroy_work(ae4);
> +	ae4_deinit(ae4);
> +}