Re: [PATCH v3 15/16] rust: platform: add basic platform device / driver abstractions

From: Danilo Krummrich
Date: Mon Oct 28 2024 - 06:19:17 EST


On Thu, Oct 24, 2024 at 11:11:50AM +0200, Dirk Behme wrote:
> > +/// IdTable type for platform drivers.
> > +pub type IdTable<T> = &'static dyn kernel::device_id::IdTable<of::DeviceId, T>;
> > +
> > +/// The platform driver trait.
> > +///
> > +/// # Example
> > +///
> > +///```
> > +/// # use kernel::{bindings, c_str, of, platform};
> > +///
> > +/// struct MyDriver;
> > +///
> > +/// kernel::of_device_table!(
> > +/// OF_TABLE,
> > +/// MODULE_OF_TABLE,
>
> It looks to me that OF_TABLE and MODULE_OF_TABLE are quite generic names
> used here. Shouldn't they be somehow driver specific, e.g. OF_TABLE_MYDRIVER
> and MODULE_OF_TABLE_MYDRIVER or whatever? Same for the other
> examples/samples in this patch series. Found that while using the *same*
> somewhere else ;)

I think the names by themselves are fine. They're local to the module. However,
we stringify `OF_TABLE` in `module_device_table` to build the export name, i.e.
"__mod_of__OF_TABLE_device_table". Hence the potential duplicate symbols.

I think we somehow need to build the module name into the symbol name as well.

>
> Best regards
>
> Dirk
>
>