RE: RE: [PATCH] can: mcp251xfd: Increase poll timeout

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Automotive Discussions]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [CAN Bus]

  Powered by Linux