Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1266519 > unrolled thread

[PATCH 1/1] can: sja1000: clear interrupts on start

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/1] can: sja1000: clear interrupts on start Mirza Krak <mirza.krak@hostmobility.com> - 2015-11-10 15:00 +0100
    Re: [PATCH 1/1] can: sja1000: clear interrupts on start Marc Kleine-Budde <mkl@pengutronix.de> - 2015-11-11 09:10 +0100
      Re: [PATCH 1/1] can: sja1000: clear interrupts on start Mirza Krak <mirza.krak@hostmobility.com> - 2015-11-11 11:00 +0100
        Re: [PATCH 1/1] can: sja1000: clear interrupts on start Marc Kleine-Budde <mkl@pengutronix.de> - 2015-11-11 11:10 +0100
          Re: [PATCH 1/1] can: sja1000: clear interrupts on start Marc Kleine-Budde <mkl@pengutronix.de> - 2015-11-11 13:40 +0100
            Re: [PATCH 1/1] can: sja1000: clear interrupts on start Marc Kleine-Budde <mkl@pengutronix.de> - 2015-11-11 13:50 +0100
            Re: [PATCH 1/1] can: sja1000: clear interrupts on start Mirza Krak <mirza.krak@hostmobility.com> - 2015-11-11 13:50 +0100
              RE: [PATCH 1/1] can: sja1000: clear interrupts on start Christian Magnusson <Christian.Magnusson@semcon.com> - 2015-11-11 14:10 +0100
          Re: [PATCH 1/1] can: sja1000: clear interrupts on start Mirza Krak <mirza.krak@hostmobility.com> - 2015-11-11 13:40 +0100

#1266519 — [PATCH 1/1] can: sja1000: clear interrupts on start

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
- 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.

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);
 }
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1267024

[Multipart message — attachments visible in raw view] — view raw

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

-- 
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   |

[toc] | [prev] | [next] | [standalone]

#1267072

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.

-- 
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
*******************************************************************
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1267079

[Multipart message — attachments visible in raw view] — view raw

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

-- 
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   |

[toc] | [prev] | [next] | [standalone]

#1267159

[Multipart message — attachments visible in raw view] — view raw

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

-- 
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   |

[toc] | [prev] | [next] | [standalone]

#1267163

[Multipart message — attachments visible in raw view] — view raw

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

-- 
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   |

[toc] | [prev] | [next] | [standalone]

#1267164

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
*******************************************************************
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1267174

VGhhbmtzLi4gSXQncyBvayBmb3IgbWUuIEdvb2QgdGhhdCB3ZSBmaW5hbGx5IGZvdW5kIHRoZSBw
cm9ibGVtIHdpdGggdGhlIGhhbmdpbmcgY2FuLWludGVyZmFjZXMuIDopDQoNCi9DaHJpc3RpYW4N
Cg0KDQpDaHJpc3RpYW4gTWFnbnVzc29uDQpjaHJpc3RpYW4ubWFnbnVzc29uQHNlbWNvbi5jb20N
Cg0KDQotLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KRnJvbTogTWlyemEgS3JhayBbbWFpbHRv
Om1pcnphLmtyYWtAaG9zdG1vYmlsaXR5LmNvbV0gDQpTZW50OiBkZW4gMTEgbm92ZW1iZXIgMjAx
NSAxMzo0Ng0KVG86IE1hcmMgS2xlaW5lLUJ1ZGRlDQpDYzogd2dAZ3JhbmRlZ2dlci5jb207IGFu
ZHJpLnluZ3Zhc29uQG1hcmVsLmNvbTsgbGludXgtY2FuQHZnZXIua2VybmVsLm9yZzsgbmV0ZGV2
QHZnZXIua2VybmVsLm9yZzsgbGludXgta2VybmVsOyBDaHJpc3RpYW4gTWFnbnVzc29uDQpTdWJq
ZWN0OiBSZTogW1BBVENIIDEvMV0gY2FuOiBzamExMDAwOiBjbGVhciBpbnRlcnJ1cHRzIG9uIHN0
YXJ0DQoNCjIwMTUtMTEtMTEgMTM6MzcgR01UKzAxOjAwIE1hcmMgS2xlaW5lLUJ1ZGRlIDxta2xA
cGVuZ3V0cm9uaXguZGU+Og0KPj4gU29tZSBjcmVkIHNob3VsZCBnbyB0byBDaHJpc3RpYW4gKGFk
ZGVkIGhpbSB0byBDQykgZm9yIGZpbmRpbmcgdGhpcyANCj4+IHByb2JsZW0gYW5kIHJlcG9ydGlu
ZyBpdCB0byBtZS4NCj4NCj4gSSdsbCBhZGQgYQ0KPg0KPiBSZXBvcnRlZC1ieTogQ2hyaXN0aWFu
IE1hZ251c3NvbiA8Q2hyaXN0aWFuLk1hZ251c3NvbkBzZW1jb24uY29tPg0KPg0KPiB0byB0aGUg
cGF0Y2guDQo+DQoNCkNhbiB3ZSBqdXN0IGdldCBhbiBBQ0sgdGhhdCBpdCBpcyBPSyBmcm9tIENo
cmlzdGlhbiBiZWZvcmUgd2UgZG8gaXQuDQoNCg0KDQotLQ0KTWVkIFbDpG5saWdhIEjDpGxzbmlu
Z2FyIC8gQmVzdCBSZWdhcmRzDQoNCioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioq
KioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioNCk1pcnphIEtyYWsNCkhvc3QgTW9iaWxp
dHkgQUINCm1pcnphLmtyYWtAaG9zdG1vYmlsaXR5LmNvbQ0KQW5kZXJzIFBlcnNvbnNnYXRhbiAx
MiwgNDE2IDY0IEfDtnRlYm9yZw0KU3dlZGVuDQpodHRwOi8vd3d3Lmhvc3Rtb2JpbGl0eS5jb20N
CkRpcmVjdDogKzQ2IDMxIDMxIDMyIDcwNA0KUGhvbmU6ICs0NiAzMSAzMSAzMiA3MDANCkZheDog
KzQ2IDMxIDgwIDY3IDUxDQpNb2JpbGU6ICs0NiA3MzAgMjggMDYgMjINCioqKioqKioqKioqKioq
KioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioNCg==
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1267160

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.

-- 
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
*******************************************************************
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web