[PATCH] ARM: OMAP2+: irq: Increase no of supported interrupts to 128

[Hiremath, Vaibhav]

On Fri, May 11, 2012 at 03:19:21, Hilman, Kevin wrote:
> "Hiremath, Vaibhav" <hvaibhav at ti.com> writes:
> 
> > On Wed, May 09, 2012 at 00:09:34, Hilman, Kevin wrote:
> >> Vaibhav Hiremath <hvaibhav at ti.com> writes:
> >> 
> >> > With addition to TI81XX, AM33XX family of devices, the number
> >> > of interrupts supported has increased to 128, compared to 96.
> >> > The current implementation for irq handling is hardcoded to use
> >> > 96 interrupts (with 3 register-sets to handle), this patch cleanups
> >> > the code, to increase maximum number of interrupts support
> >> > to 128, with dynamic detection of no of registers required for
> >> > handling all interrupts.
> >> >
> >> >
> >> > Signed-off-by: Vaibhav Hiremath <hvaibhav at ti.com>
> >> > Signed-off-by: Afzal Mohammed <afzal at ti.com>
> >> > Cc: Tony Lindgren <tony at atomide.com>
> >> > Cc: Kevin Hilman <khilman at ti.com>
> >> > Cc: Paul Walmsley <paul at pwsan.com>
> >> > ---
> >> > Ideally, we should use dynamic allocation to allocate memory
> >> > for registers/arrays, 
> >> 
> >> Yes.
> >> 
> >
> > Thanks Kevin, I will put this activity in my TODO list.
> >
> >> > may be too much cleanup for this patch,
> >> 
> >> There is no such thing as too much cleanup.  ;)  
> >> And the INTC is in need of it, IMO.
> >> 
> >
> > Indeed it is in need of cleanup...
> >
> >
> >> > so as of now restricting to minimal changes to fit devices
> >> > like, am33xx/ti81xx.
> >> 
> >> Then someone else will have to do the cleanup later.  It would be
> >> greatly appreciated if you could do the necessary cleanup in order to
> >> cleanly add support for more SoCs.  Yes, we probably should've insisted
> >> when support for TI81xx was added, but that one slipped in under the
> >> radar.
> >> 
> >
> > Yeah, I understand. As I said I will put this activity in my TODO list.
> >
> >> For starters, the notion of a banks this code is a rather messed up and
> >> needs a cleanup.  A bank is supposed to be a group of 32 interrupts, 
> >> and the INTC is made up of 3 (or 4) banks.   However, the current
> >> code creates a single "bank" of 96 (or 128) interrupts.  
> >> 
> >> It also confuses what registers are part of the bank and what are global
> >> to the INTC.  This confusion is both in init and in context save/restore.
> >> 
> >> IMO, to clean this up, first the notion of banks needs to be fixed in
> >> that code there is a distinction between what acts on banks and what
> >> works on the whole INTC.
> >> 
> >> Then, the init/alloc should be done dynamically based on the number of
> >> interrupts passed to omap_init_irq()
> >> 
> >
> > Kevin,
> > Let me finish up with am33xx baseport upstream activity which is currently 
> > going on at full swing, then next thing I will pick up is this code cleanup.
> >
> > I still feel, this is still a valid cleanup patch, and can be merged, as it 
> > is required/used when we do major cleanup.
> 
> Well, Tony can make that decision.
> 
> Personally, I think this patch continues to add confusion on top of an
> existing mess, and to me provides the proverbial straw that broke the
> camel's back.
> 
> That being said, the INTC core is an obviously important and sensitive
> piece of code so needs to be handled with care.
> 
> In case Tony decides to merge this patch and allow a future** cleanup,
> I'll provide some comments.
> 

I am OK, lets drop this patch as of now, and I will take this when I start 
cleanup activity.


> Kevin
> 
> ** since they rarely happen, we tend to not have too much faith in
>    promises of mythical "future" cleanups.  This is not because we don't
>    trust you personally, but simply based on based experience.
> 

Very nicely said :) :) :)

I agree with you, sometimes priorities changes and things doesn't happen the 
way we wanted to be.

As I said earlier, I am now full time working on upstream (no ifs and buts); else I wouldn't have committed for this activity.


Thanks,
Vaibhav