On Tue, Feb 08, 2022 at 06:59:39PM +0100, Anton Lindqvist wrote: > On Tue, Feb 08, 2022 at 07:32:38AM +0100, Alexandre Ratchov wrote: > > On Mon, Feb 07, 2022 at 06:55:21PM +0100, Anton Lindqvist wrote: > > > On Mon, Feb 07, 2022 at 11:21:43AM +0100, Alexandre Ratchov wrote: > > > > On Sun, Feb 06, 2022 at 08:40:35AM +0100, Anton Lindqvist wrote: > > > > > > > > > > Polished diff. I'm omitting a necessary refactoring commit making > > > > > audio_attach_mi() accept a new cookie argument. > > > > > > > > > > > > > I'm not sure to understand the need for the wskbd_audio structure. > > > > Couldn't we just store the cookie in audio and wskbd softc structures, > > > > then pass it in the wskbd_set_mixervolume_sc() calls ? > > > > > > Due to the device caching the data must be stored in either the audio or > > > wskbd softc and I don't want to expose the softc structures so I ended > > > up introducing wskbd_audio. Dropping the caching would probably make it > > > possible to only pass down the cookie to wskbd_set_mixervolume_sc() and > > > always do the audio device lookup. > > > > > > Is anyone in favor of this approach? I achieves the expected behavior in > > > my opinion. > > > > IMHO, handling the volume keys this way won't work in the general > > case. But for the short term we've no other options, have we? > > > > AFAICS, you're fixing a concrete use-case by tweaking what already > > exists, this won't make things more broken than they already are. I'm > > OK with it. > > Here's the complete diff including adding a cookie argument to > audio_attach_mi().
Is the caching necessary? device_lookup() seems cheap and there are at most two devices in most cases. Keeping the code minimal especially on rare and non-performace-critical code-paths would be nice. If you choose to drop the caching, this would allow to just add a a new "cookie" argument to wskbd_set_mixervolume(), similarly to what you did for audio_attach_mi()
