All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 4/4] can: flexcan: Consolidate and unify state change handling
@ 2014-09-26 17:31 Andri Yngvason
  2014-11-25 21:03 ` Wolfgang Grandegger
  0 siblings, 1 reply; 4+ messages in thread
From: Andri Yngvason @ 2014-09-26 17:31 UTC (permalink / raw)
  To: linux-can; +Cc: mkl, wg

Replacing error state change handling with the new mechanism.

Changes made since last proposal:
can: flexcan: improve rx/tx state recognition.

Note: Flexcan is problematic because there is no interrupt generated when
transitioning between error-active and error-passive. If the bus is
disconnected, there is no event that will trigger a napi poll, so the new state
will not be discovered until the bus is connected. This problem goes away if
berr-reporting is enabled, because then the ack-error takes care of it.

Another problem is that if nothing is happening on the bus, when it's connected
again, we won't see the new state until someone transmits something on the bus.

I'd say that these are minor issues because a "healthy" bus has traffic and the
"disconnected bus" issue has a work-around.

Signed-off-by: Andri Yngvason <andri.yngvason@marel.com>
---
 drivers/net/can/flexcan.c | 101 +++++++++-------------------------------------
 1 file changed, 18 insertions(+), 83 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 60f86bd..e7dea64 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -577,98 +577,30 @@ static int flexcan_poll_bus_err(struct net_device *dev, u32 reg_esr)
 	return 1;
 }
 
-static void do_state(struct net_device *dev,
-		     struct can_frame *cf, enum can_state new_state)
-{
-	struct flexcan_priv *priv = netdev_priv(dev);
-	struct can_berr_counter bec;
-
-	__flexcan_get_berr_counter(dev, &bec);
-
-	switch (priv->can.state) {
-	case CAN_STATE_ERROR_ACTIVE:
-		/*
-		 * from: ERROR_ACTIVE
-		 * to  : ERROR_WARNING, ERROR_PASSIVE, BUS_OFF
-		 * =>  : there was a warning int
-		 */
-		if (new_state >= CAN_STATE_ERROR_WARNING &&
-		    new_state <= CAN_STATE_BUS_OFF) {
-			netdev_dbg(dev, "Error Warning IRQ\n");
-			priv->can.can_stats.error_warning++;
-
-			cf->can_id |= CAN_ERR_CRTL;
-			cf->data[1] = (bec.txerr > bec.rxerr) ?
-				CAN_ERR_CRTL_TX_WARNING :
-				CAN_ERR_CRTL_RX_WARNING;
-		}
-	case CAN_STATE_ERROR_WARNING:	/* fallthrough */
-		/*
-		 * from: ERROR_ACTIVE, ERROR_WARNING
-		 * to  : ERROR_PASSIVE, BUS_OFF
-		 * =>  : error passive int
-		 */
-		if (new_state >= CAN_STATE_ERROR_PASSIVE &&
-		    new_state <= CAN_STATE_BUS_OFF) {
-			netdev_dbg(dev, "Error Passive IRQ\n");
-			priv->can.can_stats.error_passive++;
-
-			cf->can_id |= CAN_ERR_CRTL;
-			cf->data[1] = (bec.txerr > bec.rxerr) ?
-				CAN_ERR_CRTL_TX_PASSIVE :
-				CAN_ERR_CRTL_RX_PASSIVE;
-		}
-		break;
-	case CAN_STATE_BUS_OFF:
-		netdev_err(dev, "BUG! "
-			   "hardware recovered automatically from BUS_OFF\n");
-		break;
-	default:
-		break;
-	}
-
-	/* process state changes depending on the new state */
-	switch (new_state) {
-	case CAN_STATE_ERROR_WARNING:
-		netdev_dbg(dev, "Error Warning\n");
-		cf->can_id |= CAN_ERR_CRTL;
-		cf->data[1] = (bec.txerr > bec.rxerr) ?
-			CAN_ERR_CRTL_TX_WARNING :
-			CAN_ERR_CRTL_RX_WARNING;
-		break;
-	case CAN_STATE_ERROR_ACTIVE:
-		netdev_dbg(dev, "Error Active\n");
-		cf->can_id |= CAN_ERR_PROT;
-		cf->data[2] = CAN_ERR_PROT_ACTIVE;
-		break;
-	case CAN_STATE_BUS_OFF:
-		cf->can_id |= CAN_ERR_BUSOFF;
-		can_bus_off(dev);
-		break;
-	default:
-		break;
-	}
-}
-
 static int flexcan_poll_state(struct net_device *dev, u32 reg_esr)
 {
 	struct flexcan_priv *priv = netdev_priv(dev);
 	struct sk_buff *skb;
 	struct can_frame *cf;
-	enum can_state new_state;
+	enum can_state new_state = 0, rx_state = 0, tx_state = 0;
 	int flt;
+	struct can_berr_counter bec;
 
 	flt = reg_esr & FLEXCAN_ESR_FLT_CONF_MASK;
 	if (likely(flt == FLEXCAN_ESR_FLT_CONF_ACTIVE)) {
-		if (likely(!(reg_esr & (FLEXCAN_ESR_TX_WRN |
-					FLEXCAN_ESR_RX_WRN))))
-			new_state = CAN_STATE_ERROR_ACTIVE;
-		else
-			new_state = CAN_STATE_ERROR_WARNING;
-	} else if (unlikely(flt == FLEXCAN_ESR_FLT_CONF_PASSIVE))
+		tx_state = unlikely(reg_esr & FLEXCAN_ESR_TX_WRN) ?
+			   CAN_STATE_ERROR_WARNING : CAN_STATE_ERROR_ACTIVE;
+		rx_state = unlikely(reg_esr & FLEXCAN_ESR_RX_WRN) ?
+			   CAN_STATE_ERROR_WARNING : CAN_STATE_ERROR_ACTIVE;
+		new_state = max(tx_state, rx_state);
+	} else if (unlikely(flt == FLEXCAN_ESR_FLT_CONF_PASSIVE)) {
+		__flexcan_get_berr_counter(dev, &bec);
 		new_state = CAN_STATE_ERROR_PASSIVE;
-	else
+		rx_state = bec.rxerr >= bec.txerr ? new_state : 0;
+		tx_state = bec.rxerr <= bec.txerr ? new_state : 0;
+	} else {
 		new_state = CAN_STATE_BUS_OFF;
+	}
 
 	/* state hasn't changed */
 	if (likely(new_state == priv->can.state))
@@ -678,8 +610,11 @@ static int flexcan_poll_state(struct net_device *dev, u32 reg_esr)
 	if (unlikely(!skb))
 		return 0;
 
-	do_state(dev, cf, new_state);
-	priv->can.state = new_state;
+	can_change_state(dev, cf, new_state, tx_state, rx_state);
+
+	if (unlikely(new_state == CAN_STATE_BUS_OFF))
+		can_bus_off(dev);
+
 	netif_receive_skb(skb);
 
 	dev->stats.rx_packets++;
-- 
1.9.1


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

* Re: [PATCH v3 4/4] can: flexcan: Consolidate and unify state change handling
  2014-09-26 17:31 [PATCH v3 4/4] can: flexcan: Consolidate and unify state change handling Andri Yngvason
@ 2014-11-25 21:03 ` Wolfgang Grandegger
  2014-11-26 11:06   ` Andri Yngvason
  0 siblings, 1 reply; 4+ messages in thread
From: Wolfgang Grandegger @ 2014-11-25 21:03 UTC (permalink / raw)
  To: Andri Yngvason, linux-can; +Cc: mkl

On 09/26/2014 07:31 PM, Andri Yngvason wrote:
> Replacing error state change handling with the new mechanism.
> 
> Changes made since last proposal:
> can: flexcan: improve rx/tx state recognition.

Move below "---", please.

> Note: Flexcan is problematic because there is no interrupt generated when
> transitioning between error-active and error-passive. If the bus is
> disconnected, there is no event that will trigger a napi poll, so the new state
> will not be discovered until the bus is connected. This problem goes away if
> berr-reporting is enabled, because then the ack-error takes care of it.

Well, this is a known issue and should be addressed via platform data
(using the FLEXCAN_HAS_BROKEN_ERR_STATE flags).

> 
> Another problem is that if nothing is happening on the bus, when it's connected
> again, we won't see the new state until someone transmits something on the bus.
> 
> I'd say that these are minor issues because a "healthy" bus has traffic and the
> "disconnected bus" issue has a work-around.

Would be nice to do some tests on a more recent Flexcan core as well.

> 
> Signed-off-by: Andri Yngvason <andri.yngvason@marel.com>
> ---
>  drivers/net/can/flexcan.c | 101 +++++++++-------------------------------------
>  1 file changed, 18 insertions(+), 83 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 60f86bd..e7dea64 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -577,98 +577,30 @@ static int flexcan_poll_bus_err(struct net_device *dev, u32 reg_esr)
>  	return 1;
>  }
>  
> -static void do_state(struct net_device *dev,
> -		     struct can_frame *cf, enum can_state new_state)
> -{
> -	struct flexcan_priv *priv = netdev_priv(dev);
> -	struct can_berr_counter bec;
> -
> -	__flexcan_get_berr_counter(dev, &bec);
> -
> -	switch (priv->can.state) {
> -	case CAN_STATE_ERROR_ACTIVE:
> -		/*
> -		 * from: ERROR_ACTIVE
> -		 * to  : ERROR_WARNING, ERROR_PASSIVE, BUS_OFF
> -		 * =>  : there was a warning int
> -		 */
> -		if (new_state >= CAN_STATE_ERROR_WARNING &&
> -		    new_state <= CAN_STATE_BUS_OFF) {
> -			netdev_dbg(dev, "Error Warning IRQ\n");
> -			priv->can.can_stats.error_warning++;
> -
> -			cf->can_id |= CAN_ERR_CRTL;
> -			cf->data[1] = (bec.txerr > bec.rxerr) ?
> -				CAN_ERR_CRTL_TX_WARNING :
> -				CAN_ERR_CRTL_RX_WARNING;
> -		}
> -	case CAN_STATE_ERROR_WARNING:	/* fallthrough */
> -		/*
> -		 * from: ERROR_ACTIVE, ERROR_WARNING
> -		 * to  : ERROR_PASSIVE, BUS_OFF
> -		 * =>  : error passive int
> -		 */
> -		if (new_state >= CAN_STATE_ERROR_PASSIVE &&
> -		    new_state <= CAN_STATE_BUS_OFF) {
> -			netdev_dbg(dev, "Error Passive IRQ\n");
> -			priv->can.can_stats.error_passive++;
> -
> -			cf->can_id |= CAN_ERR_CRTL;
> -			cf->data[1] = (bec.txerr > bec.rxerr) ?
> -				CAN_ERR_CRTL_TX_PASSIVE :
> -				CAN_ERR_CRTL_RX_PASSIVE;
> -		}
> -		break;
> -	case CAN_STATE_BUS_OFF:
> -		netdev_err(dev, "BUG! "
> -			   "hardware recovered automatically from BUS_OFF\n");
> -		break;
> -	default:
> -		break;
> -	}
> -
> -	/* process state changes depending on the new state */
> -	switch (new_state) {
> -	case CAN_STATE_ERROR_WARNING:
> -		netdev_dbg(dev, "Error Warning\n");
> -		cf->can_id |= CAN_ERR_CRTL;
> -		cf->data[1] = (bec.txerr > bec.rxerr) ?
> -			CAN_ERR_CRTL_TX_WARNING :
> -			CAN_ERR_CRTL_RX_WARNING;
> -		break;
> -	case CAN_STATE_ERROR_ACTIVE:
> -		netdev_dbg(dev, "Error Active\n");
> -		cf->can_id |= CAN_ERR_PROT;
> -		cf->data[2] = CAN_ERR_PROT_ACTIVE;
> -		break;
> -	case CAN_STATE_BUS_OFF:
> -		cf->can_id |= CAN_ERR_BUSOFF;
> -		can_bus_off(dev);
> -		break;
> -	default:
> -		break;
> -	}
> -}
> -
>  static int flexcan_poll_state(struct net_device *dev, u32 reg_esr)
>  {
>  	struct flexcan_priv *priv = netdev_priv(dev);
>  	struct sk_buff *skb;
>  	struct can_frame *cf;
> -	enum can_state new_state;
> +	enum can_state new_state = 0, rx_state = 0, tx_state = 0;
>  	int flt;
> +	struct can_berr_counter bec;
>  
>  	flt = reg_esr & FLEXCAN_ESR_FLT_CONF_MASK;
>  	if (likely(flt == FLEXCAN_ESR_FLT_CONF_ACTIVE)) {
> -		if (likely(!(reg_esr & (FLEXCAN_ESR_TX_WRN |
> -					FLEXCAN_ESR_RX_WRN))))
> -			new_state = CAN_STATE_ERROR_ACTIVE;
> -		else
> -			new_state = CAN_STATE_ERROR_WARNING;
> -	} else if (unlikely(flt == FLEXCAN_ESR_FLT_CONF_PASSIVE))
> +		tx_state = unlikely(reg_esr & FLEXCAN_ESR_TX_WRN) ?
> +			   CAN_STATE_ERROR_WARNING : CAN_STATE_ERROR_ACTIVE;
> +		rx_state = unlikely(reg_esr & FLEXCAN_ESR_RX_WRN) ?
> +			   CAN_STATE_ERROR_WARNING : CAN_STATE_ERROR_ACTIVE;
> +		new_state = max(tx_state, rx_state);
> +	} else if (unlikely(flt == FLEXCAN_ESR_FLT_CONF_PASSIVE)) {
> +		__flexcan_get_berr_counter(dev, &bec);
>  		new_state = CAN_STATE_ERROR_PASSIVE;
> -	else
> +		rx_state = bec.rxerr >= bec.txerr ? new_state : 0;
> +		tx_state = bec.rxerr <= bec.txerr ? new_state : 0;
> +	} else {
>  		new_state = CAN_STATE_BUS_OFF;
> +	}
>  
>  	/* state hasn't changed */
>  	if (likely(new_state == priv->can.state))
> @@ -678,8 +610,11 @@ static int flexcan_poll_state(struct net_device *dev, u32 reg_esr)
>  	if (unlikely(!skb))
>  		return 0;
>  
> -	do_state(dev, cf, new_state);
> -	priv->can.state = new_state;

On SJA1000 and MSCAN you use:

	if (new_state != priv->can.state) ...

> +	can_change_state(dev, cf, new_state, tx_state, rx_state);
> +
> +	if (unlikely(new_state == CAN_STATE_BUS_OFF))
> +		can_bus_off(dev);
> +
>  	netif_receive_skb(skb);
>  
>  	dev->stats.rx_packets++;
> 


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

* Re: [PATCH v3 4/4] can: flexcan: Consolidate and unify state change handling
  2014-11-25 21:03 ` Wolfgang Grandegger
@ 2014-11-26 11:06   ` Andri Yngvason
  2014-11-26 11:28     ` Wolfgang Grandegger
  0 siblings, 1 reply; 4+ messages in thread
From: Andri Yngvason @ 2014-11-26 11:06 UTC (permalink / raw)
  To: Wolfgang Grandegger, linux-can; +Cc: mkl

Quoting Wolfgang Grandegger (2014-11-25 21:03:24)
> On 09/26/2014 07:31 PM, Andri Yngvason wrote:
> > Replacing error state change handling with the new mechanism.
> > 
> > Changes made since last proposal:
> > can: flexcan: improve rx/tx state recognition.
> 
> Move below "---", please.
> 
> > Note: Flexcan is problematic because there is no interrupt generated when
> > transitioning between error-active and error-passive. If the bus is
> > disconnected, there is no event that will trigger a napi poll, so the new state
> > will not be discovered until the bus is connected. This problem goes away if
> > berr-reporting is enabled, because then the ack-error takes care of it.
> 
> Well, this is a known issue and should be addressed via platform data
> (using the FLEXCAN_HAS_BROKEN_ERR_STATE flags).
>
I thought that was about something else.
> 
> > 
> > Another problem is that if nothing is happening on the bus, when it's connected
> > again, we won't see the new state until someone transmits something on the bus.
> > 
> > I'd say that these are minor issues because a "healthy" bus has traffic and the
> > "disconnected bus" issue has a work-around.
> 
> Would be nice to do some tests on a more recent Flexcan core as well.
> 
Isn't imx6 fairly recent?
>
> > 
> > Signed-off-by: Andri Yngvason <andri.yngvason@marel.com>
> > ---
> >  drivers/net/can/flexcan.c | 101 +++++++++-------------------------------------
> >  1 file changed, 18 insertions(+), 83 deletions(-)
> > 
> > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> > index 60f86bd..e7dea64 100644
> > --- a/drivers/net/can/flexcan.c
> > +++ b/drivers/net/can/flexcan.c
> > @@ -577,98 +577,30 @@ static int flexcan_poll_bus_err(struct net_device *dev, u32 reg_esr)
> >       return 1;
> >  }
> >  
> > -static void do_state(struct net_device *dev,
> > -                  struct can_frame *cf, enum can_state new_state)
> > -{
> > -     struct flexcan_priv *priv = netdev_priv(dev);
> > -     struct can_berr_counter bec;
> > -
> > -     __flexcan_get_berr_counter(dev, &bec);
> > -
> > -     switch (priv->can.state) {
> > -     case CAN_STATE_ERROR_ACTIVE:
> > -             /*
> > -              * from: ERROR_ACTIVE
> > -              * to  : ERROR_WARNING, ERROR_PASSIVE, BUS_OFF
> > -              * =>  : there was a warning int
> > -              */
> > -             if (new_state >= CAN_STATE_ERROR_WARNING &&
> > -                 new_state <= CAN_STATE_BUS_OFF) {
> > -                     netdev_dbg(dev, "Error Warning IRQ\n");
> > -                     priv->can.can_stats.error_warning++;
> > -
> > -                     cf->can_id |= CAN_ERR_CRTL;
> > -                     cf->data[1] = (bec.txerr > bec.rxerr) ?
> > -                             CAN_ERR_CRTL_TX_WARNING :
> > -                             CAN_ERR_CRTL_RX_WARNING;
> > -             }
> > -     case CAN_STATE_ERROR_WARNING:   /* fallthrough */
> > -             /*
> > -              * from: ERROR_ACTIVE, ERROR_WARNING
> > -              * to  : ERROR_PASSIVE, BUS_OFF
> > -              * =>  : error passive int
> > -              */
> > -             if (new_state >= CAN_STATE_ERROR_PASSIVE &&
> > -                 new_state <= CAN_STATE_BUS_OFF) {
> > -                     netdev_dbg(dev, "Error Passive IRQ\n");
> > -                     priv->can.can_stats.error_passive++;
> > -
> > -                     cf->can_id |= CAN_ERR_CRTL;
> > -                     cf->data[1] = (bec.txerr > bec.rxerr) ?
> > -                             CAN_ERR_CRTL_TX_PASSIVE :
> > -                             CAN_ERR_CRTL_RX_PASSIVE;
> > -             }
> > -             break;
> > -     case CAN_STATE_BUS_OFF:
> > -             netdev_err(dev, "BUG! "
> > -                        "hardware recovered automatically from BUS_OFF\n");
> > -             break;
> > -     default:
> > -             break;
> > -     }
> > -
> > -     /* process state changes depending on the new state */
> > -     switch (new_state) {
> > -     case CAN_STATE_ERROR_WARNING:
> > -             netdev_dbg(dev, "Error Warning\n");
> > -             cf->can_id |= CAN_ERR_CRTL;
> > -             cf->data[1] = (bec.txerr > bec.rxerr) ?
> > -                     CAN_ERR_CRTL_TX_WARNING :
> > -                     CAN_ERR_CRTL_RX_WARNING;
> > -             break;
> > -     case CAN_STATE_ERROR_ACTIVE:
> > -             netdev_dbg(dev, "Error Active\n");
> > -             cf->can_id |= CAN_ERR_PROT;
> > -             cf->data[2] = CAN_ERR_PROT_ACTIVE;
> > -             break;
> > -     case CAN_STATE_BUS_OFF:
> > -             cf->can_id |= CAN_ERR_BUSOFF;
> > -             can_bus_off(dev);
> > -             break;
> > -     default:
> > -             break;
> > -     }
> > -}
> > -
> >  static int flexcan_poll_state(struct net_device *dev, u32 reg_esr)
> >  {
> >       struct flexcan_priv *priv = netdev_priv(dev);
> >       struct sk_buff *skb;
> >       struct can_frame *cf;
> > -     enum can_state new_state;
> > +     enum can_state new_state = 0, rx_state = 0, tx_state = 0;
> >       int flt;
> > +     struct can_berr_counter bec;
> >  
> >       flt = reg_esr & FLEXCAN_ESR_FLT_CONF_MASK;
> >       if (likely(flt == FLEXCAN_ESR_FLT_CONF_ACTIVE)) {
> > -             if (likely(!(reg_esr & (FLEXCAN_ESR_TX_WRN |
> > -                                     FLEXCAN_ESR_RX_WRN))))
> > -                     new_state = CAN_STATE_ERROR_ACTIVE;
> > -             else
> > -                     new_state = CAN_STATE_ERROR_WARNING;
> > -     } else if (unlikely(flt == FLEXCAN_ESR_FLT_CONF_PASSIVE))
> > +             tx_state = unlikely(reg_esr & FLEXCAN_ESR_TX_WRN) ?
> > +                        CAN_STATE_ERROR_WARNING : CAN_STATE_ERROR_ACTIVE;
> > +             rx_state = unlikely(reg_esr & FLEXCAN_ESR_RX_WRN) ?
> > +                        CAN_STATE_ERROR_WARNING : CAN_STATE_ERROR_ACTIVE;
> > +             new_state = max(tx_state, rx_state);
> > +     } else if (unlikely(flt == FLEXCAN_ESR_FLT_CONF_PASSIVE)) {
> > +             __flexcan_get_berr_counter(dev, &bec);
> >               new_state = CAN_STATE_ERROR_PASSIVE;
> > -     else
> > +             rx_state = bec.rxerr >= bec.txerr ? new_state : 0;
> > +             tx_state = bec.rxerr <= bec.txerr ? new_state : 0;
> > +     } else {
> >               new_state = CAN_STATE_BUS_OFF;
> > +     }
> >  
> >       /* state hasn't changed */
> >       if (likely(new_state == priv->can.state))
> > @@ -678,8 +610,11 @@ static int flexcan_poll_state(struct net_device *dev, u32 reg_esr)
> >       if (unlikely(!skb))
> >               return 0;
> >  
> > -     do_state(dev, cf, new_state);
> > -     priv->can.state = new_state;
> 
> On SJA1000 and MSCAN you use:
> 
>         if (new_state != priv->can.state) ...
>
That's because they had that before. I was trying not to make more changes than
were needed.

Also, this makes sense with regard to not feeding the frame if there was no
change.
>
> > +     can_change_state(dev, cf, new_state, tx_state, rx_state);
> > +
> > +     if (unlikely(new_state == CAN_STATE_BUS_OFF))
> > +             can_bus_off(dev);
> > +
> >       netif_receive_skb(skb);
> >  
> >       dev->stats.rx_packets++;
> >

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

* Re: [PATCH v3 4/4] can: flexcan: Consolidate and unify state change  handling
  2014-11-26 11:06   ` Andri Yngvason
@ 2014-11-26 11:28     ` Wolfgang Grandegger
  0 siblings, 0 replies; 4+ messages in thread
From: Wolfgang Grandegger @ 2014-11-26 11:28 UTC (permalink / raw)
  To: Andri Yngvason; +Cc: linux-can, mkl

On Wed, 26 Nov 2014 11:06:49 +0000, Andri Yngvason
<andri.yngvason@marel.com> wrote:
> Quoting Wolfgang Grandegger (2014-11-25 21:03:24)
>> On 09/26/2014 07:31 PM, Andri Yngvason wrote:
>> > Replacing error state change handling with the new mechanism.
>> > 
>> > Changes made since last proposal:
>> > can: flexcan: improve rx/tx state recognition.
>> 
>> Move below "---", please.
>> 
>> > Note: Flexcan is problematic because there is no interrupt generated
>> > when
>> > transitioning between error-active and error-passive. If the bus is
>> > disconnected, there is no event that will trigger a napi poll, so the
>> > new state
>> > will not be discovered until the bus is connected. This problem goes
>> > away if
>> > berr-reporting is enabled, because then the ack-error takes care of
it.
>> 
>> Well, this is a known issue and should be addressed via platform data
>> (using the FLEXCAN_HAS_BROKEN_ERR_STATE flags).
>>
> I thought that was about something else.
>> 
>> > 
>> > Another problem is that if nothing is happening on the bus, when it's
>> > connected
>> > again, we won't see the new state until someone transmits something
on
>> > the bus.
>> > 
>> > I'd say that these are minor issues because a "healthy" bus has
>> > traffic and the
>> > "disconnected bus" issue has a work-around.
>> 
>> Would be nice to do some tests on a more recent Flexcan core as well.
>> 
> Isn't imx6 fairly recent?

Yes it is and I'm surprised (puzzled) that it shows that behaviour.
Anyway,
just hack the driver using FLEXCAN_HAS_BROKEN_ERR_STATE to see if it
helps.

Wolfgang.

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

end of thread, other threads:[~2014-11-26 11:28 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-26 17:31 [PATCH v3 4/4] can: flexcan: Consolidate and unify state change handling Andri Yngvason
2014-11-25 21:03 ` Wolfgang Grandegger
2014-11-26 11:06   ` Andri Yngvason
2014-11-26 11:28     ` Wolfgang Grandegger

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.