RE: Patch for putting together all the common files for OMAP3 - Patch(2/2)

Hi all,

Pls find patch(2/2). This moves the common files to a new folder /board/omap3
and modifies the main Makefile and the Makefiles in the board directories.

Pls let me know your comments.

Regards
Mani

u-boot-clnup-p2.txt (351 KB)

Hi Mani,

Pillai, Manikandan wrote:

Hi all,

Pls find the patch for moving all common files for overo, omap3evm and beagle.
I have compiled for all the boards and have run it for omap3EVM.
The first patch fixes the issues related to compilation for NAND for OMAP3EVM.

Pls send me your comments.

...

This patch puts all the common files into a separate folder for OMAP3 platforms. Signed-off-by: Manikandan Pillai <mani.pillai@ti.com>

DIFF STAT results
-------------------------------------------------------------
Makefile | 6 +-
board/omap3/common/clock.c | 313 +++++++++++++++++
board/omap3/common/lowlevel_init.S | 360 ++++++++++++++++++++
board/omap3/common/mem.c | 301 +++++++++++++++++
board/omap3/common/nand.c | 399 ++++++++++++++++++++++
board/omap3/common/syslib.c | 76 +++++
board/omap3/omap3530beagle/Makefile | 48 +++
board/omap3/omap3530beagle/config.mk | 17 +
board/omap3/omap3530beagle/omap3530beagle.c | 388 +++++++++++++++++++++
board/omap3/omap3530beagle/sys_info.c | 309 +++++++++++++++++
board/omap3/omap3530beagle/u-boot.lds | 63 ++++
board/omap3/omap3evm/Makefile | 48 +++
board/omap3/omap3evm/config.mk | 17 +
board/omap3/omap3evm/omap3evm.c | 481 +++++++++++++++++++++++++++
board/omap3/omap3evm/sys_info.c | 316 ++++++++++++++++++
board/omap3/omap3evm/u-boot.lds | 63 ++++
board/omap3/overo/Makefile | 48 +++
board/omap3/overo/config.mk | 12 +
board/omap3/overo/overo.c | 394 ++++++++++++++++++++++
board/omap3/overo/sys_info.c | 308 +++++++++++++++++
board/omap3/overo/u-boot.lds | 63 ++++
board/omap3530beagle/Makefile | 47 ---
board/omap3530beagle/clock.c | 314 -----------------
board/omap3530beagle/config.mk | 17 -
board/omap3530beagle/lowlevel_init.S | 360 --------------------
board/omap3530beagle/mem.c | 250 --------------
board/omap3530beagle/nand.c | 399 ----------------------
board/omap3530beagle/omap3530beagle.c | 388 ---------------------
board/omap3530beagle/sys_info.c | 309 -----------------
board/omap3530beagle/syslib.c | 72 ----
board/omap3530beagle/u-boot.lds | 63 ----
board/omap3evm/Makefile | 47 ---
board/omap3evm/clock.c | 313 -----------------
board/omap3evm/config.mk | 17 -
board/omap3evm/lowlevel_init.S | 360 --------------------
board/omap3evm/mem.c | 301 -----------------
board/omap3evm/nand.c | 399 ----------------------
board/omap3evm/omap3evm.c | 481 ---------------------------
board/omap3evm/sys_info.c | 316 ------------------
board/omap3evm/syslib.c | 76 -----
board/omap3evm/u-boot.lds | 63 ----
board/overo/Makefile | 47 ---
board/overo/clock.c | 314 -----------------
board/overo/config.mk | 12 -
board/overo/lowlevel_init.S | 360 --------------------
board/overo/mem.c | 288 ----------------
board/overo/nand.c | 408 -----------------------
board/overo/overo.c | 394 ----------------------
board/overo/sys_info.c | 308 -----------------
board/overo/syslib.c | 72 ----
board/overo/u-boot.lds | 63 ----
51 files changed, 4027 insertions(+), 6861 deletions(-)

We discussed this at IRC and like it :slight_smile:

So the current plan is:

1) Apply your above changes. Many thanks for this! (Hoping there are no functional changes in this patch, only code move)

2) We discussed to use board/omap3/common/ or cpu/omap3 for common files. Because DaVinci uses cpu/ directory for common files and cpu/omap3 is already there, we like to use cpu/omap3 instead additional new directory board/omap3/common/ for common files. We have a 50:50 chance to be right here. Let's see :wink:

3) Rename all files/directories in board/omap3 to not use omapXXX any more. This will result in board/omap3/evm/evm.c, board/omap3/overo/overo.c and board/omap3/beagle/beagle.c

4) Review the 3 sys_info.c files if they have common parts that can be moved to cpu/omap3 directory, too.

5) Review overo.c, beagle.c and evm.c if they have common parts that can be moved to cpu/omap3 directory, too.

6) If this is done, extract patches for U-Boot list, review them once again and if they are fine, send first patch to U-Boot list

7) Incorporate list's comments....

For (2) and (3) Steve (thanks!) will try to do the move/rename directly in git without any additional patch. File move/rename should hopefully be easy in git so that no patch is necessary for this.

Best regards

Dirk

Hi Dirk,

Thanks for your response.

1. There are no functional changes in this patch
2. I have tried to address the mv to cpu/omap3 directory the files, compile and test it. This patch addresses point (2) only.
3. I understand from the mail, that Steve will take care of (3).
4. I will work on point(4) now
5. Can work on it once point(2) of renaming files is completed.

Pls let me know if you have comments.

Regards
Mani

u-boot-clnup-p1.txt (188 KB)

I have created a new branch (common) to use for this task. I have
implemented and tested the basic renaming and common files on overo.

You can review my changes at:

http://www.sakoman.net/cgi-bin/gitweb.cgi?p=u-boot-omap3.git;a=shortlog;h=refs/heads/common

Steve

I have created a new branch (common) to use for this task. I have
implemented and tested the basic renaming and common files on overo.

You can review my changes at:

http://www.sakoman.net/cgi-bin/gitweb.cgi?p=u-boot-omap3.git;a=shortlog;h=r

efs/heads/common

Steve

Hi Steve, Dirk,

Out of interest have you considered making the same layout changes to the
x-loader codebase to make comparison and reuse easier across the OMAP3
boards or is that layout sacrosanct?

Just curious as it was something that I was considering.

Regards,

John

Hi all,
Pls find attached the patch for (4) - common parts of sys_info.c that can be
moved to cpu/omap3 directory. Both files are still named sys_info.c. Also,
the names of the directories are still the same i.e. omap3evm, overo and omap3530beagle.

Pls let me know if you have any comments.

Regards
Mani

u-boot-clnup-p2.txt (25.4 KB)

Hi all,

Pls find attached the patch for (5) - common parts of omap3evm.c,overo.c and omap3530beagle.c. The commom part is moved to a new file - cpu/omap3/omap3.c.
While making the common portion, care is taken to move only the SoC specific parts of the code to the new file. (Also note, that I have not yet renamed the files to the new names proposed)

Pls let me know if you have any comments.

I will meanwhile be working on the onenand patch(the one which failed due to upstream checkins and try to sendit out tomorrow)

Regards
Mani

u-boot-clnup-p3.txt (30.1 KB)

John,

Yes, it is on my "to do" list.

Steve

Mani: I looked at your patches to move common stuff from sys_info.c to cpu/omap3 and the comon stuff from board files. Let me answer everything in this one mail, else it becomes too confusing.

Pillai, Manikandan wrote:

Hi Dirk,

Thanks for your response.

1. There are no functional changes in this patch

Mani: Okay, thanks for confirmation.

2. I have tried to address the mv to cpu/omap3 directory the files, compile and test it. This patch addresses point (2) only.

Mani: Does the new NAND driver work for you?

3. I understand from the mail, that Steve will take care of (3).

Mani: Yes, he did it (Steve: Thanks!):

http://www.sakoman.net/cgi-bin/gitweb.cgi?p=u-boot-omap3.git;a=shortlog;h=refs/heads/common

4. I will work on point(4) now

For reference: Point (4) was: "Review the 3 sys_info.c files if they have common parts that can be
moved to cpu/omap3 directory"

First, you were faster than me :wink: I did something similiar, but slower than you...

Mani: Looking at your patch to move sys_info.c, there are still sys_info.c files in individual board directories. I don't think this is necessary. Looking at the differences between the three individiual sys_info.c files, there are only four differences, which are some strings and integers. Therefore I propose:

- Move sys_info.c completely to cpu/omap3/sys_info.c having *no* sys_info.c any more in the individual board directories.

- Change cpu/omap3/sys_info.c to use something like

u32 get_board_type(void)
{
  if (get_cpu_rev() == CPU_3430_ES2)
    return sysinfo.board_type_v2;
  else
    return sysinfo.board_type_v1;
}

char cpu_3430s = sysinfo.cpu_string;

printf("%s + %s/%s\n", sysinfo.board_string,
         mem_s, bootmode[get_gpmc0_type()]);

and then within indvidual board files

evm.c:

const omap3_sysinfo sysinfo = {
  OMAP3EVM_V1
  OMAP3EVM_V2,
  "35X-Family",
  "OMAP3 EVM board",
};

beagle.c:

const omap3_sysinfo sysinfo = {
  SDP_3430__V1
  SDP_3430__V2,
  "3530",
  "OMAP3 Beagle board",
};

overo.c:

const omap3_sysinfo overo_sysinfo = {
  SDP_3430__V1
  SDP_3430__V2,
  "3530",
  "OMAP3 overo board",
};

and a typedef like

typedef struct {
  u32 board_type_v1;
  u32 board_type_v2;
  char * cpu_string;
  char * board_string
} omap3_sysinfo;

in a header file.

5. Can work on it once point(2) of renaming files is completed.

For reference: Point (5) was: "5) Review overo.c, beagle.c and evm.c if they have common parts that
can be moved to cpu/omap3 directory"

I did something for this, too, but again, you were faster :wink: But it helps me for review:

Mani: You named the common board file cpu/omap3/omap3.c, I used cpu/omap3/board.c. But most probably this doesn't matter :wink:

Mani: Regarding what you moved from individual board files to cpu/omap3/board.c:

a) We have to check which header files we can remove from individual board files once most stuff moved to cpu/omap3/board.c

b) I think

#if (CONFIG_COMMANDS & CFG_CMD_NAND) && defined(CFG_NAND_LEGACY)
#include <linux/mtd/nand_legacy.h>
extern struct nand_chip nand_dev_desc[CFG_MAX_NAND_DEVICE];
#endif

#define NOT_EARLY 0

can be moved to common board file cpu/omap3/board.c, too (if nand_init() moves as well, see below)?

c) Functions

delay()
s_init()
dram_init()
nand_init()

can be moved, too?

For me, this results in individual board files only having

board_init()
misc_init_r()
set_muxconf_regs()

and in evm.c additionally setup_net_chip().

Conclusion:

Do you like to

- get Steve's git head with common file changes?

- move sys_info and add parameter struct configuration?

- move additional functions from individual board files?

Many thanks for your help

Dirk

Steve Sakoman wrote:

I have created a new branch (common) to use for this task. I have
implemented and tested the basic renaming and common files on overo.

You can review my changes at:

http://www.sakoman.net/cgi-bin/gitweb.cgi?p=u-boot-omap3.git;a=shortlog;h=refs/heads/common

Many thanks!

What do you think about renaming

include/configs/

omap3530beagle.h -> omap3beagle.h

overo.h -> omap3overo.h

? omap3evm.h is already there.

And one additional question:

While looking at overo.c, I found that overo's misc_init_r() has an
additional

  /* set vaux2 to 2.8V */
  byte = 0x20;
  i2c_write(0x4B, 0x76, 1, &byte, 1);
  byte = 0x09;
  i2c_write(0x4B, 0x79, 1, &byte, 1);

compared to beagle.c. Any hint what this is good for and why it isn't
needed at beagle?

Thanks

Dirk

Great first cut & suggestions! I will let you and Mani tackle this
portion unless you want me to get involved :slight_smile:

Steve

What do you think about renaming

include/configs/

Good idea! I meant to do that last night, but I guess I got sleepy and forgot.

I renamed them following the davinci convention (omap3_beagle.h,
omap3_evm.h and omap3_overo.h) and made the necessary changes to the
makefile:

http://www.sakoman.net/cgi-bin/gitweb.cgi?p=u-boot-omap3.git;a=commit;h=b6364a269aa2b3901f908bef1cf0b4f3b7fe99c9

And one additional question:

While looking at overo.c, I found that overo's misc_init_r() has an
additional

       /* set vaux2 to 2.8V */
       byte = 0x20;
       i2c_write(0x4B, 0x76, 1, &byte, 1);
       byte = 0x09;
       i2c_write(0x4B, 0x79, 1, &byte, 1);

Any hint what this is good for and why it isn't needed at beagle?

There is hw on the current Overo that needs vaux2, so this code
initializes that supply to 2.8V. Beagle doesn't use or need this
supply, so there would be no point to adding it.

Thanks Dirk!

Steve

Hi Dirk,

Thanks for all your comments. Pls see my replies inline with the tag
"Mani -------->"

Regards
Mani

Hi Mani,

Pillai, Manikandan wrote:

Hi Dirk,

Thanks for all your comments. Pls see my replies inline with the tag "Mani -------->"

Do you know Outlook (do you use Outlook?) configuration hints

http://wiki.davincidsp.com/index.php?title=Patch_upstream_sending#Outlook_config

?

Maybe this can help you to improve your mail formatting? :wink:

From: Dirk Behme [mailto:dirk.behme@googlemail.com] Sent: Wednesday, August 20, 2008 12:02 AM
To: Pillai, Manikandan
Cc: beagleboard@googlegroups.com; Steve Sakoman
Subject: Re: Patch for putting together all the common files for OMAP3 - Patch(1/1)

Mani: I looked at your patches to move common stuff from sys_info.c to cpu/omap3 and the comon stuff from board files. Let me answer everything in this one mail, else it becomes too confusing.

Pillai, Manikandan wrote:

Hi Dirk
Thanks for your response.

1. There are no functional changes in this patch

Mani: Okay, thanks for confirmation.

Mani --------> OK

2. I have tried to address the mv to cpu/omap3 directory the files, compile and test it. This patch addresses point (2) only.

Mani: Does the new NAND driver work for you?

Mani --------> No, I just fixed the compilation bug. I still don't have a NAND EVM board available with me. Should be able to test this week or early next week.

Okay, thanks for the clarification. Maybe someone can provide you a Beagle/NAND board?

3. I understand from the mail, that Steve will take care of (3).

Mani: Yes, he did it (Steve: Thanks!):
Mani --------> OK. I will download it and work on that one.(Actually, we have cloned the DENX tree here, and time to time I download Steve's git, generate a
Patch and apply it to me as a base patch and then start working on that one for
OMAP changes. That's the reason you see some patches failing)

http://www.sakoman.net/cgi-bin/gitweb.cgi?p=u-boot-omap3.git;a=shortlog;h=refs/heads/common

4. I will work on point(4) now

For reference: Point (4) was: "Review the 3 sys_info.c files if they have common parts that can be
moved to cpu/omap3 directory"

First, you were faster than me :wink: I did something similiar, but slower than you...

Mani: Looking at your patch to move sys_info.c, there are still sys_info.c files in individual board directories. I don't think this is necessary. Looking at the differences between the three individiual sys_info.c files, there are only four differences, which are some strings and integers. Therefore I propose:

- Move sys_info.c completely to cpu/omap3/sys_info.c having *no* sys_info.c any more in the individual board directories.

- Change cpu/omap3/sys_info.c to use something like

u32 get_board_type(void)
{
  if (get_cpu_rev() == CPU_3430_ES2)
    return sysinfo.board_type_v2;
  else
    return sysinfo.board_type_v1;
}

char cpu_3430s = sysinfo.cpu_string;

printf("%s + %s/%s\n", sysinfo.board_string,
         mem_s, bootmode[get_gpmc0_type()]);

and then within indvidual board files

evm.c:

const omap3_sysinfo sysinfo = {
  OMAP3EVM_V1
  OMAP3EVM_V2,
  "35X-Family",
  "OMAP3 EVM board",
};

beagle.c:

const omap3_sysinfo sysinfo = {
  SDP_3430__V1
  SDP_3430__V2,
  "3530",
  "OMAP3 Beagle board",
};

overo.c:

const omap3_sysinfo overo_sysinfo = {
  SDP_3430__V1
  SDP_3430__V2,
  "3530",
  "OMAP3 overo board",
};

and a typedef like

typedef struct {
  u32 board_type_v1;
  u32 board_type_v2;
  char * cpu_string;
  char * board_string
} omap3_sysinfo;

in a header file.

Mani --------> I noticed that we can completely remove sys_info.c. I was thinking of putting #defines and felt that if the type of board and CPU increases going forward, things would be very confusing.

Yes, #defines are no solution for this and will really become confusing. So we shouldn't do this.

I like your proposal and will try to implement it today.

:slight_smile: Okay, let us try this way, then.

5. Can work on it once point(2) of renaming files is completed.

For reference: Point (5) was: "5) Review overo.c, beagle.c and evm.c if they have common parts that
can be moved to cpu/omap3 directory"

I did something for this, too, but again, you were faster :wink: But it helps me for review:

Mani: You named the common board file cpu/omap3/omap3.c, I used cpu/omap3/board.c. But most probably this doesn't matter :wink:

Mani: Regarding what you moved from individual board files to cpu/omap3/board.c:

a) We have to check which header files we can remove from individual board files once most stuff moved to cpu/omap3/board.c

b) I think

#if (CONFIG_COMMANDS & CFG_CMD_NAND) && defined(CFG_NAND_LEGACY)
#include <linux/mtd/nand_legacy.h>
extern struct nand_chip nand_dev_desc[CFG_MAX_NAND_DEVICE];
#endif

#define NOT_EARLY 0

can be moved to common board file cpu/omap3/board.c, too (if nand_init() moves as well, see below)?

c) Functions

delay()
s_init()
dram_init()
nand_init()

can be moved, too?

Mani --------> Thought about it. But since I don't have a NAND board didn't want to play around with this one, fearing it might break something.

Please try to move above functions to common board file, too. Don't fear breaking it. First, Steve has a seperate branch for this, so breakage would be no problem. Second, I did a diff between all three existing board files and couldn't find a difference in above functions, so looking at the code it should work.

If you move above functions to common board files, too, and all three boards still compile & link, it should be fine. Then we like the patch for this.

Hopefully you will have a NAND board to test, soon.

For me, this results in individual board files only having

board_init()
misc_init_r()
set_muxconf_regs()

and in evm.c additionally setup_net_chip().

Conclusion:

Do you like to

- get Steve's git head with common file changes?
Mani --------> Yes I will download it.

- move sys_info and add parameter struct configuration?
Mani --------> I will try to complete it today. Meanwhile I will first send out the patch relating to onenand fixes(the one rejected due to upstream changes)

There is some discussion about a "Fix OneNAND read_oob/write_oob functions compatability" patch at U-Boot list. But I think this is something different?

- move additional functions from individual board files?
Mani --------> OK will take this one up after the others.

Sounds really good!

Many thanks for your contribution!

Dirk

Hi all,

Pls find attached the patch for onenand for adding scrub/block commands.
The patch takes care of changes done upstream(due to which the earlier patch
was failing)

Pls let me know your comments.

Regards
Mani

u-boot-onnd-p1.txt (21.4 KB)

This patch pushed to "common" branch today.

Steve

Mani --------> OK. I will download it and work on that one.(Actually, we have cloned the DENX tree here, and time to time I download Steve's git, generate a
Patch and apply it to me as a base patch and then start working on that one for
OMAP changes. That's the reason you see some patches failing)

You are making more work for yourself doing it this way (and also
making broken patches much more likely).

I've structured my git as a clone of DENX that is updated hourly.
Just clone my git repo and you will have the most current DENX state
as the master branch, and you will also get the work in progress as
the test and common branches. You will never will have to mess around
with generating patches to sync up, and if you ever want to see what
the diff is between our branch and the upstream branch it is as simple
as 'git diff master'

Regards,

Steve

Mani,

This patch seems to result in error messages for just about every
onenand operation. They are all of this form:

onenand_isbad_bbt: bbt info for offs 0x00260000: (block 19) 0x00

Have you seen this in your testing?

Steve

Hi all,

Pls find attached the patch for moving sys_info.c to the common folder cpu/omap3 as per the comments given by Dirk.
I have compiled for all boards - overo, beagle and omap3evm. I have tested it
on an OMAP3EVM onenand board.

Pls review this and send in your comments.

Regards
Mani

Pls find attached the patch for onenand for adding scrub/block commands.
The patch takes care of changes done upstream(due to which the earlier patch
was failing)

Mani,

Unfortunately I had to revert this patch due to a large number of
conflicts with these upstream changes:

http://www.sakoman.net/cgi-bin/gitweb.cgi?p=u-boot-omap3.git;a=commit;h=bfd7f38614e21f745b6d6845fcc616ebc5e4d36f

You'll need to redo this patch based upon the new head of tree.

Also, since this patch is not omap specific, it should probably be
submitted directly to the upstream mailing list. It really doesn't
make sense to submit it upstream along with beagle, evm, or overo
board support patches.

Steve