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

[Thomas.Kopp]

> 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.

I'd disagree as the ISO specifically says it's not part of the frame. The diagram on page PDF page 21 of the 2.0 spec: http://esd.cs.ucr.edu/webres/can20.pdf is also in the ISO and shows the Intermission outside the frame. Also the word INTERframe space suggests it shouldn't be part of the frame and lastly the definition is used for a) determining how many bits/bytes are needed to store frames which doesn't need the intermission bits and b) timing, but for those purpose the frame has ended already and if the timing of several frames is needed, complete  interframe spaces need to be added.

> > 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.
> 
> >   * 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.
I don't know where the paper got its numbers from but it also seems to be missing the SBC field completely? The ISO says: " There shall be a fixed stuff bit before the first bit of the stuff count[...]" " A further fixed stuff bit shall be inserted after each fourth bit of the CRC field" Not that the CRC field in FD frames also contains the SBC so that adds fixed stuff bits.
A good visual representation of the FSBs is on the first page you provided as a source: https://www.can-cia.org/can-knowledge/can/can-fd/ all the way on the bottom. 


Best Regards,
Thomas