Re: [PATCH v5 4/7] can: Kconfig: add CONFIG_CAN_RX_OFFLOAD

[Max Staudt]

On Tue, 7 Jun 2022 15:06:14 -0700
Jakub Kicinski <kuba@xxxxxxxxxx> wrote:

> On Tue, 7 Jun 2022 18:22:16 +0200 Max Staudt wrote:
> > > Honestly, I am totally happy to have the "default y" tag, the "if
> > > unsure, say Y" comment and the "select CAN_RX_OFFLOAD" all
> > > together.
> > > 
> > > Unless I am violating some kind of best practices, I prefer to
> > > keep it as-is. Hope this makes sense.    
> 
> AFAIU Linus likes for everything that results in code being added to
> the kernel to default to n. If the drivers hard-select that Kconfig
> why bother user with the question at all? My understanding is that
> Linus also likes to keep Kconfig as simple as possible.
> 
> > I wholeheartedly agree with Vincent's decision.
> > 
> > One example case would be users of my can327 driver, as long as it
> > is not upstream yet. They need to have RX_OFFLOAD built into their
> > distribution's can_dev.ko, otherwise they will have no choice but to
> > build their own kernel.  
> 
> Upstream mentioning out-of-tree modules may have the opposite effect 
> to what you intend :( Forgive my ignorance, what's the reason to keep
> the driver out of tree?

None, it's being upstreamed. But even with the best of intentions, it
has been in this process for a long time, and it's still going on!

For some reason, upstream tends to forget about everything that is not
upstream *yet*. I've also convinced Greg K-H to include the
N_DEVELOPMENT ldisc number for this very reason: To allow new drivers
(ldiscs in this case) to be developed comfortably out-of-tree before
they are upstreamed (and then assigned their own ldisc number).

It seems strange to me to magically build some extra features into
can_dev.ko, depending on whether some other .ko files are built in that
very same moment, or not. By "magically", I mean an invisible Kconfig
option. This is why I think Vincent's approach is best here, by making
the drivers a clearly visible subset of the RX_OFFLOAD option in
Kconfig, and RX_OFFLOAD user-selectable.


How about making RX_OFFLOAD a separate .ko file, so we don't have
various possible versions of can_dev.ko?

@Vincent, I think you suggested that some time ago, IIRC?

(I know, I was against a ton of little modules, but I'm changing my
ways here now since it seems to help...)



Max