Message-ID: <cd10630c-cfc3-4462-0ff4-0554e9ddb596@linux.intel.com>
Date: Thu, 20 Nov 2025 18:31:15 +0200 (EET)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Werner Sembach <wse@...edocomputers.com>
cc: W_Armin@....de, Hans de Goede <hansg@...nel.org>, 
    platform-driver-x86@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/2] platform/x86/uniwill: Handle more WMI events
 required for TUXEDO devices

On Thu, 20 Nov 2025, Werner Sembach wrote:
> Am 20.11.25 um 11:59 schrieb Werner Sembach:
> > Am 20.11.25 um 11:44 schrieb Ilpo Järvinen:
> > > On Wed, 19 Nov 2025, Werner Sembach wrote:
> > > 
> > > > Am 18.11.25 um 16:02 schrieb Werner Sembach:
> > > > > Handle some more WMI events that are triggered on TUXEDO devices.
> > > To avoid another round-trip and a version, please write a bit more what
> > > this is about than extremely vague "some".
> > > 
> > ofc:
> > 
> > I tested 3 more devices (InfinityBook 15 Gen9 Intel, Stellaris 16 Gen5 AMD
> > and Intel) and found that these send additional WMI events that the devices
> > Armin tested didn't.
> > 
> > These are: UNIWILL_OSD_MUTE, UNIWILL_OSD_WEBCAM_TOGGLE, and
> > UNIWILL_OSD_DC_ADAPTER_CHANGED
> > 
> > I also went through our out-of-tree driver where additional WMI events are
> > bound: UNIWILL_OSD_RADIOON, UNIWILL_OSD_RADIOOFF, and
> > UNIWILL_OSD_KB_LED_LEVEL*
> > 
> > While I don't know which devices exactly use them, at least one device does,
> > otherwise we wouldn't have them there. Also I don't see any harm in binding
> > them, so I did.
>
> Forgot the most important part: Why i bound them even when some are just
> ignored: This avoids warning spam in dmesg about unbound events.

Hi Werner,

I've actually no problem with the diff itself :-) but what I want is us 
to explain what we're doing in the changelog text. E.g., if you add code 
ignore something to avoid warning spam, please state that in the 
changelog (not just "handle" which is actually quite different than 
"ignore" if we start to really go into language details, yeah, 
"ignoring" is kind of "handling" too but must less precise, I think you 
get the point).

Some things can be read from the patch itself, but it usually means more 
work and we rarely can infer motivations behind any non-trivial change so
it's always useful to record such otherwise hidden details to the 
changelog. Nobody is perfect in this, I don't expect that, but please try 
to avoid vague expressions when writing the description.

Stating why they're ignored (beyond just warning spam itself) is even 
better when e.g. you know something else handles the same 
event already. But it's understandable if you don't always know such 
details which itself might be useful knowledge for somebody wanting to 
change the code later (was something done on purpose or just because 
we did not know better at that time? is often what I'm left to ponder 
after reading a change from history, if changelog doesn't give that 
detail, I'll haveto guess and hope for the best).

In general, if somebody asks about some detail from a submitter, it's 
worth to stop to consider if it's something should be added to the 
changelog (when no change to the diff itself is asked). It could point out 
something in the change that is not obvious enough. We're not writing 
these changelogs just for the people around now but also for the people 
that come after us (and might only see the change from the commit history
as there's no guarantee any of us will be there then and/or doesn't 
remember anyway).

That also means if I don't seem to be directly asking a question, I 
usually don't expect email reply (other than in a form of a new version of 
the patch that addresses the feedback :-)).

-- 
 i.

Powered by blists - more mailing lists