| Message ID | 1402574007-13987-6-git-send-email-r.sricharan@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jun 12, 2014 at 05:23:13PM +0530, Sricharan R wrote: > From: Nishanth Menon <nm@ti.com> > > Reverse the search algorithm to ensure that address mapping and IRQ > allocation logics are proper. This can open up new bugs which are > easily fixable rather than wait till allocation logic approaches > the limit to find new bugs. Could you expand on this logic some more? What class of bugs are you hoping to discover more easily? thx, Jason. > Signed-off-by: Nishanth Menon <nm@ti.com> > Signed-off-by: Sricharan R <r.sricharan@ti.com> > Signed-off-by: Tony Lindgren <tony@atomide.com> > --- > drivers/irqchip/irq-crossbar.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c > index 287d3ce..de021638 100644 > --- a/drivers/irqchip/irq-crossbar.c > +++ b/drivers/irqchip/irq-crossbar.c > @@ -68,7 +68,7 @@ static inline int get_prev_map_irq(int cb_no) > { > int i; > > - for (i = 0; i < cb->int_max; i++) > + for (i = cb->int_max - 1; i >= 0; i--) > if (cb->irq_map[i] == cb_no) > return i; > > @@ -79,7 +79,7 @@ static inline int allocate_free_irq(int cb_no) > { > int i; > > - for (i = 0; i < cb->int_max; i++) { > + for (i = cb->int_max - 1; i >= 0; i--) { > if (cb->irq_map[i] == IRQ_FREE) { > cb->irq_map[i] = cb_no; > return i; > -- > 1.7.9.5 >
Hi Jason, On Thursday 12 June 2014 06:26 PM, Jason Cooper wrote: > On Thu, Jun 12, 2014 at 05:23:13PM +0530, Sricharan R wrote: >> From: Nishanth Menon <nm@ti.com> >> >> Reverse the search algorithm to ensure that address mapping and IRQ >> allocation logics are proper. This can open up new bugs which are >> easily fixable rather than wait till allocation logic approaches >> the limit to find new bugs. > > Could you expand on this logic some more? What class of bugs are you > hoping to discover more easily? > class 1. address space errors -> example: reg = <a size_b> ti,max-irqs = is a wrong parameter class 2: irq-reserved list - which decides which entries in the address space is not actually wired in class 3: wrong list of routable-irqs. in general allocating from max to min tends to have benefits in ensuring the different issues that may be present in dts is easily caught at definition time, rather than at a later point in time. Regards, Sricharan
On Fri, Jun 13, 2014 at 11:39:39AM +0530, Sricharan R wrote: > Hi Jason, > > On Thursday 12 June 2014 06:26 PM, Jason Cooper wrote: > > On Thu, Jun 12, 2014 at 05:23:13PM +0530, Sricharan R wrote: > >> From: Nishanth Menon <nm@ti.com> > >> > >> Reverse the search algorithm to ensure that address mapping and IRQ > >> allocation logics are proper. This can open up new bugs which are > >> easily fixable rather than wait till allocation logic approaches > >> the limit to find new bugs. > > > > Could you expand on this logic some more? What class of bugs are you > > hoping to discover more easily? > > > > class 1. address space errors -> example: > reg = <a size_b> > ti,max-irqs = is a wrong parameter > > class 2: irq-reserved list - which decides which entries in the > address space is not actually wired in > > class 3: wrong list of routable-irqs. > > in general allocating from max to min tends to have benefits in > ensuring the different issues that may be present in dts is easily > caught at definition time, rather than at a later point in time. Perfect, thanks for clarifying. Please add this to the commit log when you respin. thx, Jason.
diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c index 287d3ce..de021638 100644 --- a/drivers/irqchip/irq-crossbar.c +++ b/drivers/irqchip/irq-crossbar.c @@ -68,7 +68,7 @@ static inline int get_prev_map_irq(int cb_no) { int i; - for (i = 0; i < cb->int_max; i++) + for (i = cb->int_max - 1; i >= 0; i--) if (cb->irq_map[i] == cb_no) return i; @@ -79,7 +79,7 @@ static inline int allocate_free_irq(int cb_no) { int i; - for (i = 0; i < cb->int_max; i++) { + for (i = cb->int_max - 1; i >= 0; i--) { if (cb->irq_map[i] == IRQ_FREE) { cb->irq_map[i] = cb_no; return i;