Message ID | 20210518165201.24376-3-tiwai@suse.de |
---|---|
State | New |
Headers | show |
Series | ALSA: Prep work for PCI rescan support | expand |
Dne 18. 05. 21 v 18:51 Takashi Iwai napsal(a): > The card power state check can be better put in the common ioctl > handler, as basically we want to prevent ioctls during the power off > state. Although this situation won't happen normally any longer (*), > it'll be helpful for catching this for the future implementation like > the faked suspend that is needed for PCI rescan. > > (*) Long long time ago, before the proper PM framework was introduced, > it was still possible to reach SNDRV_CTL_IOCTL_POWER ioctl during the > power off state. This ioctl existed as a main control for the suspend > resume state in the past, but the feature was already dropped along > with the standard PM framework. It seems like a function dup for the 5th patch which tracks in flight the power state. I think that we should drop this (and reshuffle patches) or remove this in or after the 5th patch. Jaroslav > > Signed-off-by: Takashi Iwai <tiwai@suse.de> > --- > sound/core/control.c | 3 +++ > sound/core/control_compat.c | 3 +++ > 2 files changed, 6 insertions(+) > > diff --git a/sound/core/control.c b/sound/core/control.c > index 498e3701514a..c22c3fad0c64 100644 > --- a/sound/core/control.c > +++ b/sound/core/control.c > @@ -1772,6 +1772,9 @@ static long snd_ctl_ioctl(struct file *file, unsigned int cmd, unsigned long arg > card = ctl->card; > if (snd_BUG_ON(!card)) > return -ENXIO; > + err = snd_power_wait(card, SNDRV_CTL_POWER_D0); > + if (err < 0) > + return err; > switch (cmd) { > case SNDRV_CTL_IOCTL_PVERSION: > return put_user(SNDRV_CTL_VERSION, ip) ? -EFAULT : 0; > diff --git a/sound/core/control_compat.c b/sound/core/control_compat.c > index 1d708aab9c98..d5b562ff237b 100644 > --- a/sound/core/control_compat.c > +++ b/sound/core/control_compat.c > @@ -438,6 +438,9 @@ static inline long snd_ctl_ioctl_compat(struct file *file, unsigned int cmd, uns > if (snd_BUG_ON(!ctl || !ctl->card)) > return -ENXIO; > > + err = snd_power_wait(ctl->card, SNDRV_CTL_POWER_D0); > + if (err < 0) > + return err; > switch (cmd) { > case SNDRV_CTL_IOCTL_PVERSION: > case SNDRV_CTL_IOCTL_CARD_INFO: >
On Tue, 18 May 2021 20:00:46 +0200, Jaroslav Kysela wrote: > > Dne 18. 05. 21 v 18:51 Takashi Iwai napsal(a): > > The card power state check can be better put in the common ioctl > > handler, as basically we want to prevent ioctls during the power off > > state. Although this situation won't happen normally any longer (*), > > it'll be helpful for catching this for the future implementation like > > the faked suspend that is needed for PCI rescan. > > > > (*) Long long time ago, before the proper PM framework was introduced, > > it was still possible to reach SNDRV_CTL_IOCTL_POWER ioctl during the > > power off state. This ioctl existed as a main control for the suspend > > resume state in the past, but the feature was already dropped along > > with the standard PM framework. > > It seems like a function dup for the 5th patch which tracks in flight the > power state. I think that we should drop this (and reshuffle patches) or > remove this in or after the 5th patch. A good point. With snd_power_ref_and_wait(), we achieve more fine-grained protection, so those checks could be indeed dropped. Will clean up later in the v2 patch. thanks, Takashi
diff --git a/sound/core/control.c b/sound/core/control.c index 498e3701514a..c22c3fad0c64 100644 --- a/sound/core/control.c +++ b/sound/core/control.c @@ -1772,6 +1772,9 @@ static long snd_ctl_ioctl(struct file *file, unsigned int cmd, unsigned long arg card = ctl->card; if (snd_BUG_ON(!card)) return -ENXIO; + err = snd_power_wait(card, SNDRV_CTL_POWER_D0); + if (err < 0) + return err; switch (cmd) { case SNDRV_CTL_IOCTL_PVERSION: return put_user(SNDRV_CTL_VERSION, ip) ? -EFAULT : 0; diff --git a/sound/core/control_compat.c b/sound/core/control_compat.c index 1d708aab9c98..d5b562ff237b 100644 --- a/sound/core/control_compat.c +++ b/sound/core/control_compat.c @@ -438,6 +438,9 @@ static inline long snd_ctl_ioctl_compat(struct file *file, unsigned int cmd, uns if (snd_BUG_ON(!ctl || !ctl->card)) return -ENXIO; + err = snd_power_wait(ctl->card, SNDRV_CTL_POWER_D0); + if (err < 0) + return err; switch (cmd) { case SNDRV_CTL_IOCTL_PVERSION: case SNDRV_CTL_IOCTL_CARD_INFO:
The card power state check can be better put in the common ioctl handler, as basically we want to prevent ioctls during the power off state. Although this situation won't happen normally any longer (*), it'll be helpful for catching this for the future implementation like the faked suspend that is needed for PCI rescan. (*) Long long time ago, before the proper PM framework was introduced, it was still possible to reach SNDRV_CTL_IOCTL_POWER ioctl during the power off state. This ioctl existed as a main control for the suspend resume state in the past, but the feature was already dropped along with the standard PM framework. Signed-off-by: Takashi Iwai <tiwai@suse.de> --- sound/core/control.c | 3 +++ sound/core/control_compat.c | 3 +++ 2 files changed, 6 insertions(+)