* new IRQ work status @ 2006-06-29 8:36 Benjamin Herrenschmidt 2006-06-29 13:25 ` Arnd Bergmann 0 siblings, 1 reply; 6+ messages in thread From: Benjamin Herrenschmidt @ 2006-06-29 8:36 UTC (permalink / raw) To: linuxppc-dev Here's a progress report on my new irq management work. I've ported pseries, iseries (well tested) and cell (barely tested) at this point, and tested various funky combinations of device-trees to validate as much as the core as possible. I still need to port over chrp and maple which are easy, and powermac which will be a bit more difficult as it relies a lot more on intrs/n_intrs in the device-nodes which I've now removed. Hopefully, it should be finished this week. The current serie of patch applies on top of 2.6.17-mm1. I will do a version on top of the powerpc tree as soon as the genirq port is upstream (which hopefully should happen soon, possibly tomorrow). I don't yet want to post the patches on the list (too much), they can be found at: http://gate.crashing.org/~benh/irq-WIP/ Comments welcome. Cheers, Ben. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: new IRQ work status 2006-06-29 8:36 new IRQ work status Benjamin Herrenschmidt @ 2006-06-29 13:25 ` Arnd Bergmann 2006-06-29 13:41 ` Segher Boessenkool 2006-06-29 22:57 ` Benjamin Herrenschmidt 0 siblings, 2 replies; 6+ messages in thread From: Arnd Bergmann @ 2006-06-29 13:25 UTC (permalink / raw) To: linuxppc-dev On Thursday 29 June 2006 10:36, Benjamin Herrenschmidt wrote: > http://gate.crashing.org/~benh/irq-WIP/ > > Comments welcome. > Looks great! > -} > - > /* Get an IRQ number from the pending state register of the IIC */ > -int iic_get_irq(struct pt_regs *regs) > +unsigned int iic_get_irq(struct pt_regs *regs) Is unsigned the right thing here? NO_IRQ is defined as (-1). > + > +/* > + * Hardcoded setup part to be compatible with older firmware. We don't > + * associate a device-node to the hosts in this case, which mean that > + * no mapping from the device-tree is supposed. That's ok as there is > + * none anyway > + */ > static int __init setup_iic_hardcoded(void) > { I'd prefer to throw out the hardcoded part from the official tree completely. It's only needed on the very first FW versions that were shipped and those need some other patches that we never merged upstream anyway. So better move it to an extra patch that we keep external for some time and eventually drop. > static irqreturn_t iic_ipi_action(int irq, void *dev_id, struct pt_regs > *regs) { > - smp_message_recv(iic_irq_to_ipi(irq), regs); > + int ipi = (int)(long)dev_id; > + smp_message_recv(ipi, regs); > return IRQ_HANDLED; > } Nice one. this looks a lot cleaner than looking at the irq number here. > static void iic_request_ipi(int ipi, const char *name) > { > - int irq; > - > - irq = iic_ipi_to_irq(ipi); > + int node, virq; > > - /* IPIs are marked SA_INTERRUPT as they must run with irqs > - * disabled */ > - set_irq_chip_and_handler(irq, &iic_chip, handle_percpu_irq); > - request_irq(irq, iic_ipi_action, SA_INTERRUPT, name, NULL); > + for (node = 0; node < IIC_NODE_COUNT; node++) { > + char *rname; > + if (iic_hosts[node] == NULL) > + continue; > + virq = irq_create_mapping(iic_hosts[node], > + iic_ipi_to_irq(ipi), 0); > + > + if (virq == NO_IRQ) { > + printk(KERN_ERR > + "iic: failed to map IPI %s on node %d\n", > + name, node); > + continue; > + } > + rname = kzalloc(strlen(name) + 16, GFP_KERNEL); > + if (rname) > + sprintf(rname, "%s node %d", name, node); > + else > + rname = (char *)name; Considering that name is know to be rather short and we have a fixed number of these, can't we just put the strings into struct iic instead of doing the kzalloc? > enum { > - IIC_EXT_OFFSET = 0x00, /* Start of south bridge IRQs */ > - IIC_EXT_CASCADE = 0x20, /* There is no interrupt 32 on spider */ > - IIC_NUM_EXT = 0x40, /* Number of south bridge IRQs */ > - IIC_SPE_OFFSET = 0x40, /* Start of SPE interrupts */ > - IIC_CLASS_STRIDE = 0x10, /* SPE IRQs per class */ > - IIC_IPI_OFFSET = 0x70, /* Start of IPI IRQs */ > - IIC_NUM_IPIS = 0x10, /* IRQs reserved for IPI */ > - IIC_NODE_STRIDE = 0x80, /* Total IRQs per node */ > + IIC_IRQ_INVALID = 0xff, Where is IIC_IRQ_INVALID used? > + } > + } else if (device_is_compatible(dn, "sti,platform-spider-pic") > + && (chip < 2)) { > + static long hard_coded_pics[] = > + { 0x24000008000, 0x34000008000 }; > + r.start = hard_coded_pics[chip]; > } else See above about the hardcoded stuff. > @@ -664,11 +688,18 @@ static int __init create_spu(struct devi > if (ret) > goto out_free; > > + /* XXX FIXME: Those node vs. nid things are crap, we need > + * only one information, but fixing that goes along with fixing > + * all of the node vs chip vs thread code all over the cell > + * platform. To do soon hopefully... > + */ > spu->node = find_spu_node_id(spe); > spu->nid = of_node_to_nid(spe); > if (spu->nid == -1) > spu->nid = 0; The idea at some point was to keep the notion of nid (as in address on the inter-chip protocol) separate from the NUMA node ID, which may in theory be less fine-grained. Defining them to be the same at least requires the device tree to be very careful with the numbers in ibm,associativity properties to match the bus numbers. > Index: linux-work-mm/include/asm-powerpc/spu.h > =================================================================== > --- linux-work-mm.orig/include/asm-powerpc/spu.h 2006-06-23 > 13:38:19.000000000 +1000 +++ > linux-work-mm/include/asm-powerpc/spu.h 2006-06-29 16:32:35.000000000 +1000 > @@ -117,7 +117,7 @@ struct spu { > struct list_head sched_list; > int number; > int nid; > - u32 isrc; > + unsigned int irqs[3]; > u32 node; > u64 flags; > u64 dar; We've started exporting the isrc property in sysfs recently, so I think we need to keep that in here. Arnd <>< ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: new IRQ work status 2006-06-29 13:25 ` Arnd Bergmann @ 2006-06-29 13:41 ` Segher Boessenkool 2006-06-29 22:57 ` Benjamin Herrenschmidt 1 sibling, 0 replies; 6+ messages in thread From: Segher Boessenkool @ 2006-06-29 13:41 UTC (permalink / raw) To: Arnd Bergmann; +Cc: linuxppc-dev >> /* Get an IRQ number from the pending state register of the IIC */ >> -int iic_get_irq(struct pt_regs *regs) >> +unsigned int iic_get_irq(struct pt_regs *regs) > > Is unsigned the right thing here? NO_IRQ is defined as (-1). It used to be -1, it's 0 now... Segher ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: new IRQ work status 2006-06-29 13:25 ` Arnd Bergmann 2006-06-29 13:41 ` Segher Boessenkool @ 2006-06-29 22:57 ` Benjamin Herrenschmidt 1 sibling, 0 replies; 6+ messages in thread From: Benjamin Herrenschmidt @ 2006-06-29 22:57 UTC (permalink / raw) To: Arnd Bergmann; +Cc: linuxppc-dev > > /* Get an IRQ number from the pending state register of the IIC */ > > -int iic_get_irq(struct pt_regs *regs) > > +unsigned int iic_get_irq(struct pt_regs *regs) > > Is unsigned the right thing here? NO_IRQ is defined as (-1). One of the patches changes NO_IRQ to 0 and generally virtual irq numbers to unsigned int > I'd prefer to throw out the hardcoded part from the official tree > completely. It's only needed on the very first FW versions that > were shipped and those need some other patches that we never merged > upstream anyway. So better move it to an extra patch that we keep > external for some time and eventually drop. Ok, will move it to a separate patch then. > > static irqreturn_t iic_ipi_action(int irq, void *dev_id, struct pt_regs > > *regs) { > > - smp_message_recv(iic_irq_to_ipi(irq), regs); > > + int ipi = (int)(long)dev_id; > > + smp_message_recv(ipi, regs); > > return IRQ_HANDLED; > > } > > Nice one. this looks a lot cleaner than looking at the irq number here. Yeah, I had the calculation wrong at first :) the irq number you get here is the virtual one. So you 'd need to convert it through the reverse map. With the above trick, I avoid that. > Considering that name is know to be rather short and we have a fixed > number of these, can't we just put the strings into struct iic > instead of doing the kzalloc? Good point, I'll look into it. > > enum { > > - IIC_EXT_OFFSET = 0x00, /* Start of south bridge IRQs */ > > - IIC_EXT_CASCADE = 0x20, /* There is no interrupt 32 on spider */ > > - IIC_NUM_EXT = 0x40, /* Number of south bridge IRQs */ > > - IIC_SPE_OFFSET = 0x40, /* Start of SPE interrupts */ > > - IIC_CLASS_STRIDE = 0x10, /* SPE IRQs per class */ > > - IIC_IPI_OFFSET = 0x70, /* Start of IPI IRQs */ > > - IIC_NUM_IPIS = 0x10, /* IRQs reserved for IPI */ > > - IIC_NODE_STRIDE = 0x80, /* Total IRQs per node */ > > + IIC_IRQ_INVALID = 0xff, > > Where is IIC_IRQ_INVALID used? Just to give an invalid number to the remapping core. It uses that to fill up allocated but not-yet mapped slots > See above about the hardcoded stuff. Yup. > > > @@ -664,11 +688,18 @@ static int __init create_spu(struct devi > > if (ret) > > goto out_free; > > > > + /* XXX FIXME: Those node vs. nid things are crap, we need > > + * only one information, but fixing that goes along with fixing > > + * all of the node vs chip vs thread code all over the cell > > + * platform. To do soon hopefully... > > + */ > > spu->node = find_spu_node_id(spe); > > spu->nid = of_node_to_nid(spe); > > if (spu->nid == -1) > > spu->nid = 0; > > The idea at some point was to keep the notion of nid (as in address > on the inter-chip protocol) separate from the NUMA node ID, which > may in theory be less fine-grained. Defining them to be the same > at least requires the device tree to be very careful with the numbers > in ibm,associativity properties to match the bus numbers. Yeah, I've discussed that with Paul and Anton, I want to do a generic layer to take care of all these conversions with a 4 level representation: - thread -> match linux CPU numbers - core - chip - node Anyway, that's the sort of cleanup I'll do later. > > Index: linux-work-mm/include/asm-powerpc/spu.h > > =================================================================== > > --- linux-work-mm.orig/include/asm-powerpc/spu.h 2006-06-23 > > 13:38:19.000000000 +1000 +++ > > linux-work-mm/include/asm-powerpc/spu.h 2006-06-29 16:32:35.000000000 +1000 > > @@ -117,7 +117,7 @@ struct spu { > > struct list_head sched_list; > > int number; > > int nid; > > - u32 isrc; > > + unsigned int irqs[3]; > > u32 node; > > u64 flags; > > u64 dar; > > We've started exporting the isrc property in sysfs recently, so I think > we need to keep that in here. Why would we do that ? It makes no sense on HV platforms. Is there anything that makes use of it ? Ben. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: new IRQ work status @ 2006-06-30 4:57 Bob Brose 2006-06-30 7:45 ` Benjamin Herrenschmidt 0 siblings, 1 reply; 6+ messages in thread From: Bob Brose @ 2006-06-30 4:57 UTC (permalink / raw) To: linuxppc-dev Ben Said: > > Here's a progress report on my new irq management work. . . > validate as much as the core as possible. I still need to port over chrp > and maple which are easy, and powermac which will be a bit more > difficult as it relies a lot more on intrs/n_intrs in the device-nodes > which I've now removed. > > Hopefully, it should be finished this week. The current serie of patch > applies on top of 2.6.17-mm1. I will do a version on top of the powerpc > tree as soon as the genirq port is upstream (which hopefully should > happen soon, possibly tomorrow). . . > Comments welcome. Any chance you can fix the 6 slot S900 irq problem when you do the powermac changes?? Bob ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: new IRQ work status 2006-06-30 4:57 Bob Brose @ 2006-06-30 7:45 ` Benjamin Herrenschmidt 0 siblings, 0 replies; 6+ messages in thread From: Benjamin Herrenschmidt @ 2006-06-30 7:45 UTC (permalink / raw) To: Bob Brose; +Cc: linuxppc-dev On Fri, 2006-06-30 at 04:57 +0000, Bob Brose wrote: > Ben Said: > > > > Here's a progress report on my new irq management work. > . > . > > validate as much as the core as possible. I still need to port over chrp > > and maple which are easy, and powermac which will be a bit more > > difficult as it relies a lot more on intrs/n_intrs in the device-nodes > > which I've now removed. > > > > Hopefully, it should be finished this week. The current serie of patch > > applies on top of 2.6.17-mm1. I will do a version on top of the powerpc > > tree as soon as the genirq port is upstream (which hopefully should > > happen soon, possibly tomorrow). > . > . > > Comments welcome. > > Any chance you can fix the 6 slot S900 irq problem when you do the powermac > changes?? I'll look at it after I have done the initial set of patches... it's a bug fix so there is a bit less hurry for 2.6.18 merge window Ben. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-06-30 7:45 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-06-29 8:36 new IRQ work status Benjamin Herrenschmidt 2006-06-29 13:25 ` Arnd Bergmann 2006-06-29 13:41 ` Segher Boessenkool 2006-06-29 22:57 ` Benjamin Herrenschmidt -- strict thread matches above, loose matches on Subject: below -- 2006-06-30 4:57 Bob Brose 2006-06-30 7:45 ` Benjamin Herrenschmidt
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).