linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Francesco Dolcini <francesco.dolcini@toradex.com>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: Francesco Dolcini <francesco.dolcini@toradex.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Stefan Agner <stefan@agner.ch>,
	linux-serial@vger.kernel.org, Jiri Slaby <jirislaby@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	linux-kernel@vger.kernel.org, Shawn Guo <shawnguo@kernel.org>,
	NXP Linux Team <linux-imx@nxp.com>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/2] serial: imx: fix crash when un/re-binding console
Date: Thu, 14 Oct 2021 11:12:37 +0200	[thread overview]
Message-ID: <20211014091237.GB554920@francesco-nb.int.toradex.com> (raw)
In-Reply-To: <20211014081028.xafpuzrxk3jvv5qn@pengutronix.de>

Hello Uwe,

On Thu, Oct 14, 2021 at 10:10:28AM +0200, Uwe Kleine-König wrote:
> On Thu, Oct 14, 2021 at 10:01:53AM +0200, Francesco Dolcini wrote:
> > Hello Greg,
> > 
> > On Thu, Oct 14, 2021 at 09:33:55AM +0200, Greg Kroah-Hartman wrote:
> > > On Thu, Oct 14, 2021 at 09:10:52AM +0200, Francesco Dolcini wrote:
> > > > From: Stefan Agner <stefan@agner.ch>
> > > > 
> > > > If the device used as a serial console gets un/re-binded, then
> > > > register_console() will call imx_uart_setup_console() again.
> > > > Drop __init so that imx_uart_setup_console() can be safely called
> > > > at runtime.
> > > > 
> > > > Signed-off-by: Stefan Agner <stefan@agner.ch>
> > > > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > > ---
> > > >  drivers/tty/serial/imx.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > What commit does this "fix"?
> > 
> > root@colibri-imx6ull-06746657:~# cat /sys/devices/virtual/tty/console/active 
> > tty1 ttymxc0
> > root@colibri-imx6ull-06746657:~# echo -n N > /sys/devices/virtual/tty/console/subsystem/ttymxc0/console
> > root@colibri-imx6ull-06746657:~# echo -n Y > /sys/devices/virtual/tty/console/subsystem/ttymxc0/console
> > 
> > struct console ->setup is called at that time, but imx_uart_console_setup() is gone ...
> > 
> > According to the original report from Stefan the issue is the same with unbind/bind of the console,
> > see https://marc.info/?l=linux-serial&m=154221779312036&w=2.
> > 
> > 
> > > Should this go to stable kernels?  If so, how far back?
> > This is present also in 4.19 kernel, I feel like the issue is there since it is possible to unbind
> > a console driver (since ever considering the current LTS releases?). I could
> > investigate this a little bit more.
> > 
> > We have this patch in our kernel branch since 3 years but for some reason we never managed to upstream it.
> 
> I think this is a bigger problem not only affecting imx.c. Just look at
> the output of:
> 
> 	git grep -E '__init.*setup' drivers/tty/serial/

I agree, but I think that all the earlyconsole stuff is not affected, so the amount of drivers
that should be fixed is less than it looks like.

Apart for that once you fix this issue you need to be sure that on the unbind/unregister path you undo
all you did in the setup, maybe for most of the drivers is nothing, but for serial/imx I had to
disable the clock there (see patch 2/2 in this patchset).

Francesco


  reply	other threads:[~2021-10-14  9:13 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-14  7:10 Francesco Dolcini
2021-10-14  7:10 ` [PATCH 1/2] serial: imx: fix crash when un/re-binding console Francesco Dolcini
2021-10-14  7:33   ` Greg Kroah-Hartman
2021-10-14  8:01     ` Francesco Dolcini
2021-10-14  8:10       ` Uwe Kleine-König
2021-10-14  9:12         ` Francesco Dolcini [this message]
2021-10-14 10:28       ` Francesco Dolcini
2021-10-14  7:10 ` [PATCH 2/2] serial: imx: disable console clocks on unregister Francesco Dolcini
2021-10-14  7:17 ` [PATCH 0/2] serial: imx: fix unbind/bind console Francesco Dolcini

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=20211014091237.GB554920@francesco-nb.int.toradex.com \
    --to=francesco.dolcini@toradex.com \
    --cc=festevam@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jirislaby@kernel.org \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    --cc=stefan@agner.ch \
    --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).