Re: [PATCH] ARM: OMAP: L3 interconnect: Error reporting cleanups

[Todd Poynor <toddpoynor@google.com>]

On Wed, May 25, 2011 at 12:39:56PM +0530, Santosh Shilimkar wrote:
> Tod,
> On 5/25/2011 8:20 AM, Todd Poynor wrote:
> >* Move variable declarations from header file and make these static
> >   (the entire header file should probably go away).
> >
> Infact the intial version posted on the list had all these structures
> in C file. After some comments on the list we moved them to header file.
> Let's not move it again.

I searched (a little) but didn't find the feedback that led to this.
Tell me more?  Defining variables (not externs, not even statics) in
header files is usually frowned upon.

> 
> >* Define L3 TARG instance offsets, and read/write STDERRLOG registers
> >   relative to those offsets, rather than defining STDERRLOG_MAIN
> >   instance offsets and accessing other registers via offsets from
> >   that register.
> >
> >* Use ffs() to find error source from the L3_FLAGMUX_REGERRn
> >   register.
> >
> >* Remove extra l3_base[] entry.
> >
> >* Modify L3 custom error message for consistency with standard
> >   error message.
> >
> These changes are ok.
> 
> >* Fixup indentation in one spot.
> >
> This code indentations needs more fixing and I have patch to
> clean that up. Your change is already part of that patch.
>

OK.
 
> 
> >---
> >  arch/arm/mach-omap2/omap_l3_noc.c |  122 +++++++++++++++++++++++++++++++------
> >  arch/arm/mach-omap2/omap_l3_noc.h |   88 +-------------------------
> >  2 files changed, 106 insertions(+), 104 deletions(-)
> >
> Can you update your patch with above. I will add that along with other
> L3 clean-up patch.
> 
> Thanks
> 
> regards
> Santosh