[U-Boot] [PATCH] config.mk: add drivers as default include PATH

[U-Boot] [PATCH] config.mk: add drivers as default include PATH

[Macpaul Lin]

Some device drivers and their header files will
invoke some definitions for assembly code. If a
device is required to be initilized in board.S
or in lowlevel_init.S, such memory controller,
flash controller, power control units. Sometimes
we both need to access these devices in bootstrap
process and later in normal operation period.

Hence the definitions of these drivers in the
header files is required to be found in cpu and
board folders. Moreover, these devices is usually
built into multiple different SoC with different
architectures. So the header files should be able
to be shared among the SoCs.

Add this
	CPPFLAGS += -I$(TOPDIR)/drivers
into the default include PATH will avoid such code
	"#include "../../../../../drivers/mtd/flash.h"

So this patch is suggested.

Signed-off-by: Macpaul Lin <macpaul@andestech.com>
---
 config.mk |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/config.mk b/config.mk
index 19e0a6e..52f2280 100644
--- a/config.mk
+++ b/config.mk
@@ -178,6 +178,7 @@ ifneq ($(OBJTREE),$(SRCTREE))
 CPPFLAGS += -I$(OBJTREE)/include2 -I$(OBJTREE)/include
 endif
 
+CPPFLAGS += -I$(TOPDIR)/drivers
 CPPFLAGS += -I$(TOPDIR)/include
 CPPFLAGS += -fno-builtin -ffreestanding -nostdinc	\
 	-isystem $(gccincdir) -pipe $(PLATFORM_CPPFLAGS)
-- 
1.7.3.2

[U-Boot] [PATCH] config.mk: add drivers as default include PATH

[Wolfgang Denk]

Dear Macpaul Lin,

In message <1292398049-3606-1-git-send-email-macpaul@andestech.com> you wrote:
> Some device drivers and their header files will
> invoke some definitions for assembly code. If a
> device is required to be initilized in board.S
> or in lowlevel_init.S, such memory controller,
> flash controller, power control units. Sometimes
> we both need to access these devices in bootstrap
> process and later in normal operation period.

I don't understand what you mean.  Do you have a specific example
where anything like this actually happens?

> Hence the definitions of these drivers in the
> header files is required to be found in cpu and
> board folders. Moreover, these devices is usually
> built into multiple different SoC with different
> architectures. So the header files should be able
> to be shared among the SoCs.

If this happens, these header files should be at some sufficiently
common location?

> Add this
> 	CPPFLAGS += -I$(TOPDIR)/drivers
> into the default include PATH will avoid such code
> 	"#include "../../../../../drivers/mtd/flash.h"

You should always stop, lean back and start thinking where you went
wrong whenever you encounter something that starts like this:

	#include "../../../../../xxx

This simply does not make sense.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Our way is peace.
	-- Septimus, the Son Worshiper, "Bread and Circuses",
	   stardate 4040.7.

[U-Boot] [PATCH] config.mk: add drivers as default include PATH

[Macpaul Lin]

Dear Wolfgang,

Thanks for your patience.

2010/12/15 Wolfgang Denk <wd@denx.de>

> Dear Macpaul Lin,
>
> In message <1292398049-3606-1-git-send-email-macpaul@andestech.com> you
> wrote:
> > Some device drivers and their header files will
> > invoke some definitions for assembly code. If a
> > device is required to be initilized in board.S
> > or in lowlevel_init.S, such memory controller,
> > flash controller, power control units. Sometimes
> > we both need to access these devices in bootstrap
> > process and later in normal operation period.
>
> I don't understand what you mean.  Do you have a specific example
> where anything like this actually happens?


Consider there are two different SoCs share the same peripheral devices.
That is, two different SoC licensed the same device IP.

1. Include path problem.
For example, Faraday a320 uses the same peripheral as AndesTech ag101.
A320 and Ag101 will include the same header files for the same peripheral
devices
which are built-in into the SoC.

These SoC built-in devices include,
ftsmc020.c (flash controller, arch/arm/cpu/arm920t/a320/ftsmc020.c),
ftsmc020.h (flash controller, arch/arm/include/asm/arch-a320/ftsmc020.h),

ftpmu010.h (power controller, arch/arm/include/asm/arch-a320/ftpmu010.h),
ftsdmc020.h (SDRAM controller, arch/arm/include/asm/arch-a320/ftsdmc020.h),
fttmr010.h (RTC, arch/arm/include/asm/arch-a320/fttmr010.h).

The flash controller ftsmc020 has both assembly (offset table) and C code
(structure).
The other devices as assembly code only because they will be initialized in
lowlevel_init.S or in board.S

1.1 At current situation, if ag101 want to include the device header file
above, the code
whether in "include/configs/adp-ag101.h" or in lowlevel_init.S will looks
like.

#define CONFIG_FTSMC020
#ifdef CONFIG_FTSMC020
#include "../../../arch/arm/includes/asm/arch-a320/ftsmc020.h"

1.2 Even we move those header files into proper place, this problem still
occur.
For example, if we put ftsmc020.h and ftsmc020.c into "drivers/mtd" folder.
The code in "faraday/a320evb/lowlevel_init.S" will exist
#include "../../../drivers/mtd/ftsdmc020.h"
and in "includes/configs/adp-ag101" will exist:
#ifdef CONFIG_FTSMC020
#include "../../../drivers/mtd/ftsdmc020.h"

2.
ftsmc020.c, ftsmc020.h, ftpmu010.h, ftsdmc020.h, fttmr010.h
These files and headers should be put into drivers folder since there are
multiple
SoC share the same device controllers.

3. There is possibility we initialize device in lowlevel_init.S then
reconfigure it in C level code.
Such as sdram controller and flash controller.
For example, in "arch/arm/include/asm/arch-at91/at91_mc.h" and
in "arch/arm/include/asm/arch-a320/ftsmc020.h"
There both exist assembly offset and C type structures.
Once other SoC use the same device controller whether in
lowlevel_int.S and in board.c, it will have include problem as described in
#1 and in #2.

4. If you execute 'grep -rnH "\.\./\.\./Marvell/" *' in the root of u-boot
code,
you will find the similar situation about sharing header files with same
devices.
Includes, pci.h, memory.h, core.h, i2c.h.
I guess even these devices is built-in in the SoC (chip), it is better to go
to "drivers" folder
instead of "board/Marvell/common/".
Unless the policy is that we should create a specific folder for a company
that sell SoC built-in devices.
Like "board/Faraday/common".
Otherwise it should be classified into "drivers" folder.

However, even we created a folder specific for a set of built-in devices,
we still encounter the include path like:
board/esd/cpci750/sdram_init.c:44:#include "../../Marvell/common/i2c.h"

5. Some shared header files of devices like "ftpmu010.h".
The header file "ftpmu010.h" has only register and offset definitions.
Should this kind of file go to "drivers" folder or go to "include" folder?
Since they are configured like a device drivers in a SoC (in board.c or in
lowlevel_init.S).
I guess they should be put into "drivers" folder.


> Hence the definitions of these drivers in the
> > header files is required to be found in cpu and
> > board folders. Moreover, these devices is usually
> > built into multiple different SoC with different
> > architectures. So the header files should be able
> > to be shared among the SoCs.
>
> If this happens, these header files should be at some sufficiently
> common location?
>
> > Add this
> >       CPPFLAGS += -I$(TOPDIR)/drivers
> > into the default include PATH will avoid such code
> >       "#include "../../../../../drivers/mtd/flash.h"
>
> You should always stop, lean back and start thinking where you went
> wrong whenever you encounter something that starts like this:
>
>        #include "../../../../../xxx
>
> This simply does not make sense.
>
> Best regards,
>
> Wolfgang Denk
>
>

-- 
Best regards,
Macpaul Lin

[U-Boot] [PATCH] config.mk: add drivers as default include PATH

[Wolfgang Denk]

Dear Macpaul Lin,

In message <AANLkTinWnzXRV=iUqNrp_fsbN+4uKBpJLHYuXj_KOU0w@mail.gmail.com> you wrote:
>
> 1. Include path problem.
> For example, Faraday a320 uses the same peripheral as AndesTech ag101.
> A320 and Ag101 will include the same header files for the same peripheral
> devices
> which are built-in into the SoC.
> 
> These SoC built-in devices include,
> ftsmc020.c (flash controller, arch/arm/cpu/arm920t/a320/ftsmc020.c),
> ftsmc020.h (flash controller, arch/arm/include/asm/arch-a320/ftsmc020.h),
> 
> ftpmu010.h (power controller, arch/arm/include/asm/arch-a320/ftpmu010.h),
> ftsdmc020.h (SDRAM controller, arch/arm/include/asm/arch-a320/ftsdmc020.h),
> fttmr010.h (RTC, arch/arm/include/asm/arch-a320/fttmr010.h).
> 
> The flash controller ftsmc020 has both assembly (offset table) and C code
> (structure).

The offset table should be removed. We can and should use
include/generated/*-asm-offsets.h now. Just make sure the needed
symbols are included.

> The other devices as assembly code only because they will be initialized in
> lowlevel_init.S or in board.S

This is something that should be reconsidered.  Why has this to be
done in assembler code, and cannot - for example - be delayed until we
have a C environment? 

Especially the SDRAM initialization is normally done from C anyway,
so why do we need it here?

> 1.1 At current situation, if ag101 want to include the device header file
> above, the code
> whether in "include/configs/adp-ag101.h" or in lowlevel_init.S will looks
> like.
> 
> #define CONFIG_FTSMC020
> #ifdef CONFIG_FTSMC020
> #include "../../../arch/arm/includes/asm/arch-a320/ftsmc020.h"

This is just an indication that the location of the file is not
correct.  If it's not a320 specific (which obliously is the case) it
should be moved into a common header directory.

> 1.2 Even we move those header files into proper place, this problem still
> occur.
> For example, if we put ftsmc020.h and ftsmc020.c into "drivers/mtd" folder.

It seems this is not an appropriate folder.  Why do you suggest this
one?

> 3. There is possibility we initialize device in lowlevel_init.S then
> reconfigure it in C level code.
> Such as sdram controller and flash controller.

SDRAM should not be initialized in lowlevel_init.S. Please use the
(new for ARM, shared with PPC which has always been using this) model
where RAM initialization is done in dram_init(), i. e. as part of the
init sequence in arch/arm/lib/board.c

> For example, in "arch/arm/include/asm/arch-at91/at91_mc.h" and
> in "arch/arm/include/asm/arch-a320/ftsmc020.h"
> There both exist assembly offset and C type structures.
> Once other SoC use the same device controller whether in
> lowlevel_int.S and in board.c, it will have include problem as described in
> #1 and in #2.

See above - this means that the header file needs to be moved to
acommon include directory.

> 4. If you execute 'grep -rnH "\.\./\.\./Marvell/" *' in the root of u-boot
> code,
> you will find the similar situation about sharing header files with same
> devices.
> Includes, pci.h, memory.h, core.h, i2c.h.
> I guess even these devices is built-in in the SoC (chip), it is better to go
> to "drivers" folder

drivers is not a common include directory, and thus not the right
place.

> instead of "board/Marvell/common/".

That may be true.

> However, even we created a folder specific for a set of built-in devices,
> we still encounter the include path like:

Why do you make it so complicated?  Just move common files to a _common_ 
header directory under include/ (and not drivers/).

> 5. Some shared header files of devices like "ftpmu010.h".
> The header file "ftpmu010.h" has only register and offset definitions.
> Should this kind of file go to "drivers" folder or go to "include" folder?

1) offset definitions should be removed; instead, *-asm-offsets.h
   should be used.

2) drivers/ is not a location for common header files that are
   referenced by non-driver code.  Use include/ for such common code,
   assuming it is really common to all architectures.  Use
   arch/arm/include/ if it's common to ARM systems only.

> Since they are configured like a device drivers in a SoC (in board.c or in
> lowlevel_init.S).
> I guess they should be put into "drivers" folder.

If this is common driver code, the code itself should be moved to
drivers (and then, and only then, the header files with the code).

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Nearly everyone is in favor of going  to  heaven  but  too  many  are
hoping  they'll  live  long  enough  to see an easing of the entrance
requirements. Never appeal to a man's "better nature." he  might  not
have one.

[U-Boot] [PATCH] config.mk: add drivers as default include PATH

[Macpaul Lin]

Dear Wolfgang,

2010/12/16 Wolfgang Denk <wd@denx.de>:
> Dear Macpaul Lin,

>> 3. There is possibility we initialize device in lowlevel_init.S then
>> reconfigure it in C level code.
>> Such as sdram controller and flash controller.
>
> SDRAM should not be initialized in lowlevel_init.S. Please use the
> (new for ARM, shared with PPC which has always been using this) model
> where RAM initialization is done in dram_init(), i. e. as part of the
> init sequence in arch/arm/lib/board.c
>
>> For example, in "arch/arm/include/asm/arch-at91/at91_mc.h" and
>> in "arch/arm/include/asm/arch-a320/ftsmc020.h"
>> There both exist assembly offset and C type structures.
>> Once other SoC use the same device controller whether in
>> lowlevel_int.S and in board.c, it will have include problem as described in
>> #1 and in #2.
>
> See above - this means that the header file needs to be moved to
> acommon include directory.
>
>> 4. If you execute 'grep -rnH "\.\./\.\./Marvell/" *' in the root of u-boot
>> code,
>> you will find the similar situation about sharing header files with same
>> devices.
>> Includes, pci.h, memory.h, core.h, i2c.h.
>> I guess even these devices is built-in in the SoC (chip), it is better to go
>> to "drivers" folder
>
> drivers is not a common include directory, and thus not the right
> place.
>
>> instead of "board/Marvell/common/".
>
> That may be true.
>
>
>> Since they are configured like a device drivers in a SoC (in board.c or in
>> lowlevel_init.S).
>> I guess they should be put into "drivers" folder.
>
> If this is common driver code, the code itself should be moved to
> drivers (and then, and only then, the header files with the code).
>
> Best regards,
>
> Wolfgang Denk
>

Your opinion is understood.

1. I will trying to make those shared devices being configurable as C drivers
instead of assembly definitions as possible.
2. For those devices which is hard/unnecessary to write a driver,
I will take effort on creating a common folder something like include/faraday
or include/faraday/common to collect those definitions.

-- 
Best regards,
Macpaul Lin

[U-Boot] [PATCH] config.mk: add drivers as default include PATH

[Albert ARIBAUD]

Le 16/12/2010 13:18, Wolfgang Denk a ?crit :

> SDRAM should not be initialized in lowlevel_init.S. Please use the
> (new for ARM, shared with PPC which has always been using this) model
> where RAM initialization is done in dram_init(), i. e. as part of the
> init sequence in arch/arm/lib/board.c

Just a note: some ARM SoCs may not have IRAM easily available (Marvell 
5182 for instance; it may not even be able to use its data cache for 
IRAM). In this case, even setting up a partial C environment for 
board_init_f may require initializing SDRAM in lowlevel_init.S.

Amicalement,
-- 
Albert.