Re: [PATCH - [hdspm] 0/3] Support for RME RayDAT and AIO

[Adrian Knoth <adi@drcomp.erfurt.thur.de>]

On 01/18/11 15:36, Takashi Iwai wrote:

Hi!

> Basically one patch should contain the changes that are build-clean,
> i.e. you can build cleanly after patching.  Thus changing a header
> and *.c in separate patches doesn't work well.

Ok. Better luck next time. ;) Speaking of which: should I resend
everything as a whole-in-one patch?

I could also provide a download link (given we're talking 170+kb here)

> Also, please use the generic variable types as much as possible
> for ioctl structs. Avoid use of enum or such, especially ones that
> are typedef'ed. In general, avoid typedefs in the kernel code as much
> as possible. Defining each enum type is no-go.

If've dropped all the new typedefs and only kept the existing ones now.
However, it still reads "enum foo something" in the structs for some
ioctls.

Is this acceptable or do I have to change this to int? If so, would I
manually add a couple of #defines? I somehow feel losing the semantic
connection to the enum is bad, but this might OTOH be intended, e.g. for
the sake of a stable interface.

> Second, don't define arrays in the header file. They must be in the
> body, not included headers.

Cross-checked with hdspmixer and moved to .c, except for the channel
mappings as they're included from userspace. (channel_map*)

> Also, remember checkpatch.pl is your friend.

It already was for the very last days. But I'm more excited about my new
friend Lindent. ;)


Cheers