Re: [PATCH v2 5/6] tpm: add driver for cr50 on SPI

[Guenter Roeck]

On Wed, Jul 17, 2019 at 11:21 AM Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote:
>
> Quoting Jason Gunthorpe (2019-07-17 10:25:44)
> > On Wed, Jul 17, 2019 at 10:22:20AM -0700, Stephen Boyd wrote:
> > > Quoting Jason Gunthorpe (2019-07-17 10:12:16)
> > > > On Wed, Jul 17, 2019 at 10:05:52AM -0700, Stephen Boyd wrote:
> > > > >
> > > > > Yes. The space savings comes from having the extra module 'cr50.ko' that
> > > > > holds almost nothing at all when the two drivers are modules.
> > > >
> > > > I'm not sure it is an actual savings, there is alot of minimum
> > > > overhead and alignment to have a module in the first place.
> > > >
> > >
> > > Yeah. I'm pretty sure that's why it's a bool and not a tristate for this
> > > symbol. A module has overhead that is not necessary for these little
> > > helpers.
> >
> > Linking driver stuff like that to the kernel is pretty hacky, IMHO
> >
>
> So combine lines?
>
>         obj-$(CONFIG_...) += cr50.o cr50_spi.o
>
> Sounds great.
>

Please keep in mind that cr50.c exports symbols. If cr50.o is added to
two modules, those symbols will subsequently available from both
modules. To avoid that, you might want to consider removing the
EXPORT_SYMBOL() declarations from cr50.c.

I don't know what happens if those two modules are both built into the
kernel (as happens, for example, with allyesconfig). Does the linker
try to load cr50.o twice, resulting in duplicate symbols ?

Guenter