linux-kernel - Re: [PATCH 1/1] can: sja1000: clear interrupts on start

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJ=nTstPCev0TGjZCYkzf4qX+6NUUv8+-6PeVEKL9bcOwBKc0A@mail.gmail.com>
Date:	Wed, 11 Nov 2015 13:29:59 +0100
From:	Mirza Krak <mirza.krak@...tmobility.com>
To:	Marc Kleine-Budde <mkl@...gutronix.de>
Cc:	wg@...ndegger.com, andri.yngvason@...el.com,
	"linux-can@...r.kernel.org" <linux-can@...r.kernel.org>,
	netdev@...r.kernel.org,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Christian Magnusson <Christian.Magnusson@...con.com>
Subject: Re: [PATCH 1/1] can: sja1000: clear interrupts on start

2015-11-11 11:06 GMT+01:00 Marc Kleine-Budde <mkl@...gutronix.de>:
> On 11/11/2015 10:54 AM, Mirza Krak wrote:
>> 2015-11-11 9:04 GMT+01:00 Marc Kleine-Budde <mkl@...gutronix.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@...-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@...-vcc:/opt/hm# ifconfig can0 down
root@...-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@...-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@...-vcc-14381059:~/test/can_test# cansend can0 -i 123 0 1 2
interface = can0, family = 29, type = 3, proto = 1
root@...-vcc-14381059:~/test/can_test# cansend can0 -i 123 0 1 2
interface = can0, family = 29, type = 3, proto = 1
root@...-vcc-14381059:~/test/can_test# cansend can0 -i 123 0 1 2
interface = can0, family = 29, type = 3, proto = 1
root@...-vcc-14381059:~/test/can_test# cat /proc/interrupts | grep can0
205:          0          0      GPIO  can0
root@...-vcc-14381059:~/test/can_test#


## Second unit
root@...-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@...tmobility.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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/