* [U-Boot] [PATCH v4] da830: Move common code out of da830evm.c file
@ 2010-06-03 4:25 Sudhakar Rajashekhara
2010-06-03 10:53 ` Nick Thompson
0 siblings, 1 reply; 6+ messages in thread
From: Sudhakar Rajashekhara @ 2010-06-03 4:25 UTC (permalink / raw)
To: u-boot
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
+
#ifdef CONFIG_DRIVER_TI_EMAC
/* Read ethernet MAC address from EEPROM for DVEVM compatible boards.
@@ -186,3 +204,17 @@ int davinci_configure_pin_mux_items(const struct pinmux_resource *item,
return 0;
}
+
+/*
+ * Enable PSC for various peripherals.
+ */
+int davinci_configure_lpsc_items(const struct lpsc_resource *item,
+ const int n_items)
+{
+ int i;
+
+ for (i = 0; i < n_items; i++)
+ lpsc_on(item[i].lpsc_no);
+
+ return 0;
+}
diff --git a/board/davinci/common/misc.h b/board/davinci/common/misc.h
index 329c369..ee35f01 100644
--- a/board/davinci/common/misc.h
+++ b/board/davinci/common/misc.h
@@ -45,10 +45,17 @@ struct pinmux_resource {
.n_pins = ARRAY_SIZE(item) \
}
+struct lpsc_resource {
+ const int lpsc_no;
+};
+
+void irq_init(void);
int dvevm_read_mac_address(uint8_t *buf);
void dv_configure_mac_address(uint8_t *rom_enetaddr);
int davinci_configure_pin_mux(const struct pinmux_config *pins, int n_pins);
int davinci_configure_pin_mux_items(const struct pinmux_resource *item,
int n_items);
+int davinci_configure_lpsc_items(const struct lpsc_resource *item,
+ int n_items);
#endif /* __MISC_H */
diff --git a/board/davinci/da830evm/da830evm.c b/board/davinci/da830evm/da830evm.c
index 6385443..ed89473 100644
--- a/board/davinci/da830evm/da830evm.c
+++ b/board/davinci/da830evm/da830evm.c
@@ -120,21 +120,18 @@ static const struct pinmux_resource pinmuxes[] = {
#endif
};
+static const struct lpsc_resource lpsc[] = {
+ { DAVINCI_LPSC_AEMIF }, /* NAND, NOR */
+ { DAVINCI_LPSC_SPI0 }, /* Serial Flash */
+ { DAVINCI_LPSC_EMAC }, /* image download */
+ { DAVINCI_LPSC_UART2 }, /* console */
+ { DAVINCI_LPSC_GPIO },
+};
+
int board_init(void)
{
#ifndef CONFIG_USE_IRQ
- /*
- * 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);
+ irq_init();
#endif
#ifdef CONFIG_NAND_DAVINCI
@@ -165,11 +162,8 @@ int board_init(void)
* assuming here that the DSP bootloader has set the IOPU
* such that PSC access is available to ARM
*/
- lpsc_on(DAVINCI_LPSC_AEMIF); /* NAND, NOR */
- lpsc_on(DAVINCI_LPSC_SPI0); /* Serial Flash */
- lpsc_on(DAVINCI_LPSC_EMAC); /* image download */
- lpsc_on(DAVINCI_LPSC_UART2); /* console */
- lpsc_on(DAVINCI_LPSC_GPIO);
+ if (davinci_configure_lpsc_items(lpsc, ARRAY_SIZE(lpsc)))
+ return 1;
/* setup the SUSPSRC for ARM to control emulation suspend */
writel(readl(&davinci_syscfg_regs->suspsrc) &
--
1.5.6
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v4] da830: Move common code out of da830evm.c file
2010-06-03 4:25 [U-Boot] [PATCH v4] da830: Move common code out of da830evm.c file Sudhakar Rajashekhara
@ 2010-06-03 10:53 ` Nick Thompson
2010-06-03 12:58 ` Sudhakar Rajashekhara
[not found] ` <3447546128715435825@unknownmsgid>
0 siblings, 2 replies; 6+ messages in thread
From: Nick Thompson @ 2010-06-03 10:53 UTC (permalink / raw)
To: u-boot
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?
> diff --git a/board/davinci/common/misc.h b/board/davinci/common/misc.h
> index 329c369..ee35f01 100644
> --- a/board/davinci/common/misc.h
> +++ b/board/davinci/common/misc.h
> @@ -45,10 +45,17 @@ struct pinmux_resource {
> .n_pins = ARRAY_SIZE(item) \
> }
>
> +struct lpsc_resource {
> + const int lpsc_no;
> +};
> +
> +void irq_init(void);
See comment below...
> int dvevm_read_mac_address(uint8_t *buf);
> void dv_configure_mac_address(uint8_t *rom_enetaddr);
> int davinci_configure_pin_mux(const struct pinmux_config *pins, int n_pins);
> int davinci_configure_pin_mux_items(const struct pinmux_resource *item,
> int n_items);
> +int davinci_configure_lpsc_items(const struct lpsc_resource *item,
> + int n_items);
>
> #endif /* __MISC_H */
> diff --git a/board/davinci/da830evm/da830evm.c b/board/davinci/da830evm/da830evm.c
> index 6385443..ed89473 100644
> --- a/board/davinci/da830evm/da830evm.c
> +++ b/board/davinci/da830evm/da830evm.c
> @@ -120,21 +120,18 @@ static const struct pinmux_resource pinmuxes[] = {
> #endif
> };
>
> +static const struct lpsc_resource lpsc[] = {
> + { DAVINCI_LPSC_AEMIF }, /* NAND, NOR */
> + { DAVINCI_LPSC_SPI0 }, /* Serial Flash */
> + { DAVINCI_LPSC_EMAC }, /* image download */
> + { DAVINCI_LPSC_UART2 }, /* console */
> + { DAVINCI_LPSC_GPIO },
> +};
> +
> int board_init(void)
> {
> #ifndef CONFIG_USE_IRQ
> - /*
> - * 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);
> + irq_init();
> #endif
I'm okay with this part, but does it make sense to have a weak declaration (above)
and remove the IRQ ifdef from here?
Nick.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v4] da830: Move common code out of da830evm.c file
2010-06-03 10:53 ` Nick Thompson
@ 2010-06-03 12:58 ` Sudhakar Rajashekhara
[not found] ` <3447546128715435825@unknownmsgid>
1 sibling, 0 replies; 6+ messages in thread
From: Sudhakar Rajashekhara @ 2010-06-03 12:58 UTC (permalink / raw)
To: u-boot
Hi Nick,
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?
Thanks,
Sudhakar
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v4] da830: Move common code out of da830evm.c file
[not found] ` <3447546128715435825@unknownmsgid>
@ 2010-06-04 14:26 ` Ben Gardiner
2010-06-04 15:32 ` Nick Thompson
0 siblings, 1 reply; 6+ messages in thread
From: Ben Gardiner @ 2010-06-04 14:26 UTC (permalink / raw)
To: u-boot
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?
I can confirm that compilation of da830evm_defconfig is not possible
with CONFIG_USE_IRQ; attempting this will result in a compilation
error in start.S.
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 ?
I think that the unconditional definition of
davinci_configure_lpsc_items function has caused bloat in the other
davinci u-boot binaries:
--- ../davinci-before.out 2010-06-04 09:18:44.130839762 -0400
+++ ../davinci-after.out 2010-06-04 09:28:40.241776938 -0400
@@ -1,30 +1,30 @@
Configuring for davinci_dvevm board...
text data bss dec hex filename
- 178520 5500 297984 482004 75ad4 ./u-boot
+ 178568 5500 297984 482052 75b04 ./u-boot
Configuring for davinci_schmoogie board...
text data bss dec hex filename
- 154090 9000 54172 217262 350ae ./u-boot
+ 154138 9000 54172 217310 350de ./u-boot
Configuring for davinci_sffsdr board...
text data bss dec hex filename
- 153772 9024 54172 216968 34f88 ./u-boot
+ 153820 9024 54172 217016 34fb8 ./u-boot
Configuring for davinci_sonata board...
text data bss dec hex filename
- 145308 5296 55068 205672 32368 ./u-boot
+ 145356 5296 55068 205720 32398 ./u-boot
Configuring for davinci_dm355evm board...
text data bss dec hex filename
- 207202 8516 40864 256582 3ea46 ./u-boot
+ 207250 8516 40864 256630 3ea76 ./u-boot
Configuring for davinci_dm355leopard board...
text data bss dec hex filename
- 206320 7904 40864 255088 3e470 ./u-boot
+ 206492 7904 40864 255260 3e51c ./u-boot
Configuring for davinci_dm365evm board...
text data bss dec hex filename
- 243385 8704 297752 549841 863d1 ./u-boot
+ 243557 8704 297752 550013 8647d ./u-boot
Configuring for davinci_dm6467evm board...
text data bss dec hex filename
- 91776 4776 26100 122652 1df1c ./u-boot
+ 91948 4776 26100 122824 1dfc8 ./u-boot
Configuring for da830evm board...
text data bss dec hex filename
- 147475 4888 295320 447683 6d4c3 ./u-boot
+ 147543 4888 295320 447751 6d507 ./u-boot
Whereas if the definition of the davinci_configure_lpsc_items function
is made conditional on the same CONFIG_SOC_DA8XX macro introduced for
irq_init the other davinci systems u-boot binaries stay slim.
i.e applying the following on top of your patch:
---
diff --git a/board/davinci/common/misc.c b/board/davinci/common/misc.c
index dcf3cf2..d6536f6 100644
--- a/board/davinci/common/misc.c
+++ b/board/davinci/common/misc.c
@@ -205,6 +205,7 @@ int davinci_configure_pin_mux_items(const struct
pinmux_resource *item,
return 0;
}
+#ifdef CONFIG_SOC_DA8XX
/*
* Enable PSC for various peripherals.
*/
@@ -218,3 +219,4 @@ int davinci_configure_lpsc_items(const struct
lpsc_resource *item,
return 0;
}
+#endif
results in no change in most of the davinci u-boot binaries:
--- ../davinci-before.out 2010-06-04 09:18:44.130839762 -0400
+++ ../davinci-after.out 2010-06-04 10:04:39.820802124 -0400
@@ -24,7 +24,7 @@ Configuring for davinci_dm6467evm board.
91776 4776 26100 122652 1df1c ./u-boot
Configuring for da830evm board...
text data bss dec hex filename
- 147475 4888 295320 447683 6d4c3 ./u-boot
+ 147543 4888 295320 447751 6d507 ./u-boot
This bloat could also be addressed by moving the definition of that
function into a da8xx-common file along with irq_init.
Best Regards,
Ben Gardiner
PS: binary size output comparison was based on './MAKEALL davinci'
output with the following changes:
---
diff --git a/MAKEALL b/MAKEALL
index 2527352..d5a4ee2 100755
--- a/MAKEALL
+++ b/MAKEALL
@@ -545,6 +545,21 @@ LIST_ARM7=" \
"
#########################################################################
+## DaVinci Systems
+#########################################################################
+LIST_davinci=" \
+ davinci_dvevm \
+ davinci_schmoogie \
+ davinci_sffsdr \
+ davinci_sonata \
+ davinci_dm355evm \
+ davinci_dm355leopard \
+ davinci_dm365evm \
+ davinci_dm6467evm \
+ da830evm \
+"
+
+#########################################################################
## ARM9 Systems
#########################################################################
@@ -560,7 +575,6 @@ LIST_ARM9=" \
cp926ejs \
cp946es \
cp966 \
- da830evm \
edb9301 \
edb9302 \
edb9302a \
@@ -602,14 +616,7 @@ LIST_ARM9=" \
versatileab \
versatilepb \
voiceblue \
- davinci_dvevm \
- davinci_schmoogie \
- davinci_sffsdr \
- davinci_sonata \
- davinci_dm355evm \
- davinci_dm355leopard \
- davinci_dm365evm \
- davinci_dm6467evm \
+ ${LIST_davinci} \
"
#########################################################################
@@ -996,7 +1003,7 @@ print_stats() {
for arg in $@
do
case "$arg" in
- arm|SA|ARM7|ARM9|ARM10|ARM11|ARM_CORTEX_A8|at91|ixp|pxa \
+ arm|SA|ARM7|ARM9|ARM10|ARM11|ARM_CORTEX_A8|at91|ixp|pxa|davinci \
|avr32 \
|blackfin \
|coldfire \
--
Ben Gardiner
Nanometrics Inc.
+1 (613) 592-6776 x239
http://www.nanometrics.ca
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v4] da830: Move common code out of da830evm.c file
2010-06-04 14:26 ` Ben Gardiner
@ 2010-06-04 15:32 ` Nick Thompson
2010-06-07 8:11 ` Sudhakar Rajashekhara
0 siblings, 1 reply; 6+ messages in thread
From: Nick Thompson @ 2010-06-04 15:32 UTC (permalink / raw)
To: u-boot
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.
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)?
Nick.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH v4] da830: Move common code out of da830evm.c file
2010-06-04 15:32 ` Nick Thompson
@ 2010-06-07 8:11 ` Sudhakar Rajashekhara
0 siblings, 0 replies; 6+ messages in thread
From: Sudhakar Rajashekhara @ 2010-06-07 8:11 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-06-07 8:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-06-03 4:25 [U-Boot] [PATCH v4] da830: Move common code out of da830evm.c file Sudhakar Rajashekhara
2010-06-03 10:53 ` Nick Thompson
2010-06-03 12:58 ` Sudhakar Rajashekhara
[not found] ` <3447546128715435825@unknownmsgid>
2010-06-04 14:26 ` Ben Gardiner
2010-06-04 15:32 ` Nick Thompson
2010-06-07 8:11 ` Sudhakar Rajashekhara
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox