[PATCH] ALSA: usb-audio: Fix the enable parameter behavior

[PATCH] ALSA: usb-audio: Fix the enable parameter behavior

[Tamaki Nishino]

The enable parameter doesn't seem to work as expected. Even when the
enable parameter is set to false in /etc/modprobe.d/alsa-base.conf like
"options snd-usb-audio index=-2 vid=0x1415 pid=0x1d27 enable=0",
the specified device is not disabled.

This patch fixes the enable parameter behavior to disable the device
correctly.

Signed-off-by: Tamaki Nishino <otamachan@gmail.com>
---
 sound/usb/card.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/sound/usb/card.c b/sound/usb/card.c
index 23d1d23..8018d56 100644
--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -585,15 +585,24 @@ static int usb_audio_probe(struct usb_interface *intf,
 		 * now look for an empty slot and create a new card instance
 		 */
 		for (i = 0; i < SNDRV_CARDS; i++)
-			if (enable[i] && ! usb_chip[i] &&
+			if (!usb_chip[i] &&
 			    (vid[i] == -1 || vid[i] == USB_ID_VENDOR(id)) &&
 			    (pid[i] == -1 || pid[i] == USB_ID_PRODUCT(id))) {
-				err = snd_usb_audio_create(intf, dev, i, quirk,
-							   id, &chip);
-				if (err < 0)
+				if (enable[i]) {
+					err = snd_usb_audio_create(intf, dev, i, quirk,
+								   id, &chip);
+					if (err < 0)
+						goto __error;
+					chip->pm_intf = intf;
+					break;
+				} else if (vid[i] != -1 || pid[i] != -1) {
+					dev_info(&dev->dev,
+						 "device (%04x:%04x) is disabled\n",
+						 USB_ID_VENDOR(id),
+						 USB_ID_PRODUCT(id));
+					err = -ENOENT;
 					goto __error;
-				chip->pm_intf = intf;
-				break;
+				}
 			}
 		if (!chip) {
 			dev_err(&dev->dev, "no available usb audio device\n");
-- 
2.7.4

[PATCH] ALSA: usb-audio: Fix the enable parameter behavior

[Tamaki Nishino]

The enable parameter doesn't seem to work as expected. Even when the
enable parameter is set to false in /etc/modprobe.d/alsa-base.conf like
"options snd-usb-audio index=-2 vid=0x1415 pid=0x1d27 enable=0",
the specified device is not disabled.

This patch fixes the enable parameter behavior to disable the device
correctly.

Signed-off-by: Tamaki Nishino <otamachan@gmail.com>
---
 sound/usb/card.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/sound/usb/card.c b/sound/usb/card.c
index 23d1d23..8018d56 100644
--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -585,15 +585,24 @@ static int usb_audio_probe(struct usb_interface *intf,
 		 * now look for an empty slot and create a new card instance
 		 */
 		for (i = 0; i < SNDRV_CARDS; i++)
-			if (enable[i] && ! usb_chip[i] &&
+			if (!usb_chip[i] &&
 			    (vid[i] == -1 || vid[i] == USB_ID_VENDOR(id)) &&
 			    (pid[i] == -1 || pid[i] == USB_ID_PRODUCT(id))) {
-				err = snd_usb_audio_create(intf, dev, i, quirk,
-							   id, &chip);
-				if (err < 0)
+				if (enable[i]) {
+					err = snd_usb_audio_create(intf, dev, i, quirk,
+								   id, &chip);
+					if (err < 0)
+						goto __error;
+					chip->pm_intf = intf;
+					break;
+				} else if (vid[i] != -1 || pid[i] != -1) {
+					dev_info(&dev->dev,
+						 "device (%04x:%04x) is disabled\n",
+						 USB_ID_VENDOR(id),
+						 USB_ID_PRODUCT(id));
+					err = -ENOENT;
 					goto __error;
-				chip->pm_intf = intf;
-				break;
+				}
 			}
 		if (!chip) {
 			dev_err(&dev->dev, "no available usb audio device\n");
-- 
2.7.4

Re: [PATCH] ALSA: usb-audio: Fix the enable parameter behavior

[Takashi Iwai]

On Wed, 29 Nov 2017 23:26:16 +0100,
Tamaki Nishino wrote:
> 
> The enable parameter doesn't seem to work as expected. Even when the
> enable parameter is set to false in /etc/modprobe.d/alsa-base.conf like
> "options snd-usb-audio index=-2 vid=0x1415 pid=0x1d27 enable=0",
> the specified device is not disabled.
> 
> This patch fixes the enable parameter behavior to disable the device
> correctly.

Heh, strictly speaking, this is no "fix".  The behavior of the current
enable option is "correct", per se; it disables the first-defined
slot, and in the case of USB-audio, the probe continues to the next
slot as it's a hot-plug device.

OTOH, it's just badly designed, and the option is present just for
compatibility with other modules -- e.g. YaST has put enable option
always when configuring manually.

So, I find it OK to change the semantics of this option, as it's
practically useless, and the breakage is very unlikely.
But please make it clear that the patch is rather to improve the
usability, not about the correctness.


thanks,

Takashi

Re: [PATCH] ALSA: usb-audio: Fix the enable parameter behavior

[Tamaki Nishino]

Hi Takashi,
Thanks for your feedback.

I'm sorry but I didn't know the origin of the enable option.
So is it acceptable like
---
Change the semantics of the enable option for snd-usb-audio

This patch changes the semantics of the enable option for snd-usb-audio
in order to allow users to disable a device specified by either or both of the
vendor id and the product id.
---

I would appreciate for your any feedback in advance.

Best regards,

Tamaki

On Thu, Nov 30, 2017 at 6:06 PM, Takashi Iwai <tiwai@suse.de> wrote:
> On Wed, 29 Nov 2017 23:26:16 +0100,
> Tamaki Nishino wrote:
>>
>> The enable parameter doesn't seem to work as expected. Even when the
>> enable parameter is set to false in /etc/modprobe.d/alsa-base.conf like
>> "options snd-usb-audio index=-2 vid=0x1415 pid=0x1d27 enable=0",
>> the specified device is not disabled.
>>
>> This patch fixes the enable parameter behavior to disable the device
>> correctly.
>
> Heh, strictly speaking, this is no "fix".  The behavior of the current
> enable option is "correct", per se; it disables the first-defined
> slot, and in the case of USB-audio, the probe continues to the next
> slot as it's a hot-plug device.
>
> OTOH, it's just badly designed, and the option is present just for
> compatibility with other modules -- e.g. YaST has put enable option
> always when configuring manually.
>
> So, I find it OK to change the semantics of this option, as it's
> practically useless, and the breakage is very unlikely.
> But please make it clear that the patch is rather to improve the
> usability, not about the correctness.
>
>
> thanks,
>
> Takashi

[PATCH] ALSA: usb-audio: Fix the enable parameter behavior

[Tamaki Nishino]

The enable parameter doesn't seem to work as expected. Even when the
enable parameter is set to false in /etc/modprobe.d/alsa-base.conf like
"options snd-usb-audio index=-2 vid=0x1415 pid=0x1d27 enable=0",
the specified device is not disabled.

This patch fixes the enable parameter behavior to disable the device
correctly.

Signed-off-by: Tamaki Nishino <otamachan@gmail.com>
---
 sound/usb/card.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/sound/usb/card.c b/sound/usb/card.c
index 23d1d23..8018d56 100644
--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -585,15 +585,24 @@ static int usb_audio_probe(struct usb_interface *intf,
 		 * now look for an empty slot and create a new card instance
 		 */
 		for (i = 0; i < SNDRV_CARDS; i++)
-			if (enable[i] && ! usb_chip[i] &&
+			if (!usb_chip[i] &&
 			    (vid[i] == -1 || vid[i] == USB_ID_VENDOR(id)) &&
 			    (pid[i] == -1 || pid[i] == USB_ID_PRODUCT(id))) {
-				err = snd_usb_audio_create(intf, dev, i, quirk,
-							   id, &chip);
-				if (err < 0)
+				if (enable[i]) {
+					err = snd_usb_audio_create(intf, dev, i, quirk,
+								   id, &chip);
+					if (err < 0)
+						goto __error;
+					chip->pm_intf = intf;
+					break;
+				} else if (vid[i] != -1 || pid[i] != -1) {
+					dev_info(&dev->dev,
+						 "device (%04x:%04x) is disabled\n",
+						 USB_ID_VENDOR(id),
+						 USB_ID_PRODUCT(id));
+					err = -ENOENT;
 					goto __error;
-				chip->pm_intf = intf;
-				break;
+				}
 			}
 		if (!chip) {
 			dev_err(&dev->dev, "no available usb audio device\n");
-- 
2.7.4