On 2019-12-09 11:49 p.m., Jiasen Lin wrote: > Integrated DMA engine of PEX87xx series switch support various > interrupts. According to my personal experience, I suggest that > enable error interrupt, invalid decscriptor interrupt, abort done > interrupt, graceful puse done interrupt, and > immediate pasue done interrupt by write DMA Channel x Interrupt > Control/Status register. Well, that depends on what we want to do with these interrupts: 1) We shouldn't need to handle the error/invalid descriptor interrupt. We instead just see that a specific descriptor failed (in the usual way) and handle it accordingly. (Though this isn't really documented well). An invalid descriptor should really never happen unless we have a driver bug. I suppose I could print an error message if either occur. 2) We never send an abort or immediate pause to the device, so neither interrupt can ever fire. So there's nothing to do if they do fire and thus no sense enabling them. 3) We do send a graceful pause to the device on teardown but prefer to poll for the end of the pause instead of adding the extra complexity to waiting for an interrupt. So no need for the interrupt. Logan > > Thanks, > Jiasen Lin > >> kref_init(&plxdev->ref); >> INIT_WORK(&plxdev->release_work, plx_dma_release_work); >> + spin_lock_init(&plxdev->ring_lock); >> + tasklet_init(&plxdev->desc_task, plx_dma_desc_task, >> + (unsigned long)plxdev); >> >> + RCU_INIT_POINTER(plxdev->pdev, pdev); >> plxdev->bar = pcim_iomap_table(pdev)[0]; >> >> dma = &plxdev->dma_dev; >> @@ -169,6 +501,16 @@ static void plx_dma_remove(struct pci_dev *pdev) >> >> free_irq(pci_irq_vector(pdev, 0), plxdev); >> >> + rcu_assign_pointer(plxdev->pdev, NULL); >> + synchronize_rcu(); >> + >> + spin_lock_bh(&plxdev->ring_lock); >> + plxdev->ring_active = false; >> + spin_unlock_bh(&plxdev->ring_lock); >> + >> + __plx_dma_stop(plxdev); >> + plx_dma_abort_desc(plxdev); >> + >> plxdev->bar = NULL; >> plx_dma_put(plxdev); >> >>