Thread (10 messages) flat view 10 messages, 4 authors, 2021-09-21

Re: nt: usb: USB_RTL8153_ECM should not default to y

From: Jakub Kicinski <kuba@kernel.org>
Date: 2021-09-17 19:37:16

On Fri, 17 Sep 2021 21:05:55 +0200 Maciej Żenczykowski wrote:
quoted
Yeah.. more context here:

https://lore.kernel.org/all/7fd014f2-c9a5-e7ec-f1c6-b3e4bb0f6eb6@samsung.com/ (local)

default !USB_RTL8152 would be my favorite but that probably doesn't
compute in kconfig land. Or perhaps bring back the 'y' but more clearly
mark it as a sub-option of CDCETHER? It's hard to blame people for
expecting drivers to default to n, we should make it clearer that this
is more of a "make driver X support variation Y", 'cause now it sounds
like a completely standalone driver from the Kconfig wording. At least
to a lay person like myself.  
I think:
        depends on USB_NET_CDCETHER && (USB_RTL8152 || USB_RTL8152=n)
        default y
accomplished exactly what was wanted.

USB_NET_CDCETHER is a dependency, hence:

USB_NET_CDCETHER=n forces it off - as it should - it's an addon to cdcether.

USB_NET_CDCETHER=m disallows 'y' - module implies addon must be module.

similarly USB_RTL8152 is a dependency, so it being a module disallows 'y'.
This is desired, because if CDCETHER is builtin, so this addon could
be builtin, then RTL8152 would fail to bind it by default.
ie. CDCETHER=y && RTL8152=m must force RTL8153_ECM != y  (this is the bugfix)

basically the funky 'USB_RTL8152 || USB_RTL8152=n' --> disallows 'y'
iff RTL8152=m

'default y' enables it by default as 'y' if possible, as 'm' if not,
and disables it if impossible.

So I believe this had the exact right default behaviour - and allowed
all the valid options.
Right, it was _technically_ correct but run afoul of the "drivers
should default to 'n'" policy. If it should not be treated as a driver
but more of a feature of an existing driver which user has already
selected we should refine the name of the option to make that clear.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help