Re: [PATCH v5 0/4] rust: miscdevice: Provide sample driver using the new MiscDevice bindings
From: Lee Jones
Date: Fri Dec 06 2024 - 08:15:10 EST
On Fri, 06 Dec 2024, Danilo Krummrich wrote:
> On Fri, Dec 06, 2024 at 12:54:30PM +0000, Lee Jones wrote:
> > On Fri, 06 Dec 2024, Danilo Krummrich wrote:
> >
> > > On Fri, Dec 06, 2024 at 12:42:11PM +0000, Lee Jones wrote:
> > > > It has been suggested that the driver should use dev_info() instead of
> > > > pr_info() however there is currently no scaffolding to successfully pull
> > > > a 'struct device' out from driver data post register(). This is being
> > > > worked on and we will convert this over in due course.
> > >
> > > I think you're going too fast with this series.
> > >
> > > Please address the comments you receive before sending out new versions.
> > >
> > > Also, please document the changes you have made from one version to the next,
> > > otherwise it's gonna be very hard to review this.
> >
> > I can add a change log.
> >
> > What comments were missed?
>
> I think MiscDevice should ideally use the generic `Registration` type from [1].
How can an in-tree driver use out-of-tree functionality?
> I see that you use `InPlaceModule` now, which is fine. But since this is just a
> sample, we could probably afford to wait until the generic type lands.
>
> Also, there was a comment about how we can make use of the `dev_*` macros.
>
> I really think we should fix those before we land a sample driver. It's gonna
> be hard to explain people later on that they shouldn't do what the example
> does...
We're authoring the sample based on what is available at the moment.
There will always be something better / more convenient coming down the
pipe. We don't usually put off contributors pending acceptance of
out-of-tree functionality, sample or otherwise.
As I've already mentioned, I'd be _more than_ happy to keep improving
this over time as new and improved helpers / infra. arrives.
> [1] https://lore.kernel.org/rust-for-linux/20241205141533.111830-3-dakr@xxxxxxxxxx/
--
Lee Jones [李琼斯]