Hi Oliver,

Oliver Hartkopp wrote:
> Wolfgang Grandegger wrote:
>> Oliver Hartkopp wrote:
>>> Marc Kleine-Budde wrote:
> 
>>>> okay, then the at91_can does it alright :) \o/
>>> Congrats :-)
>>>
>>> Indeed your
>>>
>>>     cf->can_dlc = min_t(__u8, (reg_msr >> 16) & 0xf, 8);
>>>
>>> looks nice.
>> min() is fine if the type does already match.
>>
>>> Wolfgang, should i update the drivers in the SVN and send a patch to netdev?
>> Yes, of course. 
> 
> Hi all,
> 
> i updated the SVN and introduced a new GET_CAN_DLC() macro, that casts the
> data length code information to __u8 and ensures the dlc not to be > 8 .

I like your approach. I'm just not happy with the name. We should use
the prefix "can_" for public functions and lower-case seems also more
appropriate. A "static inline" is less handy, I agree. Therefore I vote
for "can_get_dlc()". Also some description for that macro would be nice.
I just got some info on that topic privately:

>From chapter "Implementation Guide for the CAN Protocol" of
http://www.semiconductors.bosch.de/pdf/can2spec.pdf:

----------------------
(1) According to the CAN Specification, no transmitter may send a frame with 
DLC > 8.
The case of DLC > 8 is not covered by any of the error types defined in chapter 
7.1
"Error Detection". It is neither a Bit Error, nor a Stuff Error, nor a CRC 
Error, nor
an Acknowledge Error. It could be regarded as a Form Error, but the DLC belongs
to the stuffed Control Field and the Form Error is only defined for the 
fixed-form
bit fields (see chapter 6 "Bit Stream Coding"). So no condition for Error 
Signalling
(see chapter 7.2) is fulfilled, the reaction of a receiver to a DLC > 8 is not
defined. The Reference CAN Model defines as de-facto standard the assumption
[if received DLC > 8 then DLC := 8], expecting to receive 8 data bytes even 
when the
received Data Length Code exceeds its upper limit of 8."
----------------------

And later in the ISO 11898 spec:

----------------------
8.4.2.3 DLC field:
The number of bytes in the data field  shall be indicated by the DLC, see table 
4.
This DLC consists of four (4) bits. The data field  may be of length zero. The
admissible number of data bytes for a data frame shall range from zero (0) to 
eight
(8). Data length codes in the range of zero (0) to seven (7) shall indicate data
fields of length of zero (0) to seven (7) bytes. All other data length codes 
shall
indicate that the data field is eight (8) bytes long."
----------------------

> Especially the SJA1000 driver was happy to get this clean-up :-)
> 
> See at:
> 
> http://svn.berlios.de/wsvn/socketcan/?op=comp&compare[]=...@1092&compare[]=...@1093
> 
> I compile checked all drivers but mscan and at91_can which is not possible on 
> x86.
> 
> Please check the changes. If it's ok, i'll send a patch to netdev when the
> current merge window is nearly closed.
> 
>> Please also check for useless "dlc > 8" checks in the
>> start_xmit function.
> 
> I thought about that and i'm not really sure about it:
> What if anyone uses PF_PACKET with CAN netdevices, which is a possible 
> use-case?

OK.

Thanks, Wolfgang.
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core

Reply via email to