On 05.05.2023 08:21:36, Thomas.Kopp@xxxxxxxxxxxxx wrote:
> > On 05.05.2023 07:07:03, Thomas.Kopp@xxxxxxxxxxxxx wrote:
> > > > The datasheet "MCP25xxFD Family Reference Manual" says it needs an idle
> > > > bus.
> > >
> > > Technically what's needed is an idle condition on the bus as specified
> > > in the ISO. i.e. 11 consecutive sampled recessive bits after a full
> > > frame (if one is currently in transmission).
> >
> > Right. What happens if another CAN frames comes before there are 11
> > consecutive sampled recessive bits? The IFS for CAN is 3 bits?
>
> Not quite. Between the Frames is an IFS that's correct but the IFS
> consists of an Intermission which is 3 bits long + a bus idle
> condition of 11 bits. Regular frames have to wait for the IFS to
> elapse BUT there's an exception for error frames and overload frames.
> EF may be up to 12 bit, OF are 8 dominant + 8 recessive bits, browsing
> through the spec I think only 2 OFs can happen consecutively. Adding
> another 32 bits to the formula should cover this.
>
> > > > > diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
> > > > b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
> > > > > index 68df6d4641b5c..9908843798cef 100644
> > > > > --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
> > > > > +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
> > > > > @@ -227,6 +227,7 @@ static int
> > > > > __mcp251xfd_chip_set_mode(const struct mcp251xfd_priv *priv,
> > > > > const u8 mode_req, bool nowait)
> > > > > {
> > > > > + const struct can_bittiming *bt = &priv->can.bittiming;
> > > > > u32 con = 0, con_reqop, osc = 0;
> > > > > u8 mode;
> > > > > int err;
> > > > > @@ -251,7 +252,8 @@ __mcp251xfd_chip_set_mode(const struct
> > > > mcp251xfd_priv *priv,
> > > > >
> > > > FIELD_GET(MCP251XFD_REG_CON_OPMOD_MASK,
> > > > > con) == mode_req,
> > > > > MCP251XFD_POLL_SLEEP_US,
> > > > > - MCP251XFD_POLL_TIMEOUT_US);
> > > > > + max(MCP251XFD_POLL_TIMEOUT_US,
> > > > > + 576 * USEC_PER_SEC / bt->bitrate));
> > > >
> > > > Let's use CANFD_FRAME_LEN_MAX * BITS_PER_BYTE instead of 576. I can
> > fix
> > > > this while applying.
> > > >
> > > So, I'd suggest CANFD_FRAME_LEN_MAX * BITS_PER_BYTE + 11 + stuffbits,
> > > as far as I can tell the CANFD_FRAME_LEN_MAX ignores stuffbits, so as
> > > an overapproximation something like (CANFD_FRAME_LEN_MAX *
> > > BITS_PER_BYTE) * 1.2 + 11.
> >
> > ..and a define for the 11 and a comment for the * 1.2
> >
> Right, I think definitions for the 11 and the 1.2 make sense in the
> include/linux/can/length.h anyway.
Sounds good, make this a separate patch.
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
