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

[Basavaraj Natikar <bnatikar@amd.com>]

On 6/17/2024 10:20 PM, Bjorn Helgaas wrote:
> 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.

ya I will change it accordingly 

>
>> +		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.

Sure I will change all the lines to be like this below
       if (ae4_msix && (ae4_msix->msix_count || ae4->ae4_irq[MAX_AE4_HW_QUEUES -1]))
               pci_free_irq_vectors(pdev);

Thanks,
--
Basavaraj 

>
>> +	}
>> +}
>> +
>> +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);
>> +}