All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.