| Message ID | 1258927311-4340-12-git-send-email-albert_herranz@yahoo.es (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On Sun, Nov 22, 2009 at 3:01 PM, Albert Herranz <albert_herranz@yahoo.es> wrote: > Add support for the interrupt controller included in the "Flipper" > chipset of the Nintendo GameCube video game console. > The same interrupt controller is also present in the "Hollywood" chipset > of the Nintendo Wii. > > Signed-off-by: Albert Herranz <albert_herranz@yahoo.es> > --- > arch/powerpc/platforms/embedded6xx/Kconfig | 6 + > arch/powerpc/platforms/embedded6xx/Makefile | 1 + > arch/powerpc/platforms/embedded6xx/flipper-pic.c | 247 ++++++++++++++++++++++ > arch/powerpc/platforms/embedded6xx/flipper-pic.h | 25 +++ > 4 files changed, 279 insertions(+), 0 deletions(-) > create mode 100644 arch/powerpc/platforms/embedded6xx/flipper-pic.c > create mode 100644 arch/powerpc/platforms/embedded6xx/flipper-pic.h > > diff --git a/arch/powerpc/platforms/embedded6xx/Kconfig b/arch/powerpc/platforms/embedded6xx/Kconfig > index bfd88be..29a98c6 100644 > --- a/arch/powerpc/platforms/embedded6xx/Kconfig > +++ b/arch/powerpc/platforms/embedded6xx/Kconfig > @@ -94,6 +94,7 @@ config MPC10X_STORE_GATHERING > config GAMECUBE_COMMON > bool > select NOT_COHERENT_CACHE > + select FLIPPER_PIC > > config USBGECKO_UDBG > bool "USB Gecko udbg console for the Nintendo GameCube/Wii" > @@ -108,3 +109,8 @@ config USBGECKO_UDBG > > If in doubt, say N here. > > +config FLIPPER_PIC > + bool > + depends on GAMECUBE_COMMON > + default y You'll always want this driver when GAMECUBE common is set. Don't add another Kconfig entry. > + > diff --git a/arch/powerpc/platforms/embedded6xx/Makefile b/arch/powerpc/platforms/embedded6xx/Makefile > index 0ab7492..35258fd 100644 > --- a/arch/powerpc/platforms/embedded6xx/Makefile > +++ b/arch/powerpc/platforms/embedded6xx/Makefile > @@ -8,3 +8,4 @@ obj-$(CONFIG_PPC_HOLLY) += holly.o > obj-$(CONFIG_PPC_PRPMC2800) += prpmc2800.o > obj-$(CONFIG_PPC_C2K) += c2k.o > obj-$(CONFIG_USBGECKO_UDBG) += usbgecko_udbg.o > +obj-$(CONFIG_FLIPPER_PIC) += flipper-pic.o > +unsigned int flipper_pic_get_irq(void) > +{ > + void __iomem *io_base = flipper_irq_host->host_data; > + int irq; > + u32 irq_status; > + > + irq_status = in_be32(io_base + FLIPPER_ICR) & > + in_be32(io_base + FLIPPER_IMR); > + if (irq_status == 0) > + return -1; /* no more IRQs pending */ NO_IRQ_IGNORE > + > + __asm__ __volatile__("cntlzw %0,%1" : "=r"(irq) : "r"(irq_status)); > + return irq_linear_revmap(flipper_irq_host, 31 - irq); > +} > +
Grant Likely wrote: > On Sun, Nov 22, 2009 at 3:01 PM, Albert Herranz <albert_herranz@yahoo.es> wrote: >> Add support for the interrupt controller included in the "Flipper" >> chipset of the Nintendo GameCube video game console. >> The same interrupt controller is also present in the "Hollywood" chipset >> of the Nintendo Wii. >> >> Signed-off-by: Albert Herranz <albert_herranz@yahoo.es> >> --- >> arch/powerpc/platforms/embedded6xx/Kconfig | 6 + >> arch/powerpc/platforms/embedded6xx/Makefile | 1 + >> arch/powerpc/platforms/embedded6xx/flipper-pic.c | 247 ++++++++++++++++++++++ >> arch/powerpc/platforms/embedded6xx/flipper-pic.h | 25 +++ >> 4 files changed, 279 insertions(+), 0 deletions(-) >> create mode 100644 arch/powerpc/platforms/embedded6xx/flipper-pic.c >> create mode 100644 arch/powerpc/platforms/embedded6xx/flipper-pic.h >> >> diff --git a/arch/powerpc/platforms/embedded6xx/Kconfig b/arch/powerpc/platforms/embedded6xx/Kconfig >> index bfd88be..29a98c6 100644 >> --- a/arch/powerpc/platforms/embedded6xx/Kconfig >> +++ b/arch/powerpc/platforms/embedded6xx/Kconfig >> @@ -94,6 +94,7 @@ config MPC10X_STORE_GATHERING >> config GAMECUBE_COMMON >> bool >> select NOT_COHERENT_CACHE >> + select FLIPPER_PIC >> >> config USBGECKO_UDBG >> bool "USB Gecko udbg console for the Nintendo GameCube/Wii" >> @@ -108,3 +109,8 @@ config USBGECKO_UDBG >> >> If in doubt, say N here. >> >> +config FLIPPER_PIC >> + bool >> + depends on GAMECUBE_COMMON >> + default y > > You'll always want this driver when GAMECUBE common is set. Don't add > another Kconfig entry. > Ok. >> + >> diff --git a/arch/powerpc/platforms/embedded6xx/Makefile b/arch/powerpc/platforms/embedded6xx/Makefile >> index 0ab7492..35258fd 100644 >> --- a/arch/powerpc/platforms/embedded6xx/Makefile >> +++ b/arch/powerpc/platforms/embedded6xx/Makefile >> @@ -8,3 +8,4 @@ obj-$(CONFIG_PPC_HOLLY) += holly.o >> obj-$(CONFIG_PPC_PRPMC2800) += prpmc2800.o >> obj-$(CONFIG_PPC_C2K) += c2k.o >> obj-$(CONFIG_USBGECKO_UDBG) += usbgecko_udbg.o >> +obj-$(CONFIG_FLIPPER_PIC) += flipper-pic.o > >> +unsigned int flipper_pic_get_irq(void) >> +{ >> + void __iomem *io_base = flipper_irq_host->host_data; >> + int irq; >> + u32 irq_status; >> + >> + irq_status = in_be32(io_base + FLIPPER_ICR) & >> + in_be32(io_base + FLIPPER_IMR); >> + if (irq_status == 0) >> + return -1; /* no more IRQs pending */ > > NO_IRQ_IGNORE > I'll fix that. Thanks. I did it in the other interrupt controller but forgot about this. >> + >> + __asm__ __volatile__("cntlzw %0,%1" : "=r"(irq) : "r"(irq_status)); >> + return irq_linear_revmap(flipper_irq_host, 31 - irq); >> +} >> + > Thanks. Cheers, Albert
> config GAMECUBE_COMMON > bool > select NOT_COHERENT_CACHE > + select FLIPPER_PIC Maybe using FLIPPER (or GAMECUBE_FLIPPER) instead of GAMECUBE_COMMON is a good name? > +#define pr_fmt(fmt) DRV_MODULE_NAME ": " fmt Unused > +/* > + * Each interrupt has a corresponding bit in both > + * the Interrupt Cause (ICR) and Interrupt Mask (IMR) registers. > + * > + * Enabling/disabling an interrupt line involves asserting/clearing > + * the corresponding bit in IMR. ACK'ing a request simply involves > + * asserting the corresponding bit in ICR. > + */ > +static void flipper_pic_ack(unsigned int virq) > +{ > + int irq = virq_to_hw(virq); > + void __iomem *io_base = get_irq_chip_data(virq); > + > + set_bit(irq, io_base + FLIPPER_ICR); > +} So it should be a simple write instead of an RMW here, right? As it is you are ACKing _all_ IRQs as far as I can see. > +unsigned int flipper_pic_get_irq(void) > +{ > + void __iomem *io_base = flipper_irq_host->host_data; > + int irq; > + u32 irq_status; > + > + irq_status = in_be32(io_base + FLIPPER_ICR) & > + in_be32(io_base + FLIPPER_IMR); > + if (irq_status == 0) > + return -1; /* no more IRQs pending */ > + > + __asm__ __volatile__("cntlzw %0,%1" : "=r"(irq) : "r"(irq_status)); > + return irq_linear_revmap(flipper_irq_host, 31 - irq); > +} There are generic macros for this kind of thing, no need for asm. ffs() or something. Segher
Segher Boessenkool wrote: >> config GAMECUBE_COMMON >> bool >> select NOT_COHERENT_CACHE >> + select FLIPPER_PIC > > Maybe using FLIPPER (or GAMECUBE_FLIPPER) instead of GAMECUBE_COMMON > is a good name? > I'd prefer to not use a name that implies a specific hardware to describe two (mostly) similar but different hardwares. >> +#define pr_fmt(fmt) DRV_MODULE_NAME ": " fmt > > Unused > Nope. It is used via the pr_{err,info,...} macros. >> +/* >> + * Each interrupt has a corresponding bit in both >> + * the Interrupt Cause (ICR) and Interrupt Mask (IMR) registers. >> + * >> + * Enabling/disabling an interrupt line involves asserting/clearing >> + * the corresponding bit in IMR. ACK'ing a request simply involves >> + * asserting the corresponding bit in ICR. >> + */ > >> +static void flipper_pic_ack(unsigned int virq) >> +{ >> + int irq = virq_to_hw(virq); >> + void __iomem *io_base = get_irq_chip_data(virq); >> + >> + set_bit(irq, io_base + FLIPPER_ICR); >> +} > > So it should be a simple write instead of an RMW here, right? > As it is you are ACKing _all_ IRQs as far as I can see. > No, it acks just a single IRQ. But the simple write idea may work. I need to check if writing 0s causes no harm (which IIRC is true). >> +unsigned int flipper_pic_get_irq(void) >> +{ >> + void __iomem *io_base = flipper_irq_host->host_data; >> + int irq; >> + u32 irq_status; >> + >> + irq_status = in_be32(io_base + FLIPPER_ICR) & >> + in_be32(io_base + FLIPPER_IMR); >> + if (irq_status == 0) >> + return -1; /* no more IRQs pending */ >> + >> + __asm__ __volatile__("cntlzw %0,%1" : "=r"(irq) : "r"(irq_status)); >> + return irq_linear_revmap(flipper_irq_host, 31 - irq); >> +} > > There are generic macros for this kind of thing, no need for asm. ffs() > or something. > __ffs() matches exactly that. Thanks. Cheers, Albert
On Sun, 2009-11-22 at 23:01 +0100, Albert Herranz wrote: > +static void flipper_pic_mask_and_ack(unsigned int virq) > +{ > + int irq = virq_to_hw(virq); > + void __iomem *io_base = get_irq_chip_data(virq); > + > + clear_bit(irq, io_base + FLIPPER_IMR); > + set_bit(irq, io_base + FLIPPER_ICR); > +} Do not use clear_bit and set_bit on IOs. They will do lwarx/stwcx. which is really not what you want. You can use __clear_bit and __set_bit but it's still fishy. Those operate on unsigned long, so the size vary between 32 and 64 bit etc... not something you care that much about, but it's still the wrong tool for the job. Do those guys have more than 32 interrupts ? If not, just hand code the msak & shifts. If they do, then maybe stick with __clear_bit() and __set_bit() which are the non atomic variants. Cheers, Ben.
On Sun, 2009-11-22 at 16:28 -0700, Grant Likely wrote: > > +unsigned int flipper_pic_get_irq(void) > > +{ > > + void __iomem *io_base = flipper_irq_host->host_data; > > + int irq; > > + u32 irq_status; > > + > > + irq_status = in_be32(io_base + FLIPPER_ICR) & > > + in_be32(io_base + FLIPPER_IMR); > > + if (irq_status == 0) > > + return -1; /* no more IRQs pending */ > > NO_IRQ_IGNORE Why no just 0 ? (aka NO_IRQ) Or do you know you are getting lots of spurrious that you don't want to account ? Cheers, Ben.
Benjamin Herrenschmidt wrote: > On Sun, 2009-11-22 at 23:01 +0100, Albert Herranz wrote: > >> +static void flipper_pic_mask_and_ack(unsigned int virq) >> +{ >> + int irq = virq_to_hw(virq); >> + void __iomem *io_base = get_irq_chip_data(virq); >> + >> + clear_bit(irq, io_base + FLIPPER_IMR); >> + set_bit(irq, io_base + FLIPPER_ICR); >> +} > > Do not use clear_bit and set_bit on IOs. They will do lwarx/stwcx. which > is really not what you want. You can use __clear_bit and __set_bit but > it's still fishy. Those operate on unsigned long, so the size vary > between 32 and 64 bit etc... not something you care that much about, but > it's still the wrong tool for the job. > > Do those guys have more than 32 interrupts ? If not, just hand > code the msak & shifts. If they do, then maybe stick with __clear_bit() > and __set_bit() which are the non atomic variants. > There can be only 32 interrupt sources per pic. I'll build a mask and check if just a simple write works too (it should IMHO), instead of a RWM. > Cheers, > Ben. > Thanks, Albert
Benjamin Herrenschmidt wrote: > On Sun, 2009-11-22 at 16:28 -0700, Grant Likely wrote: >>> +unsigned int flipper_pic_get_irq(void) >>> +{ >>> + void __iomem *io_base = flipper_irq_host->host_data; >>> + int irq; >>> + u32 irq_status; >>> + >>> + irq_status = in_be32(io_base + FLIPPER_ICR) & >>> + in_be32(io_base + FLIPPER_IMR); >>> + if (irq_status == 0) >>> + return -1; /* no more IRQs pending */ >> NO_IRQ_IGNORE > > Why no just 0 ? (aka NO_IRQ) > I didn't know about 0 (I thought that was another valid interrupt). I was used to -1 to tell that no IRQs were pending (at least from the ARCH=ppc days) :) > Or do you know you are getting lots of spurrious that you don't want to > account ? > No, this is not the case here. > Cheers, > Ben. > Thanks, Albert
On Thu, 2009-11-26 at 16:30 +0100, Albert Herranz wrote: > Benjamin Herrenschmidt wrote: > > On Sun, 2009-11-22 at 23:01 +0100, Albert Herranz wrote: > > > >> +static void flipper_pic_mask_and_ack(unsigned int virq) > >> +{ > >> + int irq = virq_to_hw(virq); > >> + void __iomem *io_base = get_irq_chip_data(virq); > >> + > >> + clear_bit(irq, io_base + FLIPPER_IMR); > >> + set_bit(irq, io_base + FLIPPER_ICR); > >> +} > > > > Do not use clear_bit and set_bit on IOs. They will do lwarx/stwcx. which > > is really not what you want. You can use __clear_bit and __set_bit but > > it's still fishy. Those operate on unsigned long, so the size vary > > between 32 and 64 bit etc... not something you care that much about, but > > it's still the wrong tool for the job. > > > > Do those guys have more than 32 interrupts ? If not, just hand > > code the msak & shifts. If they do, then maybe stick with __clear_bit() > > and __set_bit() which are the non atomic variants. > > > > There can be only 32 interrupt sources per pic. > I'll build a mask and check if just a simple write works too (it should IMHO), instead of a RWM. For the ICR just writes should be ok. For the IMR, I wouldn't be surprised if you need to RMW. In which case you have two options: - One is you just do RMW :-) As long as you don't do SMP, you don't have a problem since these callbacks are generally called with local IRQs off. - Or you could explicitely add a spinlock_irqsave just out of paranoia or if you envision SMP type access to these things (from the aux processor ? But then the spinlock would have to be shared with the aux ptiocrocessor FW... fun) - In any case, clear_bit and set_bit are the wrong accessors since lwarx and stwcx. are definitely the wrong instructions to use for non cachable accesses. On other processor you probably would have blown up here in fact. Also those accessors don't provide any memory barriers which mean you may even have some ordering issues. Use readl_be() and writel_be() (or the ppc specific in_be32/out_be32) or the _le variants if you want to count bits backward :-) - You could also keep in memory cache of what the mask is supposed to be. You RMW the cache and do a simple store to the register. It won't fix the concurrency problem (that you probably don't have anyways) but it will make things faster by avoiding an MMIO load.... - However you probably need that MMIO load anyways :-) Trick is, the store you do to ack and mask an interrupt may take some time to hit the PIC before the said PIC actually lowers the CPU EE line. Enough time for your code to go back, re-enable MSR:EE and start calling into the handlers. Enough time thus to catch a "spurrious" interrupt. Old pmacs have a similar PIC and have the same problem. I would recommend that after a mask or a mask & ack, you also read back the register. The MMIO accessors (readl, etc...) will use a twi;isync construct to ensure that the load has been fully completed before execution continues which will help. - The construct and flow handler you use assume all interrupts are level sensitive, this is the case right ? Cheers, Ben. > > Cheers, > > Ben. > > > > Thanks, > Albert
On Thu, 2009-11-26 at 16:33 +0100, Albert Herranz wrote: > Benjamin Herrenschmidt wrote: > > On Sun, 2009-11-22 at 16:28 -0700, Grant Likely wrote: > >>> +unsigned int flipper_pic_get_irq(void) > >>> +{ > >>> + void __iomem *io_base = flipper_irq_host->host_data; > >>> + int irq; > >>> + u32 irq_status; > >>> + > >>> + irq_status = in_be32(io_base + FLIPPER_ICR) & > >>> + in_be32(io_base + FLIPPER_IMR); > >>> + if (irq_status == 0) > >>> + return -1; /* no more IRQs pending */ > >> NO_IRQ_IGNORE > > > > Why no just 0 ? (aka NO_IRQ) > > > > I didn't know about 0 (I thought that was another valid interrupt). 0 is a valid hw number. It's not a valid linux "virq" number. In fact, that's part of the reason why we generalized the whole virq thingy :-) Because back then, Linus made the statement that 0 should never be a valid IRQ number in linux. That also allowed us to make sure that 1..15 are never allocated to anything but a legacy 8259 to avoid all sorts of problems with crackpot legacy x86 originated drivers who hard coded some of these interrupts. > I was used to -1 to tell that no IRQs were pending (at least from the > ARCH=ppc days) :) Yup, we changed that :-) > > Or do you know you are getting lots of spurrious that you don't want to > > account ? > > > > No, this is not the case here. Cheers, Ben. > > Cheers, > > Ben. > > > > Thanks, > Albert
>> Maybe using FLIPPER (or GAMECUBE_FLIPPER) instead of GAMECUBE_COMMON >> is a good name? > > I'd prefer to not use a name that implies a specific hardware to > describe two (mostly) similar but different hardwares. Hollywood is 100% compatible to Flipper though. >>> +/* >>> + * Each interrupt has a corresponding bit in both >>> + * the Interrupt Cause (ICR) and Interrupt Mask (IMR) registers. >>> + * >>> + * Enabling/disabling an interrupt line involves asserting/clearing >>> + * the corresponding bit in IMR. ACK'ing a request simply involves >>> + * asserting the corresponding bit in ICR. >>> + */ I looked it up in YAGCD; it says that _reading_ the ICR reg already acks all interrupts (and clears the bits), you never write this reg! >>> +static void flipper_pic_ack(unsigned int virq) >>> +{ >>> + int irq = virq_to_hw(virq); >>> + void __iomem *io_base = get_irq_chip_data(virq); >>> + >>> + set_bit(irq, io_base + FLIPPER_ICR); >>> +} >> >> So it should be a simple write instead of an RMW here, right? >> As it is you are ACKing _all_ IRQs as far as I can see. >> > > No, it acks just a single IRQ. No it doesn't. Say IRQs 1 and 3 are asserted, so the reg contains 0x0a. Now you want to ack IRQ1; set_bit() will write 0x0a | 0x02, not just 0x02. Segher
Segher Boessenkool wrote: >>> Maybe using FLIPPER (or GAMECUBE_FLIPPER) instead of GAMECUBE_COMMON >>> is a good name? >> >> I'd prefer to not use a name that implies a specific hardware to >> describe two (mostly) similar but different hardwares. > > Hollywood is 100% compatible to Flipper though. > No. There's no ARAM for example :) >>>> +/* >>>> + * Each interrupt has a corresponding bit in both >>>> + * the Interrupt Cause (ICR) and Interrupt Mask (IMR) registers. >>>> + * >>>> + * Enabling/disabling an interrupt line involves asserting/clearing >>>> + * the corresponding bit in IMR. ACK'ing a request simply involves >>>> + * asserting the corresponding bit in ICR. >>>> + */ > > I looked it up in YAGCD; it says that _reading_ the ICR reg already > acks all interrupts (and clears the bits), you never write this reg! > YAGCD is not always right. You should not take it as _the truth_. I'm not telling this is the case. But I don't recall such behavior. It's been a long time since this is done this way. >>>> +static void flipper_pic_ack(unsigned int virq) >>>> +{ >>>> + int irq = virq_to_hw(virq); >>>> + void __iomem *io_base = get_irq_chip_data(virq); >>>> + >>>> + set_bit(irq, io_base + FLIPPER_ICR); >>>> +} >>> >>> So it should be a simple write instead of an RMW here, right? >>> As it is you are ACKing _all_ IRQs as far as I can see. >>> >> >> No, it acks just a single IRQ. > > No it doesn't. Say IRQs 1 and 3 are asserted, so the reg contains 0x0a. > Now you want to ack IRQ1; set_bit() will write 0x0a | 0x02, not just 0x02. > Let me rephrase it. No it should just ack a single IRQ :) So then this is working by accident. If that's the case, I guess it works because the interrupt is not cleared at the source and the ICR is set again immediately for any pending interrupt? > > Segher > > Thanks, Albert
>>> +unsigned int flipper_pic_get_irq(void) >>> +{ >>> + void __iomem *io_base = flipper_irq_host->host_data; >>> + int irq; >>> + u32 irq_status; >>> + >>> + irq_status = in_be32(io_base + FLIPPER_ICR) & >>> + in_be32(io_base + FLIPPER_IMR); >>> + if (irq_status == 0) >>> + return -1; /* no more IRQs pending */ >> >> NO_IRQ_IGNORE > > Why no just 0 ? (aka NO_IRQ) > > Or do you know you are getting lots of spurrious that you don't > want to > account ? IRQ #0 is a valid IRQ here (graphics error IIRC), it should be remapped I suppose? Segher
On Fri, 2009-11-27 at 00:00 +0100, Segher Boessenkool wrote: > >>> +unsigned int flipper_pic_get_irq(void) > >>> +{ > >>> + void __iomem *io_base = flipper_irq_host->host_data; > >>> + int irq; > >>> + u32 irq_status; > >>> + > >>> + irq_status = in_be32(io_base + FLIPPER_ICR) & > >>> + in_be32(io_base + FLIPPER_IMR); > >>> + if (irq_status == 0) > >>> + return -1; /* no more IRQs pending */ > >> > >> NO_IRQ_IGNORE > > > > Why no just 0 ? (aka NO_IRQ) > > > > Or do you know you are getting lots of spurrious that you don't > > want to > > account ? > > IRQ #0 is a valid IRQ here (graphics error IIRC), it should be > remapped I suppose? All interrupts are remapped. _get_irq() should call the appropriate revmap function to remap the HW number into a linux number. 0 is never a valid linux number and means "no interrupt". In the above case, it would seem to me that what he gets is a bitfield so 0 means no interrupt. Hence the code should be: if (irq_status == 0) return 0; Then, find first bit and return the linear revmap... Just look at what the old pmac_pic does, same stuff basically. Cheers, Ben.
>>>> Maybe using FLIPPER (or GAMECUBE_FLIPPER) instead of >>>> GAMECUBE_COMMON >>>> is a good name? >>> >>> I'd prefer to not use a name that implies a specific hardware to >>> describe two (mostly) similar but different hardwares. >> >> Hollywood is 100% compatible to Flipper though. > > No. There's no ARAM for example :) ARAM is an external device. >>>>> +/* >>>>> + * Each interrupt has a corresponding bit in both >>>>> + * the Interrupt Cause (ICR) and Interrupt Mask (IMR) registers. >>>>> + * >>>>> + * Enabling/disabling an interrupt line involves asserting/ >>>>> clearing >>>>> + * the corresponding bit in IMR. ACK'ing a request simply >>>>> involves >>>>> + * asserting the corresponding bit in ICR. >>>>> + */ >> >> I looked it up in YAGCD; it says that _reading_ the ICR reg already >> acks all interrupts (and clears the bits), you never write this reg! > > YAGCD is not always right. You should not take it as _the truth_. Oh I know. But I have no better source of information. Well I could actually test it, that's very easy using OF :-) Let's do that. >>> No, it acks just a single IRQ. >> >> No it doesn't. Say IRQs 1 and 3 are asserted, so the reg contains >> 0x0a. >> Now you want to ack IRQ1; set_bit() will write 0x0a | 0x02, not >> just 0x02. > > Let me rephrase it. No it should just ack a single IRQ :) > > So then this is working by accident. > If that's the case, I guess it works because the interrupt is not > cleared at the source and the ICR is set again immediately for any > pending interrupt? Probably, yes. Also, you will not often have more than one interrupt pending anyway (on the legacy PIC). Segher
>>>>>> +/* >>>>>> + * Each interrupt has a corresponding bit in both >>>>>> + * the Interrupt Cause (ICR) and Interrupt Mask (IMR) registers. >>>>>> + * >>>>>> + * Enabling/disabling an interrupt line involves asserting/ >>>>>> clearing >>>>>> + * the corresponding bit in IMR. ACK'ing a request simply >>>>>> involves >>>>>> + * asserting the corresponding bit in ICR. >>>>>> + */ >>> >>> I looked it up in YAGCD; it says that _reading_ the ICR reg already >>> acks all interrupts (and clears the bits), you never write this reg! >> >> YAGCD is not always right. You should not take it as _the truth_. > > Oh I know. But I have no better source of information. Well I could > actually test it, that's very easy using OF :-) Let's do that. I tested it. It turns out that neither reading or writing this register does anything; the bits are automatically cleared when the source deasserts. I didn't test all interrupts, some are harder to generate "on demand" than others. Segher
Segher Boessenkool wrote: >>>>>>> +/* >>>>>>> + * Each interrupt has a corresponding bit in both >>>>>>> + * the Interrupt Cause (ICR) and Interrupt Mask (IMR) registers. >>>>>>> + * >>>>>>> + * Enabling/disabling an interrupt line involves asserting/clearing >>>>>>> + * the corresponding bit in IMR. ACK'ing a request simply involves >>>>>>> + * asserting the corresponding bit in ICR. >>>>>>> + */ >>>> >>>> I looked it up in YAGCD; it says that _reading_ the ICR reg already >>>> acks all interrupts (and clears the bits), you never write this reg! >>> >>> YAGCD is not always right. You should not take it as _the truth_. >> >> Oh I know. But I have no better source of information. Well I could >> actually test it, that's very easy using OF :-) Let's do that. > > I tested it. It turns out that neither reading or writing this register > does anything; the bits are automatically cleared when the source > deasserts. > I checked it too on the Nintendo GameCube making the ack a no-op. It turns out that we _need_ to ack the RSW (Reset Switch) interrupt. The other interrupt sources checked (EXI, VI, DI, AI, DSP/ARAM) need no explicit ack. > I didn't test all interrupts, some are harder to generate "on demand" than > others. > Thanks, Albert
On Fri, 2009-11-27 at 18:27 +0100, Albert Herranz wrote: > > I checked it too on the Nintendo GameCube making the ack a no-op. > > It turns out that we _need_ to ack the RSW (Reset Switch) interrupt. > The other interrupt sources checked (EXI, VI, DI, AI, DSP/ARAM) need > no explicit ack. That would probably mean that the reset switch interrupt is an edge interrupt. Which means that you also use the wrong flow handler btw :-) No biggie here tho, if it's acked before the handler is called, which should be the case, and if masking it doesn't prevent the edge detector from latching, then it should be allright. But you may want to make sure you don't have another edge irq somewhere in there... Cheers, Ben.
>> It turns out that we _need_ to ack the RSW (Reset Switch) interrupt. >> The other interrupt sources checked (EXI, VI, DI, AI, DSP/ARAM) need >> no explicit ack. > > That would probably mean that the reset switch interrupt is an edge > interrupt. Nah, it means that the RSW is a flip-flop. Segher
On Sat, 2009-11-28 at 03:04 +0100, Segher Boessenkool wrote: > >> It turns out that we _need_ to ack the RSW (Reset Switch) > interrupt. > >> The other interrupt sources checked (EXI, VI, DI, AI, DSP/ARAM) > need > >> no explicit ack. > > > > That would probably mean that the reset switch interrupt is an edge > > interrupt. > > Nah, it means that the RSW is a flip-flop. Controlled by the PIC's ISR ? That would be weird. But heh, it's custom (aka crap) HW so ... :-) Cheers, Ben.
diff --git a/arch/powerpc/platforms/embedded6xx/Kconfig b/arch/powerpc/platforms/embedded6xx/Kconfig index bfd88be..29a98c6 100644 --- a/arch/powerpc/platforms/embedded6xx/Kconfig +++ b/arch/powerpc/platforms/embedded6xx/Kconfig @@ -94,6 +94,7 @@ config MPC10X_STORE_GATHERING config GAMECUBE_COMMON bool select NOT_COHERENT_CACHE + select FLIPPER_PIC config USBGECKO_UDBG bool "USB Gecko udbg console for the Nintendo GameCube/Wii" @@ -108,3 +109,8 @@ config USBGECKO_UDBG If in doubt, say N here. +config FLIPPER_PIC + bool + depends on GAMECUBE_COMMON + default y + diff --git a/arch/powerpc/platforms/embedded6xx/Makefile b/arch/powerpc/platforms/embedded6xx/Makefile index 0ab7492..35258fd 100644 --- a/arch/powerpc/platforms/embedded6xx/Makefile +++ b/arch/powerpc/platforms/embedded6xx/Makefile @@ -8,3 +8,4 @@ obj-$(CONFIG_PPC_HOLLY) += holly.o obj-$(CONFIG_PPC_PRPMC2800) += prpmc2800.o obj-$(CONFIG_PPC_C2K) += c2k.o obj-$(CONFIG_USBGECKO_UDBG) += usbgecko_udbg.o +obj-$(CONFIG_FLIPPER_PIC) += flipper-pic.o diff --git a/arch/powerpc/platforms/embedded6xx/flipper-pic.c b/arch/powerpc/platforms/embedded6xx/flipper-pic.c new file mode 100644 index 0000000..98c2e9f --- /dev/null +++ b/arch/powerpc/platforms/embedded6xx/flipper-pic.c @@ -0,0 +1,247 @@ +/* + * arch/powerpc/platforms/embedded6xx/flipper-pic.c + * + * Nintendo GameCube/Wii "Flipper" interrupt controller support. + * Copyright (C) 2004-2009 The GameCube Linux Team + * Copyright (C) 2007,2008,2009 Albert Herranz + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + */ +#define DRV_MODULE_NAME "flipper-pic" +#define pr_fmt(fmt) DRV_MODULE_NAME ": " fmt + +#include <linux/kernel.h> +#include <linux/init.h> +#include <linux/irq.h> +#include <linux/of.h> +#include <asm/io.h> + +#include "flipper-pic.h" + +#define FLIPPER_NR_IRQS 32 + +/* + * Each interrupt has a corresponding bit in both + * the Interrupt Cause (ICR) and Interrupt Mask (IMR) registers. + * + * Enabling/disabling an interrupt line involves asserting/clearing + * the corresponding bit in IMR. ACK'ing a request simply involves + * asserting the corresponding bit in ICR. + */ +#define FLIPPER_ICR 0x00 +#define FLIPPER_ICR_RSS (1<<16) /* reset switch state */ + +#define FLIPPER_IMR 0x04 + +#define FLIPPER_RESET 0x24 + + +/* + * IRQ chip hooks. + * + */ + +static void flipper_pic_mask_and_ack(unsigned int virq) +{ + int irq = virq_to_hw(virq); + void __iomem *io_base = get_irq_chip_data(virq); + + clear_bit(irq, io_base + FLIPPER_IMR); + set_bit(irq, io_base + FLIPPER_ICR); +} + +static void flipper_pic_ack(unsigned int virq) +{ + int irq = virq_to_hw(virq); + void __iomem *io_base = get_irq_chip_data(virq); + + set_bit(irq, io_base + FLIPPER_ICR); +} + +static void flipper_pic_mask(unsigned int virq) +{ + int irq = virq_to_hw(virq); + void __iomem *io_base = get_irq_chip_data(virq); + + clear_bit(irq, io_base + FLIPPER_IMR); +} + +static void flipper_pic_unmask(unsigned int virq) +{ + int irq = virq_to_hw(virq); + void __iomem *io_base = get_irq_chip_data(virq); + + set_bit(irq, io_base + FLIPPER_IMR); +} + + +static struct irq_chip flipper_pic = { + .typename = "flipper-pic", + .ack = flipper_pic_ack, + .mask_ack = flipper_pic_mask_and_ack, + .mask = flipper_pic_mask, + .unmask = flipper_pic_unmask, +}; + +/* + * IRQ host hooks. + * + */ + +static struct irq_host *flipper_irq_host; + +static int flipper_pic_map(struct irq_host *h, unsigned int virq, + irq_hw_number_t hwirq) +{ + set_irq_chip_data(virq, h->host_data); + get_irq_desc(virq)->status |= IRQ_LEVEL; + set_irq_chip_and_handler(virq, &flipper_pic, handle_level_irq); + return 0; +} + +static void flipper_pic_unmap(struct irq_host *h, unsigned int irq) +{ + set_irq_chip_data(irq, NULL); + set_irq_chip(irq, NULL); +} + +static int flipper_pic_match(struct irq_host *h, struct device_node *np) +{ + return 1; +} + + +static struct irq_host_ops flipper_irq_host_ops = { + .map = flipper_pic_map, + .unmap = flipper_pic_unmap, + .match = flipper_pic_match, +}; + +/* + * Platform hooks. + * + */ + +static void __flipper_quiesce(void __iomem *io_base) +{ + /* mask and ack all IRQs */ + out_be32(io_base + FLIPPER_IMR, 0x00000000); + out_be32(io_base + FLIPPER_ICR, 0xffffffff); +} + +struct irq_host * __init flipper_pic_init(struct device_node *np) +{ + struct irq_host *irq_host; + struct resource res; + void __iomem *io_base; + int retval; + + retval = of_address_to_resource(np, 0, &res); + if (retval) { + pr_err("no io memory range found\n"); + return NULL; + } + io_base = ioremap(res.start, res.end - res.start + 1); + + pr_info("controller at 0x%08x mapped to 0x%p\n", res.start, io_base); + + __flipper_quiesce(io_base); + + irq_host = irq_alloc_host(np, IRQ_HOST_MAP_LINEAR, FLIPPER_NR_IRQS, + &flipper_irq_host_ops, -1); + if (!irq_host) { + pr_err("failed to allocate irq_host\n"); + return NULL; + } + + irq_host->host_data = io_base; + + return irq_host; +} + +unsigned int flipper_pic_get_irq(void) +{ + void __iomem *io_base = flipper_irq_host->host_data; + int irq; + u32 irq_status; + + irq_status = in_be32(io_base + FLIPPER_ICR) & + in_be32(io_base + FLIPPER_IMR); + if (irq_status == 0) + return -1; /* no more IRQs pending */ + + __asm__ __volatile__("cntlzw %0,%1" : "=r"(irq) : "r"(irq_status)); + return irq_linear_revmap(flipper_irq_host, 31 - irq); +} + +/* + * Probe function. + * + */ + +void __init flipper_pic_probe(void) +{ + struct device_node *np; + + np = of_find_compatible_node(NULL, NULL, "nintendo,flipper-pic"); + BUG_ON(!np); + + flipper_irq_host = flipper_pic_init(np); + BUG_ON(!flipper_irq_host); + + irq_set_default_host(flipper_irq_host); + + of_node_put(np); +} + +/* + * Misc functions related to the flipper chipset. + * + */ + +/** + * flipper_quiesce() - quiesce flipper irq controller + * + * Mask and ack all interrupt sources. + * + */ +void flipper_quiesce(void) +{ + void __iomem *io_base = flipper_irq_host->host_data; + + __flipper_quiesce(io_base); +} + +/* + * Resets the platform. + */ +void flipper_platform_reset(void) +{ + void __iomem *io_base; + + if (flipper_irq_host && flipper_irq_host->host_data) { + io_base = flipper_irq_host->host_data; + out_8(io_base + FLIPPER_RESET, 0x00); + } +} + +/* + * Returns non-zero if the reset button is pressed. + */ +int flipper_is_reset_button_pressed(void) +{ + void __iomem *io_base; + u32 icr; + + if (flipper_irq_host && flipper_irq_host->host_data) { + io_base = flipper_irq_host->host_data; + icr = in_be32(io_base + FLIPPER_ICR); + return !(icr & FLIPPER_ICR_RSS); + } + return 0; +} + diff --git a/arch/powerpc/platforms/embedded6xx/flipper-pic.h b/arch/powerpc/platforms/embedded6xx/flipper-pic.h new file mode 100644 index 0000000..e339186 --- /dev/null +++ b/arch/powerpc/platforms/embedded6xx/flipper-pic.h @@ -0,0 +1,25 @@ +/* + * arch/powerpc/platforms/embedded6xx/flipper-pic.h + * + * Nintendo GameCube/Wii "Flipper" interrupt controller support. + * Copyright (C) 2004-2009 The GameCube Linux Team + * Copyright (C) 2007,2008,2009 Albert Herranz + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + */ + +#ifndef __FLIPPER_PIC_H +#define __FLIPPER_PIC_H + +unsigned int flipper_pic_get_irq(void); +void __init flipper_pic_probe(void); + +void flipper_quiesce(void); +void flipper_platform_reset(void); +int flipper_is_reset_button_pressed(void); + +#endif
Add support for the interrupt controller included in the "Flipper" chipset of the Nintendo GameCube video game console. The same interrupt controller is also present in the "Hollywood" chipset of the Nintendo Wii. Signed-off-by: Albert Herranz <albert_herranz@yahoo.es> --- arch/powerpc/platforms/embedded6xx/Kconfig | 6 + arch/powerpc/platforms/embedded6xx/Makefile | 1 + arch/powerpc/platforms/embedded6xx/flipper-pic.c | 247 ++++++++++++++++++++++ arch/powerpc/platforms/embedded6xx/flipper-pic.h | 25 +++ 4 files changed, 279 insertions(+), 0 deletions(-) create mode 100644 arch/powerpc/platforms/embedded6xx/flipper-pic.c create mode 100644 arch/powerpc/platforms/embedded6xx/flipper-pic.h