Re: [PATCH v3 1/2] rust: miscdevice: add additional data to MiscDeviceRegistration

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Sat, May 17, 2025 at 03:42:29PM +0200, Danilo Krummrich wrote:
> On Sat, May 17, 2025 at 01:33:49PM +0200, Christian Schrefl wrote:
> > +pub struct MiscDeviceRegistration<T: MiscDevice> {
> >      #[pin]
> >      inner: Opaque<bindings::miscdevice>,
> > +    #[pin]
> > +    data: UnsafePinned<T::RegistrationData>,
> >      _t: PhantomData<T>,
> >  }
> 
> I recommend not to store data within a Registration type itself.
> 
> I know that this is designed with the focus on using misc device directly from
> the module scope; and in this context it works great.
> 
> However, it becomes quite suboptimal when used from a driver scope. For
> instance, if the misc device is registered within a platform driver's probe()
> function.
> 
> I know this probably isn't supported yet. At least, I assume it isn't supported
> "officially", given that the abstraction does not provide an option to set a
> parent device. Yet I think we should consider it.

It's going to be a requirement to properly set the parent device, and
as you point out, this really should be in some sort of scope, not just
a module.

But, we have two types of users of a misc device, one like this is
written, for a module-scope, and one for the "normal" device scope.  The
device scope is going to be tricker as it can, and will, get
disconnected from the device separately from the misc device lifespan,
so when that logic is added, it's going to be tricky as you point out.

So I'll take this now, but in the future this is going to have to be
cleaned up and modified.

thanks,

greg k-h