On Fri, Jul 28, 2017 at 03:01:48AM +0200, Rafael Wysocki wrote: > On Monday, July 24, 2017 06:14:36 PM Darren Hart wrote: > > On Mon, Jul 24, 2017 at 01:41:06PM +0200, Rafael Wysocki wrote: > > > On Monday, July 24, 2017 12:19:25 PM Andy Shevchenko wrote: > > > > On Sat, Jul 22, 2017 at 2:16 AM, Darren Hart <dvhart@xxxxxxxxxxxxx> wrote: > > > > > On Thu, Jul 20, 2017 at 08:56:48PM -0700, Alex Hung wrote: > > > > >> Unsupported events is only useful for developers and does not meaningful > > > > >> for users. Using dev_dbg makes more sense and reduces noise in kernel > > > > >> messages. > > > > > > > > >> - dev_info(&device->dev, "unknown event index 0x%x\n", event); > > > > >> + dev_dbg(&device->dev, "unknown event index 0x%x\n", event); > > > > > > > > > > info is the most common log level for these events in the platform > > > > > driver x86 subsystem per 'git grep -i "unknown event"'. > > > > > > > > > > My take on this is that we want these to be reported by users, rather > > > > > than rely on developers to find them all - especially as the developers > > > > > only see a fraction of the affected hardware. > > > > > > > > > > Are you finding these to be causing a problem / or producing really > > > > > excessive log messages? > > > > > > > > > > Andy, what are your thoughts? > > > > > > > > My opinion is slightly closer to Rafael's one. > > > > > > > > I think this is a debugging context. > > > > > > > > If we really care about reporting them we might go HID way, i.e. using > > > > dev_printk(KERN_DEBUG ) with module parameter debug. > > > > > > > > As developer I'm against that. > > > > As a regular user I do not need to recompile a kernel, in case there > > > > is no dynamic debug support, and it allows me to enable debug > > > > messages. > > > > > > > > P.S. To see how important message above is, do we have any statistics > > > > how many bug reports / email we got wrt the issue and how many had > > > > been addressed? > > > > > > Well, there is one I'm aware of, but this particular one we aren't going to > > > address at all, because apparently we handle the event in question anyway > > > in a different way. > > > > > > In any case, the only situation in which this information is useful at all is > > > when some functionality is missing and you want to find out why. > > > > > > Otherwise you get messages telling you that something *may* *be* missing, > > > but as a user you have no idea what to do about that, because you don't > > > even know how to report it and to whom (in case you want to report it). > > > > My thinking was along the lines of the keymaps where we explicitly add > > KE_IGNORE when it is a key we don't care about. In that case, it is > > useful to have the messages because if that occurs we want to know so we > > can update the driver. > > Well, I'm not sure if adding every reported event to keymaps as KE_IGNORE > is a good idea, because that would require somebody to figure out what the > event is about every time and that's work which quite frankly is not very > useful (the key is still ignored if it is "unknown"). > > This kind of is blacklisting which is always painful from the maintenance > standpoint. OK, yes, I've made similar arguments in other areas. > > > This driver also has KE_IGNORE entries in the intel_vbtn_keymap. > > > > Are we talking about unknown keys - or are we talking about something > > else? > > Unknown keys. > > > If unknown keys, the additional messages will be minimal, and missing > > keys are most likely to be reported if the message is obvious. Given the > > sparse access to hardware by the developers, I prefer to have the > > message. > > Having it does not guarantee that it will be reported and even so, it is > not particularly clear where to send those reports and who is going to act on > them. We get quite a few to bugzilla.kernel.org for this subsystem, certainly more than we can handle. But, your point is taken. > > > If there is more to what is going on here - can someone provide an > > example of where this is causing a problem? Or where the event causing > > the message is not something we will add code to catch / ignore? > > The particular one that tirggered this is related to the switching between > the "laptop" and "tablet" modes on Dell 9365 which according to Mario is > handled already through the ISH driver. > > We could potentially propagate this event to user space, but we have no > idea how user space decides to handle it and we are not sure if this is going > to be used consistently for this purpose on all platforms. Yeah, this is an ongoing issue, and I don't see vendors converging on how they handle this. > Anyway, my opinion is that dev_dbg() messages are sufficient for such things > as they allow people to selectively turn the messages on and see what happens > if something seems to be missing, but otherwise they don't generate log noise. I'm convinced. Thanks. -- Darren Hart VMware Open Source Technology Center
