* Another HID problem this merge window..
@ 2018-10-27 18:13 Linus Torvalds
2018-10-27 19:36 ` Joe Perches
2018-10-28 11:16 ` drivers by default (was Re: Another HID problem this merge window..) Adam Borowski
0 siblings, 2 replies; 7+ messages in thread
From: Linus Torvalds @ 2018-10-27 18:13 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires; +Cc: linux-input, Linux Kernel Mailing List
Ok, so this is a much smaller issue than the i2c one that cause boot
problems, but it's annoying.
We do *not* enable new random drivers by default. And we most
*definitely* don't do it when they are odd-ball ones that most people
have never heard of.
Yet the new "BigBen Interactive" driver that was added this merge
window did exactly that.
Just don't do it.
Yes, yes, every developer always thinks that _their_ driver is so
special and so magically important that it should be enabled by
default. But no. When we have thousands of drivers, we don't randomly
pick one new driver to be enabled by default just because some
developer thinks it is special. It's not.
So the
default !EXPERT
was completely wrong in commit 256a90ed9e46 ("HID: hid-bigbenff:
driver for BigBen Interactive PS3OFMINIPAD gamepad"). Please don't do
things like this.
Linus
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: Another HID problem this merge window.. 2018-10-27 18:13 Another HID problem this merge window Linus Torvalds @ 2018-10-27 19:36 ` Joe Perches 2018-10-27 20:17 ` Linus Torvalds 2018-10-28 11:16 ` drivers by default (was Re: Another HID problem this merge window..) Adam Borowski 1 sibling, 1 reply; 7+ messages in thread From: Joe Perches @ 2018-10-27 19:36 UTC (permalink / raw) To: Linus Torvalds, Jiri Kosina, Benjamin Tissoires Cc: linux-input, Linux Kernel Mailing List On Sat, 2018-10-27 at 11:13 -0700, Linus Torvalds wrote: > Ok, so this is a much smaller issue than the i2c one that cause boot > problems, but it's annoying. > > We do *not* enable new random drivers by default. And we most > *definitely* don't do it when they are odd-ball ones that most people > have never heard of. > > Yet the new "BigBen Interactive" driver that was added this merge > window did exactly that. In fairness, it seems many of the HID drivers do exactly that and this could have been a "copy from example" addition. $ git grep -P -i -B2 'default\s+\!EXPERT' -- drivers/hid/Kconfig drivers/hid/Kconfig- tristate "A4 tech mice" drivers/hid/Kconfig- depends on HID drivers/hid/Kconfig: default !EXPERT -- drivers/hid/Kconfig- tristate "Apple {i,Power,Mac}Books" drivers/hid/Kconfig- depends on HID drivers/hid/Kconfig: default !EXPERT -- drivers/hid/Kconfig- tristate "Belkin Flip KVM and Wireless keyboard" drivers/hid/Kconfig- depends on HID drivers/hid/Kconfig: default !EXPERT -- drivers/hid/Kconfig- tristate "Cherry Cymotion keyboard" drivers/hid/Kconfig- depends on HID drivers/hid/Kconfig: default !EXPERT -- drivers/hid/Kconfig- tristate "Chicony devices" drivers/hid/Kconfig- depends on HID drivers/hid/Kconfig: default !EXPERT -- drivers/hid/Kconfig- tristate "Cypress mouse and barcode readers" drivers/hid/Kconfig- depends on HID drivers/hid/Kconfig: default !EXPERT -- drivers/hid/Kconfig- tristate "Ezkey BTC 8193 keyboard" drivers/hid/Kconfig- depends on HID drivers/hid/Kconfig: default !EXPERT -- drivers/hid/Kconfig- tristate "ITE devices" drivers/hid/Kconfig- depends on HID drivers/hid/Kconfig: default !EXPERT -- drivers/hid/Kconfig- tristate "Kensington Slimblade Trackball" drivers/hid/Kconfig- depends on HID drivers/hid/Kconfig: default !EXPERT -- drivers/hid/Kconfig- tristate "Logitech devices" drivers/hid/Kconfig- depends on HID drivers/hid/Kconfig: default !EXPERT ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Another HID problem this merge window.. 2018-10-27 19:36 ` Joe Perches @ 2018-10-27 20:17 ` Linus Torvalds 2018-10-27 21:45 ` Jiri Kosina 0 siblings, 1 reply; 7+ messages in thread From: Linus Torvalds @ 2018-10-27 20:17 UTC (permalink / raw) To: joe Cc: Jiri Kosina, Benjamin Tissoires, linux-input, Linux Kernel Mailing List On Sat, Oct 27, 2018 at 12:36 PM Joe Perches <joe@perches.com> wrote: > > In fairness, it seems many of the HID drivers do exactly that > and this could have been a "copy from example" addition. Interesting, and I think you're right. I wonder why I haven't noticed this before. Some of those might be hidden by other dependencies - what I do is to just check how "make oldconfig" changes my normal fairly minimal configuration, and then complain when I notice that somebody enables some odd new hardware. Anyway, those other "!EXPERT" defaults look pretty questionable too. HID itself beind default y and the HID_GENERIC driver defaulting on seems eminently sane. And HID_LOGITECH might fall under both "very common" and "enables other config options". But even then, the !EXPERT seems an odd choice. Why not just 'y' for the truly common cases? I wonder if there is some truly old historical legacy there, ie the old PC keyboard support would have been configurable out only for expert users to avoid errors, and maybe the HID Kconfig file started getting ideas from that... Linus ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Another HID problem this merge window.. 2018-10-27 20:17 ` Linus Torvalds @ 2018-10-27 21:45 ` Jiri Kosina 2018-10-29 8:49 ` Benjamin Tissoires 0 siblings, 1 reply; 7+ messages in thread From: Jiri Kosina @ 2018-10-27 21:45 UTC (permalink / raw) To: Linus Torvalds Cc: joe, Benjamin Tissoires, linux-input, Linux Kernel Mailing List On Sat, 27 Oct 2018, Linus Torvalds wrote: > I wonder if there is some truly old historical legacy there, ie the old > PC keyboard support would have been configurable out only for expert > users to avoid errors, and maybe the HID Kconfig file started getting > ideas from that... This really goes waaay back to times when we extracted all the quirks from the generic driver (which became unmaintainable exactly because quirks being sprinkled left and right) into specialized drivers, but didn't want to cause too many user surprises that all of a sudden their configuration regressed when it comes to hardware support. We've had exactly this discussion multiple times before, see for example https://lkml.org/lkml/2010/5/20/227 So I guess there is no need for replaying it, I think we're in complete agreement. That being said, benff Kconfig setting definitely escaped attention. That should never ever have been set to default y, I take blame for not noticing that while applying the patch. Thanks, -- Jiri Kosina SUSE Labs ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Another HID problem this merge window.. 2018-10-27 21:45 ` Jiri Kosina @ 2018-10-29 8:49 ` Benjamin Tissoires 0 siblings, 0 replies; 7+ messages in thread From: Benjamin Tissoires @ 2018-10-29 8:49 UTC (permalink / raw) To: Jiri Kosina Cc: Linus Torvalds, joe, open list:HID CORE LAYER, lkml, Hans de Goede Hi, On Sat, Oct 27, 2018 at 11:45 PM Jiri Kosina <jikos@kernel.org> wrote: > > On Sat, 27 Oct 2018, Linus Torvalds wrote: > > > I wonder if there is some truly old historical legacy there, ie the old > > PC keyboard support would have been configurable out only for expert > > users to avoid errors, and maybe the HID Kconfig file started getting > > ideas from that... > > This really goes waaay back to times when we extracted all the quirks from > the generic driver (which became unmaintainable exactly because quirks > being sprinkled left and right) into specialized drivers, but didn't want > to cause too many user surprises that all of a sudden their configuration > regressed when it comes to hardware support. > > We've had exactly this discussion multiple times before, see for example > > https://lkml.org/lkml/2010/5/20/227 > > So I guess there is no need for replaying it, I think we're in complete > agreement. On the things I have on my plate, I'll try to remove all of the tiny HID drivers that does nothing but some small remapping. This will probably need help from userspace ("firmware" or bpf loading), and I have not settled my plans yet. I also think we should probably clean up the Kconfig now that hid-generic can unbind itself if there is a special driver coming in. And that means that we should probably start removing the blacklist of devices that have special modules, to let them be taken by hid-generic on boot until their driver is loaded. > > That being said, benff Kconfig setting definitely escaped attention. That > should never ever have been set to default y, I take blame for not > noticing that while applying the patch. Not sure I have actually reviewed this one (I don't think so), but I'll keep this in my head for next drivers. Cheers, Benjamin > > Thanks, > > -- > Jiri Kosina > SUSE Labs > ^ permalink raw reply [flat|nested] 7+ messages in thread
* drivers by default (was Re: Another HID problem this merge window..) 2018-10-27 18:13 Another HID problem this merge window Linus Torvalds 2018-10-27 19:36 ` Joe Perches @ 2018-10-28 11:16 ` Adam Borowski 2018-10-28 20:21 ` Linus Torvalds 1 sibling, 1 reply; 7+ messages in thread From: Adam Borowski @ 2018-10-28 11:16 UTC (permalink / raw) To: Linus Torvalds; +Cc: Jiri Kosina, Linux Kernel Mailing List On Sat, Oct 27, 2018 at 11:13:17AM -0700, Linus Torvalds wrote: > Ok, so this is a much smaller issue than the i2c one that cause boot > problems, but it's annoying. > > We do *not* enable new random drivers by default. And we most > *definitely* don't do it when they are odd-ball ones that most people > have never heard of. > > Yet the new "BigBen Interactive" driver that was added this merge > window did exactly that. > > Just don't do it. Amen to that. But, perhaps you could encourage people to do enable drivers once they become very popular? For example, I just (72a9c673636) got hit by USB 3.0 being off in defconfigs, and not having keyboard is not that cool. So what about a policy that says "almost all drivers are to be =n initially, but patches enabling popular stuff by default are welcome"? Preferably if obsolete crap gets mass-disabled at least once per decade (think of the floppy driver's importance on day one vs now). > Yes, yes, every developer always thinks that _their_ driver is so > special and so magically important that it should be enabled by > default. But no. When we have thousands of drivers, we don't randomly > pick one new driver to be enabled by default just because some > developer thinks it is special. It's not. We don't know which drivers will become important and which won't, only time can tell. But not having such drivers on by default is also a problem. Meow! -- ⢀⣴⠾⠻⢶⣦⠀ ⣾⠁⢰⠒⠀⣿⡁ 10 people enter a bar: 1 who understands binary, ⢿⡄⠘⠷⠚⠋⠀ 1 who doesn't, D who prefer to write it as hex, ⠈⠳⣄⠀⠀⠀⠀ and 1 who narrowly avoided an off-by-one error. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: drivers by default (was Re: Another HID problem this merge window..) 2018-10-28 11:16 ` drivers by default (was Re: Another HID problem this merge window..) Adam Borowski @ 2018-10-28 20:21 ` Linus Torvalds 0 siblings, 0 replies; 7+ messages in thread From: Linus Torvalds @ 2018-10-28 20:21 UTC (permalink / raw) To: kilobyte; +Cc: Jiri Kosina, Linux Kernel Mailing List On Sun, Oct 28, 2018 at 4:16 AM Adam Borowski <kilobyte@angband.pl> wrote: > > Amen to that. But, perhaps you could encourage people to do enable drivers > once they become very popular? For example, I just (72a9c673636) got hit by > USB 3.0 being off in defconfigs, and not having keyboard is not that cool. Yes, that seems reasonable. I don't think we need much of an explicit "policy" for these things, though. Things that get _so_ ubiquitous that they should be marked default globally (as opposed to just being in a defconfig file for some board) are pretty rare, and them being "new" things are rarer still. Yeah, usb3 sounds like it would count, but I think that's such a rare exception that I don't think we need to make a policy for it, just a "hey, once or twice in a decade we have a new bus that became so common that we should update the 'default' to y". Linus ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-10-29 8:49 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-27 18:13 Another HID problem this merge window Linus Torvalds 2018-10-27 19:36 ` Joe Perches 2018-10-27 20:17 ` Linus Torvalds 2018-10-27 21:45 ` Jiri Kosina 2018-10-29 8:49 ` Benjamin Tissoires 2018-10-28 11:16 ` drivers by default (was Re: Another HID problem this merge window..) Adam Borowski 2018-10-28 20:21 ` Linus Torvalds
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).