Re: [PATCH 0/8] Device: generic accessors for drvdata + Driver::unbind()

[Greg KH <gregkh@linuxfoundation.org>]

On Sat, Jun 21, 2025 at 09:43:26PM +0200, Danilo Krummrich wrote:
> This patch series consists of the following three parts.
> 
>   1. Introduce the 'Internal' device context (semantically identical to the
>      'Core' device context), but only accessible for bus abstractions.
> 
>   2. Introduce generic accessors for a device's driver_data  pointer. Those are
>      implemented for the 'Internal' device context only, in order to only enable
>      bus abstractions to mess with the driver_data pointer of struct device.
> 
>   3. Implement the Driver::unbind() callback (details below).
> 
> Driver::unbind()
> ----------------
> 
> Currently, there's really only one core callback for drivers, which is
> probe().
> 
> Now, this isn't entirely true, since there is also the drop() callback of
> the driver type (serving as the driver's private data), which is returned
> by probe() and is dropped in remove().
> 
> On the C side remove() mainly serves two purposes:
> 
>   (1) Tear down the device that is operated by the driver, e.g. call bus
>       specific functions, write I/O memory to reset the device, etc.
> 
>   (2) Release the resources that have been allocated by a driver for a
>       specific device.
> 
> The drop() callback mentioned above is intended to cover (2) as the Rust
> idiomatic way.
> 
> However, it is partially insufficient and inefficient to cover (1)
> properly, since drop() can't be called with additional arguments, such as
> the reference to the corresponding device that has the correct device
> context, i.e. the Core device context.

I'm missing something, why doesn't drop() have access to the device
itself, which has the Core device context?  It's the same "object",
right?

> This makes it inefficient (but not impossible) to access device
> resources, e.g. to write device registers, and impossible to call device
> methods, which are only accessible under the Core device context.
> 
> In order to solve this, add an additional callback for (1), which we
> call unbind().
> 
> The reason for calling it unbind() is that, unlike remove(), it is *only*
> meant to be used to perform teardown operations on the device (1), but
> *not* to release resources (2).

Ick.  I get the idea, but unbind() is going to get confusing fast.
Determining what is, and is not, a "resource" is going to be hard over
time.  In fact, how would you define it?  :)

Is "teardown" only allowed to write to resources, but not free them?  If
so, why can't that happen in drop() as the resources are available there.

I'm loath to have a 2-step destroy system here for rust only, and not
for C, as maintaining this over time is going to be rough.

thanks,

greg k-h