On Thu, Feb 10, 2022 at 04:53:59PM +0100, Anton Lindqvist wrote: > On Wed, Feb 09, 2022 at 09:13:58AM +0100, Alexandre Ratchov wrote: > > 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() > > Sure, I ended up adding a new function after all since the > wskbd_set_mixervolume() prototype is not present in any header at this > point. >
ok ratchov
