Re: [PATCH] usb2can: Add support for USB2CAN interface from 8 devices

["krumboeck@universalnet.at" <krumboeck@universalnet.at>]

Am 2012-12-02 14:35, schrieb Wolfgang Grandegger:
> Hi Bernd,
>
> nice to see this driver being pushed mainline. As Oliver already pointed
> out, there are a few general naming and coding style issues:
>
> - The preferred multi line comment styles is:
>
>    /*
>     * A Comment
>     */

The Script checkpatch.pl didn't like this comment style. I'll change it again.

>
> - The patch does not yet apply to David Millers "net-next" GIT tree.
>    There are problems with Kconfig and Makefile.

I assume this is the right command and repository url:
git clone "http://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git"

>
> Could you please show the output of "candump -e any,0:0,#FFFFFFFF" when
> the device goes 1) error passive and 2) bus-off. You could provoke
> these states by 1) sending without connection to the bus or 2)
> short-circuiting CAN high and low.

bek@debian:~/software/socketcan/trunk/can-utils$ ./candump -e any,0:0,#FFFFFFFF
   can0  20000088  [8] 00 00 04 00 00 00 00 01   ERRORFRAME
         protocol-violation{{bit-stuffing-error}{}}
         bus-error
         error-counter-tx-rx{{0}{1}}
   can0  2000008C  [8] 00 04 00 00 00 00 00 63   ERRORFRAME
         controller-problem{rx-error-warning}
         protocol-violation{{}{}}
         bus-error
         error-counter-tx-rx{{0}{99}}
   can0  2000008C  [8] 00 10 00 00 00 00 00 7F   ERRORFRAME
         controller-problem{rx-error-passive}
         protocol-violation{{}{}}
         bus-error
         error-counter-tx-rx{{0}{127}}
   can0  500  [4] 00 01 02 03
   can0  500  [4] 00 01 02 03
   can0  500  [4] 00 01 02 03
   can0  500  [4] 00 01 02 03
   can0  500  [4] 00 01 02 03
   can0  500  [4] 00 01 02 03
   can0  500  [4] 00 01 02 03


In my tests I was not able to provoke "bus-off".


One of the tests from Gerd provoked "bus-off":
   can0  12345678  [8] 55 55 55 55 33 33 33 33
   can0  20000088  [8] 00 00 90 00 00 00 10 00   ERRORFRAME
   can0  2000008C  [8] 00 08 00 00 00 00 68 00   ERRORFRAME
   can0  2000008C  [8] 00 20 00 00 00 00 88 00   ERRORFRAME
   can0  20000040  [8] 00 00 00 00 00 00 F8 00   ERRORFRAME



> Another question: is it possible to switch off bus-error reporting?

No. At least I didn't found anything in the firmware.


>> +
>> +/* Send data to device */
>> +static netdev_tx_t usb2can_start_xmit(struct sk_buff *skb,
>> +                      struct net_device *netdev)
>> +{
>> +    struct usb2can *dev = netdev_priv(netdev);
>> +    struct net_device_stats *stats = &netdev->stats;
>> +    struct can_frame *cf = (struct can_frame *)skb->data;
>
> Is the cast needed?

Yes.

>
> The driver does collect similar statistics and therefore I do not see a
> need for these sysfs files... apart from printing the firmware version
> when the device is probed. BTW: what is "show_hardware" good for?
>

I don't exactly know, but I assume it is the hardware revision.


best regards,
Bernd