Re: [PATCH 1/4] serial: 8250: Avoid error message on reprobe

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Lukas Wunner <lukas@wunner.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jslaby@suse.com>,
	"Matwey V. Kornilov" <matwey@sai.msu.ru>,
	Giulio Benetti <giulio.benetti@micronovasrl.com>,
	Heiko Stuebner <heiko@sntech.de>,
	Christoph Muellner <christoph.muellner@theobroma-systems.com>,
	linux-serial@vger.kernel.org
Subject: Re: [PATCH 1/4] serial: 8250: Avoid error message on reprobe
Date: Wed, 6 May 2020 13:01:57 +0300	[thread overview]
Message-ID: <20200506100157.GB185537@smile.fi.intel.com> (raw)
In-Reply-To: <20200506060623.sf3kh3fwhoawawsd@wunner.de>

On Wed, May 06, 2020 at 08:06:23AM +0200, Lukas Wunner wrote:
> On Tue, May 05, 2020 at 07:01:01PM +0300, Andy Shevchenko wrote:
> > On Tue, May 05, 2020 at 04:42:01PM +0200, Lukas Wunner wrote:
> > > If the call to uart_add_one_port() in serial8250_register_8250_port()
> > > fails, a half-initialized entry in the serial_8250ports[] array is left
> > > behind.
> > > 
> > > A subsequent reprobe of the same serial port causes that entry to be
> > > reused.  Because uart->port.dev is set, uart_remove_one_port() is called
> > > for the half-initialized entry and bails out with an error message:
> > > 
> > > bcm2835-aux-uart 3f215040.serial: Removing wrong port: (null) != (ptrval)
> > > 
> > > The same happens on failure of mctrl_gpio_init() since commit
> > > 4a96895f74c9 ("tty/serial/8250: use mctrl_gpio helpers").
> > > 
> > > Fix by zeroing the uart->port.dev pointer in the probe error path.
> > >  
> > > Signed-off-by: Lukas Wunner <lukas@wunner.de>
> > > Cc: stable@vger.kernel.org # v2.6.10+
> > 
> > Fixes tag?
> 
> The bug was introduced in the pre-git era, so I can't provide a Fixes tag:
> 
> https://git.kernel.org/tglx/history/c/befff6f5bf5f
> 
> This commit, which went into v2.6.10, added an unconditional
> uart_remove_one_port() in serial8250_register_port().

We have history tree, but I heard you.

> In 2012, commit 835d844d1a28 ("8250_pnp: do pnp probe before legacy probe")
> made the call to uart_remove_one_port() conditional on uart->port.dev
> being non-NULL and that allows me to fix the issue by setting that
> pointer to NULL in the error path.  The commit went into v3.7, so it
> should be possible to fix the problem going back to v3.7 with my patch.
> And before that one needs to additionally make the call to
> uart_remove_one_port() conditional.

Perhaps it will be the best candidate.

Fixes, AFAIU, helps scripts to find whatever trees it might been applied
against. If somebody keeps private stable tree on unusual base it will be
helpful for them as well (but I understand that there is little care of a
such).

> However, according to www.kernel.org the oldest LTS kernel is v3.16.
> So I've given you the nitty-gritty details but it's all fairly
> irrelevant and the Cc: stable tag I've put into the commit seems the
> best I can do in this case.

-- 
With Best Regards,
Andy Shevchenko