Hi,
On Mon, Nov 2, 2009 at 8:49 PM, Wolfgang Grandegger <[email protected]> wrote:
> I assume this is v2 of the patch.
>
Yes I put the version in the subject. Will reply with v3 shortly
>> + buf[TXBDLC_OFF] = (rtr << DLC_RTR_SHIFT) | frame->can_dlc;
>
> Two spaces before "=".
>
ack. This could be a good idea for a checkpatch.pl rule. Unfortunately
I don't know much about perlre. I used some emacs regexp so I hope
this is the last time you find double whitespaces or tabs around
assignment in this patch
>> + }
>
> Here the transceiver should be switched off!?
>
ack, all the error paths now seem checked now.
>> + mcp251x_write_bits(spi, CANINTF, intf, 0x00);
>
> Assigning variables within if or while expressions is not allowed. I
> wonder why checkpatch did not spot it.
>
ack. Out of curiosity I checked checkpatch.pl, unfortunately it looks
like it looks only for ifs:
if ($c =~ /\bif\s*\(.*[^<>!=]=[^=].*/s) {
ERROR("do not use assignment in if condition\n" . $herecurr);
}
anyway I looked at this loop and tested: there is no need for it.
>> + net->flags |= IFF_ECHO;
>
> Remove spaces, please.
>
ack, sorry
>> + if (ret >= 0) {
>
> if (!ret) ?
>
ack
>> + dev_info(&spi->dev, "probed\n");
>> + return ret;
>> + }
>
> Shouldn't the power be switched off?
>
ack on the error patch
>
> We are close...
>
thank you for the patience
--
Christian Pellegrin, see http://www.evolware.org/chri/
"Real Programmers don't play tennis, or any other sport which requires
you to change clothes. Mountain climbing is OK, and Real Programmers
wear their climbing boots to work in case a mountain should suddenly
spring up in the middle of the computer room."
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core
