RE: [PATCH v3] can: m_can: add support for one shot mode

[pankj.sharma]

> From: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>
> Subject: Re: [PATCH v3] can: m_can: add support for one shot mode
> 
> On 10/21/19 2:04 PM, Pankaj Sharma wrote:
> > According to the CAN Specification (see ISO 11898-1:2015, 8.3.4
> > Recovery Management), the M_CAN provides means for automatic
> > retransmission of frames that have lost arbitration or that have been
> > disturbed by errors during transmission. By default automatic
> > retransmission is enabled.
> >
> > The Bosch MCAN controller has support for disabling automatic
> > retransmission.
> >
> > To support time-triggered communication as described in ISO
> > 11898-1:2015, chapter 9.2, the automatic retransmission may be
> > disabled via CCCR.DAR.
> >
> > CAN_CTRLMODE_ONE_SHOT is used for disabling automatic retransmission.
> >
> > Signed-off-by: Pankaj Sharma <pankj.sharma@xxxxxxxxxxx>
> > Signed-off-by: Sriram Dash <sriram.dash@xxxxxxxxxxx>
> > ---
> >
> > changes in v3:
> > - resolving build errors for net-next branch
> >
> > changes in v2:
> > - rebase to net-next
> >
> >  drivers/net/can/m_can/m_can.c | 12 +++++++++---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/can/m_can/m_can.c
> > b/drivers/net/can/m_can/m_can.c index 562c8317e3aa..75e7490c4299
> > 100644
> > --- a/drivers/net/can/m_can/m_can.c
> > +++ b/drivers/net/can/m_can/m_can.c
> > @@ -123,6 +123,7 @@ enum m_can_reg {
> >  #define CCCR_CME_CANFD_BRS	0x2
> >  #define CCCR_TXP		BIT(14)
> >  #define CCCR_TEST		BIT(7)
> > +#define CCCR_DAR		BIT(6)
> >  #define CCCR_MON		BIT(5)
> >  #define CCCR_CSR		BIT(4)
> >  #define CCCR_CSA		BIT(3)
> > @@ -1135,7 +1136,7 @@ static void m_can_chip_config(struct net_device
> *dev)
> >  	if (cdev->version == 30) {
> >  	/* Version 3.0.x */
> >
> > -		cccr &= ~(CCCR_TEST | CCCR_MON |
> > +		cccr &= ~(CCCR_TEST | CCCR_MON | CCCR_DAR |
> >  			(CCCR_CMR_MASK << CCCR_CMR_SHIFT) |
> >  			(CCCR_CME_MASK << CCCR_CME_SHIFT));
> >
> > @@ -1145,7 +1146,7 @@ static void m_can_chip_config(struct net_device
> *dev)
> >  	} else {
> >  	/* Version 3.1.x or 3.2.x */
> >  		cccr &= ~(CCCR_TEST | CCCR_MON | CCCR_BRSE | CCCR_FDOE
> |
> > -			  CCCR_NISO);
> > +			  CCCR_NISO | CCCR_DAR);
> >
> >  		/* Only 3.2.x has NISO Bit implemented */
> >  		if (cdev->can.ctrlmode & CAN_CTRLMODE_FD_NON_ISO) @@ -
> 1165,6
> > +1166,10 @@ static void m_can_chip_config(struct net_device *dev)
> >  	if (cdev->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
> >  		cccr |= CCCR_MON;
> >
> > +	/* Disable Auto Retransmission (all versions) */
> > +	if (cdev->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT)
> > +		cccr |= CCCR_DAR;
> > +
> >  	/* Write config */
> >  	m_can_write(cdev, M_CAN_CCCR, cccr);
> >  	m_can_write(cdev, M_CAN_TEST, test); @@ -1310,7 +1315,8 @@
> static
> > int m_can_dev_setup(struct m_can_classdev *m_can_dev)
> >  	m_can_dev->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
> >  					CAN_CTRLMODE_LISTENONLY |
> >  					CAN_CTRLMODE_BERR_REPORTING |
> > -					CAN_CTRLMODE_FD;
> > +					CAN_CTRLMODE_FD |
> > +					CAN_CTRLMODE_ONE_SHOT;
> >
> >  	/* Set properties depending on M_CAN version */
> >  	switch (m_can_dev->version) {
> 
> What happens if you have called netif_stop_queue() and the controller was not
> able to send a single frame?
> 
> What happens to the echo_skb, if the controller was not able to send a frame?

We are aware of this issue. For this the tx frame errors are to be first handled and 
the error stats should be updated.
Currently in MCAN code Tx errors arenât being handled. 
We are working on this issue to handle these errors. Arbitration error being the first 
patch which is already posted. Very soon will be posting the patches for tx error handling. 

> 
> Marc
> 
> --
> Pengutronix e.K.                  | Marc Kleine-Budde           |
> Industrial Linux Solutions        | Phone: +49-231-2826-924     |
> Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
> Amtsgericht Hildesheim, HRA 2686  |
> https://protect2.fireeye.com/url?k=e09921cf-bd02ddac-e098aa80-
> 0cc47a31cdbc-65578a3e12ca03be&u=http://www.pengutronix.de/   |