Re: [PATCH 1/2] rust: auxiliary: add registration data to auxiliary devices

[Gary Guo]

On Thu Apr 30, 2026 at 9:59 AM BST, Alice Ryhl wrote:
> On Tue, Apr 28, 2026 at 12:09:40AM +0200, Danilo Krummrich wrote:
>> Add a registration_data pointer to struct auxiliary_device, allowing the
>> registering (parent) driver to attach private data to the device at
>> registration time and retrieve it later when called back by the
>> auxiliary (child) driver.
>> 
>> By tying the data to the device's registration, Rust drivers can bind
>> the lifetime of device resources to it, since the auxiliary bus
>> guarantees that the parent driver remains bound while the auxiliary
>> device is bound.
>> 
>> On the Rust side, Registration<T> takes ownership of the data via
>> ForeignOwnable. A TypeId is stored alongside the data for runtime type
>> checking, making Device::registration_data<T>() a safe method.
>> 
>> Signed-off-by: Danilo Krummrich <dakr@xxxxxxxxxx>
>
> So overall I think this patch makes sense. A few comments below.
>
>> diff --git a/include/linux/auxiliary_bus.h b/include/linux/auxiliary_bus.h
>> index bc09b55e3682..4e1ad8ccbcdd 100644
>> --- a/include/linux/auxiliary_bus.h
>> +++ b/include/linux/auxiliary_bus.h
>> @@ -62,6 +62,9 @@
>>   * @sysfs.irqs: irqs xarray contains irq indices which are used by the device,
>>   * @sysfs.lock: Synchronize irq sysfs creation,
>>   * @sysfs.irq_dir_exists: whether "irqs" directory exists,
>> + * @registration_data_rust: private data owned by the registering (parent)
>> + *                          driver; valid for as long as the device is
>> + *                          registered with the driver core,
>>   *
>>   * An auxiliary_device represents a part of its parent device's functionality.
>>   * It is given a name that, combined with the registering drivers
>> @@ -148,6 +151,7 @@ struct auxiliary_device {
>>  		struct mutex lock; /* Synchronize irq sysfs creation */
>>  		bool irq_dir_exists;
>>  	} sysfs;
>> +	void *registration_data_rust;
>
> Is this really Rust-specific? Would you not want C drivers with the same
> pattern to do the same thing?
>
>> +        // SAFETY: `ptr` is non-null and was set via `into_foreign()` in `Registration::new()`;
>> +        // `RegistrationData` is `#[repr(C)]` with `type_id` at offset 0, so reading a `TypeId`
>> +        // at the start of the allocation is valid regardless of `T`.
>> +        let type_id = unsafe { ptr.cast::<TypeId>().read() };
>> +        if type_id != TypeId::of::<T>() {
>> +            return Err(EINVAL);
>> +        }
>
> Right, okay, so if you put C stuff there, we need the layout to be
> compatible with Rust type ids.
>
> Still, we could have Rust expose a couple methods to allow C code to use
> the same field with a null type id.
>
> But I guess this is all future work.
>
>> +        let data = KBox::pin_init::<Error>(
>> +            try_pin_init!(RegistrationData {
>> +                type_id: TypeId::of::<T>(),
>> +                data <- data,
>> +            }),
>> +            GFP_KERNEL,
>> +        )?;
>> +
>> +        let boxed = KBox::new(Opaque::<bindings::auxiliary_device>::zeroed(), GFP_KERNEL)?;
>
> Use __GFP_ZERO here instead?

On thing I wanted to implement in pin-init is a way to hint whether a type
prefers uninit or zero-init before initializing. Something like

    pub unsafe trait PinInit<T: ?Sized, E = Infallible>: Sized {
        const PREFER_ZERO_INIT: bool = false;

        unsafe fn __pinned_init(self, slot: *mut T) -> Result<(), E>;

        // Extra safety requirement that `slot` is zero-filled.
        unsafe fn __pinned_init_from_zero(self, slot: *mut T) -> Result<(), E> {
            self.__pinned_init(slot)
        }
    }

Then the `pin_init` macro, if it detects that `..Zeroable::zeroed()` is used,
can generate a impl where `PREFER_ZERO_INIT` is set to `true`, `__pinned_init`
that does a memset + `__pinned_init_from_zero`.

Then we can have `KBox::pin_init` checks `PREFER_ZERO_INIT`. If true, it adds a
`__GFP_ZERO` flag and invoke the `__pinned_init_from_zero` method, other wise it
uses the `__pinned_init` method without the __GFP_ZERO flag.

I've been tempting to this for a while now, but I prioritize landing the
self-referential feature before this one.

Best,
Gary