Re: [PATCH v2] rust/kernel: Add faux device bindings

[Greg Kroah-Hartman]

On Fri, Feb 07, 2025 at 01:17:45PM +0100, Danilo Krummrich wrote:
> On Fri, Feb 07, 2025 at 10:25:09AM +0100, Greg Kroah-Hartman wrote:
> > On Thu, Feb 06, 2025 at 07:40:45PM -0500, Lyude Paul wrote:
> > > +
> > > +/// The registration of a faux device.
> > > +///
> > > +/// This type represents the registration of a [`struct faux_device`]. When an instance of this type
> > > +/// is dropped, its respective faux device will be unregistered from the system.
> > > +///
> > > +/// # Invariants
> > > +///
> > > +/// `self.0` always holds a valid pointer to an initialized and registered [`struct faux_device`].
> > > +///
> > > +/// [`struct faux_device`]: srctree/include/linux/device/faux.h
> > > +#[repr(transparent)]
> > > +pub struct Registration(NonNull<bindings::faux_device>);
> > > +
> > > +impl Registration {
> > > +    /// Create and register a new faux device with the given name.
> > > +    pub fn new(name: &CStr) -> Result<Self> {
> > > +        // SAFETY:
> > > +        // - `name` is copied by this function into its own storage
> > > +        // - `faux_ops` is safe to leave NULL according to the C API
> > > +        let dev = unsafe { bindings::faux_device_create(name.as_char_ptr(), null()) };
> > 
> > I'm fine with null() here, but why wouldn't a rust binding want to allow
> > this?  What's unique here that make it this way, or is it just that the
> > current users you are thinking of don't care about it?
> 
> This is what I meant when I mentioned allowing NULL for the faux_device_ops
> allows us to simplify the Rust abstraction quite a bit.
> 
> Having probe() and remove() doesn't do a lot for us in this case in Rust other
> than needing a separate faux::Driver trait with a corresponding faux::Adapter
> implementation to handle those callbacks. It'd be an unnecessary indirection.
> 
> Do you see any advantage going through probe()?

Only reason I implemented it was that we have a real user of it in the
kernel that needed it, the regulator dummy driver.  Odds are we could
rewrite the C code to not need it, but for now let's leave it.

If over time, no one else actually needs it, I'll refactor and remove
the callbacks entirely after more of the tree is moved to the faux code.

If you all don't need the callback, wonderful, I'll not complain :)

thanks,

greg k-h