10 Jul
2023
10 Jul
'23
5:05 p.m.
On Mon, 10 Jul 2023 08:59:56 +0200, Oswald Buddenhagen wrote:
+static void emu1010_clock_work(struct work_struct *work) +{
- struct snd_emu10k1 *emu;
- struct snd_ctl_elem_id id;
- emu = container_of(work, struct snd_emu10k1,
emu1010.clock_work);- if (emu->card->shutdown)
return;+#ifdef CONFIG_PM_SLEEP
- if (emu->suspend)
return;+#endif
- // We consider this a mixer setting, so use the mixer's lock
- down_write(&emu->card->controls_rwsem);
I really don't want to see the driver touching this lock. It's basically an internal stuff of ALSA core code. And, as already discussed, the controls_rwsem wasn't intended as a lock for the mixer content protection originally. It took over the role partially, but the drivers shouldn't rely on it.
- snd_emu1010_update_clock(emu);
- downgrade_write(&emu->card->controls_rwsem);
- snd_ctl_build_ioff(&id, emu->ctl_clock_source, 0);
- snd_ctl_notify(emu->card, SNDRV_CTL_EVENT_MASK_VALUE, &id);
- up_read(&emu->card->controls_rwsem);
Err, too ugly and unnecessary change. snd_ctl_notify() doesn't take the rwsem, and it can be called at any place without fiddling the rwsem. It's snd_ctl_notify_one() that needs the downgrade to read (and maybe we should rename the function to be clearer).
thanks,
Takashi