qe: add ability to upload QE firmware
Anton Vorontsov
avorontsov at ru.mvista.com
Fri Oct 19 05:43:37 EST 2007
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 at mail.ru
backup email: ya-cbou at yandex.ru
irc://irc.freenode.net/bd2
More information about the Linuxppc-dev
mailing list