public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
From: Johan Hovold <johan@kernel.org>
To: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: "Bartosz Golaszewski" <bartosz.golaszewski@oss.qualcomm.com>,
	"Andi Shyti" <andi.shyti@kernel.org>,
	"Chen-Yu Tsai" <wens@kernel.org>,
	"Jernej Skrabec" <jernej.skrabec@gmail.com>,
	"Samuel Holland" <samuel@sholland.org>,
	"Khalil Blaiech" <kblaiech@nvidia.com>,
	"Asmaa Mnebhi" <asmaa@nvidia.com>,
	"Jean Delvare" <jdelvare@suse.com>,
	"Madhavan Srinivasan" <maddy@linux.ibm.com>,
	"Michael Ellerman" <mpe@ellerman.id.au>,
	"Nicholas Piggin" <npiggin@gmail.com>,
	"Christophe Leroy (CS GROUP)" <chleroy@kernel.org>,
	"Andreas Färber" <afaerber@suse.de>,
	"Manivannan Sadhasivam" <mani@kernel.org>,
	linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-sunxi@lists.linux.dev, linuxppc-dev@lists.ozlabs.org,
	linux-actions@lists.infradead.org,
	"Bartosz Golaszewski" <brgl@kernel.org>
Subject: Re: Big I2C core changes coming up this year (was: Re: [PATCH 00/12] i2c: add and start using i2c_adapter-specific printk helpers
Date: Mon, 19 Jan 2026 11:58:34 +0100	[thread overview]
Message-ID: <aW4OWnyYp6Vas53L@hovoldconsulting.com> (raw)
In-Reply-To: <aWYYZEPX-_1GfQtL@ninjato>

On Tue, Jan 13, 2026 at 11:03:16AM +0100, Wolfram Sang wrote:
> Johan, (and all future readers I have pointed to this mail)
> 
> > No, this is not the way to do it. You start with designing and showing
> > what the end result will look like *before* you start rewriting world
> > like you are doing here.
> 
> In general, this is correct. It does not apply here, though. I will
> describe it in detail, so I can also point other people to this mail who
> wonder about quite some intrusive changes to I2C core this year.
> 
> > We should not be making driver code less readable just to address some
> > really niche corner cases like hot pluggable i2c controllers.
> 
> It is not a niche-case for hot-plugging. Hot-plugging (which still
> should be avoided for I2C) just makes a subsystem-inherent lifecycle
> problem more obvious. All of Bart's patch series basically prepare to
> tackle this comment from the I2C core:
> 
> 1805         /* wait until all references to the device are gone
> 1806          *
> 1807          * FIXME: This is old code and should ideally be replaced by an
> 1808          * alternative which results in decoupling the lifetime of the struct
> 1809          * device from the i2c_adapter, like spi or netdev do. Any solution
> 1810          * should be thoroughly tested with DEBUG_KOBJECT_RELEASE enabled!
> 1811          */
> 1812         init_completion(&adap->dev_released);
> 1813         device_unregister(&adap->dev);
> 1814         wait_for_completion(&adap->dev_released);
> 
> This has been in the I2C core since switching to the driver model and
> the underlying problem applies to *all* i2c adapters. Simply unbind an
> I2C controller while you still have a reference to its i2c-dev
> counterpart and you are right in the problem space.

Unbinding drivers is even more of a corner case than hot-plugging i2c
controllers is since only root can do that.

Unloading modules is really just a development (debugging) tool so there
is a perfectly simple solution for any issues stemming from root
unbinding a driver while it's in use: don't do that then.

Furthermore, we even have mechanisms for preventing root from shooting
themselves in the foot with module refcounting and suppression of bind
attributes (which sometimes makes sense).

Issues stemming from unbinding drivers are not in themselves a
sufficient reason for making code harder to read and maintain or for
adding overhead to normal operation.

> The problem is known for decades(!) and nobody dared to touch it, so
> far. Even worse, the above pattern is not only present in I2C but also
> other subsystems. Bart and I have been talking about potential solutions
> for three years now. Bart brought in SRCU as a generic solution and at a
> Plumbers 2024 session with many experienced maintainers present, it was
> decided that this path is worth exploring. Greg suggested to try SRCU
> with GPIO and I2C subsystems, and if this works well, we can try to
> abstract it into something useful for other canidates as well. Now,
> recently, the 'revocable' patch series was introduced which might be
> helpful in our case.

Revocable is definitely not something we want for the very reasons I
outlined above (wrapping every access in code that's generally simply
not needed).

> It is also perfectly okay to work incrementally here, in my book. It is
> such an intrusive change that we need to touch a lot of drivers in any
> case. Yet, with the I2C core being a moving target, all the I2C drivers,
> the 'revocable' patch series, and Linux in general, I think requesting a
> fully complete patch series now is neither efficient nor maintainable.

Again, if the end result is undesirable, that's what we should be
focusing on and not "chipping away" in a direction were we most likely
do not want to go.

It doesn't have to be a complete series, an RFC reworking one driver is
more than enough to be able to judge the end result. And of course a
proper description of the problem you're trying to solve.

> Bartosz and I do have a plan. We are happy to discuss it with other
> interested people, of course. Still, despite all our efforts it might be
> a bumpy ride. Because it is such a crucial and deep-inside change to the
> subsystem core. This is part of development, though. If something
> breaks, we will fix or revert. The alternative is stagnation which I
> don't want. The above code was fine back in the days but now we have
> better mechanisms to handle lifecycle issues. And I really do not want
> Linux to have lifecycle issues. Especially not ones we *know of*.

There are lifetime issues in some subsystems related to user space
interfaces and hotplugging, but driver unbind is really not something
you need to worry about.

> > But in any case, don't get ahead of things by posting changes that we
> > most likely don't want in the end anyway.
> 
> We want helpers accessing that specific 'struct device', so we have
> central place to implement access to a protected version of it.

I really don't think we do. USB has been dealing with hotplugging since
forever without any of this. You need to keep some state around and
prevent new accesses until the last user is gone. That's about it.

Johan


  reply	other threads:[~2026-01-19 10:58 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-23 10:02 [PATCH 00/12] i2c: add and start using i2c_adapter-specific printk helpers Bartosz Golaszewski
2025-12-23 10:02 ` [PATCH 01/12] i2c: add " Bartosz Golaszewski
2025-12-25 18:30   ` kernel test robot
2025-12-25 19:33   ` kernel test robot
2025-12-23 10:02 ` [PATCH 02/12] i2c: sun6i-p2wi: use " Bartosz Golaszewski
2025-12-23 11:04   ` Chen-Yu Tsai
2025-12-23 10:02 ` [PATCH 03/12] i2c: mlxbf: " Bartosz Golaszewski
2025-12-23 10:02 ` [PATCH 04/12] i2c: isch: " Bartosz Golaszewski
2025-12-23 10:02 ` [PATCH 05/12] i2c: ali1535: " Bartosz Golaszewski
2025-12-23 10:02 ` [PATCH 06/12] i2c: scmi: " Bartosz Golaszewski
2025-12-23 10:02 ` [PATCH 07/12] i2c: ali15x3: " Bartosz Golaszewski
2025-12-23 10:02 ` [PATCH 08/12] i2c: powermac: " Bartosz Golaszewski
2025-12-23 10:02 ` [PATCH 09/12] i2c: owl: " Bartosz Golaszewski
2025-12-23 10:02 ` [PATCH 10/12] i2c: nforce2: " Bartosz Golaszewski
2025-12-23 10:02 ` [PATCH 11/12] i2c: amd756: " Bartosz Golaszewski
2025-12-23 10:02 ` [PATCH 12/12] i2c: piix4: " Bartosz Golaszewski
2025-12-23 14:23 ` [PATCH 00/12] i2c: add and start using " Johan Hovold
2025-12-23 15:11   ` Bartosz Golaszewski
2026-01-19 11:03     ` Johan Hovold
2026-01-19 11:17       ` Bartosz Golaszewski
2026-01-19 11:32         ` Johan Hovold
2026-01-13 10:03   ` Big I2C core changes coming up this year (was: " Wolfram Sang
2026-01-19 10:58     ` Johan Hovold [this message]
2026-01-13 10:44 ` Wolfram Sang
2026-01-13 10:59   ` Bartosz Golaszewski
2026-01-13 11:50     ` Wolfram Sang
2026-01-14 13:36   ` Bartosz Golaszewski
2026-01-14 15:22     ` Wolfram Sang

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=aW4OWnyYp6Vas53L@hovoldconsulting.com \
    --to=johan@kernel.org \
    --cc=afaerber@suse.de \
    --cc=andi.shyti@kernel.org \
    --cc=asmaa@nvidia.com \
    --cc=bartosz.golaszewski@oss.qualcomm.com \
    --cc=brgl@kernel.org \
    --cc=chleroy@kernel.org \
    --cc=jdelvare@suse.com \
    --cc=jernej.skrabec@gmail.com \
    --cc=kblaiech@nvidia.com \
    --cc=linux-actions@lists.infradead.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sunxi@lists.linux.dev \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=maddy@linux.ibm.com \
    --cc=mani@kernel.org \
    --cc=mpe@ellerman.id.au \
    --cc=npiggin@gmail.com \
    --cc=samuel@sholland.org \
    --cc=wens@kernel.org \
    --cc=wsa+renesas@sang-engineering.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