> -----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����?���&��