[PATCH 1/1] tty: serial: imx: disable ageing timer interrupt if dma in use

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Troy Kisky [mailto:troy.kisky@xxxxxxxxxxxxxxxxxxx]
> Sent: 11 October 2017 19:10
> To: Han, Nandor (GE Healthcare) <nandor.han@xxxxxx>;
> gregkh@xxxxxxxxxxxxxxxxxxx
> Cc: fabio.estevam@xxxxxxx; l.stach@xxxxxxxxxxxxxx; u.kleine-
> koenig@xxxxxxxxxxxxxx; linux-serial@xxxxxxxxxxxxxxx; linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx
> Subject: EXT: Re: [PATCH 1/1] tty: serial: imx: disable ageing timer interrupt if
> dma in use
> 
> On 10/10/2017 11:22 PM, Han, Nandor (GE Healthcare) wrote:

<snip>

> >> -			imx_rxint(irq, dev_id);
> >> +	if (!sport->dma_is_enabled && (sts & (USR1_RRDY |
> >> USR1_AGTIM))) {
> >> +		imx_rxint(irq, dev_id);
> >
> > I don't think we need the check `!sport->dma_is_enabled ` since RRDY and
> AGTIM interrupts should be disable when DMA is enabled.
> 
> If I AND the status with its corresponding interrupt enable. But that is just as
> complicated, and more different from the original.
> 
> 

Yes, I agree, but it would be the right thing to do. :) 

> >
> >>  		ret = IRQ_HANDLED;
> >>  	}
> >>
> >> @@ -1207,10 +1181,6 @@ static void imx_enable_dma(struct imx_port
> >> *sport)
> >>  	temp |= UCR1_RDMAEN | UCR1_TDMAEN | UCR1_ATDMAEN;
> >>  	writel(temp, sport->port.membase + UCR1);
> >>
> >> -	temp = readl(sport->port.membase + UCR2);
> >> -	temp |= UCR2_ATEN;
> >> -	writel(temp, sport->port.membase + UCR2);
> >> -
> >
> > I think this should be moved to imx_startup. At least RRDY is configured
> there.
> >
> 
> 
> Currently, non-dma mode does not use the ageing timer. You get an interrupt
> immediately when
> 1 character is in the fifo. So, enabling the ageing timer and changing fifo level,
> should really be a separate patch. But it would be a nice patch.
> 
> 

Ok but in this case, we are left with aging timer and rrdy check in the interrupt and imx-startup enables only rrdy. (which is not much different than what it was before, since aging timer didn't get a chance to be triggered)
I agree that changing the fifo level to give a chance also to the aging timer will be the right thing to do here.

Overall, I think is ok, my main concern with not enabling the aging timer in startup is that people can be confused about this.

Reviewed-by: Nandor Han <Nandor.han@xxxxxx>
��.n��������+%������w��{.n�����{��ǫ����{ay�ʇڙ���f���h������_�(�階�ݢj"��������G����?���&��




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux