[PATCH v2 0/2] snd-usb-6fire: firmware load and pulseaudio assumption

newer
[PATCH v2 0/9] soundwire: intel:...

René Herman

21 Jul 2020 21 Jul '20

11:57 a.m.

Hi Takashi.

v2 of the earlier today sent out "set" for snd-usb-6fire, although now only two patches left, incorportaing the requested changes.

The snd-usb-6fire driver for the TerraTec DMX 6Fire USB soundcard has been failing its firmware upload due to a non DMA-capable buffer on the stack. First of the patches kmallocs said bufffer instead and fixes the firmware upload; also makes it more alsa-generic by using the "goto out" structure. Note that it only looks a bit ungeneric as a patch due to also needing to at the same time unify its failure path: it's obvious when looked at post-apply.

After that first patch the driver nominally works again but still has Pulseaudio crap out due to struct snd_pcm_hardware.channels_min=1 causing it to recognize it as a mono device only. Comparing with e.g. the TerraTec Aureon 7.1 Universe driver it seems that the solution is to simply set channels_min=2 as per the second patch.

With these changes the card works again. Driver author Torsten Schenk has seen these and is fine with them: maintains an external driver with more options. I or he might time permitting start integrating more into the kernel driver over time.

Regards, René

René Herman (2): Move DMA-buffer off the stack Pulseaudio needs snd_pcm_hardware.channels_min > 1

sound/usb/6fire/firmware.c | 95 ++++++++++++++++++-------------------- sound/usb/6fire/pcm.c | 2 +- 2 files changed, 47 insertions(+), 50 deletions(-)

-- 2.17.1

Show replies by date

René Herman

21 Jul 21 Jul

11:57 a.m.

New subject: [PATCH v2 1/2] Move DMA-buffer off the stack

snd-usb-fire currently fails its firmware load with "transfer buffer not dma capable". Move said buffer off of the stack.

Signed-off-by: René Herman rene.herman@gmail.com --- sound/usb/6fire/firmware.c | 95 ++++++++++++++++++-------------------- 1 file changed, 46 insertions(+), 49 deletions(-)

diff --git a/sound/usb/6fire/firmware.c b/sound/usb/6fire/firmware.c index 69137c14d0dc..502653a89f01 100644 --- a/sound/usb/6fire/firmware.c +++ b/sound/usb/6fire/firmware.c @@ -355,63 +355,60 @@ static int usb6fire_fw_check(struct usb_interface *intf, const u8 *version)

int usb6fire_fw_init(struct usb_interface *intf) { - int i; - int ret; struct usb_device *device = interface_to_usbdev(intf); + int ret, i; + /* buffer: 8 receiving bytes from device and * sizeof(EP_W_MAX_PACKET_SIZE) bytes for non-const copy */ - u8 buffer[12]; + u8 *buffer = kmalloc(12, GFP_KERNEL); + + if (!buffer) + return -ENOMEM;

ret = usb6fire_fw_ezusb_read(device, 1, 0, buffer, 8); if (ret < 0) { dev_err(&intf->dev, "unable to receive device firmware state.\n"); - return ret; - } - if (buffer[0] != 0xeb || buffer[1] != 0xaa || buffer[2] != 0x55) { - dev_err(&intf->dev, - "unknown device firmware state received from device:"); - for (i = 0; i < 8; i++) - printk(KERN_CONT "%02x ", buffer[i]); - printk(KERN_CONT "\n"); - return -EIO; - } - /* do we need fpga loader ezusb firmware? */ - if (buffer[3] == 0x01) { - ret = usb6fire_fw_ezusb_upload(intf, - "6fire/dmx6firel2.ihx", 0, NULL, 0); - if (ret < 0) - return ret; - return FW_NOT_READY; + goto out; } - /* do we need fpga firmware and application ezusb firmware? */ - else if (buffer[3] == 0x02) { - ret = usb6fire_fw_check(intf, buffer + 4); - if (ret < 0) - return ret; - ret = usb6fire_fw_fpga_upload(intf, "6fire/dmx6firecf.bin"); - if (ret < 0) - return ret; - memcpy(buffer, ep_w_max_packet_size, - sizeof(ep_w_max_packet_size)); - ret = usb6fire_fw_ezusb_upload(intf, "6fire/dmx6fireap.ihx", - 0x0003, buffer, sizeof(ep_w_max_packet_size)); - if (ret < 0) - return ret; - return FW_NOT_READY; - } - /* all fw loaded? */ - else if (buffer[3] == 0x03) - return usb6fire_fw_check(intf, buffer + 4); - /* unknown data? */ - else { - dev_err(&intf->dev, - "unknown device firmware state received from device: "); - for (i = 0; i < 8; i++) - printk(KERN_CONT "%02x ", buffer[i]); - printk(KERN_CONT "\n"); - return -EIO; + if (buffer[0] == 0xeb && buffer[1] == 0xaa && buffer[2] == 0x55) { + /* do we need fpga loader ezusb firmware? */ + if (buffer[3] == 1) { + ret = usb6fire_fw_ezusb_upload(intf, + "6fire/dmx6firel2.ihx", 0, NULL, 0); + if (ret >= 0) + ret = FW_NOT_READY; + goto out; + } + /* do we need fpga firmware and application ezusb firmware? */ + if (buffer[3] == 2) { + ret = usb6fire_fw_check(intf, buffer + 4); + if (ret < 0) + goto out; + ret = usb6fire_fw_fpga_upload(intf, "6fire/dmx6firecf.bin"); + if (ret < 0) + goto out; + memcpy(buffer, ep_w_max_packet_size, + sizeof(ep_w_max_packet_size)); + ret = usb6fire_fw_ezusb_upload(intf, "6fire/dmx6fireap.ihx", + 0x0003, buffer, sizeof(ep_w_max_packet_size)); + if (ret >= 0) + ret = FW_NOT_READY; + goto out; + } + /* all fw loaded? */ + if (buffer[3] == 3) { + ret = usb6fire_fw_check(intf, buffer + 4); + goto out; + } } - return 0; + dev_err(&intf->dev, + "unknown device firmware state received from device: "); + for (i = 0; i < 8; i++) + printk(KERN_CONT "%02x ", buffer[i]); + printk(KERN_CONT "\n"); + ret = -EIO; + +out: kfree(buffer); + return ret; } -

-- 2.17.1

Takashi Iwai

12:03 p.m.

New subject: [PATCH v2 1/2] Move DMA-buffer off the stack

On Tue, 21 Jul 2020 11:57:47 +0200, René Herman wrote:

...

snd-usb-fire currently fails its firmware load with "transfer buffer not dma capable". Move said buffer off of the stack.

Signed-off-by: René Herman rene.herman@gmail.com

sound/usb/6fire/firmware.c | 95 ++++++++++++++++++-------------------- 1 file changed, 46 insertions(+), 49 deletions(-)

diff --git a/sound/usb/6fire/firmware.c b/sound/usb/6fire/firmware.c index 69137c14d0dc..502653a89f01 100644 --- a/sound/usb/6fire/firmware.c +++ b/sound/usb/6fire/firmware.c @@ -355,63 +355,60 @@ static int usb6fire_fw_check(struct usb_interface *intf, const u8 *version)

int usb6fire_fw_init(struct usb_interface *intf) {

int i;

int ret; struct usb_device *device = interface_to_usbdev(intf);

int ret, i;

Don't put a blank line here.

...

/* buffer: 8 receiving bytes from device and * sizeof(EP_W_MAX_PACKET_SIZE) bytes for non-const copy */

u8 buffer[12];

u8 *buffer = kmalloc(12, GFP_KERNEL);
if (!buffer)
```
return -ENOMEM;
```
ret = usb6fire_fw_ezusb_read(device, 1, 0, buffer, 8); if (ret < 0) { dev_err(&intf->dev, "unable to receive device firmware state.\n");

```
return ret;
```
}
if (buffer[0] != 0xeb || buffer[1] != 0xaa || buffer[2] != 0x55) {
```
dev_err(&intf->dev,
```

	"unknown device firmware state received from device:");

```
for (i = 0; i < 8; i++)
```
```
	printk(KERN_CONT "%02x ", buffer[i]);
```
```
printk(KERN_CONT "\n");
```
```
return -EIO;
```
}
/* do we need fpga loader ezusb firmware? */
if (buffer[3] == 0x01) {
```
ret = usb6fire_fw_ezusb_upload(intf,
```
```
		"6fire/dmx6firel2.ihx", 0, NULL, 0);
```
```
if (ret < 0)
```
```
	return ret;
```
```
return FW_NOT_READY;
```

```
goto out;
```
}

/* do we need fpga firmware and application ezusb firmware? */
else if (buffer[3] == 0x02) {

ret = usb6fire_fw_check(intf, buffer + 4);

```
if (ret < 0)
```
```
	return ret;
```

ret = usb6fire_fw_fpga_upload(intf, "6fire/dmx6firecf.bin");

```
if (ret < 0)
```
```
	return ret;
```
```
memcpy(buffer, ep_w_max_packet_size,
```
```
		sizeof(ep_w_max_packet_size));
```

ret = usb6fire_fw_ezusb_upload(intf, "6fire/dmx6fireap.ihx",

		0x0003,	buffer, sizeof(ep_w_max_packet_size));

```
if (ret < 0)
```
```
	return ret;
```
```
return FW_NOT_READY;
```
}
/* all fw loaded? */
else if (buffer[3] == 0x03)

return usb6fire_fw_check(intf, buffer + 4);

/* unknown data? */
else {
```
dev_err(&intf->dev,
```

	"unknown device firmware state received from device: ");

```
for (i = 0; i < 8; i++)
```
```
	printk(KERN_CONT "%02x ", buffer[i]);
```
```
printk(KERN_CONT "\n");
```
```
return -EIO;
```

if (buffer[0] == 0xeb && buffer[1] == 0xaa && buffer[2] == 0x55) {

/* do we need fpga loader ezusb firmware? */

```
if (buffer[3] == 1) {
```
```
	ret = usb6fire_fw_ezusb_upload(intf,
```

			"6fire/dmx6firel2.ihx", 0, NULL, 0);

```
	if (ret >= 0)
```
```
		ret = FW_NOT_READY;
```
```
	goto out;
```
```
}
```

/* do we need fpga firmware and application ezusb firmware? */

```
if (buffer[3] == 2) {
```

	ret = usb6fire_fw_check(intf, buffer + 4);

```
	if (ret < 0)
```
```
		goto out;
```

	ret = usb6fire_fw_fpga_upload(intf, "6fire/dmx6firecf.bin");

```
	if (ret < 0)
```
```
		goto out;
```
```
	memcpy(buffer, ep_w_max_packet_size,
```
```
			sizeof(ep_w_max_packet_size));
```

	ret = usb6fire_fw_ezusb_upload(intf, "6fire/dmx6fireap.ihx",

			0x0003,	buffer, sizeof(ep_w_max_packet_size));

```
	if (ret >= 0)
```
```
		ret = FW_NOT_READY;
```
```
	goto out;
```
```
}
```
```
/* all fw loaded? */
```
```
if (buffer[3] == 3) {
```

	ret = usb6fire_fw_check(intf, buffer + 4);

```
	goto out;
```
```
}
```
}

return 0;

dev_err(&intf->dev,

"unknown device firmware state received from device: ");

for (i = 0; i < 8; i++)
```
printk(KERN_CONT "%02x ", buffer[i]);
```
printk(KERN_CONT "\n");
ret = -EIO;

+out: kfree(buffer);

return ret;

Erm, please don't change the code so much. You need to replace *only* the buffer allocation / free and the error path using goto instead of the direct return. If you need to modify other code, do it in another patch. In that way, it'll be clearer what each change means.

thanks,

Takashi

René Herman

12:09 p.m.

New subject: [PATCH v2 1/2] Move DMA-buffer off the stack

I'm afraid I'll now just go do other things again then; will keep things local.

René Herman

11:57 a.m.

New subject: [PATCH v2 2/2] Pulseaudio needs snd_pcm_hardware.channels_min > 1

Pulseaudio with snd_pcm_hardware.channels_min=1 creates a mono device only.

Signed-off-by: René Herman rene.herman@gmail.com --- sound/usb/6fire/pcm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/usb/6fire/pcm.c b/sound/usb/6fire/pcm.c index 88ac1c4ee163..cce1312db93a 100644 --- a/sound/usb/6fire/pcm.c +++ b/sound/usb/6fire/pcm.c @@ -58,7 +58,7 @@ static const struct snd_pcm_hardware pcm_hw = {

.rate_min = 44100, .rate_max = 192000, - .channels_min = 1, + .channels_min = 2, .channels_max = 0, /* set in pcm_open, depending on capture/playback */ .buffer_bytes_max = MAX_BUFSIZE, .period_bytes_min = PCM_N_PACKETS_PER_URB * (PCM_MAX_PACKET_SIZE - 4),

-- 2.17.1

1616

Age (days ago)

1616

Last active (days ago)

List overview

Download

4 comments

2 participants

tags (0)

participants (2)

René Herman
Takashi Iwai