On 20.07.2013 05:44, Alan Stern wrote:
>> static int snd_usb_sb_x_fi_boot_quirk(struct usb_device *dev)
>> {
>> u16 buf = 1;
>>
>> snd_printk(KERN_ERR "X-Fi Surround 5.1 newer quirk\n");
>>
>> snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), 0x2a,
>> USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_OTHER,
>> 0, 0, &buf, 2);
>> snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), 0x2a,
>> USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_OTHER,
>> 0, 0, &buf, 2);
>> if (buf == 0) {
>> snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0), 0x29,
>> USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER,
>> 1, 2000, NULL, 0);
>> usb_reset_device(dev);
>> return -EAGAIN; //-ENODEV;
>> }
>> return 0;
>> }
>
> This is the same as the Audigy function, except for the printk string
> and the 2-byte buffer instead of the 1-byte buffer, right? Therefore I
> suggest combining them into a single function. You can pass the string
> and the buffer size as arguments.
There are the differences you mentioned (2 bytes buffer and printk
function, of course printk is for debugging only) but this is is not the
same as the Audigy since the message is sent twice (I am nor sure
whether it is necessary or not - Windows did that twice and so did I)
and a kind of "reset" is needed at the end in case of X-Fi
initialization (I made usb_reset_device(dev) but I am not sure whether
it is the only or optimal way - I have seen also other "resets" used in
other quirk functions). Without this "reset" card was not initialized
properly. This "reset" does not exist in case of Audigy quirk.
Sure, the 2 functions can be combined in one with some additional
arguments. I wonder only whether I should propose such solution - I have
no experience in kernel and drivers code, especially reading the
following in your message. But if you think I can do that I offer my
help. The only problem can be I do not have Audigy sound card to test it.
>> + static int snd_usb_sb_x_fi_boot_quirk(struct usb_device *dev)
>> + {
>> + u16 buf = 1;
>> +
>> + snd_printk(KERN_ERR "X-Fi Surround 5.1 newer quirk\n");
>> +
>> + snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), 0x2a,
>> + USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_OTHER,
>> + 0, 0, &buf, 2);
>
> There's a serious problem here. It's present in most or all of the
> quirk routines in this file, not just yours.
>
> Namely, buffers used for USB transfers must not be allocated on the
> stack; they must be allocated using kmalloc or a related function. The
> reason is because some architectures are not capable of performing DMA
> to addresses on the stack.
This is a good point which did not come to my mind due to lack of
experience. Fixing one or few functions seems reasonable easy, fixing
all in the whole file seems to be much more serious and responsible work.
> Do you feel like fixing up all those routines? I suggest allocating
> and deallocating a buffer in the function that calls the quirk
> routines, and have it pass the buffer as an extra argument.
This is probably question to the community not just to me?
--
Best regards
Mariusz Grecki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
