linux-can.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Oliver Hartkopp <socketcan@hartkopp.net>
To: Austin Schuh <austin@peloton-tech.com>
Cc: Wolfgang Grandegger <wg@grandegger.com>,
	Pavel Pisa <pisa@cmp.felk.cvut.cz>,
	Marc Kleine-Budde <mkl@pengutronix.de>,
	linux-can@vger.kernel.org
Subject: Re: sja1000 interrupt problem
Date: Sat, 21 Dec 2013 13:55:31 +0100	[thread overview]
Message-ID: <52B58FC3.50400@hartkopp.net> (raw)
In-Reply-To: <CANGgnMZNci8_B6O3JmP85ciGHG5kZvhnrBHhjTjJAN09Eea4TA@mail.gmail.com>

Hi Austin,

I was also integrating some counters for handled and non-handled interrupts
per-device - which indicated that note_interrupt() obviously is in charge to
solve the issue.

But I was not able to get further - therefore many thanks for your hint!!

I'll test the patch on Monday to run the system for at least some hours before
I leave the office for XMAS ;-)

Tnx & best regards,
Oliver

On 21.12.2013 00:13, Austin Schuh wrote:
> I have applied the fix proposed in https://lkml.org/lkml/2013/3/7/222
> for the note_interrupt function right now, and will run a test this
> weekend to see if it fixes it for sure.  I am now consistently seeing
> only 1 / 100000 of the IRQ handler calls being counted as unhandled,
> which is a lot better.
> 
> I was concerned that if the handler threads were starved, I could
> cause a bunch of unhandled interrupts, so I did a test.  I stressed
> the system by running 4 realtime tasks (= the number of hyperthreads)
> that were a higher priority than the CAN handler tasks.  I get a 'data
> overrun interrupt', but the unhandled count only climbs to 3 / 100000.
>  I'm no longer worried about that problem, at least with the PEAK
> card.
> 
> Oliver, does this patch fix it for you?
> 
> I'm going to email Thomas on Monday if the system survives the weekend
> with my results and work on getting into mainline.
> 
> Austin
> 
> On Sat, Dec 14, 2013 at 1:51 AM, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
>> Ok. I think I got it now:
>>
>> As long as the PCAN PCI adapter had only 2 channels PEAK obviously used the
>> PSB4600 PITA v1.2 (1-pita_12p.pdf) where there are two interrupt lines INT0
>> and INT1 accessible by bit 0 and bit 1 in the ICR register (see page 191).
>>
>> Due to the discontinuation of the PSB4600 in newer designs there's a Lattice
>> 4256V CPLD, see detail photo at
>>
>>         http://gridconnect.com/pcan/can-adapters/can-mini-pci.html
>>
>> The CPLD now obviously uses the formerly reserved bits 6+7 for the channels
>> 3+4 in a backward compatible manner. So everything with peak_pci_icr_masks[]
>> is fine but it's no real PITA anymore :-)
>>
>> Sorry for the confusion.
>>
>> Looks like there's some more investigation of the -rt irq threads to do :-(
>>
>> Regards,
>> Oliver
>>
>>
>> On 13.12.2013 22:14, Oliver Hartkopp wrote:
>>> Hi all,
>>>
>>> after some more investigation of the two PITA specifications
>>>
>>> https://www.google.de/#q=PCI+Interface+for+Telephony+infineon
>>>
>>> http://pdf.datasheetcatalog.com/datasheet/infineon/1-pita_22p.pdf
>>>
>>> and
>>>
>>> http://pdf.datasheetcatalog.com/datasheet/infineon/1-pita_12p.pdf
>>>
>>> I'm not sure *why* the driver works at all.
>>>
>>> It's the mix between byte and word accesses and especially the per-device
>>> interrupt bits in the PITA_ICR (Interrupt Control Register).
>>>
>>> The interrupt bits in this register GP[0123]_INT are located in the bits
>>> 2,3,4,5 in the ICR (1-pita_12p.pdf, p. 191 / 1-pita_22p.pdf p.202)
>>>
>>> If my interpretation is correct the
>>>
>>>       static const u16 peak_pci_icr_masks[PEAK_PCI_CHAN_MAX] = {
>>>               0x02, 0x01, 0x40, 0x80
>>>       };
>>>
>>> would be completely bogus and would not hit the right bit in
>>>
>>> static void peak_pci_post_irq(const struct sja1000_priv *priv)
>>> {
>>>         struct peak_pci_chan *chan = priv->priv;
>>>         u16 icr;
>>>
>>>         /* Select and clear in PITA stored interrupt */
>>>         icr = readw(chan->cfg_base + PITA_ICR);
>>>         if (icr & chan->icr_mask)
>>>                 writew(chan->icr_mask, chan->cfg_base + PITA_ICR);
>>> }
>>>
>>> at all.
>>>
>>> Am I wrong? Or is this the wrong specification?
>>> The code in ems_pci.c seems to fit to this PITA spec ...
>>>
>>> Regards,
>>> Oliver
>>>
>>>
>>> On 13.12.2013 18:14, Oliver Hartkopp wrote:
>>>> Answering myself ...
>>>>
>>>>>
>>>>> Maybe it's time to look into other implementations than PEAK/mainline ...
>>>>>
>>>>
>>>> E.g. a EMS_PCI adapter has a PITA-2 too (depending on it's HW revicsion).
>>>> There's a EMS PCI driver in mainline and (at least) can4linux.
>>>>
>>>> Both access the registers with 32 bit read/write functions but the peak_pci
>>>> only writes 16 bit values?!?
>>>>
>>>> Checking
>>>>
>>>>      http://pdf.datasheetcatalog.com/datasheet/infineon/1-pita_22p.pdf
>>>>
>>>> 32 bit should be the right way to do it.
>>>>
>>>> Regards,
>>>> Oliver
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-can" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-can" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>

  parent reply	other threads:[~2013-12-21 12:55 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-08  0:47 sja1000 interrupt problem Austin Schuh
2013-10-08  6:32 ` Wolfgang Grandegger
2013-10-08  6:58   ` Oliver Hartkopp
2013-10-08 18:48     ` Austin Schuh
2013-10-08 19:44       ` Wolfgang Grandegger
2013-10-08 20:47         ` Austin Schuh
2013-10-09  6:21           ` Wolfgang Grandegger
2013-10-09  6:31           ` Wolfgang Grandegger
2013-10-09  6:47           ` Wolfgang Grandegger
     [not found]             ` <CANGgnMZpPGctUWGcg7Lp-QFPc7d6A5GeL9KQYnpeYMR8WukgdA@mail.gmail.com>
2013-11-07  8:15               ` Wolfgang Grandegger
2013-11-07 23:43                 ` Austin Schuh
2013-11-09 14:21                   ` Oliver Hartkopp
2013-11-12  2:59                     ` Austin Schuh
2013-11-12 21:26                       ` Oliver Hartkopp
2013-11-12 23:22                         ` Austin Schuh
2013-11-13  3:41                           ` Austin Schuh
2013-11-13  6:58                             ` Oliver Hartkopp
2013-11-13  9:48                               ` Kurt Van Dijck
2013-11-13  6:44                           ` Oliver Hartkopp
2013-11-13  8:11                             ` Wolfgang Grandegger
2013-11-13  9:08                               ` Pavel Pisa
2013-11-13  9:52                                 ` Wolfgang Grandegger
2013-11-13 18:41                                   ` Oliver Hartkopp
2013-11-13 19:29                                     ` Wolfgang Grandegger
2013-11-13 22:00                                       ` Oliver Hartkopp
2013-11-13 11:02                                 ` Kurt Van Dijck
2013-11-16 21:42                                 ` Oliver Hartkopp
2013-11-17  8:18                                   ` Wolfgang Grandegger
2013-11-17 14:27                                     ` Oliver Hartkopp
2013-11-17 17:23                                       ` Wolfgang Grandegger
2013-11-17 20:46                                         ` Wolfgang Grandegger
2013-11-18 17:08                                           ` Austin Schuh
2013-12-09 21:54                                             ` Austin Schuh
2013-12-09 21:54                                               ` Austin Schuh
2013-12-10  7:49                                               ` Wolfgang Grandegger
2013-12-10  8:05                                                 ` Austin Schuh
2013-12-10  9:32                                                   ` Wolfgang Grandegger
2013-12-10 13:47                                                     ` Oliver Hartkopp
2013-12-10 14:23                                                       ` Oliver Hartkopp
2013-12-10 14:41                                                       ` Wolfgang Grandegger
2013-12-10 16:05                                                         ` Oliver Hartkopp
2013-12-10 21:12                                                           ` Wolfgang Grandegger
2013-12-11 16:59                                                             ` Oliver Hartkopp
2013-12-11 19:27                                                               ` Wolfgang Grandegger
2013-12-12  6:13                                                                 ` Oliver Hartkopp
2013-12-12 17:38                                                                   ` Oliver Hartkopp
2013-12-12 22:56                                                                     ` Wolfgang Grandegger
2013-12-13  0:07                                                                       ` Austin Schuh
2013-12-13 16:16                                                                         ` Oliver Hartkopp
2013-12-13  9:38                                                                       ` Oliver Hartkopp
2013-12-13 10:04                                                                         ` Wolfgang Grandegger
2013-12-13 10:09                                                                           ` Wolfgang Grandegger
2013-12-13 16:25                                                                             ` Oliver Hartkopp
2013-12-13 17:33                                                                               ` Wolfgang Grandegger
2013-12-13 10:07                                                                         ` Marc Kleine-Budde
2013-12-13 16:22                                                                           ` Oliver Hartkopp
2013-12-13 17:14                                                                             ` Oliver Hartkopp
2013-12-13 21:14                                                                               ` Oliver Hartkopp
2013-12-14  9:51                                                                                 ` Oliver Hartkopp
2013-12-20 23:13                                                                                   ` Austin Schuh
2013-12-21  8:29                                                                                     ` Wolfgang Grandegger
2013-12-21 13:12                                                                                       ` Oliver Hartkopp
2013-12-21 12:55                                                                                     ` Oliver Hartkopp [this message]
2013-12-23 15:58                                                                                       ` Oliver Hartkopp
2013-11-09 19:42                   ` Wolfgang Grandegger
     [not found]                     ` <CANGgnMbb+VResUC6h+cK6Hfe5PLJx9R9ao6bMdJM2e5BPaDamw@mail.gmail.com>
2013-11-12 22:15                       ` Wolfgang Grandegger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52B58FC3.50400@hartkopp.net \
    --to=socketcan@hartkopp.net \
    --cc=austin@peloton-tech.com \
    --cc=linux-can@vger.kernel.org \
    --cc=mkl@pengutronix.de \
    --cc=pisa@cmp.felk.cvut.cz \
    --cc=wg@grandegger.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).