Re: [PATCH] rme96 synchronization support

[Knut Petersen <Knut_Petersen@t-online.de>]

On 13.08.2013 09:19, Takashi Iwai wrote:
> Well, looking at rme96.c, the rme96 driver code can be cleaned up and
> lots of redundant checks can be reduced, then the sync-support patch
> will be the half size.  For example, it's almost useless to check
> RME96_ISPLAYING() in trigger, or doing in close or prepare callback.
> (It's still valid to do it in snd_rme96_create(), though, since the
>   register values are unknown at the first initialization state,
>   though.)
>
> But it's no serious problem, and we can live with the current code.
> So the only problem is the coding-style issues in your patch.

Ok, I admit that I never used scripts/checkpatch.pl ;-) Alsa, feeding the current
rme96.c code to scripts/checkpatch.pl gives  235 errors and 205 warnings:

99 * ERROR: trailing whitespace
88 * ERROR: code indent should use tabs where possible
76 * WARNING: please, no spaces at the start of a line
72 * WARNING: line over 80 characters
39 * WARNING: braces {} are not necessary for single statement blocks
21 * ERROR: do not use assignment in if condition
18 * ERROR: that open brace { should be on the previous line
13 * WARNING: braces {} are not necessary for any arm of this statement
8 * ERROR: space required after that ',' (ctx:VxV)
3 * WARNING: quoted string split across lines
1 * ERROR: space prohibited after that '!' (ctx:BxW)
1 * WARNING: Use #include <linux/io.h> instead of <asm/io.h>
1 * WARNING: Avoid CamelCase: <snd_BUG>

Comments to the list of errors and warnings:

The CamelCase snd_BUG() isn´t a problem of rme96.c.

I don´t agree that code like "if ((err = pci_request_regions(pci, "RME96")) < 0)"
is a problem.

I don´t agree with the script that lines over 80 characters are _always_ a problem.

Nevertheless, I really don´t like code alternating between different styles, that´s the reason
for some unnecessary braces {} I used in the submitted patches.

So I suggest that I will (re)submit three patches:
- first a patch cleaning the current rme96.c source
- the PM and SYNC patches based on the first patch.

After that additional patches removing useless checks, etc could follow. But this
should be patch 4(+) as it´s not always obvious for me if a check is really useless.

Do you agree?

cu,
  Knut