It's been another year of discussing the object life-time problems at
conferences. I2C is one of the offenders and its problems are more
complex than those of some other subsystems. It seems the revocable[1]
API may make its way into the kernel this year but even with it in
place, I2C won't be able to use it as there's currently nothing to
*revoke*. The struct device is embedded within the i2c_adapter struct
whose lifetime is tied to the provider device being bound to its driver.

Fixing this won't be fast and easy but nothing's going to happen if we
don't start chipping away at it. The ultimate goal in order to be able
to use an SRCU-based solution (revocable or otherwise) is to convert the
embedded struct device in struct i2c_adapter into an __rcu pointer that
can be *revoked*. To that end we need to hide all dereferences of
adap->dev in drivers.

This series addresses the usage of adap->dev in device printk() helpers
(dev_err() et al). It introduces a set of i2c-specific helpers and
starts using them across bus drivers. For now just 12 patches but I'll
keep on doing it if these get accepted. Once these get upstream for
v6.20/7.0, we'll be able to also start converting i2c drivers outside of
drivers/i2c/.

Link: [1] https://lore.kernel.org/all/20251106152330.11733-1-tzungbi@kernel.org/
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
Changes in v2:
- Add a patch renaming an existing i2c_dbg() macro in a media driver
- Link to v1: https://lore.kernel.org/r/20251223-i2c-printk-helpers-v1-0-46a08306afdb@oss.qualcomm.com

---
Bartosz Golaszewski (13):
      media: saa7134: rename i2c_dbg() to saa7134_i2c_dbg()
      i2c: add i2c_adapter-specific printk helpers
      i2c: sun6i-p2wi: use i2c_adapter-specific printk helpers
      i2c: mlxbf: use i2c_adapter-specific printk helpers
      i2c: isch: use i2c_adapter-specific printk helpers
      i2c: ali1535: use i2c_adapter-specific printk helpers
      i2c: scmi: use i2c_adapter-specific printk helpers
      i2c: ali15x3: use i2c_adapter-specific printk helpers
      i2c: powermac: use i2c_adapter-specific printk helpers
      i2c: owl: use i2c_adapter-specific printk helpers
      i2c: nforce2: use i2c_adapter-specific printk helpers
      i2c: amd756: use i2c_adapter-specific printk helpers
      i2c: piix4: use i2c_adapter-specific printk helpers

 drivers/i2c/busses/i2c-ali1535.c        | 20 ++++++++++----------
 drivers/i2c/busses/i2c-ali15x3.c        | 20 ++++++++++----------
 drivers/i2c/busses/i2c-amd756.c         | 24 ++++++++++++------------
 drivers/i2c/busses/i2c-isch.c           | 32 ++++++++++++++++----------------
 drivers/i2c/busses/i2c-mlxbf.c          | 19 +++++++++----------
 drivers/i2c/busses/i2c-nforce2.c        | 14 +++++++-------
 drivers/i2c/busses/i2c-owl.c            |  4 ++--
 drivers/i2c/busses/i2c-piix4.c          |  8 ++++----
 drivers/i2c/busses/i2c-powermac.c       | 26 +++++++++++++-------------
 drivers/i2c/busses/i2c-scmi.c           |  6 +++---
 drivers/i2c/busses/i2c-sun6i-p2wi.c     |  8 ++++----
 drivers/media/pci/saa7134/saa7134-i2c.c | 26 +++++++++++++-------------
 include/linux/i2c.h                     |  6 ++++++
 13 files changed, 109 insertions(+), 104 deletions(-)
---
base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
change-id: 20251222-i2c-printk-helpers-a69f4403ca70

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

On Mon, Feb 23, 2026 at 09:59:29AM +0100, Bartosz Golaszewski wrote:
> It's been another year of discussing the object life-time problems at
> conferences. I2C is one of the offenders and its problems are more
> complex than those of some other subsystems. It seems the revocable[1]
> API may make its way into the kernel this year but even with it in
> place, I2C won't be able to use it as there's currently nothing to
> *revoke*. The struct device is embedded within the i2c_adapter struct
> whose lifetime is tied to the provider device being bound to its driver.
> 
> Fixing this won't be fast and easy but nothing's going to happen if we
> don't start chipping away at it. The ultimate goal in order to be able
> to use an SRCU-based solution (revocable or otherwise) is to convert the
> embedded struct device in struct i2c_adapter into an __rcu pointer that
> can be *revoked*. To that end we need to hide all dereferences of
> adap->dev in drivers.
> 
> This series addresses the usage of adap->dev in device printk() helpers
> (dev_err() et al). It introduces a set of i2c-specific helpers and
> starts using them across bus drivers. For now just 12 patches but I'll
> keep on doing it if these get accepted. Once these get upstream for
> v6.20/7.0, we'll be able to also start converting i2c drivers outside of
> drivers/i2c/.

I applied the series to for-current but squashed the user conversions
into patch 1. Changes are trivial enough and I don't want the pull
request to look excessive, so it can go in smoothly. Hope you are fine
with it.

On Thu, 26 Feb 2026 21:21:35 +0100, Wolfram Sang
<wsa+renesas@sang-engineering.com> said:
> On Mon, Feb 23, 2026 at 09:59:29AM +0100, Bartosz Golaszewski wrote:
>> It's been another year of discussing the object life-time problems at
>> conferences. I2C is one of the offenders and its problems are more
>> complex than those of some other subsystems. It seems the revocable[1]
>> API may make its way into the kernel this year but even with it in
>> place, I2C won't be able to use it as there's currently nothing to
>> *revoke*. The struct device is embedded within the i2c_adapter struct
>> whose lifetime is tied to the provider device being bound to its driver.
>>
>> Fixing this won't be fast and easy but nothing's going to happen if we
>> don't start chipping away at it. The ultimate goal in order to be able
>> to use an SRCU-based solution (revocable or otherwise) is to convert the
>> embedded struct device in struct i2c_adapter into an __rcu pointer that
>> can be *revoked*. To that end we need to hide all dereferences of
>> adap->dev in drivers.
>>
>> This series addresses the usage of adap->dev in device printk() helpers
>> (dev_err() et al). It introduces a set of i2c-specific helpers and
>> starts using them across bus drivers. For now just 12 patches but I'll
>> keep on doing it if these get accepted. Once these get upstream for
>> v6.20/7.0, we'll be able to also start converting i2c drivers outside of
>> drivers/i2c/.
>
> I applied the series to for-current but squashed the user conversions
> into patch 1. Changes are trivial enough and I don't want the pull
> request to look excessive, so it can go in smoothly. Hope you are fine
> with it.
>

Sure, do you still want me to send these changes in separate patches for
review?

Bart

> > I applied the series to for-current but squashed the user conversions
> > into patch 1. Changes are trivial enough and I don't want the pull
> > request to look excessive, so it can go in smoothly. Hope you are fine
> > with it.
> >
> 
> Sure, do you still want me to send these changes in separate patches for
> review?

Yes, this is totally fine with me. I think we get more tags when the
patches are broken out.

On Mon, Feb 23, 2026 at 09:59:29AM +0100, Bartosz Golaszewski wrote:
> It's been another year of discussing the object life-time problems at
> conferences. I2C is one of the offenders and its problems are more
> complex than those of some other subsystems. It seems the revocable[1]
> API may make its way into the kernel this year but even with it in
> place, I2C won't be able to use it as there's currently nothing to
> *revoke*. The struct device is embedded within the i2c_adapter struct
> whose lifetime is tied to the provider device being bound to its driver.
> 
> Fixing this won't be fast and easy but nothing's going to happen if we
> don't start chipping away at it. The ultimate goal in order to be able
> to use an SRCU-based solution (revocable or otherwise) is to convert the
> embedded struct device in struct i2c_adapter into an __rcu pointer that
> can be *revoked*. To that end we need to hide all dereferences of
> adap->dev in drivers.

Again, this is not the way to do this. You don't need to wrap every
access to struct device in random subsystem specific helpers to address
the lifetime issues. You're just creating a big mess here for no good
reason.

I asked you to show where you think you're going with this because none
of this looks right. Same comment applies to your other series.

Wolfram, I noticed you merged these last night. Please think again and
let's discuss the end result here. There's no question that there are
lifetime issues in i2c, but this is not the way to solve it.

Johan

Johan,

> Wolfram, I noticed you merged these last night. Please think again and
> let's discuss the end result here. There's no question that there are
> lifetime issues in i2c, but this is not the way to solve it.

I did think again and do not see a way how the life cycle problems can
be solved while drivers happily access the device struct of the adapter.
Whatever the solution to the core problem is (revocable, custom SRCU,
something else), I still think this step is needed in any case. If I am
wrong with this opinion, please enlighten me. Pointer to some existing
thread is OK, too. I didn't have the bandwidth to read the revocable
mail threads.

Happy hacking,

   Wolfram

On Fri, Feb 27, 2026 at 10:08:34AM +0100, Wolfram Sang wrote:

> > Wolfram, I noticed you merged these last night. Please think again and
> > let's discuss the end result here. There's no question that there are
> > lifetime issues in i2c, but this is not the way to solve it.
> 
> I did think again and do not see a way how the life cycle problems can
> be solved while drivers happily access the device struct of the adapter.

There's nothing special about the struct device. What matters is that
drivers don't free memory that's still in use by the core.

> Whatever the solution to the core problem is (revocable, custom SRCU,
> something else), I still think this step is needed in any case. If I am
> wrong with this opinion, please enlighten me. Pointer to some existing
> thread is OK, too. I didn't have the bandwidth to read the revocable
> mail threads.

It's not even about revocable or SRCU, that's just an implementation
detail.

It seems all that is needed is to decouple the struct i2c_adapter from
the driver data and have core manage the lifetime of the former using
the reference count of the embedded struct device.

Then you can use an rwsem, SRCU, revocable or something else to handle
devices going away while they are in use.

Johan

On Fri, Feb 27, 2026 at 11:06 AM Johan Hovold <johan@kernel.org> wrote:
>
> On Fri, Feb 27, 2026 at 10:08:34AM +0100, Wolfram Sang wrote:
>
> > > Wolfram, I noticed you merged these last night. Please think again and
> > > let's discuss the end result here. There's no question that there are
> > > lifetime issues in i2c, but this is not the way to solve it.
> >
> > I did think again and do not see a way how the life cycle problems can
> > be solved while drivers happily access the device struct of the adapter.
>
> There's nothing special about the struct device. What matters is that
> drivers don't free memory that's still in use by the core.
>
> > Whatever the solution to the core problem is (revocable, custom SRCU,
> > something else), I still think this step is needed in any case. If I am
> > wrong with this opinion, please enlighten me. Pointer to some existing
> > thread is OK, too. I didn't have the bandwidth to read the revocable
> > mail threads.
>
> It's not even about revocable or SRCU, that's just an implementation
> detail.
>
> It seems all that is needed is to decouple the struct i2c_adapter from
> the driver data and have core manage the lifetime of the former using
> the reference count of the embedded struct device.
>

I feel like we've discussed it already under v1 or elsewhere.

This is a weird pattern you sometimes see where a driver allocates
something and passes the ownership to the subsystem.  This often
causes confusion among driver authors, who logically assume that if
you allocate something, you are responsible for freeing it. Since this
is C and not Rust (where such things are tracked by the compiler), I
strongly believe we should strive to keep ownership consistent: the
driver should free resources it allocated within the bounds of the
lifetime of the device it controls. The subsystem should manage the
data it allocated - in this case the i2c adapter struct device.

I know there are a lot of places where this is done in the kernel but
let's not introduce new ones. This is a bad pattern.

But even if you decided this is the way to go, I fail to see how it
would be easier than what I'm trying to do. You would have to modify
*all* I2C bus drivers as opposed to only modifying those that access
the underlying struct device. Or am I missing something?

Bartosz

> Then you can use an rwsem, SRCU, revocable or something else to handle
> devices going away while they are in use.
>

On Fri, Feb 27, 2026 at 04:42:09PM +0100, Bartosz Golaszewski wrote:
> On Fri, Feb 27, 2026 at 11:06 AM Johan Hovold <johan@kernel.org> wrote:

> > It seems all that is needed is to decouple the struct i2c_adapter from
> > the driver data and have core manage the lifetime of the former using
> > the reference count of the embedded struct device.

> This is a weird pattern you sometimes see where a driver allocates
> something and passes the ownership to the subsystem.

It's not weird at all, this is the standard way to handle this. We have
these things called reference counts for a reason.

> This often
> causes confusion among driver authors, who logically assume that if
> you allocate something, you are responsible for freeing it.Since this
> is C and not Rust (where such things are tracked by the compiler), I
> strongly believe we should strive to keep ownership consistent: the
> driver should free resources it allocated within the bounds of the
> lifetime of the device it controls. The subsystem should manage the
> data it allocated - in this case the i2c adapter struct device.

Drivers are responsible for dropping *their* reference, it doesn't mean
that the resource is necessarily freed immediately as someone else may
be holding a reference. Anyone surprised by this should not be doing
kernel development.

> I know there are a lot of places where this is done in the kernel but
> let's not introduce new ones. This is a bad pattern.

No, it's not. It's literally the standard way of doing this.

> But even if you decided this is the way to go, I fail to see how it
> would be easier than what I'm trying to do. You would have to modify
> *all* I2C bus drivers as opposed to only modifying those that access
> the underlying struct device. Or am I missing something?

Yes, you have to update the allocation and replace container_of() with
dev_get_drvdata() but it's a straight-forward transformation that brings
the i2c subsystem more in line with the driver model (unlike whatever it
is you're trying to do).

Johan

On Fri, Feb 27, 2026 at 5:41 PM Johan Hovold <johan@kernel.org> wrote:
>
> On Fri, Feb 27, 2026 at 04:42:09PM +0100, Bartosz Golaszewski wrote:
> > On Fri, Feb 27, 2026 at 11:06 AM Johan Hovold <johan@kernel.org> wrote:
>
> > > It seems all that is needed is to decouple the struct i2c_adapter from
> > > the driver data and have core manage the lifetime of the former using
> > > the reference count of the embedded struct device.
>
> > This is a weird pattern you sometimes see where a driver allocates
> > something and passes the ownership to the subsystem.
>
> It's not weird at all, this is the standard way to handle this. We have
> these things called reference counts for a reason.
>

I wouldn't say it's *the* standard way. There are at least several different
ways driver subsystems handle resource ownership. And even so: the fact that
something's done a lot does not make it automatically correct.

> > This often
> > causes confusion among driver authors, who logically assume that if
> > you allocate something, you are responsible for freeing it.Since this
> > is C and not Rust (where such things are tracked by the compiler), I
> > strongly believe we should strive to keep ownership consistent: the
> > driver should free resources it allocated within the bounds of the
> > lifetime of the device it controls. The subsystem should manage the
> > data it allocated - in this case the i2c adapter struct device.
>
> Drivers are responsible for dropping *their* reference, it doesn't mean
> that the resource is necessarily freed immediately as someone else may
> be holding a reference. Anyone surprised by this should not be doing
> kernel development.
>

I disagree. For some reason, you're defending a suboptimal programming
interface. I'm all for reference counting but mixing reference-counted data
with non-counted is simply not a good idea. An API should be easy to use and
hard to misuse. Given the amount of issues, this approach is definitely easy
to misuse.

I'm advocating for a hard split between the subsystem data (reference-counted)
and driver data (living from probe() until remove()). A logical struct device
managed entirely by the subsystem should live in a separate structure than
driver data and be allocated - and freed - by the subsystem module.

Let's put aside kernel code for a minute and work with an abstract C example,
where the equivalent of what you're proposing would look like this:

struct bar {
	struct foo foo;
	...
};

struct bar *bar = malloc(sizeof(*bar));

ret = foo_register(&bar->foo);

And the corresponding free() lives who knows where because foo_register()
automagically introduces reference counting (nevermind the need to calculate
where bar is in relations to foo).

I strongly believe that this makes more sense:

struct bar {
	...
};

struct bar *bar = malloc();

struct foo *foo = foo_register(bar);

// foo is reference counted and allocated in the provider of foo_register()

foo_put(foo);
free(bar);

The equivalent of which is moving struct device out of struct i2c_adapter.
In fact: I would love to see i2c_adapter become a truly reference-counted
object detached from driver data but due to it being embedded in every bus
driver data structure it realistically won't happen.

> > I know there are a lot of places where this is done in the kernel but
> > let's not introduce new ones. This is a bad pattern.
>
> No, it's not. It's literally the standard way of doing this.
>
> > But even if you decided this is the way to go, I fail to see how it
> > would be easier than what I'm trying to do. You would have to modify
> > *all* I2C bus drivers as opposed to only modifying those that access
> > the underlying struct device. Or am I missing something?
>
> Yes, you have to update the allocation and replace container_of() with
> dev_get_drvdata() but it's a straight-forward transformation that brings
> the i2c subsystem more in line with the driver model (unlike whatever it
> is you're trying to do).
>

No, it's not that simple. The .release() callback of struct device embedded
in struct i2c_adapter is assigned from the bus type and only calls complete()
(yeah, I too don't think it looks right, one would expect to see the associated
kfree() here, right?). It relies on the bus driver freeing the data in its
remove() path. That's why we wait until all references to said struct device
are dropped. After your proposed change, if your new release() lives in the
driver module, it must not be removed until all the references are dropped
- basically where we are now. If on the other hand, the release() callback's
functionality is moved into i2c-core, how would you handle the fact i2c_adapter
can be embedded in a larger driver data structure? Provide yet another callback
in i2c_adapter called from the device's .release()? Sure, can be done but I
doubt it's a better solution.

Bartosz

On Mon, Mar 02, 2026 at 12:03:19PM -0600, Bartosz Golaszewski wrote:
> On Fri, Feb 27, 2026 at 5:41 PM Johan Hovold <johan@kernel.org> wrote:
> >
> > On Fri, Feb 27, 2026 at 04:42:09PM +0100, Bartosz Golaszewski wrote:
> > > On Fri, Feb 27, 2026 at 11:06 AM Johan Hovold <johan@kernel.org> wrote:
> >
> > > > It seems all that is needed is to decouple the struct i2c_adapter from
> > > > the driver data and have core manage the lifetime of the former using
> > > > the reference count of the embedded struct device.
> >
> > > This is a weird pattern you sometimes see where a driver allocates
> > > something and passes the ownership to the subsystem.
> >
> > It's not weird at all, this is the standard way to handle this. We have
> > these things called reference counts for a reason.
> >
> 
> I wouldn't say it's *the* standard way. There are at least several different
> ways driver subsystems handle resource ownership. And even so: the fact that
> something's done a lot does not make it automatically correct.

It's the way the driver model works.

> > > This often
> > > causes confusion among driver authors, who logically assume that if
> > > you allocate something, you are responsible for freeing it.Since this
> > > is C and not Rust (where such things are tracked by the compiler), I
> > > strongly believe we should strive to keep ownership consistent: the
> > > driver should free resources it allocated within the bounds of the
> > > lifetime of the device it controls. The subsystem should manage the
> > > data it allocated - in this case the i2c adapter struct device.
> >
> > Drivers are responsible for dropping *their* reference, it doesn't mean
> > that the resource is necessarily freed immediately as someone else may
> > be holding a reference. Anyone surprised by this should not be doing
> > kernel development.
> 
> I disagree. For some reason, you're defending a suboptimal programming
> interface. I'm all for reference counting but mixing reference-counted data
> with non-counted is simply not a good idea. An API should be easy to use and
> hard to misuse. Given the amount of issues, this approach is definitely easy
> to misuse.
> 
> I'm advocating for a hard split between the subsystem data (reference-counted)
> and driver data (living from probe() until remove()). A logical struct device
> managed entirely by the subsystem should live in a separate structure than
> driver data and be allocated - and freed - by the subsystem module.

It doesn't really matter what you think. You can't just go around
making up new subsystem specific rules at your whim. The linux driver
model uses reference counting and that's what developers expect to be
used.

> Let's put aside kernel code for a minute and work with an abstract C example,
> where the equivalent of what you're proposing would look like this:
> 
> struct bar {
> 	struct foo foo;
> 	...
> };
> 
> struct bar *bar = malloc(sizeof(*bar));
> 
> ret = foo_register(&bar->foo);
> 
> And the corresponding free() lives who knows where because foo_register()
> automagically introduces reference counting (nevermind the need to calculate
> where bar is in relations to foo).

No, that's not what I'm suggesting here, but it would be compatible with
the driver model (ever heard of struct device which works exactly like
this?).

> I strongly believe that this makes more sense:
> 
> struct bar {
> 	...
> };
> 
> struct bar *bar = malloc();
> 
> struct foo *foo = foo_register(bar);
> 
> // foo is reference counted and allocated in the provider of foo_register()
> 
> foo_put(foo);
> free(bar);
> 
> The equivalent of which is moving struct device out of struct i2c_adapter.

No, it's not.

> In fact: I would love to see i2c_adapter become a truly reference-counted
> object detached from driver data but due to it being embedded in every bus
> driver data structure it realistically won't happen.

And this is what I've been suggesting all along, separating the driver
data and making the adapter reference counted.

The idiomatic way to handle this is:

	xyz_probe()
	{
		adap = i2c_adapter_alloc();
		// initialise driver data, store pointer in adap
		i2c_adapter_register(adap);
	}

	xyz_remove()
	{
		i2c_adapter_deregister(adap);
		i2c_adapter_put(adap);
	}

Exactly where the driver data is stored is secondary, it could be memory
allocated by core or by the driver.

But the adapter is reference counted and kept around until all users are
gone.

> > > I know there are a lot of places where this is done in the kernel but
> > > let's not introduce new ones. This is a bad pattern.
> >
> > No, it's not. It's literally the standard way of doing this.
> >
> > > But even if you decided this is the way to go, I fail to see how it
> > > would be easier than what I'm trying to do. You would have to modify
> > > *all* I2C bus drivers as opposed to only modifying those that access
> > > the underlying struct device. Or am I missing something?
> >
> > Yes, you have to update the allocation and replace container_of() with
> > dev_get_drvdata() but it's a straight-forward transformation that brings
> > the i2c subsystem more in line with the driver model (unlike whatever it
> > is you're trying to do).
> >
> 
> No, it's not that simple. The .release() callback of struct device embedded
> in struct i2c_adapter is assigned from the bus type and only calls complete()
> (yeah, I too don't think it looks right, one would expect to see the associated
> kfree() here, right?). It relies on the bus driver freeing the data in its
> remove() path. That's why we wait until all references to said struct device
> are dropped. After your proposed change, if your new release() lives in the
> driver module, it must not be removed until all the references are dropped
> - basically where we are now. If on the other hand, the release() callback's
> functionality is moved into i2c-core, how would you handle the fact i2c_adapter
> can be embedded in a larger driver data structure? Provide yet another callback
> in i2c_adapter called from the device's .release()? Sure, can be done but I
> doubt it's a better solution.

You seem to be constructing some kind of straw man here. Obviously, the
release function would free the memory allocated for the adapter struct.

An adapter driver can free its driver data on unbind as core will
guarantee that there are no further callbacks after the adapter has been
deregistered.

Johan

On Tue, Mar 3, 2026 at 4:57 PM Johan Hovold <johan@kernel.org> wrote:
>
> On Mon, Mar 02, 2026 at 12:03:19PM -0600, Bartosz Golaszewski wrote:
> > On Fri, Feb 27, 2026 at 5:41 PM Johan Hovold <johan@kernel.org> wrote:
> > >
> > > On Fri, Feb 27, 2026 at 04:42:09PM +0100, Bartosz Golaszewski wrote:
> > > > On Fri, Feb 27, 2026 at 11:06 AM Johan Hovold <johan@kernel.org> wrote:
> > >
> > > > > It seems all that is needed is to decouple the struct i2c_adapter from
> > > > > the driver data and have core manage the lifetime of the former using
> > > > > the reference count of the embedded struct device.
> > >
> > > > This is a weird pattern you sometimes see where a driver allocates
> > > > something and passes the ownership to the subsystem.
> > >
> > > It's not weird at all, this is the standard way to handle this. We have
> > > these things called reference counts for a reason.
> > >
> >
> > I wouldn't say it's *the* standard way. There are at least several different
> > ways driver subsystems handle resource ownership. And even so: the fact that
> > something's done a lot does not make it automatically correct.
>
> It's the way the driver model works.
>

No, it does not impose any specific pattern to use for subsystems other than
requiring each device that's been *initialized* to provide a .release() callback
called when the last reference is dropped.

> > > > This often
> > > > causes confusion among driver authors, who logically assume that if
> > > > you allocate something, you are responsible for freeing it.Since this
> > > > is C and not Rust (where such things are tracked by the compiler), I
> > > > strongly believe we should strive to keep ownership consistent: the
> > > > driver should free resources it allocated within the bounds of the
> > > > lifetime of the device it controls. The subsystem should manage the
> > > > data it allocated - in this case the i2c adapter struct device.
> > >
> > > Drivers are responsible for dropping *their* reference, it doesn't mean
> > > that the resource is necessarily freed immediately as someone else may
> > > be holding a reference. Anyone surprised by this should not be doing
> > > kernel development.
> >
> > I disagree. For some reason, you're defending a suboptimal programming
> > interface. I'm all for reference counting but mixing reference-counted data
> > with non-counted is simply not a good idea. An API should be easy to use and
> > hard to misuse. Given the amount of issues, this approach is definitely easy
> > to misuse.
> >
> > I'm advocating for a hard split between the subsystem data (reference-counted)
> > and driver data (living from probe() until remove()). A logical struct device
> > managed entirely by the subsystem should live in a separate structure than
> > driver data and be allocated - and freed - by the subsystem module.
>
> It doesn't really matter what you think. You can't just go around
> making up new subsystem specific rules at your whim. The linux driver
> model uses reference counting and that's what developers expect to be
> used.
>

And I've never said that it should not use reference counting. I'm not sure
what you're implying here.

> > Let's put aside kernel code for a minute and work with an abstract C example,
> > where the equivalent of what you're proposing would look like this:
> >
> > struct bar {
> >	   struct foo foo;
> >	   ...
> > };
> >
> > struct bar *bar = malloc(sizeof(*bar));
> >
> > ret = foo_register(&bar->foo);
> >
> > And the corresponding free() lives who knows where because foo_register()
> > automagically introduces reference counting (nevermind the need to calculate
> > where bar is in relations to foo).
>
> No, that's not what I'm suggesting here, but it would be compatible with
> the driver model (ever heard of struct device which works exactly like
> this?).
>

I know how struct device works. I'm pointing out that this is a bad API (just
to be absolutely clear: not the reference counting of struct device itself but
using it in a way tha looks like it's not refcounted but becomes so after an
API call) because it's confusing. I'm not buying the argument that if it
confuses you then you should not be doing kernel development because it's not
the goal of API design to make it as complex and confusing as possible - quite
the contrary. And it *is* confusing given the amount of misuse present. I've
heard Greg KH say on multiple occasions during his talks that we try to offload
complex code to subsystems so that drivers can remain fairly simple. I agree
with that.

> > I strongly believe that this makes more sense:
> >
> > struct bar {
> >	   ...
> > };
> >
> > struct bar *bar = malloc();
> >
> > struct foo *foo = foo_register(bar);
> >
> > // foo is reference counted and allocated in the provider of foo_register()
> >
> > foo_put(foo);
> > free(bar);
> >
> > The equivalent of which is moving struct device out of struct i2c_adapter.
>
> No, it's not.
>
> > In fact: I would love to see i2c_adapter become a truly reference-counted
> > object detached from driver data but due to it being embedded in every bus
> > driver data structure it realistically won't happen.
>
> And this is what I've been suggesting all along, separating the driver
> data and making the adapter reference counted.
>
> The idiomatic way to handle this is:
>
>		 xyz_probe()
>		 {
>				 adap = i2c_adapter_alloc();
>				 // initialise driver data, store pointer in adap
>				 i2c_adapter_register(adap);
>		 }
>
>		 xyz_remove()
>		 {
>				 i2c_adapter_deregister(adap);
>				 i2c_adapter_put(adap);
>		 }
>
> Exactly where the driver data is stored is secondary, it could be memory
> allocated by core or by the driver.
>
> But the adapter is reference counted and kept around until all users are
> gone.
>

Yeah, that's awesome but that's not what's being done in i2c. We do:

struct foo_i2c_driver_data {
	struct i2c_adapter adap {
		struct device dev;
		...
	};
	...
};

instead which is a completely different story. It makes foo_i2c_driver_data
implicitly reference counted despite its lifetime being tied to the bound-state
of the device. It becomes painfully obvious in rust when the compiler starts
enforcing proper lifetime management.

What you showed above is totally fine. For an even simpler API, my personal
preference would be to:

xyz_probe()
{
	struct drv_data *data = kzalloc();
	/*
	 * ... stands for any other arguments or a config struct. In the concrete
	 * example of i2c, we'd supply the algo struct, fwnode, etc.
	 */
	struct i2c_adapter *adap = i2c_adapter_register(data, ...);
}

xyz_remove()
{
	kfree(data);
	i2c_adapter_unregister();
}

The reference counting of i2c_adapter happens behind the scenes in the
subsystem code. We're hiding the implementation details from the driver as it
has no business knowing it - it always only needs a single reference.

This way you have a kfree() corresponding with the kmalloc() and an
unregister() corresponding with the register().

But sure, your example works fine too. My point is: getting to that state would
require more churn than allowing drivers to continue allocating i2c_adapter
struct themselves with struct device being moved out of it - making reference
counting of it work properly.

And I agree: doing the above would be even better but you'd need - for every
driver - to move the i2c_adapter struct out of driver data and make it a
pointer. That's in addition to providing new APIs and using them. I2C drivers
are spread treewide. There's a reason why nobody attempted it for decades. I'm
proposing something a bit less complex: allow drivers to free i2c_adapter at
unbind but make i2c core keep a private, reference-counted structure for as
long as it's needed.

> > > > I know there are a lot of places where this is done in the kernel but
> > > > let's not introduce new ones. This is a bad pattern.
> > >
> > > No, it's not. It's literally the standard way of doing this.
> > >
> > > > But even if you decided this is the way to go, I fail to see how it
> > > > would be easier than what I'm trying to do. You would have to modify
> > > > *all* I2C bus drivers as opposed to only modifying those that access
> > > > the underlying struct device. Or am I missing something?
> > >
> > > Yes, you have to update the allocation and replace container_of() with
> > > dev_get_drvdata() but it's a straight-forward transformation that brings
> > > the i2c subsystem more in line with the driver model (unlike whatever it
> > > is you're trying to do).
> > >
> >
> > No, it's not that simple. The .release() callback of struct device embedded
> > in struct i2c_adapter is assigned from the bus type and only calls complete()
> > (yeah, I too don't think it looks right, one would expect to see the associated
> > kfree() here, right?). It relies on the bus driver freeing the data in its
> > remove() path. That's why we wait until all references to said struct device
> > are dropped. After your proposed change, if your new release() lives in the
> > driver module, it must not be removed until all the references are dropped
> > - basically where we are now. If on the other hand, the release() callback's
> > functionality is moved into i2c-core, how would you handle the fact i2c_adapter
> > can be embedded in a larger driver data structure? Provide yet another callback
> > in i2c_adapter called from the device's .release()? Sure, can be done but I
> > doubt it's a better solution.
>
> You seem to be constructing some kind of straw man here. Obviously, the
> release function would free the memory allocated for the adapter struct.
>
> An adapter driver can free its driver data on unbind as core will
> guarantee that there are no further callbacks after the adapter has been
> deregistered.
>

Sure, but my point all along has been that - with struct device currently
embedded in struct i2c_adapter - that's not the case. Driver data *and*
i2c_adapter are tied together. You need a major rework in either case.

I'm frustrated because I'm spending time working on an actual solution. I've
explained what I'm doing and what the end result will look like based on what
works for GPIO (struct gpio_chip's lifetime is bound to device's "bound" state,
struct gpio_device is refcounted, I want to mirror it with i2c_adapter and
whatever we eventually call its refcounted counterpart - let's say:
i2c_bus_device). If you claim you have a better alternative - will you do the
work to make it happen?

Bartosz

On Wed, Mar 04, 2026 at 01:55:14AM -0800, Bartosz Golaszewski wrote:
> On Tue, Mar 3, 2026 at 4:57 PM Johan Hovold <johan@kernel.org> wrote:
> >
> > On Mon, Mar 02, 2026 at 12:03:19PM -0600, Bartosz Golaszewski wrote:
> > > On Fri, Feb 27, 2026 at 5:41 PM Johan Hovold <johan@kernel.org> wrote:
> > > >
> > > > On Fri, Feb 27, 2026 at 04:42:09PM +0100, Bartosz Golaszewski wrote:
> > > > > On Fri, Feb 27, 2026 at 11:06 AM Johan Hovold <johan@kernel.org> wrote:
> > > >
> > > > > > It seems all that is needed is to decouple the struct i2c_adapter from
> > > > > > the driver data and have core manage the lifetime of the former using
> > > > > > the reference count of the embedded struct device.
> > > >
> > > > > This is a weird pattern you sometimes see where a driver allocates
> > > > > something and passes the ownership to the subsystem.
> > > >
> > > > It's not weird at all, this is the standard way to handle this. We have
> > > > these things called reference counts for a reason.
> > >
> > > I wouldn't say it's *the* standard way. There are at least several different
> > > ways driver subsystems handle resource ownership. And even so: the fact that
> > > something's done a lot does not make it automatically correct.
> >
> > It's the way the driver model works.
> 
> No, it does not impose any specific pattern to use for subsystems other than
> requiring each device that's been *initialized* to provide a .release() callback
> called when the last reference is dropped.

Reference counting is a core part of the driver model and this is
reflected in the way subsystems manage lifetime.

> > > I'm advocating for a hard split between the subsystem data (reference-counted)
> > > and driver data (living from probe() until remove()). A logical struct device
> > > managed entirely by the subsystem should live in a separate structure than
> > > driver data and be allocated - and freed - by the subsystem module.
> >
> > It doesn't really matter what you think. You can't just go around
> > making up new subsystem specific rules at your whim. The linux driver
> > model uses reference counting and that's what developers expect to be
> > used.
> >
> 
> And I've never said that it should not use reference counting. I'm not sure
> what you're implying here.

You have posted changes that will prevent driver from accessing the
struct device of core i2c structures. This is unexpected, non-idiomatic
and subsystem specific and therefore a bad idea.

> > > Let's put aside kernel code for a minute and work with an abstract C example,
> > > where the equivalent of what you're proposing would look like this:
> > >
> > > struct bar {
> > >	   struct foo foo;
> > >	   ...
> > > };
> > >
> > > struct bar *bar = malloc(sizeof(*bar));
> > >
> > > ret = foo_register(&bar->foo);
> > >
> > > And the corresponding free() lives who knows where because foo_register()
> > > automagically introduces reference counting (nevermind the need to calculate
> > > where bar is in relations to foo).
> >
> > No, that's not what I'm suggesting here, but it would be compatible with
> > the driver model (ever heard of struct device which works exactly like
> > this?).
> 
> I know how struct device works. I'm pointing out that this is a bad API (just
> to be absolutely clear: not the reference counting of struct device itself but
> using it in a way tha looks like it's not refcounted but becomes so after an
> API call) because it's confusing. I'm not buying the argument that if it
> confuses you then you should not be doing kernel development because it's not
> the goal of API design to make it as complex and confusing as possible - quite
> the contrary. And it *is* confusing given the amount of misuse present. I've
> heard Greg KH say on multiple occasions during his talks that we try to offload
> complex code to subsystems so that drivers can remain fairly simple. I agree
> with that.

Again, this is a core feature of the driver model. You can't just ignore
it and come up with random ways to work around just because you disagree
with design decisions that were made 25 years ago.

> > > I strongly believe that this makes more sense:
> > >
> > > struct bar {
> > >	   ...
> > > };
> > >
> > > struct bar *bar = malloc();
> > >
> > > struct foo *foo = foo_register(bar);
> > >
> > > // foo is reference counted and allocated in the provider of foo_register()
> > >
> > > foo_put(foo);
> > > free(bar);
> > >
> > > The equivalent of which is moving struct device out of struct i2c_adapter.
> >
> > No, it's not.
> >
> > > In fact: I would love to see i2c_adapter become a truly reference-counted
> > > object detached from driver data but due to it being embedded in every bus
> > > driver data structure it realistically won't happen.
> >
> > And this is what I've been suggesting all along, separating the driver
> > data and making the adapter reference counted.
> >
> > The idiomatic way to handle this is:
> >
> >		 xyz_probe()
> >		 {
> >				 adap = i2c_adapter_alloc();
> >				 // initialise driver data, store pointer in adap
> >				 i2c_adapter_register(adap);
> >		 }
> >
> >		 xyz_remove()
> >		 {
> >				 i2c_adapter_deregister(adap);
> >				 i2c_adapter_put(adap);
> >		 }
> >
> > Exactly where the driver data is stored is secondary, it could be memory
> > allocated by core or by the driver.
> >
> > But the adapter is reference counted and kept around until all users are
> > gone.
> 
> Yeah, that's awesome but that's not what's being done in i2c. We do:
> 
> struct foo_i2c_driver_data {
> 	struct i2c_adapter adap {
> 		struct device dev;
> 		...
> 	};
> 	...
> };
> 
> instead which is a completely different story. It makes foo_i2c_driver_data
> implicitly reference counted despite its lifetime being tied to the bound-state
> of the device. It becomes painfully obvious in rust when the compiler starts
> enforcing proper lifetime management.

I'm quite aware of that and that's why we are discussing how to change
it.

> But sure, your example works fine too. My point is: getting to that state would
> require more churn than allowing drivers to continue allocating i2c_adapter
> struct themselves with struct device being moved out of it - making reference
> counting of it work properly.
> 
> And I agree: doing the above would be even better but you'd need - for every
> driver - to move the i2c_adapter struct out of driver data and make it a
> pointer. That's in addition to providing new APIs and using them. I2C drivers
> are spread treewide. There's a reason why nobody attempted it for decades. I'm
> proposing something a bit less complex: allow drivers to free i2c_adapter at
> unbind but make i2c core keep a private, reference-counted structure for as
> long as it's needed.

But its non-idiomatic and therefore not a good idea. Sometimes you just
have to dig in and fix the real problem instead of trying to work around
it.

> > > > Yes, you have to update the allocation and replace container_of() with
> > > > dev_get_drvdata() but it's a straight-forward transformation that brings
> > > > the i2c subsystem more in line with the driver model (unlike whatever it
> > > > is you're trying to do).
> > > >
> > >
> > > No, it's not that simple. The .release() callback of struct device embedded
> > > in struct i2c_adapter is assigned from the bus type and only calls complete()
> > > (yeah, I too don't think it looks right, one would expect to see the associated
> > > kfree() here, right?). It relies on the bus driver freeing the data in its
> > > remove() path. That's why we wait until all references to said struct device
> > > are dropped. After your proposed change, if your new release() lives in the
> > > driver module, it must not be removed until all the references are dropped
> > > - basically where we are now. If on the other hand, the release() callback's
> > > functionality is moved into i2c-core, how would you handle the fact i2c_adapter
> > > can be embedded in a larger driver data structure? Provide yet another callback
> > > in i2c_adapter called from the device's .release()? Sure, can be done but I
> > > doubt it's a better solution.
> >
> > You seem to be constructing some kind of straw man here. Obviously, the
> > release function would free the memory allocated for the adapter struct.
> >
> > An adapter driver can free its driver data on unbind as core will
> > guarantee that there are no further callbacks after the adapter has been
> > deregistered.
> >
> 
> Sure, but my point all along has been that - with struct device currently
> embedded in struct i2c_adapter - that's not the case. Driver data *and*
> i2c_adapter are tied together. You need a major rework in either case.

And I've being saying that the driver data should be *decoupled* from
the i2c_adapter.

> I'm frustrated because I'm spending time working on an actual solution. I've
> explained what I'm doing and what the end result will look like based on what
> works for GPIO (struct gpio_chip's lifetime is bound to device's "bound" state,
> struct gpio_device is refcounted, I want to mirror it with i2c_adapter and
> whatever we eventually call its refcounted counterpart - let's say:
> i2c_bus_device). If you claim you have a better alternative - will you do the
> work to make it happen?

Sure. I'll fix this properly.

Johan

On Fri, Mar 6, 2026 at 4:39 PM Johan Hovold <johan@kernel.org> wrote:
>
> On Wed, Mar 04, 2026 at 01:55:14AM -0800, Bartosz Golaszewski wrote:
> > On Tue, Mar 3, 2026 at 4:57 PM Johan Hovold <johan@kernel.org> wrote:
> > >
> > > On Mon, Mar 02, 2026 at 12:03:19PM -0600, Bartosz Golaszewski wrote:
> > > > On Fri, Feb 27, 2026 at 5:41 PM Johan Hovold <johan@kernel.org> wrote:
> > > > >
> > > > > On Fri, Feb 27, 2026 at 04:42:09PM +0100, Bartosz Golaszewski wrote:
> > > > > > On Fri, Feb 27, 2026 at 11:06 AM Johan Hovold <johan@kernel.org> wrote:
> > > > >
> > > > > > > It seems all that is needed is to decouple the struct i2c_adapter from
> > > > > > > the driver data and have core manage the lifetime of the former using
> > > > > > > the reference count of the embedded struct device.
> > > > >
> > > > > > This is a weird pattern you sometimes see where a driver allocates
> > > > > > something and passes the ownership to the subsystem.
> > > > >
> > > > > It's not weird at all, this is the standard way to handle this. We have
> > > > > these things called reference counts for a reason.
> > > >
> > > > I wouldn't say it's *the* standard way. There are at least several different
> > > > ways driver subsystems handle resource ownership. And even so: the fact that
> > > > something's done a lot does not make it automatically correct.
> > >
> > > It's the way the driver model works.
> >
> > No, it does not impose any specific pattern to use for subsystems other than
> > requiring each device that's been *initialized* to provide a .release() callback
> > called when the last reference is dropped.
>
> Reference counting is a core part of the driver model and this is
> reflected in the way subsystems manage lifetime.
>

Seems like we've reached an agreement and can stop arguing but you
make it sound here like I'm somehow against reference counting. I've
never said anything like that and here, I just explained how reference
counting works and what it imposes on users.

> > > > I'm advocating for a hard split between the subsystem data (reference-counted)
> > > > and driver data (living from probe() until remove()). A logical struct device
> > > > managed entirely by the subsystem should live in a separate structure than
> > > > driver data and be allocated - and freed - by the subsystem module.
> > >
> > > It doesn't really matter what you think. You can't just go around
> > > making up new subsystem specific rules at your whim. The linux driver
> > > model uses reference counting and that's what developers expect to be
> > > used.
> > >
> >
> > And I've never said that it should not use reference counting. I'm not sure
> > what you're implying here.
>
> You have posted changes that will prevent driver from accessing the
> struct device of core i2c structures. This is unexpected, non-idiomatic
> and subsystem specific and therefore a bad idea.
>

That's not true, the changes provide a helper to that end.

> > > > Let's put aside kernel code for a minute and work with an abstract C example,
> > > > where the equivalent of what you're proposing would look like this:
> > > >
> > > > struct bar {
> > > >      struct foo foo;
> > > >      ...
> > > > };
> > > >
> > > > struct bar *bar = malloc(sizeof(*bar));
> > > >
> > > > ret = foo_register(&bar->foo);
> > > >
> > > > And the corresponding free() lives who knows where because foo_register()
> > > > automagically introduces reference counting (nevermind the need to calculate
> > > > where bar is in relations to foo).
> > >
> > > No, that's not what I'm suggesting here, but it would be compatible with
> > > the driver model (ever heard of struct device which works exactly like
> > > this?).
> >
> > I know how struct device works. I'm pointing out that this is a bad API (just
> > to be absolutely clear: not the reference counting of struct device itself but
> > using it in a way tha looks like it's not refcounted but becomes so after an
> > API call) because it's confusing. I'm not buying the argument that if it
> > confuses you then you should not be doing kernel development because it's not
> > the goal of API design to make it as complex and confusing as possible - quite
> > the contrary. And it *is* confusing given the amount of misuse present. I've
> > heard Greg KH say on multiple occasions during his talks that we try to offload
> > complex code to subsystems so that drivers can remain fairly simple. I agree
> > with that.
>
> Again, this is a core feature of the driver model. You can't just ignore
> it and come up with random ways to work around just because you disagree
> with design decisions that were made 25 years ago.
>

It absolutely *can* be done differently. There's nothing that imposes
a certain API design on susbsystems. If you design the subsystem code
well, provider drivers don't need more than one reference (taken in
probe(), released in remove(), for instance via the
register()/unregister() pair) so the counting can be hidden within the
subsystems that control them.

Bartosz

On Fri, Mar 06, 2026 at 06:34:43PM +0100, Bartosz Golaszewski wrote:
> On Fri, Mar 6, 2026 at 4:39 PM Johan Hovold <johan@kernel.org> wrote:

> > You have posted changes that will prevent driver from accessing the
> > struct device of core i2c structures. This is unexpected, non-idiomatic
> > and subsystem specific and therefore a bad idea.
> 
> That's not true, the changes provide a helper to that end.

That was supposed to say "prevent drivers from accessing the struct
device *directly*".

> > Again, this is a core feature of the driver model. You can't just ignore
> > it and come up with random ways to work around just because you disagree
> > with design decisions that were made 25 years ago.
> 
> It absolutely *can* be done differently. There's nothing that imposes
> a certain API design on susbsystems. If you design the subsystem code
> well, provider drivers don't need more than one reference (taken in
> probe(), released in remove(), for instance via the
> register()/unregister() pair) so the counting can be hidden within the
> subsystems that control them.

Yes, there is nothing preventing you from diverting from the idiomatic
way of doing things. But my point is that that's not a good idea.

Johan

On Mon, Mar 9, 2026 at 11:31 AM Johan Hovold <johan@kernel.org> wrote:
>
> On Fri, Mar 06, 2026 at 06:34:43PM +0100, Bartosz Golaszewski wrote:
> > On Fri, Mar 6, 2026 at 4:39 PM Johan Hovold <johan@kernel.org> wrote:
>
> > > You have posted changes that will prevent driver from accessing the
> > > struct device of core i2c structures. This is unexpected, non-idiomatic
> > > and subsystem specific and therefore a bad idea.
> >
> > That's not true, the changes provide a helper to that end.
>
> That was supposed to say "prevent drivers from accessing the struct
> device *directly*".
>
> > > Again, this is a core feature of the driver model. You can't just ignore
> > > it and come up with random ways to work around just because you disagree
> > > with design decisions that were made 25 years ago.
> >
> > It absolutely *can* be done differently. There's nothing that imposes
> > a certain API design on susbsystems. If you design the subsystem code
> > well, provider drivers don't need more than one reference (taken in
> > probe(), released in remove(), for instance via the
> > register()/unregister() pair) so the counting can be hidden within the
> > subsystems that control them.
>
> Yes, there is nothing preventing you from diverting from the idiomatic
> way of doing things. But my point is that that's not a good idea.
>

"Idiomatic" is a just buzz-word. I don't know why you insist on it
being the only "correct" way. People have been doing all kinds of
driver data management for a long time. You recently looked at my
series for nvmem - did you see that nvmem_register() only takes a
config struct (which may be a stack variable in probe() for all it
cares) and copies all the data it needs into refcounted struct
nvmem_device that the subsystem allocates and manages?

An nvmem provider driver only has to do
nvmem_register()/nvmem_unregister() and, while it can access the
internal struct device, it never has to in practice.

There's no:
  nvmem_alloc()
  nvmem_register()
  nvmem_unregister()
  nvmem_put()

I don't see why we wouldn't do the same in i2c:

  struct i2c_adapter_config cfg = { ... /* dev id, driver data,
whatever... */ };
  adap = i2c_adapter_register(&cfg);
  i2c_adapter_unregister(adap);

I understand that you doing the work will give you some discretion as
to the implementation details but I'm asking you to at least consider
this as a viable solution when used by others elsewhere. While you
have criticised my proposal for i2c rework - fair enough - I don't
think you have ever given an actual argument against this simpler
register/unregister-sans-alloc-and-put pattern other than it not being
"idiomatic" which is honestly quite vague.

Bart

On Tue, Mar 10, 2026 at 10:28:17AM +0100, Bartosz Golaszewski wrote:
> On Mon, Mar 9, 2026 at 11:31 AM Johan Hovold <johan@kernel.org> wrote:
> >
> > On Fri, Mar 06, 2026 at 06:34:43PM +0100, Bartosz Golaszewski wrote:
> > > On Fri, Mar 6, 2026 at 4:39 PM Johan Hovold <johan@kernel.org> wrote:
> >
> > > > You have posted changes that will prevent driver from accessing the
> > > > struct device of core i2c structures. This is unexpected, non-idiomatic
> > > > and subsystem specific and therefore a bad idea.
> > >
> > > That's not true, the changes provide a helper to that end.
> >
> > That was supposed to say "prevent drivers from accessing the struct
> > device *directly*".
> >
> > > > Again, this is a core feature of the driver model. You can't just ignore
> > > > it and come up with random ways to work around just because you disagree
> > > > with design decisions that were made 25 years ago.
> > >
> > > It absolutely *can* be done differently. There's nothing that imposes
> > > a certain API design on susbsystems. If you design the subsystem code
> > > well, provider drivers don't need more than one reference (taken in
> > > probe(), released in remove(), for instance via the
> > > register()/unregister() pair) so the counting can be hidden within the
> > > subsystems that control them.
> >
> > Yes, there is nothing preventing you from diverting from the idiomatic
> > way of doing things. But my point is that that's not a good idea.
> 
> "Idiomatic" is a just buzz-word.

No, it has a meaning.

> I don't know why you insist on it
> being the only "correct" way. People have been doing all kinds of
> driver data management for a long time.

Yes, subsystems do things differently, and unfortunately also gets
things wrong occasionally.

By separating allocation, registration, deregistration and put you can
avoid some of the mistakes that can result from combining these
operations.

> You recently looked at my
> series for nvmem - did you see that nvmem_register() only takes a
> config struct (which may be a stack variable in probe() for all it
> cares) and copies all the data it needs into refcounted struct
> nvmem_device that the subsystem allocates and manages?
> An nvmem provider driver only has to do
> nvmem_register()/nvmem_unregister() and, while it can access the
> internal struct device, it never has to in practice.

nit: It looks like drivers can't access the internal struct device
currently.
 
> There's no:
>   nvmem_alloc()
>   nvmem_register()
>   nvmem_unregister()
>   nvmem_put()
> 
> I don't see why we wouldn't do the same in i2c:
> 
>   struct i2c_adapter_config cfg = { ... /* dev id, driver data,
> whatever... */ };
>   adap = i2c_adapter_register(&cfg);
>   i2c_adapter_unregister(adap);

Because it's unnecessary, would amount to a lot of churn, and has no
clear benefits. We already have an adapter object, it just needs to
refcounted.

Johan

Hi Bart, hi Johan,

> And I agree: doing the above would be even better but you'd need - for every
> driver - to move the i2c_adapter struct out of driver data and make it a
> pointer. That's in addition to providing new APIs and using them. I2C drivers
> are spread treewide. There's a reason why nobody attempted it for decades. I'm
> proposing something a bit less complex: allow drivers to free i2c_adapter at
> unbind but make i2c core keep a private, reference-counted structure for as
> long as it's needed.

I am still with Bart, the above paragraph sums it up extremly well IMO.
I also recall that the outcome of the Plumbers session 2024 was "go for
it!". Nobody said the approach would be "fighting" the driver model.
There were a lot of experienced developers in the room.

> I'm frustrated because I'm spending time working on an actual solution. I've
> explained what I'm doing and what the end result will look like based on what
> works for GPIO (struct gpio_chip's lifetime is bound to device's "bound" state,
> struct gpio_device is refcounted, I want to mirror it with i2c_adapter and
> whatever we eventually call its refcounted counterpart - let's say:
> i2c_bus_device).

I am super-happy and thankful that Bart volunteers to spend all this
time on fixing this decade old problem. I know this alone is not a
reason to accept a technically bad solution. But I think it isn't. I
think it is a viable approach to keep the churn and potential
regressions lower than a theoretically ideal solution which is nobody to
do anyways because you'd need to refactor drivers from the 90s in a
quite intrusive way.

All the best,

   Wolfram

On Wed, Mar 04, 2026 at 12:07:39PM +0100, Wolfram Sang wrote:
> Hi Bart, hi Johan,
> 
> > And I agree: doing the above would be even better but you'd need - for every
> > driver - to move the i2c_adapter struct out of driver data and make it a
> > pointer. That's in addition to providing new APIs and using them. I2C drivers
> > are spread treewide. There's a reason why nobody attempted it for decades. I'm
> > proposing something a bit less complex: allow drivers to free i2c_adapter at
> > unbind but make i2c core keep a private, reference-counted structure for as
> > long as it's needed.
> 
> I am still with Bart, the above paragraph sums it up extremly well IMO.
> I also recall that the outcome of the Plumbers session 2024 was "go for
> it!". Nobody said the approach would be "fighting" the driver model.
> There were a lot of experienced developers in the room.

I don't know what was said a conference some years ago or whether there
was any misunderstanding on either side. What matters is what was
posted.

> > I'm frustrated because I'm spending time working on an actual solution. I've
> > explained what I'm doing and what the end result will look like based on what
> > works for GPIO (struct gpio_chip's lifetime is bound to device's "bound" state,
> > struct gpio_device is refcounted, I want to mirror it with i2c_adapter and
> > whatever we eventually call its refcounted counterpart - let's say:
> > i2c_bus_device).
> 
> I am super-happy and thankful that Bart volunteers to spend all this
> time on fixing this decade old problem. I know this alone is not a
> reason to accept a technically bad solution. But I think it isn't. I
> think it is a viable approach to keep the churn and potential
> regressions lower than a theoretically ideal solution which is nobody to
> do anyways because you'd need to refactor drivers from the 90s in a
> quite intrusive way.

We've done bigger refactoring than this, and after a looking a this a
bit further today, I don't think it's going to be that intrusive at all.

Bartosz seems to agree that my suggestion to decouple the driver data
from the i2c_adapter would be better, and I'm willing to do the job.

Johan

Hi Johan,

> I don't know what was said a conference some years ago or whether there
> was any misunderstanding on either side. What matters is what was
> posted.

What was posted was in accordance with what was discussed at said
conference. The people in the room (including gkh) were OK with the
compromise solution. If you are not and willing to provide a better
solution...

> Bartosz seems to agree that my suggestion to decouple the driver data
> from the i2c_adapter would be better, and I'm willing to do the job.

... you get all my support.

Thanks and happy hacking,

   Wolfram

On Fri, Mar 6, 2026 at 4:50 PM Johan Hovold <johan@kernel.org> wrote:
>
> Bartosz seems to agree that my suggestion to decouple the driver data
> from the i2c_adapter would be better, and I'm willing to do the job.
>

Fair enough, I'll leave this for a couple of months then.

Bartosz