From: swarren@wwwdotorg•org (Stephen Warren)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH V3 2/5] ARM: bcm2708: add interrupt controller driver
Date: Thu, 13 Sep 2012 20:21:14 -0600 [thread overview]
Message-ID: <5052949A.1020909@wwwdotorg.org> (raw)
In-Reply-To: <201209131045.20730.arnd@arndb.de>
On 09/13/2012 04:45 AM, Arnd Bergmann wrote:
> On Thursday 13 September 2012, Stephen Warren wrote:
>> On 09/12/2012 04:37 AM, Arnd Bergmann wrote:
>>>> @@ -0,0 +1,110 @@
>>>> +BCM2708 Top-Level ("ARMCTRL") Interrupt Controller
>>>> +
>>>> +The BCM2708 contains a custom top-level interrupt controller, which supports
>>>> +72 interrupt sources using a 2-level register scheme. The interrupt
>>>> +controller, or the HW block containing it, is referred to occasionally
>>>> +as "armctrl" in the SoC documentation, hence naming of this binding.
>>>
>>> Do we actually know that BCM2708 has the same one, or could it be present
>>> just on bcm2835? It seem hard to find any information about bcm2708,
>>> so I don't feel too good about using that name in bindings.
>>
>> I don't know anything at all about the BCM2708 really. Perhaps Dom at
>> Broadcom can fill in some details?
>>
>> A similar discussion was apparently held downstream, and IIRC the
>> reported decision there was that BCM2708 was the "parent" of a family of
>> SoCs, so they made all the DT stuff compatible with both 2708 and 2835.
>> Given the lack of documentation, I'd be quite happy to rework all of
>> this to say just BCM2835 instead, and drop any reference to BCM2708 at
>> all. Should I just go ahead and do that?
>
> That's probably safer, yes.
Sounds good. For reference, I found:
https://github.com/raspberrypi/linux/issues/22
... where it sounds like BCM2708 isn't actually a chip at all, but a
family name, so that supports this change.
>>>> +asmlinkage void __exception_irq_entry bcm2708_armctrl_handle_irq(
>>>> + struct pt_regs *regs)
>>>> +{
>>>> + u32 stat, irq;
>>>> +
>>>> + while ((stat = readl_relaxed(intc.pending[0]) & BANK0_VALID_MASK)) {
>>>> + if (stat & BANK0_HWIRQ_MASK) {
>>>> + irq = MAKE_HWIRQ(0, ffs(stat & BANK0_HWIRQ_MASK) - 1);
>>>> + handle_IRQ(irq_linear_revmap(intc.domain, irq), regs);
>>>> + } else if (stat & SHORTCUT1_MASK) {
>>>> + armctrl_handle_shortcut(1, regs, stat & SHORTCUT1_MASK);
>>>> + } else if (stat & SHORTCUT2_MASK) {
>>>> + armctrl_handle_shortcut(2, regs, stat & SHORTCUT2_MASK);
>>>> + } else if (stat & BANK1_HWIRQ) {
>>>> + armctrl_handle_bank(1, regs);
>>>> + } else if (stat & BANK2_HWIRQ) {
>>>> + armctrl_handle_bank(2, regs);
>>>> + } else {
>>>> + BUG();
>>>> + }
>>>> + }
>>>> +}
>>>
>>> I'm not sure if readl_relaxed() is appropriate here, or if you need readl().
>>> If you have an MSI type interrupt signaling the completion of a DMA, you
>>> need to ensure ordering between the data transfer and the interrupt
>>> notification.
>>
>> I did wonder about this. I suppose it would be safe to globally replace
>> all readl/writel_relaxed with plain readl/writel, and fix this up later
>> if we can justify it. Should I go ahead and do that?
>
> The synchronizations can be a bit expensive, so in the interrupt controller
> driver it makes sense to use at least writel_relaxed, which should always
> be fine because you don't have to worry about outgoing DMAs.
Thinking some more about this - I doubt there are any MSI-style
interrupts; there's certainly no PCI/PCIe on the Raspberry Pi board, and
none documented in the SoC itself (although admittedly only a small
subset of the SoC is publicly documented). I guess it's easiest just to
leave that code as-is, and fix it up if the hardware ever turns out to
be more complex, and actually have MSI.
next prev parent reply other threads:[~2012-09-14 2:21 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-09-12 4:18 [PATCH V3 1/5] ARM: add infra-structure for BCM2708/BCM2835 and Raspberry Pi Stephen Warren
2012-09-12 4:18 ` [PATCH V3 2/5] ARM: bcm2708: add interrupt controller driver Stephen Warren
2012-09-12 10:37 ` Arnd Bergmann
2012-09-13 1:12 ` Stephen Warren
2012-09-13 10:45 ` Arnd Bergmann
2012-09-14 2:21 ` Stephen Warren [this message]
2012-09-12 4:18 ` [PATCH V3 3/5] ARM: bcm2708: add system timer Stephen Warren
2012-09-12 10:40 ` Arnd Bergmann
2012-09-12 4:18 ` [PATCH V3 4/5] ARM: bcm2708: add stub clock driver Stephen Warren
2012-09-12 11:27 ` Arnd Bergmann
2012-09-12 4:18 ` [PATCH V3 5/5] ARM: bcm2708: instantiate console UART Stephen Warren
2012-09-12 10:45 ` Arnd Bergmann
2012-09-13 1:15 ` Stephen Warren
2012-09-13 10:36 ` Arnd Bergmann
2012-09-12 9:01 ` [PATCH V3 1/5] ARM: add infra-structure for BCM2708/BCM2835 and Raspberry Pi Arnd Bergmann
2012-09-13 19:51 ` Domenico Andreoli
2012-09-14 2:16 ` Stephen Warren
2012-09-16 0:34 ` Olof Johansson
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=5052949A.1020909@wwwdotorg.org \
--to=swarren@wwwdotorg$(echo .)org \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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