[PATCH v2 0/3] ALSA: hdsp and hdspm, don't disable device if not enabled
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(-)
hdsp wants to disable a not enabled pci device, which makes kernel throw a warning. Make sure the device is enabled before calling disable.
[ 1.758292] snd_hdsp 0000:00:03.0: disabling already-disabled device [ 1.758327] WARNING: CPU: 0 PID: 180 at drivers/pci/pci.c:2146 pci_disable_device+0x91/0xb0 [ 1.766985] Call Trace: [ 1.767121] snd_hdsp_card_free+0x94/0xf0 [snd_hdsp] [ 1.767388] release_card_device+0x4b/0x80 [snd] [ 1.767639] device_release+0x3b/0xa0 [ 1.767838] kobject_put+0x94/0x1b0 [ 1.768027] put_device+0x13/0x20 [ 1.768207] snd_card_free+0x61/0x90 [snd] [ 1.768430] snd_hdsp_probe+0x524/0x5e0 [snd_hdsp]
Signed-off-by: Tong Zhang ztong0001@gmail.com --- sound/pci/rme9652/hdsp.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/sound/pci/rme9652/hdsp.c b/sound/pci/rme9652/hdsp.c index 4cf879c42dc4..d9879a5bd60e 100644 --- a/sound/pci/rme9652/hdsp.c +++ b/sound/pci/rme9652/hdsp.c @@ -5285,8 +5285,10 @@ static int snd_hdsp_create(struct snd_card *card,
pci_set_master(hdsp->pci);
- if ((err = pci_request_regions(pci, "hdsp")) < 0) + if ((err = pci_request_regions(pci, "hdsp")) < 0) { + pci_disable_device(pci); return err; + } hdsp->port = pci_resource_start(pci, 0); if ((hdsp->iobase = ioremap(hdsp->port, HDSP_IO_EXTENT)) == NULL) { dev_err(hdsp->card->dev, "unable to remap region 0x%lx-0x%lx\n", @@ -5387,10 +5389,10 @@ static int snd_hdsp_free(struct hdsp *hdsp) vfree(hdsp->fw_uploaded); iounmap(hdsp->iobase);
- if (hdsp->port) + if (hdsp->port) { pci_release_regions(hdsp->pci); - - pci_disable_device(hdsp->pci); + pci_disable_device(hdsp->pci); + } return 0; }
hdspm wants to disable a not enabled pci device, which makes kernel throw a warning. Make sure the device is enabled before calling disable.
[ 1.786391] snd_hdspm 0000:00:03.0: disabling already-disabled device [ 1.786400] WARNING: CPU: 0 PID: 182 at drivers/pci/pci.c:2146 pci_disable_device+0x91/0xb0 [ 1.795181] Call Trace: [ 1.795320] snd_hdspm_card_free+0x58/0xa0 [snd_hdspm] [ 1.795595] release_card_device+0x4b/0x80 [snd] [ 1.795860] device_release+0x3b/0xa0 [ 1.796072] kobject_put+0x94/0x1b0 [ 1.796260] put_device+0x13/0x20 [ 1.796438] snd_card_free+0x61/0x90 [snd] [ 1.796659] snd_hdspm_probe+0x97b/0x1440 [snd_hdspm]
Signed-off-by: Tong Zhang ztong0001@gmail.com --- sound/pci/rme9652/hdspm.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/sound/pci/rme9652/hdspm.c b/sound/pci/rme9652/hdspm.c index 8d900c132f0f..af3898c88bba 100644 --- a/sound/pci/rme9652/hdspm.c +++ b/sound/pci/rme9652/hdspm.c @@ -6582,8 +6582,10 @@ static int snd_hdspm_create(struct snd_card *card, pci_set_master(hdspm->pci);
err = pci_request_regions(pci, "hdspm"); - if (err < 0) + if (err < 0) { + pci_disable_device(pci); return err; + }
hdspm->port = pci_resource_start(pci, 0); io_extent = pci_resource_len(pci, 0); @@ -6880,10 +6882,10 @@ static int snd_hdspm_free(struct hdspm * hdspm) kfree(hdspm->mixer); iounmap(hdspm->iobase);
- if (hdspm->port) + if (hdspm->port) { pci_release_regions(hdspm->pci); - - pci_disable_device(hdspm->pci); + pci_disable_device(hdspm->pci); + } return 0; }
rme9652 wants to disable a not enabled pci device, which makes kernel throw a warning. Make sure the device is enabled before calling disable.
[ 1.751595] snd_rme9652 0000:00:03.0: disabling already-disabled device [ 1.751605] WARNING: CPU: 0 PID: 174 at drivers/pci/pci.c:2146 pci_disable_device+0x91/0xb0 [ 1.759968] Call Trace: [ 1.760145] snd_rme9652_card_free+0x76/0xa0 [snd_rme9652] [ 1.760434] release_card_device+0x4b/0x80 [snd] [ 1.760679] device_release+0x3b/0xa0 [ 1.760874] kobject_put+0x94/0x1b0 [ 1.761059] put_device+0x13/0x20 [ 1.761235] snd_card_free+0x61/0x90 [snd] [ 1.761454] snd_rme9652_probe+0x3be/0x700 [snd_rme9652]
Signed-off-by: Tong Zhang ztong0001@gmail.com --- sound/pci/rme9652/rme9652.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/sound/pci/rme9652/rme9652.c b/sound/pci/rme9652/rme9652.c index 4df992e846f2..f9c9b8a80797 100644 --- a/sound/pci/rme9652/rme9652.c +++ b/sound/pci/rme9652/rme9652.c @@ -1728,10 +1728,10 @@ static int snd_rme9652_free(struct snd_rme9652 *rme9652) if (rme9652->irq >= 0) free_irq(rme9652->irq, (void *)rme9652); iounmap(rme9652->iobase); - if (rme9652->port) + if (rme9652->port) { pci_release_regions(rme9652->pci); - - pci_disable_device(rme9652->pci); + pci_disable_device(rme9652->pci); + } return 0; }
@@ -2454,8 +2454,10 @@ static int snd_rme9652_create(struct snd_card *card,
spin_lock_init(&rme9652->lock);
- if ((err = pci_request_regions(pci, "rme9652")) < 0) + if ((err = pci_request_regions(pci, "rme9652")) < 0) { + pci_disable_device(pci); return err; + } rme9652->port = pci_resource_start(pci, 0); rme9652->iobase = ioremap(rme9652->port, RME9652_IO_EXTENT); if (rme9652->iobase == NULL) {
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
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 v3: change checks to pci_is_enabled()
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 | 3 ++- sound/pci/rme9652/hdspm.c | 3 ++- sound/pci/rme9652/rme9652.c | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-)
On Sun, 21 Mar 2021 16:38:37 +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 v3: change checks to pci_is_enabled()
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
Now applied all three patches. Thanks.
Takashi
hdsp wants to disable a not enabled pci device, which makes kernel throw a warning. Make sure the device is enabled before calling disable.
[ 1.758292] snd_hdsp 0000:00:03.0: disabling already-disabled device [ 1.758327] WARNING: CPU: 0 PID: 180 at drivers/pci/pci.c:2146 pci_disable_device+0x91/0xb0 [ 1.766985] Call Trace: [ 1.767121] snd_hdsp_card_free+0x94/0xf0 [snd_hdsp] [ 1.767388] release_card_device+0x4b/0x80 [snd] [ 1.767639] device_release+0x3b/0xa0 [ 1.767838] kobject_put+0x94/0x1b0 [ 1.768027] put_device+0x13/0x20 [ 1.768207] snd_card_free+0x61/0x90 [snd] [ 1.768430] snd_hdsp_probe+0x524/0x5e0 [snd_hdsp]
Suggested-by: Takashi Iwai tiwai@suse.de Signed-off-by: Tong Zhang ztong0001@gmail.com --- sound/pci/rme9652/hdsp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/pci/rme9652/hdsp.c b/sound/pci/rme9652/hdsp.c index 4cf879c42dc4..720297cbdf87 100644 --- a/sound/pci/rme9652/hdsp.c +++ b/sound/pci/rme9652/hdsp.c @@ -5390,7 +5390,8 @@ static int snd_hdsp_free(struct hdsp *hdsp) if (hdsp->port) pci_release_regions(hdsp->pci);
- pci_disable_device(hdsp->pci); + if (pci_is_enabled(hdsp->pci)) + pci_disable_device(hdsp->pci); return 0; }
hdspm wants to disable a not enabled pci device, which makes kernel throw a warning. Make sure the device is enabled before calling disable.
[ 1.786391] snd_hdspm 0000:00:03.0: disabling already-disabled device [ 1.786400] WARNING: CPU: 0 PID: 182 at drivers/pci/pci.c:2146 pci_disable_device+0x91/0xb0 [ 1.795181] Call Trace: [ 1.795320] snd_hdspm_card_free+0x58/0xa0 [snd_hdspm] [ 1.795595] release_card_device+0x4b/0x80 [snd] [ 1.795860] device_release+0x3b/0xa0 [ 1.796072] kobject_put+0x94/0x1b0 [ 1.796260] put_device+0x13/0x20 [ 1.796438] snd_card_free+0x61/0x90 [snd] [ 1.796659] snd_hdspm_probe+0x97b/0x1440 [snd_hdspm]
Suggested-by: Takashi Iwai tiwai@suse.de Signed-off-by: Tong Zhang ztong0001@gmail.com --- sound/pci/rme9652/hdspm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/pci/rme9652/hdspm.c b/sound/pci/rme9652/hdspm.c index 8d900c132f0f..00cbf81ab2a6 100644 --- a/sound/pci/rme9652/hdspm.c +++ b/sound/pci/rme9652/hdspm.c @@ -6883,7 +6883,8 @@ static int snd_hdspm_free(struct hdspm * hdspm) if (hdspm->port) pci_release_regions(hdspm->pci);
- pci_disable_device(hdspm->pci); + if (pci_is_enabled(hdspm->pci)) + pci_disable_device(hdspm->pci); return 0; }
rme9652 wants to disable a not enabled pci device, which makes kernel throw a warning. Make sure the device is enabled before calling disable.
[ 1.751595] snd_rme9652 0000:00:03.0: disabling already-disabled device [ 1.751605] WARNING: CPU: 0 PID: 174 at drivers/pci/pci.c:2146 pci_disable_device+0x91/0xb0 [ 1.759968] Call Trace: [ 1.760145] snd_rme9652_card_free+0x76/0xa0 [snd_rme9652] [ 1.760434] release_card_device+0x4b/0x80 [snd] [ 1.760679] device_release+0x3b/0xa0 [ 1.760874] kobject_put+0x94/0x1b0 [ 1.761059] put_device+0x13/0x20 [ 1.761235] snd_card_free+0x61/0x90 [snd] [ 1.761454] snd_rme9652_probe+0x3be/0x700 [snd_rme9652]
Suggested-by: Takashi Iwai tiwai@suse.de Signed-off-by: Tong Zhang ztong0001@gmail.com --- sound/pci/rme9652/rme9652.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/pci/rme9652/rme9652.c b/sound/pci/rme9652/rme9652.c index 4df992e846f2..f407a95fc81f 100644 --- a/sound/pci/rme9652/rme9652.c +++ b/sound/pci/rme9652/rme9652.c @@ -1731,7 +1731,8 @@ static int snd_rme9652_free(struct snd_rme9652 *rme9652) if (rme9652->port) pci_release_regions(rme9652->pci);
- pci_disable_device(rme9652->pci); + if (pci_is_enabled(rme9652->pci)) + pci_disable_device(rme9652->pci); return 0; }
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
participants (2)
-
Takashi Iwai -
Tong Zhang