| Message ID | 1447163974-24514-1-git-send-email-mirza.krak@hostmobility.com |
|---|---|
| State | Awaiting Upstream, archived |
| Delegated to: | David Miller |
| Headers | show |
On 11/10/2015 02:59 PM, Mirza Krak wrote: > From: Mirza Krak <mirza.krak@hostmobility.com> > > According to SJA1000 data sheet error-warning (EI) interrupt is not > cleared by setting the controller in to reset-mode. > > Then if we have the following case: > - system is suspended (echo mem > /sys/power/state) and SJA1000 is left > in operating state This problem occurs only an hardware, where the SJA1000 is powered during system suspend? > - A bus error condition occurs which activates EI interrupt, system is > still suspended which means EI interrupt will be not be handled nor > cleared. > > If the above two events occur, on resume there is no way to return the > SJA1000 to operating state, except to cycle power to it. > > By simply reading the IR register on start we will clear any previous > conditions that could be present. Doesn't the SJA1000 trigger an interrupt that is detected after resuming? You add the fix to the open() function, which is triggered during $(ifconfig up), how is related to suspend/resume? Does the network layer call $(ifconfig down) during shutdown? There isn't any suspend/resume code in the sja1000 driver, for me it seems that we should add resume code that handles this problem. > > Signed-off-by: Mirza Krak <mirza.krak@hostmobility.com> > --- > drivers/net/can/sja1000/sja1000.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/net/can/sja1000/sja1000.c b/drivers/net/can/sja1000/sja1000.c > index 7b92e911a616..f10834be48a5 100644 > --- a/drivers/net/can/sja1000/sja1000.c > +++ b/drivers/net/can/sja1000/sja1000.c > @@ -218,6 +218,9 @@ static void sja1000_start(struct net_device *dev) > priv->write_reg(priv, SJA1000_RXERR, 0x0); > priv->read_reg(priv, SJA1000_ECC); > > + /* clear interrupt flags */ > + priv->read_reg(priv, SJA1000_IR); > + > /* leave reset mode */ > set_normal_mode(dev); > } > Marc
2015-11-11 9:04 GMT+01:00 Marc Kleine-Budde <mkl@pengutronix.de>: > This problem occurs only an hardware, where the SJA1000 is powered > during system suspend? Correct, and the controller is in UP state. > Doesn't the SJA1000 trigger an interrupt that is detected after > resuming? You add the fix to the open() function, which is triggered > during $(ifconfig up), how is related to suspend/resume? Does the > network layer call $(ifconfig down) during shutdown? It does not trigger another interrupt. In our tests we have seen that IE and EPI flags are set in the IR registered when we resume the system. If we do not clear these two flags it does not produce any other interrupts. Setting the controller in to reset mode does not clear the IE and EPI flags, I can understand that IE flag is not cleared as this is stated in the data-sheet, can not explain why EPI flag is not cleared though as data-sheet states 0 (reset) for both hardware reset and SETTING MOD.0 BY SOFTWARE (which is the reset-mode) Reason I put it in open() is so at least a DOWN/UP procedure returns the controller to an operating state. Also if we do an UP we should clear any earlier states that might exist in the registers like we do with error counters and error code capture. Network layer does not call $(ifconfig down) during shutdown. > There isn't any suspend/resume code in the sja1000 driver, for me it > seems that we should add resume code that handles this problem. Yes, resume code should be implemented to handle this and other problems (receive data). But still a DOWN/UP procedure should clear any previous state that could exist in the controller registers.
On 11/11/2015 10:54 AM, Mirza Krak wrote: > 2015-11-11 9:04 GMT+01:00 Marc Kleine-Budde <mkl@pengutronix.de>: >> This problem occurs only an hardware, where the SJA1000 is powered >> during system suspend? > > Correct, and the controller is in UP state. > >> Doesn't the SJA1000 trigger an interrupt that is detected after >> resuming? You add the fix to the open() function, which is triggered >> during $(ifconfig up), how is related to suspend/resume? Does the >> network layer call $(ifconfig down) during shutdown? > > It does not trigger another interrupt. Can you put a scope on the interrupt pin and see if it's a problem of the SoC and/or Linux or the SJA1000 not pulling the IRQ line. > In our tests we have seen that IE and EPI flags are set in the IR > registered when we resume the system. If we do not clear these two > flags it does not produce any other interrupts. Setting the controller > in to reset mode does not clear the IE and EPI flags, I can understand > that IE flag is not cleared as this is stated in the data-sheet, can > not explain why EPI flag is not cleared though as data-sheet states 0 > (reset) for both hardware reset and SETTING MOD.0 BY SOFTWARE (which > is the reset-mode) > > Reason I put it in open() is so at least a DOWN/UP procedure returns > the controller to an operating state. Also if we do an UP we should > clear any earlier states that might exist in the registers like we do > with error counters and error code capture. Yes - makes sense to have it in open(). > Network layer does not call $(ifconfig down) during shutdown. > >> There isn't any suspend/resume code in the sja1000 driver, for me it >> seems that we should add resume code that handles this problem. > > Yes, resume code should be implemented to handle this and other > problems (receive data). But still a DOWN/UP procedure should clear > any previous state that could exist in the controller registers. I'll add your patch to can/master and add stable on Cc. Looking forward for some suspend/resume code :) Thanks, Marc
2015-11-11 11:06 GMT+01:00 Marc Kleine-Budde <mkl@pengutronix.de>: > On 11/11/2015 10:54 AM, Mirza Krak wrote: >> 2015-11-11 9:04 GMT+01:00 Marc Kleine-Budde <mkl@pengutronix.de>: >>> This problem occurs only an hardware, where the SJA1000 is powered >>> during system suspend? >> >> Correct, and the controller is in UP state. >> >>> Doesn't the SJA1000 trigger an interrupt that is detected after >>> resuming? You add the fix to the open() function, which is triggered >>> during $(ifconfig up), how is related to suspend/resume? Does the >>> network layer call $(ifconfig down) during shutdown? >> >> It does not trigger another interrupt. > > Can you put a scope on the interrupt pin and see if it's a problem of > the SoC and/or Linux or the SJA1000 not pulling the IRQ line. A little demonstration on what it looks like when the system is resumed (added a printk in sja1000_start without my patch, though my printk also clears IR register): root@mx4-vcc:/opt/hm# ip -det link show can0 33: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN mode DEFAULT qlen 10 link/can can state ERROR-ACTIVE (berr-counter tx 0 rx 128) restart-ms 0 bitrate 500000 sample-point 0.875 tq 250 prop-seg 3 phase-seg1 3 phase-seg2 1 sjw 1 sja1000: tseg1 1..16 tseg2 1..8 sjw 1..4 brp 1..64 brp-inc 1 clock 12000000 root@mx4-vcc:/opt/hm# ifconfig can0 down root@mx4-vcc:/opt/hm# ifconfig can0 up [ 1612.621187] sja1000: MOD: 0x1 [ 1612.640992] sja1000: SR: 0x7c [ 1612.660039] sja1000: IR: 0x24 Also did a measurement with the scope, it is the SJA1000 chip not pulling the IRQ line. I can actually send one frame when the controller is in this state but never get a TI interrupt. root@mx4-vcc-14381059:~/test/can_test# ip -det link show can0 17: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN mode DEFAULT qlen 10 link/can can state ERROR-ACTIVE (berr-counter tx 0 rx 0) restart-ms 0 bitrate 125000 sample-point 0.875 tq 500 prop-seg 6 phase-seg1 7 phase-seg2 2 sjw 1 sja1000: tseg1 1..16 tseg2 1..8 sjw 1..4 brp 1..64 brp-inc 1 clock 12000000 root@mx4-vcc-14381059:~/test/can_test# cansend can0 -i 123 0 1 2 interface = can0, family = 29, type = 3, proto = 1 root@mx4-vcc-14381059:~/test/can_test# cansend can0 -i 123 0 1 2 interface = can0, family = 29, type = 3, proto = 1 root@mx4-vcc-14381059:~/test/can_test# cansend can0 -i 123 0 1 2 interface = can0, family = 29, type = 3, proto = 1 root@mx4-vcc-14381059:~/test/can_test# cat /proc/interrupts | grep can0 205: 0 0 GPIO can0 root@mx4-vcc-14381059:~/test/can_test# ## Second unit root@mx4-t20-1000000:~/test/can_test# candump can0 interface = can0, family = 29, type = 3, proto = 1 <0x07b> [3] 00 01 02 >>> There isn't any suspend/resume code in the sja1000 driver, for me it >>> seems that we should add resume code that handles this problem. >> >> Yes, resume code should be implemented to handle this and other >> problems (receive data). But still a DOWN/UP procedure should clear >> any previous state that could exist in the controller registers. > > I'll add your patch to can/master and add stable on Cc. Looking forward > for some suspend/resume code :) Looking forward to writing suspend/resume code :). Some cred should go to Christian (added him to CC) for finding this problem and reporting it to me.
On 11/11/2015 01:29 PM, Mirza Krak wrote: >> Can you put a scope on the interrupt pin and see if it's a problem of >> the SoC and/or Linux or the SJA1000 not pulling the IRQ line. > > A little demonstration on what it looks like when the system is > resumed (added a printk in sja1000_start without my patch, though my > printk also clears IR register): > > root@mx4-vcc:/opt/hm# ip -det link show can0 > 33: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state > UNKNOWN mode DEFAULT qlen 10 > link/can > can state ERROR-ACTIVE (berr-counter tx 0 rx 128) restart-ms 0 > bitrate 500000 sample-point 0.875 > tq 250 prop-seg 3 phase-seg1 3 phase-seg2 1 sjw 1 > sja1000: tseg1 1..16 tseg2 1..8 sjw 1..4 brp 1..64 brp-inc 1 > clock 12000000 > root@mx4-vcc:/opt/hm# ifconfig can0 down > root@mx4-vcc:/opt/hm# ifconfig can0 up > [ 1612.621187] sja1000: MOD: 0x1 > [ 1612.640992] sja1000: SR: 0x7c > [ 1612.660039] sja1000: IR: 0x24 > > Also did a measurement with the scope, it is the SJA1000 chip not > pulling the IRQ line. > > I can actually send one frame when the controller is in this state but > never get a TI interrupt. Thanks for testing. [...] >>> Yes, resume code should be implemented to handle this and other >>> problems (receive data). But still a DOWN/UP procedure should clear >>> any previous state that could exist in the controller registers. >> >> I'll add your patch to can/master and add stable on Cc. Looking forward >> for some suspend/resume code :) > > Looking forward to writing suspend/resume code :). > > Some cred should go to Christian (added him to CC) for finding this > problem and reporting it to me. I'll add a Reported-by: Christian Magnusson <Christian.Magnusson@semcon.com> to the patch. Marc
2015-11-11 13:37 GMT+01:00 Marc Kleine-Budde <mkl@pengutronix.de>: >> Some cred should go to Christian (added him to CC) for finding this >> problem and reporting it to me. > > I'll add a > > Reported-by: Christian Magnusson <Christian.Magnusson@semcon.com> > > to the patch. > Can we just get an ACK that it is OK from Christian before we do it.
On 11/11/2015 01:46 PM, Mirza Krak wrote: > 2015-11-11 13:37 GMT+01:00 Marc Kleine-Budde <mkl@pengutronix.de>: >>> Some cred should go to Christian (added him to CC) for finding this >>> problem and reporting it to me. >> >> I'll add a >> >> Reported-by: Christian Magnusson <Christian.Magnusson@semcon.com> >> >> to the patch. > > Can we just get an ACK that it is OK from Christian before we do it. Sure. Marc
Thanks.. It's ok for me. Good that we finally found the problem with the hanging can-interfaces. :) /Christian Christian Magnusson christian.magnusson@semcon.com -----Original Message----- From: Mirza Krak [mailto:mirza.krak@hostmobility.com] Sent: den 11 november 2015 13:46 To: Marc Kleine-Budde Cc: wg@grandegger.com; andri.yngvason@marel.com; linux-can@vger.kernel.org; netdev@vger.kernel.org; linux-kernel; Christian Magnusson Subject: Re: [PATCH 1/1] can: sja1000: clear interrupts on start 2015-11-11 13:37 GMT+01:00 Marc Kleine-Budde <mkl@pengutronix.de>: >> Some cred should go to Christian (added him to CC) for finding this >> problem and reporting it to me. > > I'll add a > > Reported-by: Christian Magnusson <Christian.Magnusson@semcon.com> > > to the patch. > Can we just get an ACK that it is OK from Christian before we do it. -- Med Vänliga Hälsningar / Best Regards ******************************************************************* Mirza Krak Host Mobility AB mirza.krak@hostmobility.com Anders Personsgatan 12, 416 64 Göteborg Sweden http://www.hostmobility.com Direct: +46 31 31 32 704 Phone: +46 31 31 32 700 Fax: +46 31 80 67 51 Mobile: +46 730 28 06 22 *******************************************************************
diff --git a/drivers/net/can/sja1000/sja1000.c b/drivers/net/can/sja1000/sja1000.c index 7b92e911a616..f10834be48a5 100644 --- a/drivers/net/can/sja1000/sja1000.c +++ b/drivers/net/can/sja1000/sja1000.c @@ -218,6 +218,9 @@ static void sja1000_start(struct net_device *dev) priv->write_reg(priv, SJA1000_RXERR, 0x0); priv->read_reg(priv, SJA1000_ECC); + /* clear interrupt flags */ + priv->read_reg(priv, SJA1000_IR); + /* leave reset mode */ set_normal_mode(dev); }