Linux Serial subsystem development
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: "Uwe Kleine-König" <u.kleine-koenig@baylibre.com>
Cc: "Hugo Villeneuve" <hugo@hugovil.com>,
	"Jiri Slaby" <jirislaby@kernel.org>,
	"Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
	"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
	"Hugo Villeneuve" <hvilleneuve@dimonoff.com>,
	linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org
Subject: Re: [PATCH] serial: 8250_hub6: Fix double definition for hub6_match_port()
Date: Thu, 30 Jul 2026 16:49:45 +0200	[thread overview]
Message-ID: <2026073028-anointer-unlinked-c0ea@gregkh> (raw)
In-Reply-To: <amn8Hb2dbv_Sah_Y@monoceros>

On Wed, Jul 29, 2026 at 03:30:57PM +0200, Uwe Kleine-König wrote:
> Hello Hugo,
> 
> On Mon, Jul 27, 2026 at 11:26:43AM -0400, Hugo Villeneuve wrote:
> > On Mon, 27 Jul 2026 11:22:10 -0400
> > Hugo Villeneuve <hugo@hugovil.com> wrote:
> > 
> > > Hi Uwe,
> > > 
> > > On Fri, 24 Jul 2026 09:58:13 +0200
> > > Uwe Kleine-König <u.kleine-koenig@baylibre.com> wrote:
> > > 
> > > > On Thu, Jul 23, 2026 at 11:13:59PM -0400, Hugo Villeneuve wrote:
> > > > > > For me the open question remains if the driver works in the
> > > > > > configuration CONFIG_SERIAL_8250=y (and thus CONFIG_SERIAL_CORE=y) +
> > > > > > CONFIG_SERIAL_8250_HUB6=m. In this case at least commit
> > > > > > 3d406299d8829747fe2e8692f4c29fe3dc1d101f +
> > > > > > https://lore.kernel.org/linux-serial/20260715153707.4181828-1-hugo@hugovil.com/
> > > > > > introduces a logical change in uart_match_port() that isn't explained in
> > > > > > the commit log.
> > > > > 
> > > > > Not sure what you mean by that logical change?
> > > > 
> > > > With said configuration and before
> > > > 3d406299d8829747fe2e8692f4c29fe3dc1d101f uart_match_port() returned
> > > > 
> > > > 	port1->iobase == port2->iobase && port1->hub6 == port2->hub6
> > > > 
> > > > and with 3d406299d8829747fe2e8692f4c29fe3dc1d101f (and your fix on top)
> > > > it returns false (because when drivers/tty/serial/serial_core.c is
> > > > compiled IS_REACHABLE(CONFIG_SERIAL_8250_HUB6) evaluates to false).
> > > 
> > > this change was certainly not intended, sorry about that. Looks like
> > > there are still subtle Kconfig-isms (and 8250-isms) that I still need to
> > > master...
> > > 
> > > So to be sure i understand this correctly:
> > > I will submit a patch to replace IS_REACHABLE with IS_ENABLED, which
> > > means that the configuration CONFIG_SERIAL_8250=y and
> > > CONFIG_SERIAL_8250_HUB6=m is not supported, as you stated that
> > > it currently cannot happen?
> 
> No, that can happen. e.g. ARCH=parisc allmodconfig has something similar
> (and this is how I stumbled over the breakage of
> 3d406299d8829747fe2e8692f4c29fe3dc1d101f).
> 
> [similar = SERIAL_CORE=y + SERIAL_8250_HUB6=m]
> 
> > Or we leave it as is, since this combination is not really valid?
> 
> Not sure what you're saying here. You want to keep the IS_REACHABLE and
> so be able to compile CONFIG_SERIAL_8250=y + CONFIG_SERIAL_8250_HUB6=m
> but have that broken at runtime? What does make CONFIG_SERIAL_8250=y +
> CONFIG_SERIAL_8250_HUB6=m "not really valid"? Or do you mean something
> else?
> 
> I think the real fix would be to just copy uart_match_port() into the
> two drivers that actually use it (and remove the then dead code paths).
> Then it would be drivers/tty/serial/8250/8250_core.c using
> hub6_match_port() only and that can be handled by a proper dependency.

I'm totally confused, so I'll drop this patch from my review queue and
wait for you all to figure it out :)

  reply	other threads:[~2026-07-30 16:20 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-20  8:08 [PATCH] serial: 8250_hub6: Fix double definition for hub6_match_port() Uwe Kleine-König (The Capable Hub)
2026-07-20 10:31 ` Uwe Kleine-König (The Capable Hub)
2026-07-20 14:07   ` Hugo Villeneuve
2026-07-20 21:18     ` Uwe Kleine-König
2026-07-20 22:53       ` Hugo Villeneuve
2026-07-23  4:37         ` Uwe Kleine-König
2026-07-24  3:13           ` Hugo Villeneuve
2026-07-24  7:58             ` Uwe Kleine-König
2026-07-27 15:22               ` Hugo Villeneuve
2026-07-27 15:26                 ` Hugo Villeneuve
2026-07-29 13:30                   ` Uwe Kleine-König
2026-07-30 14:49                     ` Greg Kroah-Hartman [this message]
2026-07-30 19:57                       ` Uwe Kleine-König
2026-07-30 20:15                         ` Hugo Villeneuve

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=2026073028-anointer-unlinked-c0ea@gregkh \
    --to=gregkh@linuxfoundation.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=hugo@hugovil.com \
    --cc=hvilleneuve@dimonoff.com \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=jirislaby@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=u.kleine-koenig@baylibre.com \
    /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