linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Sherry Sun <sherry.sun@nxp.com>
Cc: "jirislaby@kernel.org" <jirislaby@kernel.org>,
	"u.kleine-koenig@pengutronix.de" <u.kleine-koenig@pengutronix.de>,
	"linux-serial@vger.kernel.org" <linux-serial@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	dl-linux-imx <linux-imx@nxp.com>
Subject: Re: [PATCH V2] tty: serial: imx: disable UCR4_OREN in .stop_rx() instead of .shutdown()
Date: Wed, 24 Nov 2021 12:24:41 +0100	[thread overview]
Message-ID: <YZ4g+U7PZ4bdjVjm@kroah.com> (raw)
In-Reply-To: <DB9PR04MB841142300744686983652AB992619@DB9PR04MB8411.eurprd04.prod.outlook.com>

On Wed, Nov 24, 2021 at 10:56:43AM +0000, Sherry Sun wrote:
> Hi Greg,
> 
> > -----Original Message-----
> > From: Greg KH <gregkh@linuxfoundation.org>
> > Sent: 2021年11月24日 17:59
> > To: Sherry Sun <sherry.sun@nxp.com>
> > Cc: jirislaby@kernel.org; u.kleine-koenig@pengutronix.de; linux-
> > serial@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx <linux-
> > imx@nxp.com>
> > Subject: Re: [PATCH V2] tty: serial: imx: disable UCR4_OREN in .stop_rx()
> > instead of .shutdown()
> > 
> > On Tue, Nov 23, 2021 at 06:51:22PM +0800, Sherry Sun wrote:
> > > From: Fugang Duan <fugang.duan@nxp.com>
> > >
> > > Disable the UCR4_OREN bit in .stop_rx() before the uart receiver is
> > > disabled maybe better than in the .shutdown() function.
> > 
> > Why is it better?  What does this "fix"?
> 
> Since I am not clear about the history of this patch, so I haven't found any obvious issues without this patch so far.

Then why submit it?

> But after discussing with the IP owner, it is still recommended to disable the receiver-related interrupts like OREN before disabling the receiver.

recommended by what?  The hardware designers?

> If not, when we have the overrun error during the receiver disable process, the overrun interrupt will keep trigging until we disable the OREN interrupt in the .shutdown(), because the ORE status can only be cleared when read the rx FIFO or reset the controller.  Although the called time between the receiver disable and OREN disable in .shutdown() is very short, there is still the risk of endless interrupt during this short period of time. So had better to disable OREN before the receiver been disabled in .stop_rx().

Please document this in the changelog, otherwise we have no idea why
this is needed.

thanks,

greg k-h

  reply	other threads:[~2021-11-24 11:24 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-23 10:51 [PATCH V2] tty: serial: imx: disable UCR4_OREN in .stop_rx() instead of .shutdown() Sherry Sun
2021-11-23 13:03 ` Uwe Kleine-König
2021-11-24  9:59 ` Greg KH
2021-11-24 10:56   ` Sherry Sun
2021-11-24 11:24     ` Greg KH [this message]
2021-11-24 11:35       ` Sherry Sun

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YZ4g+U7PZ4bdjVjm@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=jirislaby@kernel.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=sherry.sun@nxp.com \
    --cc=u.kleine-koenig@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).