Re: RE: [PATCH] can: length: add definitions for frame lengths in bits

[Marc Kleine-Budde]

On 08.05.2023 08:54:18, Thomas.Kopp@xxxxxxxxxxxxx wrote:
> I was working on the same thing on Friday but didn't get around to
> sending it off, so here are a couple thoughts I had when working on
> the defines in length.h
> 
> The definitions for IFS in here are called intermission in the
> standard

ACK, and IMF seems to be a common abbreviation.

> and I'd argue they shouldn't be part of the frame at all.

The diagram in https://www.can-cia.org/can-knowledge/can/can-fd/
suggests that IMF is part of the frame.

> To
> quote the ISO: "DFs and RFs shall be separated from preceding frames,
> whatever frame type they are (DF, RF, EF, OF), by a time period called
> inter-frame space."
> 
> So, my suggestion would be to pull out the 3 bit IFS definition that's
> currently in and introduce 11 bit Bus idle and if necessary 3 bit
> Intermission separately.
> 
> index 6995092b774ec..62e92c1553376 100644
> --- a/include/linux/can/length.h
> +++ b/include/linux/can/length.h
> @@ -6,6 +6,26 @@
>  #ifndef _CAN_LENGTH_H
>  #define _CAN_LENGTH_H
> 
> +/*
> + * First part of the Inter Frame Space
> + */
> +#define CAN_INTERMISSION_BITS 3
> +
> +/*
> + * Number of consecutive recessive bits on the bus for integration etc.
> + */
> +#define CAN_IDLE_CONDITION_BITS 11
> +
> 
> The field currently called Stuff bit count (SBC) is also not correct
> I'd say. I'm not sure about the history but given that this is
> dependent on the DLC I think what's meant is the number of Fixed Stuff
> bits (FSB) . The ISO does not define a term for the Stuff bit Count
> but the CiA did define/document it this way. What's meant though is
> not the number of fixed stuff bits (FSB) which the comment implies
> here but the modulo 8 3 bit gray-code followed by the parity bit. So
> for the FD frame definitions I'd propose something like this: Renaming
> the current SBC to FSB and adding the SBC.

> /*
>   * Size of a CAN-FD Standard Frame
> @@ -69,17 +87,17 @@
>   * Error Status Indicator (ESI)                1
>   * Data length code (DLC)              4
>   * Data field                          0...512
> - * Stuff Bit Count (SBC)               0...16: 4 20...64:5
> + * Stuff Bit Count (SBC)               4

ACK

>   * CRC                                 0...16: 17 20...64:21
>   * CRC delimiter (CD)                  1
> + * Fixed Stuff bits (FSB)              0...16: 6 20...64:7

As far as I understand
https://ieeexplore.ieee.org/stamp/stamp.jsp?tp=&arnumber=8338047 the FSB
is 5 or 6.

>   * ACK slot (AS)                       1
>   * ACK delimiter (AD)                  1
>   * End-of-frame (EOF)                  7
> - * Inter frame spacing                 3
>   *
> - * assuming CRC21, rounded up and ignoring bitstuffing
> + * assuming CRC21, rounded up and ignoring dynamic bitstuffing
>   */
> 
> Best Regards,
> Thomas
> 

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

Attachment: signature.asc
Description: PGP signature