Re: qe: add ability to upload QE firmware

[Anton Vorontsov <avorontsov@ru.mvista.com>]

On Thu, Oct 18, 2007 at 02:02:38PM -0500, Timur Tabi wrote:
> Anton Vorontsov wrote:
>>> +	if (firmware->length == (length + sizeof(u32))) {
>>> +		/* Length is valid, and there's a CRC */
>>> +		crc = be32_to_cpu(*((__be32 *) ((void *) firmware + length)));
>> Spaces are not needed after "(__be32 *)" and "(void *)". The whole
>> construction isn't easy to parse, thus spaces are only distracting.
>
> I have to disagree.  I added the spaces to make it easier to read.
>
>>> +	/* If there's only one microcode, then we assume it's common for all
>>> +	   RISCs, so we set the CERCR.CIR bit to share the IRAM with all RISCs.
>>> +	   This should be safe even on SOCs with only one RISC.
>>> +
>>> +	   If there are multiple 'microcode' structures, but each one points
>>> +	   to the same microcode binary (ignoring offsets), then we also assume
>>> +	   that we want share RAM.
>>> +	 */
>> Comment style is unorthodox.
>
> Should I prefix each line with a "*"?

Yup.

Heh... well, ideal multiline comment is:

/*
 * Multiline comment here.
 */

But this doesn't matter much, the salt is in "*"s. ;-)

>>> +		code = (void *) firmware + be32_to_cpu(ucode->code_offset);
>> space after (void *).
>
> Really?  This:
>
> 	code = (void *)firmware + be32_to_cpu(ucode->code_offset);
>
> is harder to read.  Without the space, it looks like *)firmware is one 
> word.

:-) I'm not about to argue, this is each own preference.

But to complete my point: I don't care what style exactly is used, what
I care about is consistency across the code I'm looking in. 100%
consistency is not affordable, of course. But we're all trying, aren't we?
(type *)variable is orthodox style, and when I see unusual style, my eyes
are itching, and this code is hard to read (to me). So, if everybody will
switch to "(type *) variable" style -- I'll just follow.


And again: this is all of minor importance, just my $0.02 for the code
consistency.

Thanks,

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2