On 12/15/20 1:40 PM, Vincent MAILHOL wrote:
[...]
>>> This is just an overview, the data bitrate is also applied to some
>>> other fields of the frame (e.g. the CRC). So when creating the macro
>>> for the CANFD_FRAME_OVERHEAD, it should be split between the nominal
>>> and data bitrates.
>>
>> I think we shouldn't mess with the number of bytes depending on BRS. When it
>> turns out that for CAN-FD this is not exact enough, we should look into the wifi
>> drivers. As wifi doesn't even have a constant channel speed.
>
> I have to admit that I was also not a fan of that idea, but it was the
> only thing I could think of.
>
> I am curious to see how this issue could be handled in a smarter way.
Let's wait until we have a "problem" with that :)
[...]
>> Talking about CAN-FD and naming: For the CAN-FD frame/dll length we need the
>> actual length of the data field.
>>
>> The code looks like this:
>>
>>> static const u8 len2XXX[] = {
>>> 0, 1, 2, 3, 4, 5, 6, 7, 8, /* 0 - 8 */
>>> 12, 12, 12, 12, /* 9 - 12 */
>>> 16, 16, 16, 16, /* 13 - 16 */
>>> 20, 20, 20, 20, /* 17 - 20 */
>>> 24, 24, 24, 24, /* 21 - 24 */
>>> 32, 32, 32, 32, 32, 32, 32, 32, /* 25 - 32 */
>>> 40, 40, 40, 40, 40, 40, 40, 40, /* 33 - 40 */
>>> 48, 48, 48, 48, 48, 48, 48, 48 /* 41 - 48 */
>>> };
>>>
>>> /* map the sanitized data length to an appropriate XXX length */
>>> u8 can_len2XXX(u8 len)
>>> {
>>> if (unlikely(len > ARRAY_SIZE(len2XXX)))
>>> return 64;
>>>
>>> return len2XXX[len];
>>> }
>>
>> Have you got a good name for this, too?
I've send a v2...(hmmm, but missed the v2)
> Before speaking of the naming, let me discuss the how to.
>
> For CAN-FD, the overhead length (i.e. all fields of the frame minus
> the data) might vary depending on:
> * Whether the frame is SFF or EFF
ACK, included in the v2
> * Size of the CRC: either 15, 17 or 21 depending only on the length
> of the data.
I haven't looked into the actual standard, but from what I have found, the CRC
for CAN-FD frames is 17 (for data length 0...16 bytes) or 21 (for data length >
16 byes). Also there are 4 or 5 fixed stuff bits (corresponding to 17 or 21 bits
of CRC) in the CRC.
I got the impression that even 0...8 byte CAN-FD frames use a CRC of 17.
> I did not fully check the ISO standard, but I hope I did not forget another
> field of variable length in the above list.
>
> The CRC field will be handled in the len2XXX[] table, the SFF/EFF will
> be handled in the can_len2XXX() function.
For now I've selected the 21 bit CRC, for CAN-FD:
> #define CANFD_DLL_OVERHEAD_SFF DIV_ROUND_UP(61, 8)
== 8 bytes
for CRC17 it would be 5 bits less => 56 bit => 7 bytes
> #define CANFD_DLL_OVERHEAD_EFF DIV_ROUND_UP(80, 8)
== 10 bytes
for CRC17 it would be 5 bits less => 75 bits => 8 bytes
I think that 1 bit is not worth the trouble.
> The other fields (sof, dlc, eof, ...) are constant, and can be
> squeezed in either the len2XXX[] table or the can_len2XXX() function.
I want to use the can_fd_data_len2frame_data_len() in the mcp251xfd driver, so
I'd like to keep that.
> The choice of how to group things is fully arbitrary.
>
> Some candidates would be:
> * can_fd_data_len2crc_len[] which would just do return the length on
> the CRC and leave all other calculation to the can_len2XXX()
> function.
> * can_fd_data_len2ssf_len[] which will return the full frame length
> as if it was a SFF. The can_len2XXX() function would only do the
> very last step and add the length difference between the SFF and
> EFF if relevant.
>
> All in between mix are also possible but would result in more
> convoluted naming (e.g. can_fd_data_len2data_crc_len[] that would
> return the data length plus the CRC length).
>
> In all scenarios, the can_len2XXX() function would serve the same
> purpose: calculate the full frame length. So I propose to name it
> can_fd_data_len2frame_len().
What about: can_fd_data_len2frame_data_len()
As it only the frame's data length, not the complete frame len. (I don't mind
long function names :))
> All that said, it is hard for me to pick a favorite. Out of all above
> idea, I propose to keep the second candidate:
> * can_fd_data_len2ssf_len[]
> But this is with no big conviction.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Attachment:
signature.asc
Description: OpenPGP digital signature
