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

[Jakub Kicinski]

On Wed, 8 Jun 2022 08:40:19 +0900 Vincent MAILHOL wrote:
> On Wed. 8 Jun 2022 à 07:06, Jakub Kicinski <kuba@xxxxxxxxxx> wrote:
> > AFAIU Linus likes for everything that results in code being added to
> > the kernel to default to n.  
> 
> A "make defconfig" would not select CONFIG_CAN (on which
> CAN_RX_OFFLOAD indirectly depends) and so by default this code is not
> added to the kernel.
> 
> > 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 do not think that this is so convoluted. What would bother me is
> that RX offload is not a new feature. Before this series, RX offload
> is built-in the can-dev.o by default. If this new CAN_RX_OFFLOAD does
> not default to yes, then the default features built-in can-dev.o would
> change before and after this series.
> 
> But you being one of the maintainers, if you insist I will go in your
> direction. So will removing the "default yes" and the comment "If
> unsure, say yes" from the CAN_RX_OFFLOAD satisfy you?

I'm mostly trying to make sure Linus won't complain and block the entire
net-next PR. Unfortunately I don't think the rules are written down
anywhere.

I could well be missing some CAN-specific context here but I see no
practical benefit to exposing a knob for enabling driver framework and
then selecting that framework in drivers as well. The only beneficiary
I can think of is out-of-tree code.

If the framework is optional (covers only parts of the driver's
functionality) we make the knob configurable and drivers should work
with or without it (e.g. PTP).

If the framework is important / fundamental - hide it completely from 
the user / Kconfig and have the drivers 'select' it as a dependency
(e.g. DEVLINK, PAGE_POOL).

I'm not familiar with examples of the middle ground where we'd both
expose the Kconfig, _and_ select in the drivers. Are there any?

I don't want you to rage-quit over this tho, so we can merge as is and
deal with the consequences.

> > 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?  
> 
> I can answer for Max. The can327 patch is under review with the clear
> intent to have it upstream. c.f.:
> https://lore.kernel.org/linux-can/20220602213544.68273-1-max@xxxxxxxxx/
> 
> But until the patch gets accepted, it is defacto an out of tree module.

Good to hear, then it will get upstream in due course, and the problem
will disappear, no?