All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] can: m_can: add support for handling arbitration error
       [not found] <CGME20191021121350epcas5p3313e54a3bc5c8600c52a6db299893f78@epcas5p3.samsung.com>
@ 2019-10-21 12:13 ` Pankaj Sharma
  2019-10-25  9:44     ` pankj.sharma
                     ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Pankaj Sharma @ 2019-10-21 12:13 UTC (permalink / raw)
  To: linux-can, netdev, linux-kernel
  Cc: wg, mkl, davem, eugen.hristev, ludovic.desroches, pankaj.dubey,
	rcsekar, jhofstee, simon.horman, Pankaj Sharma, Sriram Dash

The Bosch MCAN hardware (3.1.0 and above) supports interrupt flag to
detect Protocol error in arbitration phase.

Transmit error statistics is currently not updated from the MCAN driver.
Protocol error in arbitration phase is a TX error and the network
statistics should be updated accordingly.

The member "tx_error" of "struct net_device_stats" should be incremented
as arbitration is a transmit protocol error. Also "arbitration_lost" of
"struct can_device_stats" should be incremented to report arbitration
lost.

Signed-off-by: Pankaj Sharma <pankj.sharma@samsung.com>
Signed-off-by: Sriram Dash <sriram.dash@samsung.com>
---

changes in v2:
- common m_can_ prefix for is_protocol_err function
- handling stats even if the allocation of the skb fails
- resolving build errors on net-next branch

 drivers/net/can/m_can/m_can.c | 37 +++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 75e7490c4299..a736297a875f 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -778,6 +778,38 @@ static inline bool is_lec_err(u32 psr)
 	return psr && (psr != LEC_UNUSED);
 }
 
+static inline bool m_can_is_protocol_err(u32 irqstatus)
+{
+	return irqstatus & IR_ERR_LEC_31X;
+}
+
+static int m_can_handle_protocol_error(struct net_device *dev, u32 irqstatus)
+{
+	struct net_device_stats *stats = &dev->stats;
+	struct m_can_classdev *cdev = netdev_priv(dev);
+	struct can_frame *cf;
+	struct sk_buff *skb;
+
+	/* propagate the error condition to the CAN stack */
+	skb = alloc_can_err_skb(dev, &cf);
+	if (unlikely(!skb)) {
+		netdev_dbg(dev, "allocation of skb failed\n");
+		stats->tx_errors++;
+		return 0;
+	}
+	if (cdev->version >= 31 && (irqstatus & IR_PEA)) {
+		netdev_dbg(dev, "Protocol error in Arbitration fail\n");
+		stats->tx_errors++;
+		cdev->can.can_stats.arbitration_lost++;
+		cf->can_id |= CAN_ERR_LOSTARB;
+		cf->data[0] |= CAN_ERR_LOSTARB_UNSPEC;
+	}
+
+	netif_receive_skb(skb);
+
+	return 1;
+}
+
 static int m_can_handle_bus_errors(struct net_device *dev, u32 irqstatus,
 				   u32 psr)
 {
@@ -792,6 +824,11 @@ static int m_can_handle_bus_errors(struct net_device *dev, u32 irqstatus,
 	    is_lec_err(psr))
 		work_done += m_can_handle_lec_err(dev, psr & LEC_UNUSED);
 
+	/* handle protocol errors in arbitration phase */
+	if ((cdev->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING) &&
+	    m_can_is_protocol_err(irqstatus))
+		work_done += m_can_handle_protocol_error(dev, irqstatus);
+
 	/* other unproccessed error interrupts */
 	m_can_handle_other_err(dev, irqstatus);
 
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* RE: [PATCH v2] can: m_can: add support for handling arbitration error
  2019-10-21 12:13 ` [PATCH v2] can: m_can: add support for handling arbitration error Pankaj Sharma
@ 2019-10-25  9:44     ` pankj.sharma
  2019-10-25 11:16   ` Simon Horman
  2019-10-29 14:23   ` Marc Kleine-Budde
  2 siblings, 0 replies; 7+ messages in thread
From: pankj.sharma @ 2019-10-25  9:44 UTC (permalink / raw)
  To: linux-can, netdev, linux-kernel, wg, mkl
  Cc: davem, eugen.hristev, ludovic.desroches, pankaj.dubey, rcsekar,
	jhofstee, simon.horman, 'Sriram Dash'

Gentle Ping!

> From: Pankaj Sharma <pankj.sharma@samsung.com>
> Subject: [PATCH v2] can: m_can: add support for handling arbitration error
> 
> The Bosch MCAN hardware (3.1.0 and above) supports interrupt flag to detect
> Protocol error in arbitration phase.
> 
> Transmit error statistics is currently not updated from the MCAN driver.
> Protocol error in arbitration phase is a TX error and the network statistics should
> be updated accordingly.
> 
> The member "tx_error" of "struct net_device_stats" should be incremented as
> arbitration is a transmit protocol error. Also "arbitration_lost" of "struct
> can_device_stats" should be incremented to report arbitration lost.
> 
> Signed-off-by: Pankaj Sharma <pankj.sharma@samsung.com>
> Signed-off-by: Sriram Dash <sriram.dash@samsung.com>
> ---
> 
> changes in v2:
> - common m_can_ prefix for is_protocol_err function
> - handling stats even if the allocation of the skb fails
> - resolving build errors on net-next branch
> 
>  drivers/net/can/m_can/m_can.c | 37
> +++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index 75e7490c4299..a736297a875f 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -778,6 +778,38 @@ static inline bool is_lec_err(u32 psr)
>  	return psr && (psr != LEC_UNUSED);
>  }
> 
> +static inline bool m_can_is_protocol_err(u32 irqstatus) {
> +	return irqstatus & IR_ERR_LEC_31X;
> +}
> +
> +static int m_can_handle_protocol_error(struct net_device *dev, u32
> +irqstatus) {
> +	struct net_device_stats *stats = &dev->stats;
> +	struct m_can_classdev *cdev = netdev_priv(dev);
> +	struct can_frame *cf;
> +	struct sk_buff *skb;
> +
> +	/* propagate the error condition to the CAN stack */
> +	skb = alloc_can_err_skb(dev, &cf);
> +	if (unlikely(!skb)) {
> +		netdev_dbg(dev, "allocation of skb failed\n");
> +		stats->tx_errors++;
> +		return 0;
> +	}
> +	if (cdev->version >= 31 && (irqstatus & IR_PEA)) {
> +		netdev_dbg(dev, "Protocol error in Arbitration fail\n");
> +		stats->tx_errors++;
> +		cdev->can.can_stats.arbitration_lost++;
> +		cf->can_id |= CAN_ERR_LOSTARB;
> +		cf->data[0] |= CAN_ERR_LOSTARB_UNSPEC;
> +	}
> +
> +	netif_receive_skb(skb);
> +
> +	return 1;
> +}
> +
>  static int m_can_handle_bus_errors(struct net_device *dev, u32 irqstatus,
>  				   u32 psr)
>  {
> @@ -792,6 +824,11 @@ static int m_can_handle_bus_errors(struct net_device
> *dev, u32 irqstatus,
>  	    is_lec_err(psr))
>  		work_done += m_can_handle_lec_err(dev, psr & LEC_UNUSED);
> 
> +	/* handle protocol errors in arbitration phase */
> +	if ((cdev->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING) &&
> +	    m_can_is_protocol_err(irqstatus))
> +		work_done += m_can_handle_protocol_error(dev, irqstatus);
> +
>  	/* other unproccessed error interrupts */
>  	m_can_handle_other_err(dev, irqstatus);
> 
> --
> 2.17.1

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH v2] can: m_can: add support for handling arbitration error
@ 2019-10-25  9:44     ` pankj.sharma
  0 siblings, 0 replies; 7+ messages in thread
From: pankj.sharma @ 2019-10-25  9:44 UTC (permalink / raw)
  To: linux-can, netdev, linux-kernel, wg, mkl
  Cc: davem, eugen.hristev, ludovic.desroches, pankaj.dubey, rcsekar,
	jhofstee, simon.horman, 'Sriram Dash'

Gentle Ping!

> From: Pankaj Sharma <pankj.sharma@samsung.com>
> Subject: [PATCH v2] can: m_can: add support for handling arbitration error
> 
> The Bosch MCAN hardware (3.1.0 and above) supports interrupt flag to detect
> Protocol error in arbitration phase.
> 
> Transmit error statistics is currently not updated from the MCAN driver.
> Protocol error in arbitration phase is a TX error and the network statistics should
> be updated accordingly.
> 
> The member "tx_error" of "struct net_device_stats" should be incremented as
> arbitration is a transmit protocol error. Also "arbitration_lost" of "struct
> can_device_stats" should be incremented to report arbitration lost.
> 
> Signed-off-by: Pankaj Sharma <pankj.sharma@samsung.com>
> Signed-off-by: Sriram Dash <sriram.dash@samsung.com>
> ---
> 
> changes in v2:
> - common m_can_ prefix for is_protocol_err function
> - handling stats even if the allocation of the skb fails
> - resolving build errors on net-next branch
> 
>  drivers/net/can/m_can/m_can.c | 37
> +++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index 75e7490c4299..a736297a875f 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -778,6 +778,38 @@ static inline bool is_lec_err(u32 psr)
>  	return psr && (psr != LEC_UNUSED);
>  }
> 
> +static inline bool m_can_is_protocol_err(u32 irqstatus) {
> +	return irqstatus & IR_ERR_LEC_31X;
> +}
> +
> +static int m_can_handle_protocol_error(struct net_device *dev, u32
> +irqstatus) {
> +	struct net_device_stats *stats = &dev->stats;
> +	struct m_can_classdev *cdev = netdev_priv(dev);
> +	struct can_frame *cf;
> +	struct sk_buff *skb;
> +
> +	/* propagate the error condition to the CAN stack */
> +	skb = alloc_can_err_skb(dev, &cf);
> +	if (unlikely(!skb)) {
> +		netdev_dbg(dev, "allocation of skb failed\n");
> +		stats->tx_errors++;
> +		return 0;
> +	}
> +	if (cdev->version >= 31 && (irqstatus & IR_PEA)) {
> +		netdev_dbg(dev, "Protocol error in Arbitration fail\n");
> +		stats->tx_errors++;
> +		cdev->can.can_stats.arbitration_lost++;
> +		cf->can_id |= CAN_ERR_LOSTARB;
> +		cf->data[0] |= CAN_ERR_LOSTARB_UNSPEC;
> +	}
> +
> +	netif_receive_skb(skb);
> +
> +	return 1;
> +}
> +
>  static int m_can_handle_bus_errors(struct net_device *dev, u32 irqstatus,
>  				   u32 psr)
>  {
> @@ -792,6 +824,11 @@ static int m_can_handle_bus_errors(struct net_device
> *dev, u32 irqstatus,
>  	    is_lec_err(psr))
>  		work_done += m_can_handle_lec_err(dev, psr & LEC_UNUSED);
> 
> +	/* handle protocol errors in arbitration phase */
> +	if ((cdev->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING) &&
> +	    m_can_is_protocol_err(irqstatus))
> +		work_done += m_can_handle_protocol_error(dev, irqstatus);
> +
>  	/* other unproccessed error interrupts */
>  	m_can_handle_other_err(dev, irqstatus);
> 
> --
> 2.17.1



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] can: m_can: add support for handling arbitration error
  2019-10-21 12:13 ` [PATCH v2] can: m_can: add support for handling arbitration error Pankaj Sharma
  2019-10-25  9:44     ` pankj.sharma
@ 2019-10-25 11:16   ` Simon Horman
  2019-10-29 14:23   ` Marc Kleine-Budde
  2 siblings, 0 replies; 7+ messages in thread
From: Simon Horman @ 2019-10-25 11:16 UTC (permalink / raw)
  To: Pankaj Sharma
  Cc: linux-can, netdev, linux-kernel, wg, mkl, davem, eugen.hristev,
	ludovic.desroches, pankaj.dubey, rcsekar, jhofstee, Sriram Dash

On Mon, Oct 21, 2019 at 05:43:36PM +0530, Pankaj Sharma wrote:
> The Bosch MCAN hardware (3.1.0 and above) supports interrupt flag to
> detect Protocol error in arbitration phase.
> 
> Transmit error statistics is currently not updated from the MCAN driver.
> Protocol error in arbitration phase is a TX error and the network
> statistics should be updated accordingly.
> 
> The member "tx_error" of "struct net_device_stats" should be incremented
> as arbitration is a transmit protocol error. Also "arbitration_lost" of
> "struct can_device_stats" should be incremented to report arbitration
> lost.
> 
> Signed-off-by: Pankaj Sharma <pankj.sharma@samsung.com>
> Signed-off-by: Sriram Dash <sriram.dash@samsung.com>
> ---
> 
> changes in v2:
> - common m_can_ prefix for is_protocol_err function
> - handling stats even if the allocation of the skb fails
> - resolving build errors on net-next branch

No objections from my side.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] can: m_can: add support for handling arbitration error
  2019-10-21 12:13 ` [PATCH v2] can: m_can: add support for handling arbitration error Pankaj Sharma
  2019-10-25  9:44     ` pankj.sharma
  2019-10-25 11:16   ` Simon Horman
@ 2019-10-29 14:23   ` Marc Kleine-Budde
  2019-10-30  5:55       ` pankj.sharma
  2 siblings, 1 reply; 7+ messages in thread
From: Marc Kleine-Budde @ 2019-10-29 14:23 UTC (permalink / raw)
  To: Pankaj Sharma, linux-can, netdev, linux-kernel
  Cc: wg, davem, eugen.hristev, ludovic.desroches, pankaj.dubey,
	rcsekar, jhofstee, simon.horman, Sriram Dash


[-- Attachment #1.1: Type: text/plain, Size: 3238 bytes --]

On 10/21/19 2:13 PM, Pankaj Sharma wrote:
> The Bosch MCAN hardware (3.1.0 and above) supports interrupt flag to
> detect Protocol error in arbitration phase.
> 
> Transmit error statistics is currently not updated from the MCAN driver.
> Protocol error in arbitration phase is a TX error and the network
> statistics should be updated accordingly.
> 
> The member "tx_error" of "struct net_device_stats" should be incremented
> as arbitration is a transmit protocol error. Also "arbitration_lost" of
> "struct can_device_stats" should be incremented to report arbitration
> lost.
> 
> Signed-off-by: Pankaj Sharma <pankj.sharma@samsung.com>
> Signed-off-by: Sriram Dash <sriram.dash@samsung.com>
> ---
> 
> changes in v2:
> - common m_can_ prefix for is_protocol_err function
> - handling stats even if the allocation of the skb fails
> - resolving build errors on net-next branch
> 
>  drivers/net/can/m_can/m_can.c | 37 +++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index 75e7490c4299..a736297a875f 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -778,6 +778,38 @@ static inline bool is_lec_err(u32 psr)
>  	return psr && (psr != LEC_UNUSED);
>  }
>  
> +static inline bool m_can_is_protocol_err(u32 irqstatus)
> +{
> +	return irqstatus & IR_ERR_LEC_31X;
> +}
> +
> +static int m_can_handle_protocol_error(struct net_device *dev, u32 irqstatus)
> +{
> +	struct net_device_stats *stats = &dev->stats;
> +	struct m_can_classdev *cdev = netdev_priv(dev);
> +	struct can_frame *cf;
> +	struct sk_buff *skb;
> +
> +	/* propagate the error condition to the CAN stack */
> +	skb = alloc_can_err_skb(dev, &cf);
> +	if (unlikely(!skb)) {
> +		netdev_dbg(dev, "allocation of skb failed\n");
> +		stats->tx_errors++;
> +		return 0;
> +	}
> +	if (cdev->version >= 31 && (irqstatus & IR_PEA)) {
> +		netdev_dbg(dev, "Protocol error in Arbitration fail\n");
> +		stats->tx_errors++;
> +		cdev->can.can_stats.arbitration_lost++;

If the skb allocation fails, you miss the stats here.

> +		cf->can_id |= CAN_ERR_LOSTARB;
> +		cf->data[0] |= CAN_ERR_LOSTARB_UNSPEC;
> +	}
> +
> +	netif_receive_skb(skb);
> +
> +	return 1;
> +}
> +
>  static int m_can_handle_bus_errors(struct net_device *dev, u32 irqstatus,
>  				   u32 psr)
>  {
> @@ -792,6 +824,11 @@ static int m_can_handle_bus_errors(struct net_device *dev, u32 irqstatus,
>  	    is_lec_err(psr))
>  		work_done += m_can_handle_lec_err(dev, psr & LEC_UNUSED);
>  
> +	/* handle protocol errors in arbitration phase */
> +	if ((cdev->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING) &&
> +	    m_can_is_protocol_err(irqstatus))
> +		work_done += m_can_handle_protocol_error(dev, irqstatus);
> +
>  	/* other unproccessed error interrupts */
>  	m_can_handle_other_err(dev, irqstatus);

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  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH v2] can: m_can: add support for handling arbitration error
  2019-10-29 14:23   ` Marc Kleine-Budde
@ 2019-10-30  5:55       ` pankj.sharma
  0 siblings, 0 replies; 7+ messages in thread
From: pankj.sharma @ 2019-10-30  5:55 UTC (permalink / raw)
  To: 'Marc Kleine-Budde'
  Cc: wg, davem, eugen.hristev, ludovic.desroches, pankaj.dubey,
	rcsekar, jhofstee, simon.horman, 'Sriram Dash',
	linux-can, netdev, linux-kernel

> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Subject: Re: [PATCH v2] can: m_can: add support for handling arbitration error
> 
> On 10/21/19 2:13 PM, Pankaj Sharma wrote:
> > The Bosch MCAN hardware (3.1.0 and above) supports interrupt flag to
> > detect Protocol error in arbitration phase.
> >
> > Transmit error statistics is currently not updated from the MCAN driver.
> > Protocol error in arbitration phase is a TX error and the network
> > statistics should be updated accordingly.
> >
> > The member "tx_error" of "struct net_device_stats" should be
> > incremented as arbitration is a transmit protocol error. Also
> > "arbitration_lost" of "struct can_device_stats" should be incremented
> > to report arbitration lost.
> >
> > Signed-off-by: Pankaj Sharma <pankj.sharma@samsung.com>
> > Signed-off-by: Sriram Dash <sriram.dash@samsung.com>
> > ---
> >
> > changes in v2:
> > - common m_can_ prefix for is_protocol_err function
> > - handling stats even if the allocation of the skb fails
> > - resolving build errors on net-next branch
> >
> >  drivers/net/can/m_can/m_can.c | 37
> > +++++++++++++++++++++++++++++++++++
> >  1 file changed, 37 insertions(+)
> >
> > diff --git a/drivers/net/can/m_can/m_can.c
> > b/drivers/net/can/m_can/m_can.c index 75e7490c4299..a736297a875f
> > 100644
> > --- a/drivers/net/can/m_can/m_can.c
> > +++ b/drivers/net/can/m_can/m_can.c
> > @@ -778,6 +778,38 @@ static inline bool is_lec_err(u32 psr)
> >  	return psr && (psr != LEC_UNUSED);
> >  }
> >
> > +static inline bool m_can_is_protocol_err(u32 irqstatus) {
> > +	return irqstatus & IR_ERR_LEC_31X;
> > +}
> > +
> > +static int m_can_handle_protocol_error(struct net_device *dev, u32
> > +irqstatus) {
> > +	struct net_device_stats *stats = &dev->stats;
> > +	struct m_can_classdev *cdev = netdev_priv(dev);
> > +	struct can_frame *cf;
> > +	struct sk_buff *skb;
> > +
> > +	/* propagate the error condition to the CAN stack */
> > +	skb = alloc_can_err_skb(dev, &cf);
> > +	if (unlikely(!skb)) {
> > +		netdev_dbg(dev, "allocation of skb failed\n");
> > +		stats->tx_errors++;
> > +		return 0;
> > +	}
> > +	if (cdev->version >= 31 && (irqstatus & IR_PEA)) {
> > +		netdev_dbg(dev, "Protocol error in Arbitration fail\n");
> > +		stats->tx_errors++;
> > +		cdev->can.can_stats.arbitration_lost++;
> 
> If the skb allocation fails, you miss the stats here.

Alright. We shall handle the stats even when skb fails. 
Shall post in upcoming revision.

> 
> > +		cf->can_id |= CAN_ERR_LOSTARB;
> > +		cf->data[0] |= CAN_ERR_LOSTARB_UNSPEC;
> > +	}
> > +
> > +	netif_receive_skb(skb);
> > +
> > +	return 1;
> > +}
> > +
> >  static int m_can_handle_bus_errors(struct net_device *dev, u32 irqstatus,
> >  				   u32 psr)
> >  {
> > @@ -792,6 +824,11 @@ static int m_can_handle_bus_errors(struct
> net_device *dev, u32 irqstatus,
> >  	    is_lec_err(psr))
> >  		work_done += m_can_handle_lec_err(dev, psr & LEC_UNUSED);
> >
> > +	/* handle protocol errors in arbitration phase */
> > +	if ((cdev->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING) &&
> > +	    m_can_is_protocol_err(irqstatus))
> > +		work_done += m_can_handle_protocol_error(dev, irqstatus);
> > +
> >  	/* other unproccessed error interrupts */
> >  	m_can_handle_other_err(dev, irqstatus);
> 
> 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=cb5c4e6130fb99cd.cb5dc52e-
> 7d616d604aa6cbcf&u=http://www.pengutronix.de   |

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH v2] can: m_can: add support for handling arbitration error
@ 2019-10-30  5:55       ` pankj.sharma
  0 siblings, 0 replies; 7+ messages in thread
From: pankj.sharma @ 2019-10-30  5:55 UTC (permalink / raw)
  To: 'Marc Kleine-Budde'
  Cc: wg, davem, eugen.hristev, ludovic.desroches, pankaj.dubey,
	rcsekar, jhofstee, simon.horman, 'Sriram Dash',
	linux-can, netdev, linux-kernel

> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Subject: Re: [PATCH v2] can: m_can: add support for handling arbitration error
> 
> On 10/21/19 2:13 PM, Pankaj Sharma wrote:
> > The Bosch MCAN hardware (3.1.0 and above) supports interrupt flag to
> > detect Protocol error in arbitration phase.
> >
> > Transmit error statistics is currently not updated from the MCAN driver.
> > Protocol error in arbitration phase is a TX error and the network
> > statistics should be updated accordingly.
> >
> > The member "tx_error" of "struct net_device_stats" should be
> > incremented as arbitration is a transmit protocol error. Also
> > "arbitration_lost" of "struct can_device_stats" should be incremented
> > to report arbitration lost.
> >
> > Signed-off-by: Pankaj Sharma <pankj.sharma@samsung.com>
> > Signed-off-by: Sriram Dash <sriram.dash@samsung.com>
> > ---
> >
> > changes in v2:
> > - common m_can_ prefix for is_protocol_err function
> > - handling stats even if the allocation of the skb fails
> > - resolving build errors on net-next branch
> >
> >  drivers/net/can/m_can/m_can.c | 37
> > +++++++++++++++++++++++++++++++++++
> >  1 file changed, 37 insertions(+)
> >
> > diff --git a/drivers/net/can/m_can/m_can.c
> > b/drivers/net/can/m_can/m_can.c index 75e7490c4299..a736297a875f
> > 100644
> > --- a/drivers/net/can/m_can/m_can.c
> > +++ b/drivers/net/can/m_can/m_can.c
> > @@ -778,6 +778,38 @@ static inline bool is_lec_err(u32 psr)
> >  	return psr && (psr != LEC_UNUSED);
> >  }
> >
> > +static inline bool m_can_is_protocol_err(u32 irqstatus) {
> > +	return irqstatus & IR_ERR_LEC_31X;
> > +}
> > +
> > +static int m_can_handle_protocol_error(struct net_device *dev, u32
> > +irqstatus) {
> > +	struct net_device_stats *stats = &dev->stats;
> > +	struct m_can_classdev *cdev = netdev_priv(dev);
> > +	struct can_frame *cf;
> > +	struct sk_buff *skb;
> > +
> > +	/* propagate the error condition to the CAN stack */
> > +	skb = alloc_can_err_skb(dev, &cf);
> > +	if (unlikely(!skb)) {
> > +		netdev_dbg(dev, "allocation of skb failed\n");
> > +		stats->tx_errors++;
> > +		return 0;
> > +	}
> > +	if (cdev->version >= 31 && (irqstatus & IR_PEA)) {
> > +		netdev_dbg(dev, "Protocol error in Arbitration fail\n");
> > +		stats->tx_errors++;
> > +		cdev->can.can_stats.arbitration_lost++;
> 
> If the skb allocation fails, you miss the stats here.

Alright. We shall handle the stats even when skb fails. 
Shall post in upcoming revision.

> 
> > +		cf->can_id |= CAN_ERR_LOSTARB;
> > +		cf->data[0] |= CAN_ERR_LOSTARB_UNSPEC;
> > +	}
> > +
> > +	netif_receive_skb(skb);
> > +
> > +	return 1;
> > +}
> > +
> >  static int m_can_handle_bus_errors(struct net_device *dev, u32 irqstatus,
> >  				   u32 psr)
> >  {
> > @@ -792,6 +824,11 @@ static int m_can_handle_bus_errors(struct
> net_device *dev, u32 irqstatus,
> >  	    is_lec_err(psr))
> >  		work_done += m_can_handle_lec_err(dev, psr & LEC_UNUSED);
> >
> > +	/* handle protocol errors in arbitration phase */
> > +	if ((cdev->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING) &&
> > +	    m_can_is_protocol_err(irqstatus))
> > +		work_done += m_can_handle_protocol_error(dev, irqstatus);
> > +
> >  	/* other unproccessed error interrupts */
> >  	m_can_handle_other_err(dev, irqstatus);
> 
> 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=cb5c4e6130fb99cd.cb5dc52e-
> 7d616d604aa6cbcf&u=http://www.pengutronix.de   |



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-10-30  5:55 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20191021121350epcas5p3313e54a3bc5c8600c52a6db299893f78@epcas5p3.samsung.com>
2019-10-21 12:13 ` [PATCH v2] can: m_can: add support for handling arbitration error Pankaj Sharma
2019-10-25  9:44   ` pankj.sharma
2019-10-25  9:44     ` pankj.sharma
2019-10-25 11:16   ` Simon Horman
2019-10-29 14:23   ` Marc Kleine-Budde
2019-10-30  5:55     ` pankj.sharma
2019-10-30  5:55       ` pankj.sharma

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.