mbox series

[v2,0/3] ALSA: hdsp and hdspm, don't disable device if not enabled

Message ID 20210320222337.243368-1-ztong0001@gmail.com
Headers show
Series ALSA: hdsp and hdspm, don't disable device if not enabled | expand

Message

Tong Zhang March 20, 2021, 10:23 p.m. UTC 
This series fixes issues in hdsp and hdspm. The drivers in question want
to disable a device that is not enabled on error path.

v2: add fix to rme9652

Tong Zhang (3):
  ALSA: hdsp: don't disable if not enabled
  ALSA: hdspm: don't disable if not enabled
  ALSA: rme9652: don't disable if not enabled

 sound/pci/rme9652/hdsp.c    | 10 ++++++----
 sound/pci/rme9652/hdspm.c   | 10 ++++++----
 sound/pci/rme9652/rme9652.c | 10 ++++++----
 3 files changed, 18 insertions(+), 12 deletions(-)

Comments

Takashi Iwai March 21, 2021, 8:16 a.m. UTC | #1 
On Sat, 20 Mar 2021 23:23:33 +0100,
Tong Zhang wrote:
> 
> This series fixes issues in hdsp and hdspm. The drivers in question want
> to disable a device that is not enabled on error path.
> 
> v2: add fix to rme9652
> 
> Tong Zhang (3):
>   ALSA: hdsp: don't disable if not enabled
>   ALSA: hdspm: don't disable if not enabled
>   ALSA: rme9652: don't disable if not enabled

Thanks for the patches.

IMO, a safer way for this is to add pci_is_enabled() check in *_free()
functions around the call of pci_disable_device().  The point is that
*_free() is the sole destructor function that manages all stuff, hence
it's better to do all there.  And, of course, it'll be less changes.

Care to resend v3 patches with that?


thanks,

Takashi
Tong Zhang March 21, 2021, 3:40 p.m. UTC | #2 
On Sun, Mar 21, 2021 at 4:16 AM Takashi Iwai <tiwai@suse.de> wrote:
>
> On Sat, 20 Mar 2021 23:23:33 +0100,
> Tong Zhang wrote:
> >
> > This series fixes issues in hdsp and hdspm. The drivers in question want
> > to disable a device that is not enabled on error path.
> >
> > v2: add fix to rme9652
> >
> > Tong Zhang (3):
> >   ALSA: hdsp: don't disable if not enabled
> >   ALSA: hdspm: don't disable if not enabled
> >   ALSA: rme9652: don't disable if not enabled
>
> Thanks for the patches.
>
> IMO, a safer way for this is to add pci_is_enabled() check in *_free()
> functions around the call of pci_disable_device().  The point is that
> *_free() is the sole destructor function that manages all stuff, hence
> it's better to do all there.  And, of course, it'll be less changes.
>
> Care to resend v3 patches with that?
>
>
> thanks,
>
> Takashi

Thanks Takashi.
I made some changes and sent them as v3.
- Tong