* [PATCH v4 6/6] can: handle bus-off in can_change_state()
@ 2014-11-28 12:12 Andri Yngvason
2014-11-28 12:21 ` Wolfgang Grandegger
0 siblings, 1 reply; 3+ messages in thread
From: Andri Yngvason @ 2014-11-28 12:12 UTC (permalink / raw)
To: linux-can; +Cc: mkl, wg
Signed-off-by: Andri Yngvason <andri.yngvason@marel.com>
---
drivers/net/can/dev.c | 1 +
drivers/net/can/flexcan.c | 3 ---
drivers/net/can/mscan/mscan.c | 11 +++++------
drivers/net/can/sja1000/sja1000.c | 3 ---
4 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 3e2da66..17fce72 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -340,6 +340,7 @@ void can_change_state(struct net_device *dev, struct can_frame *cf,
if (unlikely(new_state == CAN_STATE_BUS_OFF)) {
cf->can_id |= CAN_ERR_BUSOFF;
+ can_bus_off(dev);
return;
}
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 9f91735..0823956 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -612,9 +612,6 @@ static int flexcan_poll_state(struct net_device *dev, u32 reg_esr)
can_change_state(dev, cf, tx_state, rx_state);
- if (unlikely(new_state == CAN_STATE_BUS_OFF))
- can_bus_off(dev);
-
netif_receive_skb(skb);
dev->stats.rx_packets++;
diff --git a/drivers/net/can/mscan/mscan.c b/drivers/net/can/mscan/mscan.c
index e36b740..2eb84f1 100644
--- a/drivers/net/can/mscan/mscan.c
+++ b/drivers/net/can/mscan/mscan.c
@@ -362,11 +362,7 @@ static void mscan_get_err_frame(struct net_device *dev, struct can_frame *frame,
new_state = get_new_state(dev, canrflg);
if (new_state != priv->can.state) {
- can_change_state(dev, frame,
- state_map[MSCAN_STATE_TX(canrflg)],
- state_map[MSCAN_STATE_RX(canrflg)]);
-
- if (priv->can.state == CAN_STATE_BUS_OFF) {
+ if (new_state == CAN_STATE_BUS_OFF) {
/*
* The MSCAN on the MPC5200 does recover from bus-off
* automatically. To avoid that we stop the chip doing
@@ -378,8 +374,11 @@ static void mscan_get_err_frame(struct net_device *dev, struct can_frame *frame,
setbits8(®s->canctl0,
MSCAN_SLPRQ | MSCAN_INITRQ);
}
- can_bus_off(dev);
}
+
+ can_change_state(dev, frame,
+ state_map[MSCAN_STATE_TX(canrflg)],
+ state_map[MSCAN_STATE_RX(canrflg)]);
}
priv->shadow_statflg = canrflg & MSCAN_STAT_MSK;
frame->can_dlc = CAN_ERR_DLC;
diff --git a/drivers/net/can/sja1000/sja1000.c b/drivers/net/can/sja1000/sja1000.c
index 32bd7f4..37b6392 100644
--- a/drivers/net/can/sja1000/sja1000.c
+++ b/drivers/net/can/sja1000/sja1000.c
@@ -479,9 +479,6 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
rx_state = txerr <= rxerr ? state : 0;
can_change_state(dev, cf, tx_state, rx_state);
-
- if(state == CAN_STATE_BUS_OFF)
- can_bus_off(dev);
}
netif_rx(skb);
--
1.9.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v4 6/6] can: handle bus-off in can_change_state()
2014-11-28 12:12 [PATCH v4 6/6] can: handle bus-off in can_change_state() Andri Yngvason
@ 2014-11-28 12:21 ` Wolfgang Grandegger
2014-11-28 12:33 ` Andri Yngvason
0 siblings, 1 reply; 3+ messages in thread
From: Wolfgang Grandegger @ 2014-11-28 12:21 UTC (permalink / raw)
To: Andri Yngvason; +Cc: linux-can, mkl
On Fri, 28 Nov 2014 12:12:28 +0000, Andri Yngvason
<andri.yngvason@marel.com> wrote:
> Signed-off-by: Andri Yngvason <andri.yngvason@marel.com>
> ---
> drivers/net/can/dev.c | 1 +
> drivers/net/can/flexcan.c | 3 ---
> drivers/net/can/mscan/mscan.c | 11 +++++------
> drivers/net/can/sja1000/sja1000.c | 3 ---
> 4 files changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
> index 3e2da66..17fce72 100644
> --- a/drivers/net/can/dev.c
> +++ b/drivers/net/can/dev.c
> @@ -340,6 +340,7 @@ void can_change_state(struct net_device *dev, struct
> can_frame *cf,
>
> if (unlikely(new_state == CAN_STATE_BUS_OFF)) {
> cf->can_id |= CAN_ERR_BUSOFF;
> + can_bus_off(dev);
> return;
> }
>
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 9f91735..0823956 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -612,9 +612,6 @@ static int flexcan_poll_state(struct net_device
*dev,
> u32 reg_esr)
>
> can_change_state(dev, cf, tx_state, rx_state);
>
> - if (unlikely(new_state == CAN_STATE_BUS_OFF))
> - can_bus_off(dev);
> -
> netif_receive_skb(skb);
>
> dev->stats.rx_packets++;
> diff --git a/drivers/net/can/mscan/mscan.c
b/drivers/net/can/mscan/mscan.c
> index e36b740..2eb84f1 100644
> --- a/drivers/net/can/mscan/mscan.c
> +++ b/drivers/net/can/mscan/mscan.c
> @@ -362,11 +362,7 @@ static void mscan_get_err_frame(struct net_device
> *dev, struct can_frame *frame,
>
> new_state = get_new_state(dev, canrflg);
> if (new_state != priv->can.state) {
> - can_change_state(dev, frame,
> - state_map[MSCAN_STATE_TX(canrflg)],
> - state_map[MSCAN_STATE_RX(canrflg)]);
> -
> - if (priv->can.state == CAN_STATE_BUS_OFF) {
> + if (new_state == CAN_STATE_BUS_OFF) {
> /*
> * The MSCAN on the MPC5200 does recover from bus-off
> * automatically. To avoid that we stop the chip doing
> @@ -378,8 +374,11 @@ static void mscan_get_err_frame(struct net_device
> *dev, struct can_frame *frame,
> setbits8(®s->canctl0,
> MSCAN_SLPRQ | MSCAN_INITRQ);
> }
> - can_bus_off(dev);
> }
> +
> + can_change_state(dev, frame,
> + state_map[MSCAN_STATE_TX(canrflg)],
> + state_map[MSCAN_STATE_RX(canrflg)]);
> }
> priv->shadow_statflg = canrflg & MSCAN_STAT_MSK;
> frame->can_dlc = CAN_ERR_DLC;
> diff --git a/drivers/net/can/sja1000/sja1000.c
> b/drivers/net/can/sja1000/sja1000.c
> index 32bd7f4..37b6392 100644
> --- a/drivers/net/can/sja1000/sja1000.c
> +++ b/drivers/net/can/sja1000/sja1000.c
> @@ -479,9 +479,6 @@ static int sja1000_err(struct net_device *dev,
uint8_t
> isrc, uint8_t status)
> rx_state = txerr <= rxerr ? state : 0;
>
> can_change_state(dev, cf, tx_state, rx_state);
> -
> - if(state == CAN_STATE_BUS_OFF)
> - can_bus_off(dev);
Ah, no, that's wrong because not all devices use this function. I thought
just moving the counter incrementation out of can_bus_off() back to
the driver (for all drivers not yet using can_change_state). Sorry for
confusion.
Wolfgang.
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v4 6/6] can: handle bus-off in can_change_state()
2014-11-28 12:21 ` Wolfgang Grandegger
@ 2014-11-28 12:33 ` Andri Yngvason
0 siblings, 0 replies; 3+ messages in thread
From: Andri Yngvason @ 2014-11-28 12:33 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: linux-can, mkl
Quoting Wolfgang Grandegger (2014-11-28 12:21:53)
> On Fri, 28 Nov 2014 12:12:28 +0000, Andri Yngvason
> <andri.yngvason@marel.com> wrote:
> > Signed-off-by: Andri Yngvason <andri.yngvason@marel.com>
> > ---
> > drivers/net/can/dev.c | 1 +
> > drivers/net/can/flexcan.c | 3 ---
> > drivers/net/can/mscan/mscan.c | 11 +++++------
> > drivers/net/can/sja1000/sja1000.c | 3 ---
> > 4 files changed, 6 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
> > index 3e2da66..17fce72 100644
> > --- a/drivers/net/can/dev.c
> > +++ b/drivers/net/can/dev.c
> > @@ -340,6 +340,7 @@ void can_change_state(struct net_device *dev, struct
> > can_frame *cf,
> >
> > if (unlikely(new_state == CAN_STATE_BUS_OFF)) {
> > cf->can_id |= CAN_ERR_BUSOFF;
> > + can_bus_off(dev);
> > return;
> > }
> >
> > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> > index 9f91735..0823956 100644
> > --- a/drivers/net/can/flexcan.c
> > +++ b/drivers/net/can/flexcan.c
> > @@ -612,9 +612,6 @@ static int flexcan_poll_state(struct net_device
> *dev,
> > u32 reg_esr)
> >
> > can_change_state(dev, cf, tx_state, rx_state);
> >
> > - if (unlikely(new_state == CAN_STATE_BUS_OFF))
> > - can_bus_off(dev);
> > -
> > netif_receive_skb(skb);
> >
> > dev->stats.rx_packets++;
> > diff --git a/drivers/net/can/mscan/mscan.c
> b/drivers/net/can/mscan/mscan.c
> > index e36b740..2eb84f1 100644
> > --- a/drivers/net/can/mscan/mscan.c
> > +++ b/drivers/net/can/mscan/mscan.c
> > @@ -362,11 +362,7 @@ static void mscan_get_err_frame(struct net_device
> > *dev, struct can_frame *frame,
> >
> > new_state = get_new_state(dev, canrflg);
> > if (new_state != priv->can.state) {
> > - can_change_state(dev, frame,
> > - state_map[MSCAN_STATE_TX(canrflg)],
> > - state_map[MSCAN_STATE_RX(canrflg)]);
> > -
> > - if (priv->can.state == CAN_STATE_BUS_OFF) {
> > + if (new_state == CAN_STATE_BUS_OFF) {
> > /*
> > * The MSCAN on the MPC5200 does recover from bus-off
> > * automatically. To avoid that we stop the chip doing
> > @@ -378,8 +374,11 @@ static void mscan_get_err_frame(struct net_device
> > *dev, struct can_frame *frame,
> > setbits8(®s->canctl0,
> > MSCAN_SLPRQ | MSCAN_INITRQ);
> > }
> > - can_bus_off(dev);
> > }
> > +
> > + can_change_state(dev, frame,
> > + state_map[MSCAN_STATE_TX(canrflg)],
> > + state_map[MSCAN_STATE_RX(canrflg)]);
> > }
> > priv->shadow_statflg = canrflg & MSCAN_STAT_MSK;
> > frame->can_dlc = CAN_ERR_DLC;
> > diff --git a/drivers/net/can/sja1000/sja1000.c
> > b/drivers/net/can/sja1000/sja1000.c
> > index 32bd7f4..37b6392 100644
> > --- a/drivers/net/can/sja1000/sja1000.c
> > +++ b/drivers/net/can/sja1000/sja1000.c
> > @@ -479,9 +479,6 @@ static int sja1000_err(struct net_device *dev,
> uint8_t
> > isrc, uint8_t status)
> > rx_state = txerr <= rxerr ? state : 0;
> >
> > can_change_state(dev, cf, tx_state, rx_state);
> > -
> > - if(state == CAN_STATE_BUS_OFF)
> > - can_bus_off(dev);
>
> Ah, no, that's wrong because not all devices use this function. I thought
> just moving the counter incrementation out of can_bus_off() back to
> the driver (for all drivers not yet using can_change_state). Sorry for
> confusion.
There wasn't any confusion. I just felt like this might be a good idea, so I
included it. Turns out it isn't. It's a separate patch, so it can be easily
omitted.
What you described is in patch 5.
--
Andri.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-11-28 12:33 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-28 12:12 [PATCH v4 6/6] can: handle bus-off in can_change_state() Andri Yngvason
2014-11-28 12:21 ` Wolfgang Grandegger
2014-11-28 12:33 ` Andri Yngvason
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox