* [PATCH v6] can: sja1000: fix {pre,post}_irq() handling and IRQ handler return value
@ 2013-11-24 23:03 Marc Kleine-Budde
2013-11-25 8:54 ` Marc Kleine-Budde
2013-12-05 15:50 ` Richard Andrysek
0 siblings, 2 replies; 27+ messages in thread
From: Marc Kleine-Budde @ 2013-11-24 23:03 UTC (permalink / raw)
To: linux-can; +Cc: kernel, Oliver Hartkopp, Marc Kleine-Budde
From: Oliver Hartkopp <socketcan@hartkopp.net>
It has been reported that on PREEMPT_RT enabled system the peak SJA1000
hardware can trigger "irq X: nobody cared" errors.
This patch fixes the issue that the sja1000_interrupt() function may have
returned IRQ_NONE without processing the optional pre_irq() and post_irq()
function before. Further the irq processing counter 'n' is moved to the end of
the while statement to return correct IRQ_[NONE|HANDLED] values at error
conditions.
Reported-by: Wolfgang Grandegger <wg@grandegger.com>
Acked-by: Wolfgang Grandegger <wg@grandegger.com>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
Changes since v5: make it one patch again.
drivers/net/can/sja1000/sja1000.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/net/can/sja1000/sja1000.c b/drivers/net/can/sja1000/sja1000.c
index 7164a99..f17c301 100644
--- a/drivers/net/can/sja1000/sja1000.c
+++ b/drivers/net/can/sja1000/sja1000.c
@@ -494,20 +494,20 @@ irqreturn_t sja1000_interrupt(int irq, void *dev_id)
uint8_t isrc, status;
int n = 0;
- /* Shared interrupts and IRQ off? */
- if (priv->read_reg(priv, SJA1000_IER) == IRQ_OFF)
- return IRQ_NONE;
-
if (priv->pre_irq)
priv->pre_irq(priv);
+ /* Shared interrupts and IRQ off? */
+ if (priv->read_reg(priv, SJA1000_IER) == IRQ_OFF)
+ goto out;
+
while ((isrc = priv->read_reg(priv, SJA1000_IR)) &&
(n < SJA1000_MAX_IRQ)) {
- n++;
+
status = priv->read_reg(priv, SJA1000_SR);
/* check for absent controller due to hw unplug */
if (status == 0xFF && sja1000_is_absent(priv))
- return IRQ_NONE;
+ goto out;
if (isrc & IRQ_WUI)
netdev_warn(dev, "wakeup interrupt\n");
@@ -535,7 +535,7 @@ irqreturn_t sja1000_interrupt(int irq, void *dev_id)
status = priv->read_reg(priv, SJA1000_SR);
/* check for absent controller */
if (status == 0xFF && sja1000_is_absent(priv))
- return IRQ_NONE;
+ goto out;
}
}
if (isrc & (IRQ_DOI | IRQ_EI | IRQ_BEI | IRQ_EPI | IRQ_ALI)) {
@@ -543,8 +543,9 @@ irqreturn_t sja1000_interrupt(int irq, void *dev_id)
if (sja1000_err(dev, isrc, status))
break;
}
+ n++;
}
-
+out:
if (priv->post_irq)
priv->post_irq(priv);
--
1.8.5.rc0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v6] can: sja1000: fix {pre,post}_irq() handling and IRQ handler return value
2013-11-24 23:03 [PATCH v6] can: sja1000: fix {pre,post}_irq() handling and IRQ handler return value Marc Kleine-Budde
@ 2013-11-25 8:54 ` Marc Kleine-Budde
2013-11-25 18:12 ` Oliver Hartkopp
` (2 more replies)
2013-12-05 15:50 ` Richard Andrysek
1 sibling, 3 replies; 27+ messages in thread
From: Marc Kleine-Budde @ 2013-11-25 8:54 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-can, kernel, Oliver Hartkopp, austin
[-- Attachment #1: Type: text/plain, Size: 909 bytes --]
On 11/25/2013 12:03 AM, Marc Kleine-Budde wrote:
> From: Oliver Hartkopp <socketcan@hartkopp.net>
>
> It has been reported that on PREEMPT_RT enabled system the peak SJA1000
> hardware can trigger "irq X: nobody cared" errors.
>
> This patch fixes the issue that the sja1000_interrupt() function may have
> returned IRQ_NONE without processing the optional pre_irq() and post_irq()
> function before. Further the irq processing counter 'n' is moved to the end of
> the while statement to return correct IRQ_[NONE|HANDLED] values at error
> conditions.
Austin, can you test if this patch solves your problem.
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 |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v6] can: sja1000: fix {pre,post}_irq() handling and IRQ handler return value
2013-11-25 8:54 ` Marc Kleine-Budde
@ 2013-11-25 18:12 ` Oliver Hartkopp
2013-11-25 22:05 ` Austin Schuh
2013-12-09 19:48 ` Austin Schuh
2 siblings, 0 replies; 27+ messages in thread
From: Oliver Hartkopp @ 2013-11-25 18:12 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-can, kernel, austin
On 25.11.2013 09:54, Marc Kleine-Budde wrote:
> On 11/25/2013 12:03 AM, Marc Kleine-Budde wrote:
>> From: Oliver Hartkopp <socketcan@hartkopp.net>
>>
>> It has been reported that on PREEMPT_RT enabled system the peak SJA1000
>> hardware can trigger "irq X: nobody cared" errors.
This sentence of the description is not really related to the patch.
We all looked into the sja1000 driver due to the issue Austin reported and
detected the general problem in error handling then.
With PREEMPT_RT there are irq threads which might lead to Austins specific
problem ...
>>
>> This patch fixes the issue that the sja1000_interrupt() function may have
>> returned IRQ_NONE without processing the optional pre_irq() and post_irq()
>> function before. Further the irq processing counter 'n' is moved to the end of
>> the while statement to return correct IRQ_[NONE|HANDLED] values at error
>> conditions.
>
> Austin, can you test if this patch solves your problem.
Austin should test it, right.
But that should not delay sending this patch to net/stable tree.
Sorry for the confusion. Following the mails must have been hard ;-)
Best regards,
Oliver
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v6] can: sja1000: fix {pre,post}_irq() handling and IRQ handler return value
2013-11-25 8:54 ` Marc Kleine-Budde
2013-11-25 18:12 ` Oliver Hartkopp
@ 2013-11-25 22:05 ` Austin Schuh
2013-12-09 19:48 ` Austin Schuh
2 siblings, 0 replies; 27+ messages in thread
From: Austin Schuh @ 2013-11-25 22:05 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-can, kernel, Oliver Hartkopp
I'll test the patch on the the 2nd when I have access to the machine
again. Thanks!
Austin
On Mon, Nov 25, 2013 at 12:54 AM, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> On 11/25/2013 12:03 AM, Marc Kleine-Budde wrote:
>> From: Oliver Hartkopp <socketcan@hartkopp.net>
>>
>> It has been reported that on PREEMPT_RT enabled system the peak SJA1000
>> hardware can trigger "irq X: nobody cared" errors.
>>
>> This patch fixes the issue that the sja1000_interrupt() function may have
>> returned IRQ_NONE without processing the optional pre_irq() and post_irq()
>> function before. Further the irq processing counter 'n' is moved to the end of
>> the while statement to return correct IRQ_[NONE|HANDLED] values at error
>> conditions.
>
> Austin, can you test if this patch solves your problem.
>
> 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 |
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v6] can: sja1000: fix {pre,post}_irq() handling and IRQ handler return value
2013-11-25 8:54 ` Marc Kleine-Budde
2013-11-25 18:12 ` Oliver Hartkopp
2013-11-25 22:05 ` Austin Schuh
@ 2013-12-09 19:48 ` Austin Schuh
2013-12-09 21:07 ` Marc Kleine-Budde
2 siblings, 1 reply; 27+ messages in thread
From: Austin Schuh @ 2013-12-09 19:48 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-can, kernel, Oliver Hartkopp
Hi Marc,
I have applied the patch. I ran a test with 75% bus load @500000
bits/sec (15% sending from the SJA1000 and 60% from another device on
the bus, running candump can1 to grab the data). It failed within
minutes. I'll add the code back in to disable tracing when the IRQ
gets disabled.
Austin
On Mon, Nov 25, 2013 at 12:54 AM, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> On 11/25/2013 12:03 AM, Marc Kleine-Budde wrote:
>> From: Oliver Hartkopp <socketcan@hartkopp.net>
>>
>> It has been reported that on PREEMPT_RT enabled system the peak SJA1000
>> hardware can trigger "irq X: nobody cared" errors.
>>
>> This patch fixes the issue that the sja1000_interrupt() function may have
>> returned IRQ_NONE without processing the optional pre_irq() and post_irq()
>> function before. Further the irq processing counter 'n' is moved to the end of
>> the while statement to return correct IRQ_[NONE|HANDLED] values at error
>> conditions.
>
> Austin, can you test if this patch solves your problem.
>
> 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 |
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v6] can: sja1000: fix {pre,post}_irq() handling and IRQ handler return value
2013-12-09 19:48 ` Austin Schuh
@ 2013-12-09 21:07 ` Marc Kleine-Budde
2013-12-09 23:50 ` Austin Schuh
0 siblings, 1 reply; 27+ messages in thread
From: Marc Kleine-Budde @ 2013-12-09 21:07 UTC (permalink / raw)
To: Austin Schuh; +Cc: linux-can, kernel, Oliver Hartkopp
[-- Attachment #1: Type: text/plain, Size: 693 bytes --]
On 12/09/2013 08:48 PM, Austin Schuh wrote:
> Hi Marc,
>
> I have applied the patch. I ran a test with 75% bus load @500000
> bits/sec (15% sending from the SJA1000 and 60% from another device on
> the bus, running candump can1 to grab the data). It failed within
> minutes. I'll add the code back in to disable tracing when the IRQ
> gets disabled.
Does the patch make any difference at all?
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 |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v6] can: sja1000: fix {pre,post}_irq() handling and IRQ handler return value
2013-12-09 21:07 ` Marc Kleine-Budde
@ 2013-12-09 23:50 ` Austin Schuh
0 siblings, 0 replies; 27+ messages in thread
From: Austin Schuh @ 2013-12-09 23:50 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-can, kernel, Oliver Hartkopp
If anything, it increased the frequency of failure (on my RT kernel).
It took 14 minutes to fail the first time, and a little under 2 hours
the second time.
That doesn't mean that it doesn't fix another problem, just that it
doesn't fix the problem that I am having. I don't see any obvious new
problems caused by the patch.
Austin
On Mon, Dec 9, 2013 at 1:07 PM, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> On 12/09/2013 08:48 PM, Austin Schuh wrote:
>> Hi Marc,
>>
>> I have applied the patch. I ran a test with 75% bus load @500000
>> bits/sec (15% sending from the SJA1000 and 60% from another device on
>> the bus, running candump can1 to grab the data). It failed within
>> minutes. I'll add the code back in to disable tracing when the IRQ
>> gets disabled.
>
> Does the patch make any difference at all?
>
> 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 |
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v6] can: sja1000: fix {pre,post}_irq() handling and IRQ handler return value
2013-11-24 23:03 [PATCH v6] can: sja1000: fix {pre,post}_irq() handling and IRQ handler return value Marc Kleine-Budde
2013-11-25 8:54 ` Marc Kleine-Budde
@ 2013-12-05 15:50 ` Richard Andrysek
2013-12-05 17:50 ` Wolfgang Grandegger
1 sibling, 1 reply; 27+ messages in thread
From: Richard Andrysek @ 2013-12-05 15:50 UTC (permalink / raw)
To: linux-can
Marc Kleine-Budde <mkl <at> pengutronix.de> writes:
>
> From: Oliver Hartkopp <socketcan <at> hartkopp.net>
>
> It has been reported that on PREEMPT_RT enabled system the peak SJA1000
> hardware can trigger "irq X: nobody cared" errors.
>
> This patch fixes the issue that the sja1000_interrupt() function may have
> returned IRQ_NONE without processing the optional pre_irq() and post_irq()
> function before. Further the irq processing counter 'n' is moved to the end of
> the while statement to return correct IRQ_[NONE|HANDLED] values at error
> conditions.
>
> Reported-by: Wolfgang Grandegger <wg <at> grandegger.com>
> Acked-by: Wolfgang Grandegger <wg <at> grandegger.com>
> Signed-off-by: Oliver Hartkopp <socketcan <at> hartkopp.net>
> Signed-off-by: Marc Kleine-Budde <mkl <at> pengutronix.de>
> ---
> Changes since v5: make it one patch again.
>
> drivers/net/can/sja1000/sja1000.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/can/sja1000/sja1000.c
b/drivers/net/can/sja1000/sja1000.c
> index 7164a99..f17c301 100644
> --- a/drivers/net/can/sja1000/sja1000.c
> +++ b/drivers/net/can/sja1000/sja1000.c
> <at> <at> -494,20 +494,20 <at> <at> irqreturn_t
sja1000_interrupt(int irq, void *dev_id)
> uint8_t isrc, status;
> int n = 0;
>
> - /* Shared interrupts and IRQ off? */
> - if (priv->read_reg(priv, SJA1000_IER) == IRQ_OFF)
> - return IRQ_NONE;
> -
> if (priv->pre_irq)
> priv->pre_irq(priv);
>
> + /* Shared interrupts and IRQ off? */
> + if (priv->read_reg(priv, SJA1000_IER) == IRQ_OFF)
> + goto out;
> +
> while ((isrc = priv->read_reg(priv, SJA1000_IR)) &&
> (n < SJA1000_MAX_IRQ)) {
> - n++;
> +
> status = priv->read_reg(priv, SJA1000_SR);
> /* check for absent controller due to hw unplug */
> if (status == 0xFF && sja1000_is_absent(priv))
> - return IRQ_NONE;
> + goto out;
>
> if (isrc & IRQ_WUI)
> netdev_warn(dev, "wakeup interrupt\n");
> <at> <at> -535,7 +535,7 <at> <at> irqreturn_t sja1000_interrupt(int
irq, void *dev_id)
> status = priv->read_reg(priv, SJA1000_SR);
> /* check for absent controller */
> if (status == 0xFF && sja1000_is_absent(priv))
> - return IRQ_NONE;
> + goto out;
> }
> }
> if (isrc & (IRQ_DOI | IRQ_EI | IRQ_BEI | IRQ_EPI | IRQ_ALI)) {
> <at> <at> -543,8 +543,9 <at> <at> irqreturn_t sja1000_interrupt(int
irq, void *dev_id)
> if (sja1000_err(dev, isrc, status))
> break;
> }
> + n++;
> }
> -
> +out:
> if (priv->post_irq)
> priv->post_irq(priv);
>
I've used a patch and now I see an another effect.
I see with a dmesg like this: [16773.935134] peak_pci 0000:03:0b.0 can0:
data overrun interrupt
1)Why there are reported as a frame error, by calling ifconfig –a?
$ sudo dmesg | grep "data overrun interrupt" | wc
17 136 1122
~$ sudo ifconfig can0
can0 Link encap:UNSPEC HWaddr
00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00
UP RUNNING NOARP MTU:16 Metric:1
RX packets:12711052 errors:17 dropped:0 overruns:0 frame:17
TX packets:15023217 errors:0 dropped:0 overruns:0 carrier:0
collisions:0 txqueuelen:10
RX bytes:101688416 (96.9 MiB) TX bytes:109206486 (104.1 MiB)
Interrupt:19
Rx errors:17 == wc over "data overrun interrupt": 17 == RX frame
: 17.
2) How the data overrun can happen, if the can bus load is ~25%?
$ sudo cat /proc/net/can/stats
[sudo] password for ran:
1421819 transmitted frames (TXF)
2606493 received frames (RXF)
473873 matched frames (RXMF)
18 % total match ratio (RXMR)
2400 frames/s total tx rate (TXR)
4400 frames/s total rx rate (RXR)
18 % current match ratio (CRXMR)
2400 frames/s current tx rate (CTXR)
4400 frames/s current rx rate (CRXR)
18 % max match ratio (MRXMR)
2427 frames/s max tx rate (MTXR)
4427 frames/s max rx rate (MRXR)
4 current receive list entries (CRCV)
9 maximum receive list entries (MRCV)
2 statistic resets (STR)
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v6] can: sja1000: fix {pre,post}_irq() handling and IRQ handler return value
2013-12-05 15:50 ` Richard Andrysek
@ 2013-12-05 17:50 ` Wolfgang Grandegger
2013-12-05 19:37 ` Richard Andrysek
0 siblings, 1 reply; 27+ messages in thread
From: Wolfgang Grandegger @ 2013-12-05 17:50 UTC (permalink / raw)
To: Richard Andrysek, linux-can
Hi Richard,
On 12/05/2013 04:50 PM, Richard Andrysek wrote:
> Marc Kleine-Budde <mkl <at> pengutronix.de> writes:
>
>>
>> From: Oliver Hartkopp <socketcan <at> hartkopp.net>
>>
>> It has been reported that on PREEMPT_RT enabled system the peak SJA1000
>> hardware can trigger "irq X: nobody cared" errors.
>>
>> This patch fixes the issue that the sja1000_interrupt() function may have
>> returned IRQ_NONE without processing the optional pre_irq() and post_irq()
>> function before. Further the irq processing counter 'n' is moved to the end of
>> the while statement to return correct IRQ_[NONE|HANDLED] values at error
>> conditions.
>>
>> Reported-by: Wolfgang Grandegger <wg <at> grandegger.com>
>> Acked-by: Wolfgang Grandegger <wg <at> grandegger.com>
>> Signed-off-by: Oliver Hartkopp <socketcan <at> hartkopp.net>
>> Signed-off-by: Marc Kleine-Budde <mkl <at> pengutronix.de>
>> ---
>> Changes since v5: make it one patch again.
>>
>> drivers/net/can/sja1000/sja1000.c | 17 +++++++++--------
>> 1 file changed, 9 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/net/can/sja1000/sja1000.c
> b/drivers/net/can/sja1000/sja1000.c
>> index 7164a99..f17c301 100644
>> --- a/drivers/net/can/sja1000/sja1000.c
>> +++ b/drivers/net/can/sja1000/sja1000.c
>> <at> <at> -494,20 +494,20 <at> <at> irqreturn_t
> sja1000_interrupt(int irq, void *dev_id)
>> uint8_t isrc, status;
>> int n = 0;
>>
>> - /* Shared interrupts and IRQ off? */
>> - if (priv->read_reg(priv, SJA1000_IER) == IRQ_OFF)
>> - return IRQ_NONE;
>> -
>> if (priv->pre_irq)
>> priv->pre_irq(priv);
>>
>> + /* Shared interrupts and IRQ off? */
>> + if (priv->read_reg(priv, SJA1000_IER) == IRQ_OFF)
>> + goto out;
>> +
>> while ((isrc = priv->read_reg(priv, SJA1000_IR)) &&
>> (n < SJA1000_MAX_IRQ)) {
>> - n++;
>> +
>> status = priv->read_reg(priv, SJA1000_SR);
>> /* check for absent controller due to hw unplug */
>> if (status == 0xFF && sja1000_is_absent(priv))
>> - return IRQ_NONE;
>> + goto out;
>>
>> if (isrc & IRQ_WUI)
>> netdev_warn(dev, "wakeup interrupt\n");
>> <at> <at> -535,7 +535,7 <at> <at> irqreturn_t sja1000_interrupt(int
> irq, void *dev_id)
>> status = priv->read_reg(priv, SJA1000_SR);
>> /* check for absent controller */
>> if (status == 0xFF && sja1000_is_absent(priv))
>> - return IRQ_NONE;
>> + goto out;
>> }
>> }
>> if (isrc & (IRQ_DOI | IRQ_EI | IRQ_BEI | IRQ_EPI | IRQ_ALI)) {
>> <at> <at> -543,8 +543,9 <at> <at> irqreturn_t sja1000_interrupt(int
> irq, void *dev_id)
>> if (sja1000_err(dev, isrc, status))
>> break;
>> }
>> + n++;
>> }
>> -
>> +out:
>> if (priv->post_irq)
>> priv->post_irq(priv);
>>
>
> I've used a patch and now I see an another effect.
You mean, the data overruns do not happen without the pach above?
> I see with a dmesg like this: [16773.935134] peak_pci 0000:03:0b.0 can0:
> data overrun interrupt
>
> 1)Why there are reported as a frame error, by calling ifconfig –a?
> $ sudo dmesg | grep "data overrun interrupt" | wc
> 17 136 1122
>
> ~$ sudo ifconfig can0
> can0 Link encap:UNSPEC HWaddr
> 00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00
> UP RUNNING NOARP MTU:16 Metric:1
> RX packets:12711052 errors:17 dropped:0 overruns:0 frame:17
> TX packets:15023217 errors:0 dropped:0 overruns:0 carrier:0
> collisions:0 txqueuelen:10
> RX bytes:101688416 (96.9 MiB) TX bytes:109206486 (104.1 MiB)
> Interrupt:19
>
>
> Rx errors:17 == wc over "data overrun interrupt": 17 == RX frame
> : 17.
>
> 2) How the data overrun can happen, if the can bus load is ~25%?
If a bunch of messages is sent in a short time. The 25% is an average
load, I assume.
>
> $ sudo cat /proc/net/can/stats
> [sudo] password for ran:
>
> 1421819 transmitted frames (TXF)
> 2606493 received frames (RXF)
> 473873 matched frames (RXMF)
>
> 18 % total match ratio (RXMR)
> 2400 frames/s total tx rate (TXR)
> 4400 frames/s total rx rate (RXR)
>
> 18 % current match ratio (CRXMR)
> 2400 frames/s current tx rate (CTXR)
> 4400 frames/s current rx rate (CRXR)
>
> 18 % max match ratio (MRXMR)
> 2427 frames/s max tx rate (MTXR)
> 4427 frames/s max rx rate (MRXR)
>
> 4 current receive list entries (CRCV)
> 9 maximum receive list entries (MRCV)
>
> 2 statistic resets (STR)
# ip -d -s link show can0
will list the CAN statistics.
Wolfgang.
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v6] can: sja1000: fix {pre,post}_irq() handling and IRQ handler return value
2013-12-05 17:50 ` Wolfgang Grandegger
@ 2013-12-05 19:37 ` Richard Andrysek
2013-12-05 20:26 ` Wolfgang Grandegger
0 siblings, 1 reply; 27+ messages in thread
From: Richard Andrysek @ 2013-12-05 19:37 UTC (permalink / raw)
To: linux-can
Wolfgang Grandegger <wg <at> grandegger.com> writes:
> 2) How the data overrun can happen, if the can bus load is ~25%?
>
> If a bunch of messages is sent in a short time. The 25% is an average
> load, I assume.
>
> >
> > $ sudo cat /proc/net/can/stats
> > [sudo] password for ran:
> >
> > 1421819 transmitted frames (TXF)
> > 2606493 received frames (RXF)
> > 473873 matched frames (RXMF)
> >
> > 18 % total match ratio (RXMR)
> > 2400 frames/s total tx rate (TXR)
> > 4400 frames/s total rx rate (RXR)
> >
> > 18 % current match ratio (CRXMR)
> > 2400 frames/s current tx rate (CTXR)
> > 4400 frames/s current rx rate (CRXR)
> >
> > 18 % max match ratio (MRXMR)
> > 2427 frames/s max tx rate (MTXR)
> > 4427 frames/s max rx rate (MRXR)
> >
> > 4 current receive list entries (CRCV)
> > 9 maximum receive list entries (MRCV)
> >
> > 2 statistic resets (STR)
>
> # ip -d -s link show can0
>
> will list the CAN statistics.
>
> Wolfgang.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-can" in
> the body of a message to majordomo <at> vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
Hi Wolfgang,
thank you for help. I've recognized that these two lines comes from a
network world:
RX packets:12462166 errors:121380 dropped:0 overruns:0 frame:121380
TX packets:11604619 errors:678 dropped:0 overruns:0 carrier:0
Then each event "arbitration lost" generates an IRQ.
Is it OK to disable it, just now for tests. With that I want to reduce an
amount of IRQs. Is it possible to disable it without a new compilation of a
kernel?
We are running quite fast, in few ms. So we test your driver quite well:)
by
Richard
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v6] can: sja1000: fix {pre,post}_irq() handling and IRQ handler return value
2013-12-05 19:37 ` Richard Andrysek
@ 2013-12-05 20:26 ` Wolfgang Grandegger
2013-12-06 9:27 ` Richard Andrysek
0 siblings, 1 reply; 27+ messages in thread
From: Wolfgang Grandegger @ 2013-12-05 20:26 UTC (permalink / raw)
To: Richard Andrysek, linux-can
On 12/05/2013 08:37 PM, Richard Andrysek wrote:
> Wolfgang Grandegger <wg <at> grandegger.com> writes:
>
>> 2) How the data overrun can happen, if the can bus load is ~25%?
>>
>> If a bunch of messages is sent in a short time. The 25% is an average
>> load, I assume.
>>
>>>
>>> $ sudo cat /proc/net/can/stats
>>> [sudo] password for ran:
>>>
>>> 1421819 transmitted frames (TXF)
>>> 2606493 received frames (RXF)
>>> 473873 matched frames (RXMF)
>>>
>>> 18 % total match ratio (RXMR)
>>> 2400 frames/s total tx rate (TXR)
>>> 4400 frames/s total rx rate (RXR)
>>>
>>> 18 % current match ratio (CRXMR)
>>> 2400 frames/s current tx rate (CTXR)
>>> 4400 frames/s current rx rate (CRXR)
>>>
>>> 18 % max match ratio (MRXMR)
>>> 2427 frames/s max tx rate (MTXR)
>>> 4427 frames/s max rx rate (MRXR)
>>>
>>> 4 current receive list entries (CRCV)
>>> 9 maximum receive list entries (MRCV)
>>>
>>> 2 statistic resets (STR)
>>
>> # ip -d -s link show can0
>>
>> will list the CAN statistics.
>>
>> Wolfgang.
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-can" in
>> the body of a message to majordomo <at> vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>
> Hi Wolfgang,
>
> thank you for help. I've recognized that these two lines comes from a
> network world:
>
> RX packets:12462166 errors:121380 dropped:0 overruns:0 frame:121380
> TX packets:11604619 errors:678 dropped:0 overruns:0 carrier:0
What do you mean? CAN belongs to the network world. What does the
following command report:
# ip -d -s link show can0
> Then each event "arbitration lost" generates an IRQ.
From a CAN device?
> Is it OK to disable it, just now for tests. With that I want to reduce an
> amount of IRQs. Is it possible to disable it without a new compilation of a
> kernel?
"ifconfig <device> down" should do the job. You may get in trouble if
it's your ethernet device, though.
> We are running quite fast, in few ms. So we test your driver quite well:)
With "-rt" or with vanilla Linux?
Wolfgang.
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v6] can: sja1000: fix {pre,post}_irq() handling and IRQ handler return value
2013-12-05 20:26 ` Wolfgang Grandegger
@ 2013-12-06 9:27 ` Richard Andrysek
2013-12-06 9:56 ` Wolfgang Grandegger
2013-12-06 10:12 ` Marc Kleine-Budde
0 siblings, 2 replies; 27+ messages in thread
From: Richard Andrysek @ 2013-12-06 9:27 UTC (permalink / raw)
To: linux-can
Wolfgang Grandegger <wg <at> grandegger.com> writes:
>
> What do you mean? CAN belongs to the network world. What does the
> following command report:
>
> # ip -d -s link show can0
I have now already modified driver, but before it shown the overrun &
arbitration lost errors. Perfect command:)
>
> > Then each event "arbitration lost" generates an IRQ.
>
> From a CAN device?
Yes you can see it in a code of driver/net/can/sja1000/sja1000.c,
function:
static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
{
...
if (isrc & IRQ_ALI) {
/* arbitration lost interrupt */
netdev_dbg(dev, "arbitration lost interrupt\n");
alc = priv->read_reg(priv, SJA1000_ALC);
priv->can.can_stats.arbitration_lost++;
stats->tx_errors++;
cf->can_id |= CAN_ERR_LOSTARB;
cf->data[0] = alc & 0x1f;
}
...
}
I've just studied sja1000.c I do not know yet, if it is healthy.
May be something in upper levels will be not anymore well handled.
I've changed a code in a function (see IRQ_ALI part):
static void set_normal_mode(struct net_device *dev)
{
...
if (priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING)
priv->write_reg(priv, SJA1000_IER, IRQ_ALL & ~IRQ_ALI);
else
priv->write_reg(priv, SJA1000_IER,
IRQ_ALL & ~(IRQ_BEI | IRQ_ALI));
...
}
But I prefare to make some kind of "ioctl" support for that. There are
applications, where it shall not happend. Concurrently I've played with
taskset and priorities.
$ ps -e -o pid,rtprio,comm | grep "irq/19"
56 89 irq/19-uhci_hcd
1877 88 irq/19-can0
1922 88 irq/19-can1
I do not still understand, what does it mean "irq/19-uhci_hcd". Which is
also active, even I do not have any USB device. Mouse and keyboard are
connected through PS2.
$ lsusb
Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 002 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 003 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 004 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 005 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Now I have NO FAILURE. To make 100% confirmation, I have to improve tests.
Until now:
$ ip -d -s link show can0
3: 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 1000000 sample-point 0.750
tq 250 prop-seg 1 phase-seg1 1 phase-seg2 1 sjw 1
sja1000: tseg1 1..16 tseg2 1..8 sjw 1..4 brp 1..64 brp-inc 1
clock 8000000
re-started bus-errors arbit-lost error-warn error-pass bus-off
0 0 0 0 0 0
RX: bytes packets errors dropped overrun mcast
1874449440 234306180 0 0 0 0
TX: bytes packets errors dropped carrier collsns
771292494 107205646 0 0 0 0
$ ip -d -s link show can1
4: can1: <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 1000000 sample-point 0.750
tq 250 prop-seg 1 phase-seg1 1 phase-seg2 1 sjw 1
sja1000: tseg1 1..16 tseg2 1..8 sjw 1..4 brp 1..64 brp-inc 1
clock 8000000
re-started bus-errors arbit-lost error-warn error-pass bus-off
0 0 0 0 0 0
RX: bytes packets errors dropped overrun mcast
384541802 59160277 0 0 0 0
TX: bytes packets errors dropped carrier collsns
77591108 9843156 0 0 0 0
Please keep in mind arbit-lost is now zero forever.
>
> > Is it OK to disable it, just now for tests. With that I want to reduce an
> > amount of IRQs. Is it possible to disable it without a new compilation of a
> > kernel?
>
> "ifconfig <device> down" should do the job. You may get in trouble if
> it's your ethernet device, though.
>
I do not want to disable a can, just an interrupt "arbitration lost" for TX.
> > We are running quite fast, in few ms. So we test your driver quite well:)
>
> With "-rt" or with vanilla Linux?
3.12.1-custom6-rt4
> Wolfgang.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-can" in
> the body of a message to majordomo <at> vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
What do you think about support for enabling/disabling selected IRQs? Is it
already done?
By
Richard
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v6] can: sja1000: fix {pre,post}_irq() handling and IRQ handler return value
2013-12-06 9:27 ` Richard Andrysek
@ 2013-12-06 9:56 ` Wolfgang Grandegger
2013-12-06 10:32 ` Richard Andrysek
2013-12-06 10:12 ` Marc Kleine-Budde
1 sibling, 1 reply; 27+ messages in thread
From: Wolfgang Grandegger @ 2013-12-06 9:56 UTC (permalink / raw)
To: Richard Andrysek; +Cc: linux-can
On Fri, 6 Dec 2013 09:27:27 +0000 (UTC), Richard Andrysek
<richard.andrysek@rg-mechatronics.com> wrote:
> Wolfgang Grandegger <wg <at> grandegger.com> writes:
>
>>
>> What do you mean? CAN belongs to the network world. What does the
>> following command report:
>>
>> # ip -d -s link show can0
>
> I have now already modified driver, but before it shown the overrun &
> arbitration lost errors. Perfect command:)
>>
>> > Then each event "arbitration lost" generates an IRQ.
>>
>> From a CAN device?
>
> Yes you can see it in a code of driver/net/can/sja1000/sja1000.c,
> function:
>
> static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t
> status)
> {
> ...
> if (isrc & IRQ_ALI) {
> /* arbitration lost interrupt */
> netdev_dbg(dev, "arbitration lost interrupt\n");
> alc = priv->read_reg(priv, SJA1000_ALC);
> priv->can.can_stats.arbitration_lost++;
> stats->tx_errors++;
> cf->can_id |= CAN_ERR_LOSTARB;
> cf->data[0] = alc & 0x1f;
> }
> ...
> }
>
>
> I've just studied sja1000.c I do not know yet, if it is healthy.
This is normally due to hardware/electrical problems. It has nothing to do
with software. Well, is there *any* relation to the patch mentioned in the
subject? I mean, does the problem *not* show up without that patch.
> May be something in upper levels will be not anymore well handled.
> I've changed a code in a function (see IRQ_ALI part):
>
> static void set_normal_mode(struct net_device *dev)
> {
> ...
> if (priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING)
> priv->write_reg(priv, SJA1000_IER, IRQ_ALL &
~IRQ_ALI);
> else
> priv->write_reg(priv, SJA1000_IER,
> IRQ_ALL & ~(IRQ_BEI | IRQ_ALI));
> ...
> }
>
> But I prefare to make some kind of "ioctl" support for that. There are
> applications, where it shall not happend. Concurrently I've played with
> taskset and priorities.
>
> $ ps -e -o pid,rtprio,comm | grep "irq/19"
> 56 89 irq/19-uhci_hcd
> 1877 88 irq/19-can0
> 1922 88 irq/19-can1
>
> I do not still understand, what does it mean "irq/19-uhci_hcd". Which is
> also active, even I do not have any USB device. Mouse and keyboard are
> connected through PS2.
Seem that the interrupts are shared. "$ cat /pric/interrupts" provide
further
information.
Wolfgang.
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v6] can: sja1000: fix {pre,post}_irq() handling and IRQ handler return value
2013-12-06 9:56 ` Wolfgang Grandegger
@ 2013-12-06 10:32 ` Richard Andrysek
2013-12-06 18:32 ` Wolfgang Grandegger
0 siblings, 1 reply; 27+ messages in thread
From: Richard Andrysek @ 2013-12-06 10:32 UTC (permalink / raw)
To: linux-can
Wolfgang Grandegger <wg <at> grandegger.com> writes:
>
>...
>
> > I've just studied sja1000.c I do not know yet, if it is healthy.
>
> This is normally due to hardware/electrical problems. It has nothing to do
>
> with software. Well, is there *any* relation to the patch mentioned in the
>
> subject? I mean, does the problem *not* show up without that patch.
Patch solved a problem with unhandled IRQs. The original problem is solved.
CAN arbitration itself is for me a feature, which I as a programmer want to
control and use it as I want. Similar other IRQs.
>
> > May be something in upper levels will be not anymore well handled.
>
> > I've changed a code in a function (see IRQ_ALI part):
>
> >
>
> > static void set_normal_mode(struct net_device *dev)
>
> > {
>
> > ...
>
> > if (priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING)
>
> > priv->write_reg(priv, SJA1000_IER, IRQ_ALL &
>
> ~IRQ_ALI);
>
> > else
>
> > priv->write_reg(priv, SJA1000_IER,
>
> > IRQ_ALL & ~(IRQ_BEI | IRQ_ALI));
>
> > ...
>
> > }
>
> >
>
> > But I prefare to make some kind of "ioctl" support for that. There are
>
> > applications, where it shall not happend. Concurrently I've played with
>
> > taskset and priorities.
>
> >
>
> > $ ps -e -o pid,rtprio,comm | grep "irq/19"
>
> > 56 89 irq/19-uhci_hcd
>
> > 1877 88 irq/19-can0
>
> > 1922 88 irq/19-can1
>
> >
>
> > I do not still understand, what does it mean "irq/19-uhci_hcd". Which is
>
> > also active, even I do not have any USB device. Mouse and keyboard are
>
> > connected through PS2.
>
> Seem that the interrupts are shared. "$ cat /pric/interrupts" provide
>
> further
>
> information.
Yes, it is so.
$ cat /proc/interrupts
CPU0 CPU1
0: 127 0 IO-APIC-edge timer
1: 5582 0 IO-APIC-edge i8042
6: 3 0 IO-APIC-edge floppy
7: 1 0 IO-APIC-edge parport0
8: 1 0 IO-APIC-edge rtc0
9: 0 0 IO-APIC-fasteoi acpi
12: 30627 0 IO-APIC-edge i8042
14: 43351 0 IO-APIC-edge ata_piix
15: 286242 0 IO-APIC-edge ata_piix
16: 0 0 IO-APIC-fasteoi uhci_hcd:usb2, uhci_hcd:usb5
17: 370 0 IO-APIC-fasteoi i801_smbus, snd_intel8x0
18: 102206 0 IO-APIC-fasteoi uhci_hcd:usb4, eth0
19: 397673976 0 IO-APIC-fasteoi uhci_hcd:usb3, can0, can1
23: 0 0 IO-APIC-fasteoi ehci_hcd:usb1
NMI: 1 1 Non-maskable interrupts
LOC: 15370305 11437013 Local timer interrupts
SPU: 0 0 Spurious interrupts
PMI: 0 0 Performance monitoring interrupts
IWI: 0 0 IRQ work interrupts
RTR: 0 0 APIC ICR read retries
RES: 283821 18429091 Rescheduling interrupts
CAL: 9623 3662 Function call interrupts
TLB: 7408 66943 TLB shootdowns
THR: 0 0 Threshold APIC interrupts
MCE: 0 0 Machine check exceptions
MCP: 0 0 Machine check polls
ERR: 0
MIS: 0
I prefer to switch off it from a linux side. I will look for that.
>
> Wolfgang.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-can" in
> the body of a message to majordomo <at> vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v6] can: sja1000: fix {pre,post}_irq() handling and IRQ handler return value
2013-12-06 10:32 ` Richard Andrysek
@ 2013-12-06 18:32 ` Wolfgang Grandegger
2013-12-09 9:29 ` Richard Andrysek
0 siblings, 1 reply; 27+ messages in thread
From: Wolfgang Grandegger @ 2013-12-06 18:32 UTC (permalink / raw)
To: Richard Andrysek, linux-can
On 12/06/2013 11:32 AM, Richard Andrysek wrote:
> Wolfgang Grandegger <wg <at> grandegger.com> writes:
>
>>
>> ...
>>
>>> I've just studied sja1000.c I do not know yet, if it is healthy.
>>
>> This is normally due to hardware/electrical problems. It has nothing to do
>>
>> with software. Well, is there *any* relation to the patch mentioned in the
>>
>> subject? I mean, does the problem *not* show up without that patch.
>
>
> Patch solved a problem with unhandled IRQs. The original problem is solved.
What did it fix? Could you elaborate on that issue. You actually never
reported a problem.
Wolfgang.
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v6] can: sja1000: fix {pre,post}_irq() handling and IRQ handler return value
2013-12-06 18:32 ` Wolfgang Grandegger
@ 2013-12-09 9:29 ` Richard Andrysek
0 siblings, 0 replies; 27+ messages in thread
From: Richard Andrysek @ 2013-12-09 9:29 UTC (permalink / raw)
To: linux-can
Wolfgang Grandegger <wg <at> grandegger.com> writes:
>
> On 12/06/2013 11:32 AM, Richard Andrysek wrote:
> > Wolfgang Grandegger <wg <at> grandegger.com> writes:
> >
> >>
> >> ...
> >>
> >>> I've just studied sja1000.c I do not know yet, if it is healthy.
> >>
> >> This is normally due to hardware/electrical problems. It has nothing to do
> >>
> >> with software. Well, is there *any* relation to the patch mentioned in the
> >>
> >> subject? I mean, does the problem *not* show up without that patch.
> >
> >
> > Patch solved a problem with unhandled IRQs. The original problem is solved.
>
> What did it fix? Could you elaborate on that issue. You actually never
> reported a problem.
>
> Wolfgang.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-can" in
> the body of a message to majordomo <at> vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
I've reported a problem by peak-systems and they recommend me this patch.
Without this patch with gotos, it leaves unhandled IRQs, e.g.:
Dec 1 04:59:33 xxx kernel: [141189.277965] irq 19: nobody cared (try
booting with the "irqpoll" option)
...
Dec 1 04:59:33 xxx kernel: [141189.278131] Disabling IRQ #19
With this patch the sja1000 IRQs are not lost.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v6] can: sja1000: fix {pre,post}_irq() handling and IRQ handler return value
2013-12-06 9:27 ` Richard Andrysek
2013-12-06 9:56 ` Wolfgang Grandegger
@ 2013-12-06 10:12 ` Marc Kleine-Budde
2013-12-06 10:57 ` Richard Andrysek
1 sibling, 1 reply; 27+ messages in thread
From: Marc Kleine-Budde @ 2013-12-06 10:12 UTC (permalink / raw)
To: Richard Andrysek; +Cc: linux-can
[-- Attachment #1: Type: text/plain, Size: 4837 bytes --]
On 12/06/2013 10:27 AM, Richard Andrysek wrote:
>> What do you mean? CAN belongs to the network world. What does the
>> following command report:
>>
>> # ip -d -s link show can0
>
> I have now already modified driver, but before it shown the overrun &
> arbitration lost errors. Perfect command:)
>>
>>> Then each event "arbitration lost" generates an IRQ.
>>
>> From a CAN device?
>
> Yes you can see it in a code of driver/net/can/sja1000/sja1000.c,
> function:
>
> static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
> {
> ...
> if (isrc & IRQ_ALI) {
> /* arbitration lost interrupt */
> netdev_dbg(dev, "arbitration lost interrupt\n");
> alc = priv->read_reg(priv, SJA1000_ALC);
> priv->can.can_stats.arbitration_lost++;
> stats->tx_errors++;
> cf->can_id |= CAN_ERR_LOSTARB;
> cf->data[0] = alc & 0x1f;
> }
> ...
> }
>
>
> I've just studied sja1000.c I do not know yet, if it is healthy.
> May be something in upper levels will be not anymore well handled.
> I've changed a code in a function (see IRQ_ALI part):
You can send a diff, to illustrate your changes.
> static void set_normal_mode(struct net_device *dev)
> {
> ...
> if (priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING)
> priv->write_reg(priv, SJA1000_IER, IRQ_ALL & ~IRQ_ALI);
> else
> priv->write_reg(priv, SJA1000_IER,
> IRQ_ALL & ~(IRQ_BEI | IRQ_ALI));
> ...
> }
>
> But I prefare to make some kind of "ioctl" support for that. There are
> applications, where it shall not happend. Concurrently I've played with
> taskset and priorities.
ioctl() is not a option here, but there are two options:
1) We can put the arbitration lost error to the bus errors,
but I think that's wrong.
2) You can add another ctrlmode to disable arbitration lost error
reporting.
> $ ps -e -o pid,rtprio,comm | grep "irq/19"
> 56 89 irq/19-uhci_hcd
> 1877 88 irq/19-can0
> 1922 88 irq/19-can1
>
> I do not still understand, what does it mean "irq/19-uhci_hcd". Which is
> also active, even I do not have any USB device. Mouse and keyboard are
> connected through PS2.
The USB interrupt is needed, even if you don't have a device plugged
into an USB port. You have to disable you USB Device in your BIOS,
disable the driver or something like that.
> $ lsusb
> Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
> Bus 002 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
> Bus 003 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
> Bus 004 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
> Bus 005 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
>
> Now I have NO FAILURE. To make 100% confirmation, I have to improve tests.
> Until now:
>
> $ ip -d -s link show can0
> 3: 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 1000000 sample-point 0.750
> tq 250 prop-seg 1 phase-seg1 1 phase-seg2 1 sjw 1
> sja1000: tseg1 1..16 tseg2 1..8 sjw 1..4 brp 1..64 brp-inc 1
> clock 8000000
> re-started bus-errors arbit-lost error-warn error-pass bus-off
> 0 0 0 0 0 0
> RX: bytes packets errors dropped overrun mcast
> 1874449440 234306180 0 0 0 0
> TX: bytes packets errors dropped carrier collsns
> 771292494 107205646 0 0 0 0
>
> $ ip -d -s link show can1
> 4: can1: <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 1000000 sample-point 0.750
> tq 250 prop-seg 1 phase-seg1 1 phase-seg2 1 sjw 1
> sja1000: tseg1 1..16 tseg2 1..8 sjw 1..4 brp 1..64 brp-inc 1
> clock 8000000
> re-started bus-errors arbit-lost error-warn error-pass bus-off
> 0 0 0 0 0 0
> RX: bytes packets errors dropped overrun mcast
> 384541802 59160277 0 0 0 0
> TX: bytes packets errors dropped carrier collsns
> 77591108 9843156 0 0 0 0
>
>
> Please keep in mind arbit-lost is now zero forever.
Because you disabled it, right?
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 |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v6] can: sja1000: fix {pre,post}_irq() handling and IRQ handler return value
2013-12-06 10:12 ` Marc Kleine-Budde
@ 2013-12-06 10:57 ` Richard Andrysek
2013-12-06 11:45 ` arbitration lost error reporting (was: Re: [PATCH v6] can: sja1000: fix {pre,post}_irq() handling and IRQ handler return value) Marc Kleine-Budde
0 siblings, 1 reply; 27+ messages in thread
From: Richard Andrysek @ 2013-12-06 10:57 UTC (permalink / raw)
To: linux-can
Marc Kleine-Budde <mkl <at> pengutronix.de> writes:
>
> You can send a diff, to illustrate your changes.
$ diff /cygdrive/X/bug1/sja1000.c /cygdrive/X/modified_sja1000.c
151a152
> /* RAN 5.12.2013: Disable arbitration IRQ */
153c154
< priv->write_reg(priv, SJA1000_IER, IRQ_ALL);
---
> priv->write_reg(priv, SJA1000_IER, IRQ_ALL &
~IRQ_ALI);
156c157
< IRQ_ALL & ~IRQ_BEI);
---
> IRQ_ALL & ~(IRQ_BEI | IRQ_ALI));
> > But I prefare to make some kind of "ioctl" support for that. There are
> > applications, where it shall not happend. Concurrently I've played with
> > taskset and priorities.
>
> ioctl() is not a option here, but there are two options:
> 1) We can put the arbitration lost error to the bus errors,
> but I think that's wrong.
> 2) You can add another ctrlmode to disable arbitration lost error
> reporting.
> ...
> > 4: can1: <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 1000000 sample-point 0.750
> > tq 250 prop-seg 1 phase-seg1 1 phase-seg2 1 sjw 1
> > sja1000: tseg1 1..16 tseg2 1..8 sjw 1..4 brp 1..64 brp-inc 1
> > clock 8000000
> > re-started bus-errors arbit-lost error-warn error-pass bus-off
> > 0 0 0 0 0 0
> > RX: bytes packets errors dropped overrun mcast
> > 384541802 59160277 0 0 0 0
> > TX: bytes packets errors dropped carrier collsns
> > 77591108 9843156 0 0 0 0
> >
> >
> > Please keep in mind arbit-lost is now zero forever.
>
> Because you disabled it, right?
Yes, that's right.
>
> Marc
As Wolgang mentioned it has nothing to do with the patch v6.
Enabling/diabling IRQs is a new feature. So I don't know, if here it is a
right place to continue about that.
By
Richard
^ permalink raw reply [flat|nested] 27+ messages in thread* arbitration lost error reporting (was: Re: [PATCH v6] can: sja1000: fix {pre,post}_irq() handling and IRQ handler return value)
2013-12-06 10:57 ` Richard Andrysek
@ 2013-12-06 11:45 ` Marc Kleine-Budde
2013-12-06 12:02 ` arbitration lost error reporting Oliver Hartkopp
0 siblings, 1 reply; 27+ messages in thread
From: Marc Kleine-Budde @ 2013-12-06 11:45 UTC (permalink / raw)
To: Richard Andrysek; +Cc: linux-can
[-- Attachment #1: Type: text/plain, Size: 1542 bytes --]
On 12/06/2013 11:57 AM, Richard Andrysek wrote:
>> You can send a diff, to illustrate your changes.
>
> $ diff /cygdrive/X/bug1/sja1000.c /cygdrive/X/modified_sja1000.c
"diff -u" is preferred, but no need to resend it.
>>> But I prefare to make some kind of "ioctl" support for that. There are
>>> applications, where it shall not happend. Concurrently I've played with
>>> taskset and priorities.
>>
>> ioctl() is not a option here, but there are two options:
>> 1) We can put the arbitration lost error to the bus errors,
>> but I think that's wrong.
>> 2) You can add another ctrlmode to disable arbitration lost error
>> reporting.
> As Wolgang mentioned it has nothing to do with the patch v6.
> Enabling/diabling IRQs is a new feature. So I don't know, if here it is a
> right place to continue about that.
Yes, why not? :)
As I outlined above you have two options. I prefer option 2 and it has a
big change of going mainline. The implementation is similar to
CAN_CTRLMODE_BERR_REPORTING, just look for it in the kernel source tree
and add a new define for arbitration lost error reporting.
Do you have a preferred name for the define?
- CAN_CTRLMODE_AERR_REPORTING
- CAN_CTRLMODE_ARBITRATIONERR_REPORTING to
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 |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: arbitration lost error reporting
2013-12-06 11:45 ` arbitration lost error reporting (was: Re: [PATCH v6] can: sja1000: fix {pre,post}_irq() handling and IRQ handler return value) Marc Kleine-Budde
@ 2013-12-06 12:02 ` Oliver Hartkopp
2013-12-06 12:16 ` Marc Kleine-Budde
2013-12-06 17:59 ` Wolfgang Grandegger
0 siblings, 2 replies; 27+ messages in thread
From: Oliver Hartkopp @ 2013-12-06 12:02 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: Richard Andrysek, linux-can
On 06.12.2013 12:45, Marc Kleine-Budde wrote:
> On 12/06/2013 11:57 AM, Richard Andrysek wrote:
>>> You can send a diff, to illustrate your changes.
>>
>> $ diff /cygdrive/X/bug1/sja1000.c /cygdrive/X/modified_sja1000.c
>
> "diff -u" is preferred, but no need to resend it.
>
>>>> But I prefare to make some kind of "ioctl" support for that. There are
>>>> applications, where it shall not happend. Concurrently I've played with
>>>> taskset and priorities.
>>>
>>> ioctl() is not a option here, but there are two options:
>>> 1) We can put the arbitration lost error to the bus errors,
>>> but I think that's wrong.
>>> 2) You can add another ctrlmode to disable arbitration lost error
>>> reporting.
>
>> As Wolgang mentioned it has nothing to do with the patch v6.
>> Enabling/diabling IRQs is a new feature. So I don't know, if here it is a
>> right place to continue about that.
>
> Yes, why not? :)
> As I outlined above you have two options. I prefer option 2 and it has a
> big change of going mainline. The implementation is similar to
> CAN_CTRLMODE_BERR_REPORTING, just look for it in the kernel source tree
> and add a new define for arbitration lost error reporting.
>
> Do you have a preferred name for the define?
> - CAN_CTRLMODE_AERR_REPORTING
> - CAN_CTRLMODE_ARBITRATIONERR_REPORTING to
Loosing the arbitration is not an error. It just can happen from time to time.
If you thing about a define, what about:
CAN_CTRLMODE_ARB_LOST_REPORTING
CAN_CTRLMODE_ARBITRATION_LOST_REPORTING
Regards,
Oliver
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: arbitration lost error reporting
2013-12-06 12:02 ` arbitration lost error reporting Oliver Hartkopp
@ 2013-12-06 12:16 ` Marc Kleine-Budde
2013-12-06 13:21 ` Richard Andrysek
2013-12-06 17:59 ` Wolfgang Grandegger
1 sibling, 1 reply; 27+ messages in thread
From: Marc Kleine-Budde @ 2013-12-06 12:16 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: Richard Andrysek, linux-can
[-- Attachment #1: Type: text/plain, Size: 699 bytes --]
On 12/06/2013 01:02 PM, Oliver Hartkopp wrote:
>> Do you have a preferred name for the define?
>> - CAN_CTRLMODE_AERR_REPORTING
>> - CAN_CTRLMODE_ARBITRATIONERR_REPORTING to
>
> Loosing the arbitration is not an error. It just can happen from time to time.
Details :)
> If you thing about a define, what about:
>
> [ ] CAN_CTRLMODE_ARB_LOST_REPORTING
> [x] CAN_CTRLMODE_ARBITRATION_LOST_REPORTING
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 |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: arbitration lost error reporting
2013-12-06 12:02 ` arbitration lost error reporting Oliver Hartkopp
2013-12-06 12:16 ` Marc Kleine-Budde
@ 2013-12-06 17:59 ` Wolfgang Grandegger
2013-12-07 13:13 ` Oliver Hartkopp
2013-12-09 9:01 ` Richard Andrysek
1 sibling, 2 replies; 27+ messages in thread
From: Wolfgang Grandegger @ 2013-12-06 17:59 UTC (permalink / raw)
To: Oliver Hartkopp, Marc Kleine-Budde; +Cc: Richard Andrysek, linux-can
On 12/06/2013 01:02 PM, Oliver Hartkopp wrote:
>
>
> On 06.12.2013 12:45, Marc Kleine-Budde wrote:
>> On 12/06/2013 11:57 AM, Richard Andrysek wrote:
>>>> You can send a diff, to illustrate your changes.
>>>
>>> $ diff /cygdrive/X/bug1/sja1000.c /cygdrive/X/modified_sja1000.c
>>
>> "diff -u" is preferred, but no need to resend it.
>>
>>>>> But I prefare to make some kind of "ioctl" support for that. There are
>>>>> applications, where it shall not happend. Concurrently I've played with
>>>>> taskset and priorities.
>>>>
>>>> ioctl() is not a option here, but there are two options:
>>>> 1) We can put the arbitration lost error to the bus errors,
>>>> but I think that's wrong.
>>>> 2) You can add another ctrlmode to disable arbitration lost error
>>>> reporting.
>>
>>> As Wolgang mentioned it has nothing to do with the patch v6.
>>> Enabling/diabling IRQs is a new feature. So I don't know, if here it is a
>>> right place to continue about that.
>>
>> Yes, why not? :)
>> As I outlined above you have two options. I prefer option 2 and it has a
>> big change of going mainline. The implementation is similar to
>> CAN_CTRLMODE_BERR_REPORTING, just look for it in the kernel source tree
>> and add a new define for arbitration lost error reporting.
>>
>> Do you have a preferred name for the define?
>> - CAN_CTRLMODE_AERR_REPORTING
>> - CAN_CTRLMODE_ARBITRATIONERR_REPORTING to
>
> Loosing the arbitration is not an error. It just can happen from time to time.
Well, if it does not happen often, why do we want to suppress reporting
this error? I still do not see what it's good for.
Wolfgang.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: arbitration lost error reporting
2013-12-06 17:59 ` Wolfgang Grandegger
@ 2013-12-07 13:13 ` Oliver Hartkopp
2013-12-09 9:01 ` Richard Andrysek
1 sibling, 0 replies; 27+ messages in thread
From: Oliver Hartkopp @ 2013-12-07 13:13 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: Marc Kleine-Budde, Richard Andrysek, linux-can
On 06.12.2013 18:59, Wolfgang Grandegger wrote:
> On 12/06/2013 01:02 PM, Oliver Hartkopp wrote:
>> On 06.12.2013 12:45, Marc Kleine-Budde wrote:
>>> As I outlined above you have two options. I prefer option 2 and it has a
>>> big change of going mainline. The implementation is similar to
>>> CAN_CTRLMODE_BERR_REPORTING, just look for it in the kernel source tree
>>> and add a new define for arbitration lost error reporting.
>>>
>>> Do you have a preferred name for the define?
>>> - CAN_CTRLMODE_AERR_REPORTING
>>> - CAN_CTRLMODE_ARBITRATIONERR_REPORTING to
>>
>> Loosing the arbitration is not an error. It just can happen from time to time.
>
> Well, if it does not happen often, why do we want to suppress reporting
> this error? I still do not see what it's good for.
Sometimes it's good to let it settle.
Indeed the CAN_CTRLMODE_BERR_REPORTING was introduced to reduce the irq storms
that can occur when bus errors happen on the bus.
All other states are promoted by the driver when this information can be
retrieved from the hardware.
Btw. it seems reasonable to ask whether it makes sense to create error message
skbs when there's no consumer of the information.
E.g. the networking timestamps are only enabled when (at least) one
application requires timestamps on the host.
Therefore I would suggest to create the error message skb only when someone is
requesting this information (== an error message filter is enabled).
I'll send a RFC patch for that ...
Oliver
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: arbitration lost error reporting
2013-12-06 17:59 ` Wolfgang Grandegger
2013-12-07 13:13 ` Oliver Hartkopp
@ 2013-12-09 9:01 ` Richard Andrysek
2013-12-09 10:32 ` Marc Kleine-Budde
1 sibling, 1 reply; 27+ messages in thread
From: Richard Andrysek @ 2013-12-09 9:01 UTC (permalink / raw)
To: linux-can
Wolfgang Grandegger <wg <at> grandegger.com> writes:
...
> >> big change of going mainline. The implementation is similar to
> >> CAN_CTRLMODE_BERR_REPORTING, just look for it in the kernel source tree
> >> and add a new define for arbitration lost error reporting.
> >>
> >> Do you have a preferred name for the define?
> >> - CAN_CTRLMODE_AERR_REPORTING
> >> - CAN_CTRLMODE_ARBITRATIONERR_REPORTING to
> >
> > Loosing the arbitration is not an error. It just can happen from time to
time.
>
> Well, if it does not happen often, why do we want to suppress reporting
> this error? I still do not see what it's good for.
>
> Wolfgang.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-can" in
> the body of a message to majordomo <at> vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
How do you define not often? Is it not often one time per ~130us? For X
channels you should divide it through X. Please assume this case stochastic
=> not always.
Next if the information is for an application irrelevant, why I need to
generate an IRQ and use CPU resources?
How to fight with such case:
1) Use time triggered protocol - not always possible
2) Slow done communication - not always possible
3) Buy faster machine - not always possible
4) Combination of 1..4 - ...
or
5) Let a freedom to a programmer, for the price that he shall to know it.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: arbitration lost error reporting
2013-12-09 9:01 ` Richard Andrysek
@ 2013-12-09 10:32 ` Marc Kleine-Budde
0 siblings, 0 replies; 27+ messages in thread
From: Marc Kleine-Budde @ 2013-12-09 10:32 UTC (permalink / raw)
To: Richard Andrysek; +Cc: linux-can
[-- Attachment #1: Type: text/plain, Size: 1920 bytes --]
On 12/09/2013 10:01 AM, Richard Andrysek wrote:
> Wolfgang Grandegger <wg <at> grandegger.com> writes:
>
> ...
>>>> big change of going mainline. The implementation is similar to
>>>> CAN_CTRLMODE_BERR_REPORTING, just look for it in the kernel source tree
>>>> and add a new define for arbitration lost error reporting.
>>>>
>>>> Do you have a preferred name for the define?
>>>> - CAN_CTRLMODE_AERR_REPORTING
>>>> - CAN_CTRLMODE_ARBITRATIONERR_REPORTING to
>>>
>>> Loosing the arbitration is not an error. It just can happen from time to
> time.
>>
>> Well, if it does not happen often, why do we want to suppress reporting
>> this error? I still do not see what it's good for.
>>
>> Wolfgang.
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-can" in
>> the body of a message to majordomo <at> vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>
> How do you define not often? Is it not often one time per ~130us? For X
> channels you should divide it through X. Please assume this case stochastic
> => not always.
> Next if the information is for an application irrelevant, why I need to
> generate an IRQ and use CPU resources?
>
> How to fight with such case:
>
> 1) Use time triggered protocol - not always possible
> 2) Slow done communication - not always possible
> 3) Buy faster machine - not always possible
> 4) Combination of 1..4 - ...
>
> or
>
> 5) Let a freedom to a programmer, for the price that he shall to know it.
+1
Feel free to implement it, as outlined in the previous mails.
regards,
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 |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2013-12-09 23:50 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-24 23:03 [PATCH v6] can: sja1000: fix {pre,post}_irq() handling and IRQ handler return value Marc Kleine-Budde
2013-11-25 8:54 ` Marc Kleine-Budde
2013-11-25 18:12 ` Oliver Hartkopp
2013-11-25 22:05 ` Austin Schuh
2013-12-09 19:48 ` Austin Schuh
2013-12-09 21:07 ` Marc Kleine-Budde
2013-12-09 23:50 ` Austin Schuh
2013-12-05 15:50 ` Richard Andrysek
2013-12-05 17:50 ` Wolfgang Grandegger
2013-12-05 19:37 ` Richard Andrysek
2013-12-05 20:26 ` Wolfgang Grandegger
2013-12-06 9:27 ` Richard Andrysek
2013-12-06 9:56 ` Wolfgang Grandegger
2013-12-06 10:32 ` Richard Andrysek
2013-12-06 18:32 ` Wolfgang Grandegger
2013-12-09 9:29 ` Richard Andrysek
2013-12-06 10:12 ` Marc Kleine-Budde
2013-12-06 10:57 ` Richard Andrysek
2013-12-06 11:45 ` arbitration lost error reporting (was: Re: [PATCH v6] can: sja1000: fix {pre,post}_irq() handling and IRQ handler return value) Marc Kleine-Budde
2013-12-06 12:02 ` arbitration lost error reporting Oliver Hartkopp
2013-12-06 12:16 ` Marc Kleine-Budde
2013-12-06 13:21 ` Richard Andrysek
2013-12-06 13:23 ` Marc Kleine-Budde
2013-12-06 17:59 ` Wolfgang Grandegger
2013-12-07 13:13 ` Oliver Hartkopp
2013-12-09 9:01 ` Richard Andrysek
2013-12-09 10:32 ` Marc Kleine-Budde
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).