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 "*"?
>> + 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 shall apply the other changes. Thanks.
--
Timur Tabi
Linux Kernel Developer @ Freescale
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
