Re: [PATCH v2 00/13] i2c: add and start using i2c_adapter-specific printk helpers

[Johan Hovold <johan@kernel.org>]

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