[Sound-open-firmware] [PATCH 2/2] cnl: dma: refine dma interrupt processing on cnl

[rander.wang]

hi Liam,

On 3/6/2018 7:40 PM, Liam Girdwood wrote:
> On Tue, 2018-03-06 at 13:26 +0800, Rander Wang wrote:
>> +/*All the dma share the same interrupt on CNL, so check each dma*/
>> +/*controller when interrupt coming, then do the work            */
>> +static void dw_dma_irq_cnl(void *data)
>> +{
>> +	struct dma *dma;
>> +	uint32_t status_intr;
>> +
>> +	/*check interrupt status of DMA controller 0*/
>> +	dma = dma_get(DMA_GP_LP_DMAC0);
>> +	status_intr = dw_read(dma, DW_INTR_STATUS);
>> +	if (status_intr)
>> +		dw_dma_irq_handler(dma);
>> +
>> +	/*check interrupt status of DMA controller 1*/
>> +	dma = dma_get(DMA_GP_LP_DMAC1);
>> +	status_intr = dw_read(dma, DW_INTR_STATUS);
>> +	if (status_intr)
>> +		dw_dma_irq_handler(dma);
>
> Btw, shouldn't the CNL IRQ hanler be using the same code as APL since
> the DMAC is second level on both APL and CNL ?

for APL, there is a different irq for each DMA channel, but for CNL two
dma controller share the same irq, and dma status should be checked for
which channal. So it is a three level irq on CNL

>> +}
>> +
>>   static int dw_dma_probe(struct dma *dma)
>>   {
>>   	struct dma_pdata *dw_pdata;
>> @@ -1157,7 +1177,11 @@ static int dw_dma_probe(struct dma *dma)
>>   	}
>>   
>>   	/* register our IRQ handler */
>> +#ifdef CONFIG_CANNONLAKE
>> +	interrupt_register(dma_irq(dma), dw_dma_irq_cnl, dma);
>> +#else
>>   	interrupt_register(dma_irq(dma), dw_dma_irq_handler, dma);
>> +#endif
> You dont need this #ifdef if both IRQ handler functions have the same
> name (and we select the correct one at build time).
     sorry, i don't understand this issue.
>
> Liam
>
>>   	interrupt_enable(dma_irq(dma));
>>   
>>   	return 0;
> _______________________________________________
> Sound-open-firmware mailing list
> Sound-open-firmware at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware