Re: Driver for TerraTec DMX 6Fire USB

[Torsten Schenk <torsten.schenk@zoho.com>]

[-- Attachment #1: Type: text/plain, Size: 3094 bytes --]

Hello,

Thanks very much for the review. I corrected the issues you wrote about (except the last one, see below). checkpatch.pl reports now 0 errors and warnings.

Another thing about the driver's features:
I'm currently in contact with a guy that also owns a TT DMX6FireUSB. He says, firmware is not loading for him. I'm trying to figure out how to make
it work, but I already know, that my driver cannot provide fw uploading for every device out there by now. I think it's just a matter of removing some checks, but I want to make as sure as I can before I just do so.

If the device was previously used in windows and not unplugged from power, it will work also with no firmware upload.

>First off, there are lots of coding-style issues to be fixed. 
>Try to run scripts/checkpatch.pl once with your patch. Not all warnings 
>have to be fixed (e.g. 80-chars) but would be nice if fixed.
I though so. checkpatch.pl is a useful script, good to know it exists.

>Adding a short prefix (e.g. usb6fire_probe()) is better in general. 
>Otherwise the stack trace would be difficult to read. Of course, 
>too long names are also annoying, so it's a matter of taste. 
How is it supposed to be? I reviewed my code and added a prefix to every function. Hopefully
it didn't get too long now... Some, especially functions with not much content, seem to have a too long name by now for my flavour. 

>NULLs are automatically filled, so these can be skipped. 
Which NULLs are automatically filled? Just the area create by snd_card_create or
every area allocated via kmalloc? If last is true, I will have to remove some more NULLs.

>Don't use own bool type. Use the generic "bool" type, and use 
>true/false instead of TRUE/FALSE (unless the size really matters). 
>Use standard min() / max() macros. 
>Use u8 and u16. 
Didn't know these exists...
When I used C once, 'bool' and also 'min'/'max' weren't available.

>Remove all, clean, install things. 
Just forgot ;)

>Do you really need hwdep? 
>Rather select FW_LOADER, if any. 
> 
>> +    select SND_PCM 
Thanks, just copied this from another definition without exactly knowing what it does...
> 
>Missing SND_RAWMIDI dependency. 
Realized that after posting.

> 
> 
>> @@ -65,7 +76,6 @@ 
>>      * Native Instruments Guitar Rig Session I/O 
>>      * Native Instruments Guitar Rig mobile 
>>      * Native Instruments Traktor Kontrol X1 
>> -     * Native Instruments Traktor Kontrol S4 
> 
>>      To compile this driver as a module, choose M here: the module 
>>      will be called snd-usb-caiaq. 
>> @@ -83,7 +93,6 @@ 
>>      * Native Instruments Kore Controller 
>>      * Native Instruments Kore Controller 2 
>>      * Native Instruments Audio Kontrol 1 
>> -     * Native Instruments Traktor Kontrol S4 
> 
>Don't bother others ;) 
I'd love not to ;) But how can I remove it? It's automatically generated by diff. Perhaps wrong arguments? Mine are -Nur
Or should I simply remove these lines with a text editor?

Greetings,
Torsten

[-- Attachment #2: usb6fire.tar.bz2 --]
[-- Type: application/octet-stream, Size: 13210 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel