* [PATCH] powerpc/cpm2 Fix set interrupt type @ 2009-01-27 8:44 paulfax 2009-01-27 14:31 ` Kumar Gala 2009-02-06 16:44 ` Kumar Gala 0 siblings, 2 replies; 6+ messages in thread From: paulfax @ 2009-01-27 8:44 UTC (permalink / raw) To: linuxppc-dev This is a simple change to correct problems when using set_irq_type on platforms using CPM2. This code correct the problem on most platform but still fails on 8272 derived platforms for some interrupts. On 8272 PC2 & 3 are missing and PC 23 & 29 are added, which this patch does not address. Signed-off-by: Paul Bilke <paul at conspiracy.net> --- diff --git a/arch/powerpc/sysdev/cpm2_pic.c b/arch/powerpc/sysdev/cpm2_pic.c index b16ca3e..78f1f7c 100644 --- a/arch/powerpc/sysdev/cpm2_pic.c +++ b/arch/powerpc/sysdev/cpm2_pic.c @@ -165,7 +165,7 @@ static int cpm2_set_irq_type(unsigned int virq, unsigned int flow_type) edibit = (14 - (src - CPM2_IRQ_EXT1)); else if (src >= CPM2_IRQ_PORTC15 && src <= CPM2_IRQ_PORTC0) - edibit = (31 - (src - CPM2_IRQ_PORTC15)); + edibit = (31 - (CPM2_IRQ_PORTC0 - src)); else return (flow_type & IRQ_TYPE_LEVEL_LOW) ? 0 : -EINVAL; ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc/cpm2 Fix set interrupt type 2009-01-27 8:44 [PATCH] powerpc/cpm2 Fix set interrupt type paulfax @ 2009-01-27 14:31 ` Kumar Gala 2009-01-27 18:14 ` Anton Vorontsov 2009-02-06 16:44 ` Kumar Gala 1 sibling, 1 reply; 6+ messages in thread From: Kumar Gala @ 2009-01-27 14:31 UTC (permalink / raw) To: Anton Vorontsov; +Cc: linuxppc-dev list, paulfax On Jan 27, 2009, at 2:44 AM, paulfax wrote: > This is a simple change to correct problems when using set_irq_type > on platforms using CPM2. > This code correct the problem on most platform but still fails on > 8272 derived platforms for some interrupts. > On 8272 PC2 & 3 are missing and PC 23 & 29 are added, which this > patch does not address. > > Signed-off-by: Paul Bilke <paul at conspiracy.net> > --- Anton, Is this something you can review and look into the 8272 issue? - k > diff --git a/arch/powerpc/sysdev/cpm2_pic.c b/arch/powerpc/sysdev/ > cpm2_pic.c > index b16ca3e..78f1f7c 100644 > --- a/arch/powerpc/sysdev/cpm2_pic.c > +++ b/arch/powerpc/sysdev/cpm2_pic.c > @@ -165,7 +165,7 @@ static int cpm2_set_irq_type(unsigned int virq, > unsigned int flow_type) > edibit = (14 - (src - CPM2_IRQ_EXT1)); > else > if (src >= CPM2_IRQ_PORTC15 && src <= CPM2_IRQ_PORTC0) > - edibit = (31 - (src - CPM2_IRQ_PORTC15)); > + edibit = (31 - (CPM2_IRQ_PORTC0 - src)); > else > return (flow_type & IRQ_TYPE_LEVEL_LOW) ? 0 : > -EINVAL; > > _______________________________________________ > Linuxppc-dev mailing list > Linuxppc-dev@ozlabs.org > https://ozlabs.org/mailman/listinfo/linuxppc-dev ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc/cpm2 Fix set interrupt type 2009-01-27 14:31 ` Kumar Gala @ 2009-01-27 18:14 ` Anton Vorontsov 2009-01-28 1:16 ` Kumar Gala 0 siblings, 1 reply; 6+ messages in thread From: Anton Vorontsov @ 2009-01-27 18:14 UTC (permalink / raw) To: Kumar Gala; +Cc: linuxppc-dev list, paulfax On Tue, Jan 27, 2009 at 08:31:16AM -0600, Kumar Gala wrote: > > On Jan 27, 2009, at 2:44 AM, paulfax wrote: > >> This is a simple change to correct problems when using set_irq_type on >> platforms using CPM2. >> This code correct the problem on most platform but still fails on 8272 >> derived platforms for some interrupts. >> On 8272 PC2 & 3 are missing and PC 23 & 29 are added, which this patch >> does not address. This is the same as in MPC8555. >> Signed-off-by: Paul Bilke <paul at conspiracy.net> >> --- > > Anton, > > Is this something you can review Sure. According to MPC8555 and MPC8272 reference manuals, current code indeed has the issue, and the patch correctly fixes it. SIEXR register map looks like this (bits are in big-endian numbering scheme): [0:1] - PC0..PC1 [2:13] - PC4..PC15 [14:15] - PC23 and PC29 Let's look into fixed version, assuming we got src 48 (PC29): edibit = (31 - (CPM2_IRQ_PORTC0 - src)); edibit = (31 - (63 - 48)) = 16 1 << 16 points to PC29 (bit 15 in big-endian bit numbering scheme), just as it should be. The same code should work for MPC8560 too (where there are no PC23 & PC29, but PC2 and PC3). > and look into the 8272 issue? I don't actually see any issues wrt 8272. To me, it looks like the fixed code should work fine... Reviewed-by: Anton Vorontsov <avorontsov@ru.mvista.com> >> diff --git a/arch/powerpc/sysdev/cpm2_pic.c b/arch/powerpc/sysdev/ >> cpm2_pic.c >> index b16ca3e..78f1f7c 100644 >> --- a/arch/powerpc/sysdev/cpm2_pic.c >> +++ b/arch/powerpc/sysdev/cpm2_pic.c >> @@ -165,7 +165,7 @@ static int cpm2_set_irq_type(unsigned int virq, >> unsigned int flow_type) >> edibit = (14 - (src - CPM2_IRQ_EXT1)); >> else >> if (src >= CPM2_IRQ_PORTC15 && src <= CPM2_IRQ_PORTC0) >> - edibit = (31 - (src - CPM2_IRQ_PORTC15)); >> + edibit = (31 - (CPM2_IRQ_PORTC0 - src)); >> else >> return (flow_type & IRQ_TYPE_LEVEL_LOW) ? 0 : >> -EINVAL; Thanks, -- Anton Vorontsov email: cbouatmailru@gmail.com irc://irc.freenode.net/bd2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc/cpm2 Fix set interrupt type 2009-01-27 18:14 ` Anton Vorontsov @ 2009-01-28 1:16 ` Kumar Gala 2009-01-28 2:05 ` paulfax 0 siblings, 1 reply; 6+ messages in thread From: Kumar Gala @ 2009-01-28 1:16 UTC (permalink / raw) To: avorontsov; +Cc: linuxppc-dev list, paulfax On Jan 27, 2009, at 12:14 PM, Anton Vorontsov wrote: > On Tue, Jan 27, 2009 at 08:31:16AM -0600, Kumar Gala wrote: >> >> On Jan 27, 2009, at 2:44 AM, paulfax wrote: >> >>> This is a simple change to correct problems when using >>> set_irq_type on >>> platforms using CPM2. >>> This code correct the problem on most platform but still fails on >>> 8272 >>> derived platforms for some interrupts. >>> On 8272 PC2 & 3 are missing and PC 23 & 29 are added, which this >>> patch >>> does not address. > > This is the same as in MPC8555. I guess I'm concerned about Paul's comments related to 8272 still failing and the patch not addressing PC2/3. >>> Signed-off-by: Paul Bilke <paul at conspiracy.net> >>> --- >> >> Anton, >> >> Is this something you can review > > Sure. According to MPC8555 and MPC8272 reference manuals, > current code indeed has the issue, and the patch correctly > fixes it. SIEXR register map looks like this (bits are in > big-endian numbering scheme): > > [0:1] - PC0..PC1 > [2:13] - PC4..PC15 > [14:15] - PC23 and PC29 > > Let's look into fixed version, assuming we got src 48 (PC29): > > edibit = (31 - (CPM2_IRQ_PORTC0 - src)); > edibit = (31 - (63 - 48)) = 16 > > 1 << 16 points to PC29 (bit 15 in big-endian bit numbering scheme), > just as it should be. > > The same code should work for MPC8560 too (where there are no > PC23 & PC29, but PC2 and PC3). > >> and look into the 8272 issue? > > I don't actually see any issues wrt 8272. To me, it looks like > the fixed code should work fine... Paul? - k ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc/cpm2 Fix set interrupt type 2009-01-28 1:16 ` Kumar Gala @ 2009-01-28 2:05 ` paulfax 0 siblings, 0 replies; 6+ messages in thread From: paulfax @ 2009-01-28 2:05 UTC (permalink / raw) To: Kumar Gala; +Cc: linuxppc-dev list Kumar Gala wrote: > > On Jan 27, 2009, at 12:14 PM, Anton Vorontsov wrote: > >> On Tue, Jan 27, 2009 at 08:31:16AM -0600, Kumar Gala wrote: >>> >>> On Jan 27, 2009, at 2:44 AM, paulfax wrote: >>> >>>> This is a simple change to correct problems when using set_irq_type on >>>> platforms using CPM2. >>>> This code correct the problem on most platform but still fails on 8272 >>>> derived platforms for some interrupts. >>>> On 8272 PC2 & 3 are missing and PC 23 & 29 are added, which this patch >>>> does not address. >> >> This is the same as in MPC8555. > > I guess I'm concerned about Paul's comments related to 8272 still > failing and the patch not addressing PC2/3. > >>>> Signed-off-by: Paul Bilke <paul at conspiracy.net> >>>> --- >>> >>> Anton, >>> >>> Is this something you can review >> >> Sure. According to MPC8555 and MPC8272 reference manuals, >> current code indeed has the issue, and the patch correctly >> fixes it. SIEXR register map looks like this (bits are in >> big-endian numbering scheme): >> >> [0:1] - PC0..PC1 >> [2:13] - PC4..PC15 >> [14:15] - PC23 and PC29 >> >> Let's look into fixed version, assuming we got src 48 (PC29): >> >> edibit = (31 - (CPM2_IRQ_PORTC0 - src)); >> edibit = (31 - (63 - 48)) = 16 >> >> 1 << 16 points to PC29 (bit 15 in big-endian bit numbering scheme), >> just as it should be. >> >> The same code should work for MPC8560 too (where there are no >> PC23 & PC29, but PC2 and PC3). >> >>> and look into the 8272 issue? >> >> I don't actually see any issues wrt 8272. To me, it looks like >> the fixed code should work fine... > > Paul? > > - k > _______________________________________________ > Linuxppc-dev mailing list > Linuxppc-dev@ozlabs.org > https://ozlabs.org/mailman/listinfo/linuxppc-dev It does actually produce the right bit pattern for the SIUEXR based on the interrupt being passed in even on the 8272 family as Anton point out. My concern is that throughout the tree there are defines made based on the code's 8260 lineage that are confusing to anyone trying to discern functionality on slightly different processor families. In this case the define for CPM_IRQ_PORTC15 & CPM_IRQ_PORTC0 are isolated within the cpm2_pic source file instead of a public header which presents less of a problem. Of course port c 15's interrupt on the 8272 is 50 not 48 as the define states. Not sure making the defines generic will make it any clearer, just force the person to dig deeper to see exactly what is going on. I am trying to avoid the situation like the one where we don't have defines in cpm2.h for the 8272's variations in SIUMCR. We have code that looks like it changing external cache configuration but its really setting where the burst addresses appear on a 8272. Don't know the "right" way to make the code obvious, but this patch does fix a long lingering problem. My thought is to apply it as is, its better than what is there. I suspect the the right fix for all the minor variations in the CPM on different devices will never happen. I don't want to go ifdef crazy for sure, but in my u-boot tree I have them around the bits that differ between devices. This allows me to find errors caused by unknown routines setting hardware functions that are invalid at compile time. Can't wait for all the surprises in this area on my next project, an 8568. Paul ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc/cpm2 Fix set interrupt type 2009-01-27 8:44 [PATCH] powerpc/cpm2 Fix set interrupt type paulfax 2009-01-27 14:31 ` Kumar Gala @ 2009-02-06 16:44 ` Kumar Gala 1 sibling, 0 replies; 6+ messages in thread From: Kumar Gala @ 2009-02-06 16:44 UTC (permalink / raw) To: paulfax; +Cc: linuxppc-dev On Jan 27, 2009, at 2:44 AM, paulfax wrote: > This is a simple change to correct problems when using set_irq_type > on platforms using CPM2. > This code correct the problem on most platform but still fails on > 8272 derived platforms for some interrupts. > On 8272 PC2 & 3 are missing and PC 23 & 29 are added, which this > patch does not address. > > Signed-off-by: Paul Bilke <paul at conspiracy.net> > --- > > > > > diff --git a/arch/powerpc/sysdev/cpm2_pic.c b/arch/powerpc/sysdev/ > cpm2_pic.c > index b16ca3e..78f1f7c 100644 > --- a/arch/powerpc/sysdev/cpm2_pic.c > +++ b/arch/powerpc/sysdev/cpm2_pic.c > @@ -165,7 +165,7 @@ static int cpm2_set_irq_type(unsigned int virq, > unsigned int flow_type) > edibit = (14 - (src - CPM2_IRQ_EXT1)); > else > if (src >= CPM2_IRQ_PORTC15 && src <= CPM2_IRQ_PORTC0) > - edibit = (31 - (src - CPM2_IRQ_PORTC15)); > + edibit = (31 - (CPM2_IRQ_PORTC0 - src)); > else > return (flow_type & IRQ_TYPE_LEVEL_LOW) ? 0 : > -EINVAL; applied - k ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-02-06 16:44 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-01-27 8:44 [PATCH] powerpc/cpm2 Fix set interrupt type paulfax 2009-01-27 14:31 ` Kumar Gala 2009-01-27 18:14 ` Anton Vorontsov 2009-01-28 1:16 ` Kumar Gala 2009-01-28 2:05 ` paulfax 2009-02-06 16:44 ` Kumar Gala
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).