[U-Boot] [PATCH v4] da830: Move common code out of da830evm.c file

[Sudhakar Rajashekhara <sudhakar.raj@ti.com>]

Hi,

On Fri, Jun 04, 2010 at 21:02:31, Nick Thompson wrote:
> On 04/06/10 15:26, Ben Gardiner wrote:
> > On Thu, Jun 3, 2010 at 8:58 AM, Sudhakar Rajashekhara
> > <sudhakar.raj@ti.com> wrote:
> >> On Thu, Jun 03, 2010 at 16:23:36, Nick Thompson wrote:
> >>> On 03/06/10 05:25, Sudhakar Rajashekhara wrote:
> >>>> TI's DA850/OMAP-L138 platform is similar to DA830/OMAP-L137
> >>>> in many aspects. So instead of repeating the same code in
> >>>> multiple files, move the common code to a different file
> >>>> and call those functions from the respective da830/da850
> >>>> files.
> >>>>
> >>>> Signed-off-by: Sudhakar Rajashekhara <sudhakar.raj@ti.com>
> >>>> Acked-by: Nick Thompson <nick.thompson@ge.com>
> >>>> Acked-by: Ben Gardiner <bengardiner@nanometrics.ca>
> >>>> ---
> >>>> Since v3:
> >>>> Fixes the following compiler error for other davinci targets:
> >>>>
> >>>> misc.c: In function 'irq_init':
> >>>> misc.c:51: error: 'davinci_aintc_regs' undeclared (first use in this function)
> >>>> misc.c:51: error: (Each undeclared identifier is reported only once
> >>>> misc.c:51: error: for each function it appears in.)
> >>>> make[1]: *** [.../build/board/davinci/common/misc.o] Error 1
> >>>> make: *** [.../build/board/davinci/common/libdavinci.a] Error 2
> >>>> make: *** Waiting for unfinished jobs....
> >>>>
> >>>>  board/davinci/common/misc.c       |   32 ++++++++++++++++++++++++++++++++
> >>>>  board/davinci/common/misc.h       |    7 +++++++
> >>>>  board/davinci/da830evm/da830evm.c |   28 +++++++++++-----------------
> >>>>  3 files changed, 50 insertions(+), 17 deletions(-)
> >>>>
> >>>> diff --git a/board/davinci/common/misc.c b/board/davinci/common/misc.c
> >>>> index 25ca326..dcf3cf2 100644
> >>>> --- a/board/davinci/common/misc.c
> >>>> +++ b/board/davinci/common/misc.c
> >>>> @@ -41,6 +41,24 @@ int dram_init(void)
> >>>>     return(0);
> >>>>  }
> >>>>
> >>>> +#ifdef CONFIG_SOC_DA8XX
> >>>> +void irq_init(void)
> >>>> +{
> >>>> +   /*
> >>>> +    * Mask all IRQs by clearing the global enable and setting
> >>>> +    * the enable clear for all the 90 interrupts.
> >>>> +    */
> >>>> +
> >>>> +   writel(0, &davinci_aintc_regs->ger);
> >>>> +
> >>>> +   writel(0, &davinci_aintc_regs->hier);
> >>>> +
> >>>> +   writel(0xffffffff, &davinci_aintc_regs->ecr1);
> >>>> +   writel(0xffffffff, &davinci_aintc_regs->ecr2);
> >>>> +   writel(0xffffffff, &davinci_aintc_regs->ecr3);
> >>>> +}
> >>>> +#endif
> >>>
> >>> In the current code base, this code is not included in the da830 compilation
> >>> if IRQs are not used. With this patch the code is included, but not called
> >>> if IRQs are not used. IRQs are often not used, so this change causes bloat.
> >>>
> >>> Could you please make this conditional on IRQs?
> >>>
> >>
> >> I added the code between CONFIG_SOC_DA8XX macro because davinci_aintc_regs
> >> declaration is in between this macro in hardware.h file. So they'll not be
> >> available for targets other than DA830 and DA850. Also, AINTC register
> >> mapping is different on DM644x, DM646x, DM355 and DM365. Shall I consider
> >> moving the irq_init function out of misc.c?
> 
> Just to be clear on my part: I meant that it should be conditional on both
> DA8XX *and* IRQs.
> 
> > 
> > Since it is da8XX specific, irq_init might be best placed somewhere in
> > the board/davinci/da8xxevm directory that is being introduced in the
> > da850 support series? Perhaps for this patch it could be extracted to
> > board/davinci/da830evm/common.c ?
> 
> Agreed.
> 

I'll be moving the irq_init and davinci_configure_lpsc_items functions to
common.c under board/davinci/da830.

> Also if the register mappings are different across all platforms, maybe
> davinci_aintc_regs should be renamed as da8xx_aintc_regs (and the struct
> definition name changed as well)?
> 

Yes, AINTC is specific to da8xx platforms, but in future there may be some
davinci platforms which may have this AINTC module. Also, in hardrware.h
the DA8XX specific register definitions have DAVINCI string in them. As of
now I'll retain the name davinci_aintc_regs as it will not cause any harm.

Regards,
Sudhakar