[U-Boot] [PATCH v3 5/5] New board support: Nokia RX-51 aka N900

[Marek Vasut <marex@denx.de>]

Dear Pali Roh?r,

> Based on previous work by: Alistair Buxton <a.j.buxton@gmail.com>
> 
> Signed-off-by: Pali Roh?r <pali.rohar@gmail.com>
> Cc: ?????? ???????? <freemangordon@abv.bg>

Can we please stick to ASCII instead of UTF8?

Cc: Ivaylo Dimitrov (if I'm not mistaken)

> ---
> Changes since v2:
>    - Added documentation in doc/README.nokia_rx51
>    - Updated MAINTAINERS
>    - Fixed Fix omap_mmc_init
>    - Code for errata 430973 workaround
>    - Added board specified atag support
>    - Generate omap atag table instead reusing from NOLO
>    - Load bootmenu.src always
>    - Enabled ext4 support
>    - Fixed comments
>    - Reserve protected RAM for attached kernel
>    - Rewritten assembler code in lowlevel_init.S
>    - Reset lp5523 led on init
> 
> Changes since v1:
>    - Set correct configs for Memory Map
>    - Enable passing memory tag to kernel atags
>    - Use gpio input command for detecting keyboard slide
>    - Restore powerbus state after calling twl4030 regulator code
>    - Renamed command noloboot to attachboot
>    - Atag address must be always 0x80000100, removed code from lowlevel
>    - Added usb vendor, product id and product name
>    - Enabled command line editing
>    - Fixed keymap and cursor keys
> 
> Changes since original version:
>    - Removed Makefile targets: clean and distclean
>    - Rewrited bootcommand and env variables in nokia_rx51.h
>    - Removed useless CONFIG defines in nokia_rx51.h
>    - Disable L2 cache with CONFIG_SYS_L2CACHE_OFF - fixed battery draining
>    - Added onenand support (default disabled due to big u-boot size)
>    - Moved inlined asm code to new file lowlevel_init.S
>    - Fixed commit message
> 
>  MAINTAINERS                      |    4 +
>  board/nokia/rx51/Makefile        |   46 +++
>  board/nokia/rx51/lowlevel_init.S |  209 ++++++++++++
>  board/nokia/rx51/rx51.c          |  673
> ++++++++++++++++++++++++++++++++++++++ board/nokia/rx51/rx51.h          | 
> 389 ++++++++++++++++++++++
>  board/nokia/rx51/tag_omap.h      |  311 ++++++++++++++++++
>  boards.cfg                       |    1 +
>  doc/README.nokia_rx51            |  104 ++++++
>  include/configs/nokia_rx51.h     |  452 +++++++++++++++++++++++++
>  9 files changed, 2189 insertions(+)
>  create mode 100644 board/nokia/rx51/Makefile
>  create mode 100644 board/nokia/rx51/lowlevel_init.S
>  create mode 100644 board/nokia/rx51/rx51.c
>  create mode 100644 board/nokia/rx51/rx51.h
>  create mode 100644 board/nokia/rx51/tag_omap.h
>  create mode 100644 doc/README.nokia_rx51
>  create mode 100644 include/configs/nokia_rx51.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 971235b..613d8cd 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1009,6 +1009,10 @@ Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>
> 
>  	armadillo-800eva	R8A7740 (RMOBILE SoC)
> 
> +Pali Roh?r <pali.rohar@gmail.com>
> +
> +	nokia_rx51	ARM ARMV7 (OMAP3xx SoC)

OMAP3xxx or OMAP34xx

> +
>  -------------------------------------------------------------------------
> 
[..]

> +relocaddr:		/* address of this relocaddr section after coping */
> +	.word .		/* address of section (calculated at compile time) */
> +
> +startaddr:		/* address of u-boot after copying */
> +	.word CONFIG_SYS_TEXT_BASE
> +
> +kernaddr:		/* address of kernel after copying */
> +	.word KERNEL_ADDRESS
> +
> +kernsize:		/* maximal size of kernel image */
> +	.word KERNEL_MAXSIZE
> +
> +kernoffs:		/* offset of kernel image in loaded u-boot */
> +	.word KERNEL_OFFSET
> +
> +imagesize:		/* maximal size of image */
> +	.word IMAGE_MAXSIZE
> +
> +ih_magic:		/* IH_MAGIC in big endian from include/image.h */
> +	.word 0x56190527
> +
> +/*

Try using the new kerneldoc style, the tools are now in.

See eg. here about the annotations:
http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/144173

Also see:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/kernel-
doc-nano-HOWTO.txt

> + * Routine: save_boot_params (called after reset from start.S)
> + * Description: Copy attached kernel to address KERNEL_ADDRESS
> + *              Copy u-boot to address CONFIG_SYS_TEXT_BASE
> + *              Return to copied u-boot address
> + */
> +
> +.global save_boot_params
> +save_boot_params:
[...]

How much of the assembler crap can be made C?

[...]

> +static char *boot_reason_ptr;
> +static char *hw_build_ptr;
> +static char *nolo_version_ptr;
> +static char *boot_mode_ptr;
> +
> +/*
> + * Routine: init_omap_tags
> + * Description: Initialize pointers to values in tag_omap
> + */
> +static void init_omap_tags(void)
> +{
> +	char *component;
> +	char *version;
> +	int i = 0;
> +	while (omap[i].hdr.tag) {
> +		switch (omap[i].hdr.tag) {
> +		case OMAP_TAG_BOOT_REASON:
> +			boot_reason_ptr = omap[i].u.boot_reason.reason_str;
> +			break;
> +		case OMAP_TAG_VERSION_STR:
> +			component = omap[i].u.version.component;
> +			version = omap[i].u.version.version;
> +			if (strcmp(component, "hw-build") == 0)
> +				hw_build_ptr = version;
> +			else if (strcmp(component, "nolo") == 0)
> +				nolo_version_ptr = version;
> +			else if (strcmp(component, "boot-mode") == 0)
> +				boot_mode_ptr = version;
> +			break;

default: missing.

> +		}
> +		++i;

i++;

> +	}
> +}

[...]

> +	/* append omap atag only if env setup_omap_atag is set to 1 */
> +	str = getenv("setup_omap_atag");
> +	if (!str || strcmp(str, "1") != 0)

str[0] == '1' ? But still, you only want to check if it's defined, no?

[...]

> +/*
> + * Routine: twl4030_regulator_set_mode
> + * Description: Set twl4030 regulator mode over i2c powerbus.
> + */
> +static void twl4030_regulator_set_mode(u8 id, u8 mode)
> +{
> +	u16 msg = MSG_SINGULAR(DEV_GRP_P1, id, mode);
> +	twl4030_i2c_write_u8(TWL4030_CHIP_PM_MASTER, msg >> 8,
> +			TWL4030_PM_MASTER_PB_WORD_MSB);
> +	twl4030_i2c_write_u8(TWL4030_CHIP_PM_MASTER, msg & 0xff,
> +			TWL4030_PM_MASTER_PB_WORD_LSB);

Uh, is this somehow special that you can't do longer transfer?

> +}
> +
> +static void omap3_emu_romcode_call(u32 service_id, u32 *parameters)
> +{
> +	u32 i, num_params = *parameters;
> +	u32 *sram_scratch_space = (u32 *)OMAP3_PUBLIC_SRAM_SCRATCH_AREA;
> +
> +	/*
> +	 * copy the parameters to an un-cached area to avoid coherency
> +	 * issues
> +	 */

_WHAT_ ?!

Use dcache_flush(). The Omap3 rom won't cope with cache memory? Actually -- why 
do you even do call into ROM ?

> +	for (i = 0; i < num_params; i++) {
> +		__raw_writel(*parameters, sram_scratch_space);
> +		parameters++;
> +		sram_scratch_space++;
> +	}
> +
> +	/* Now make the PPA call */
> +	do_omap3_emu_romcode_call(service_id, OMAP3_PUBLIC_SRAM_SCRATCH_AREA);
> +}
> +

[...]

> +	/* set env variable attkernaddr for relocated kernel */
> +	sprintf(buf, "%#x", KERNEL_ADDRESS);
> +	setenv("attkernaddr", buf);

Uhhh ? This definitelly isn't right! What are you trying to achieve here?

> +
> +	/* initialize omap tags */
> +	init_omap_tags();
> +
> +	/* reuse atags from previous bootloader */
> +	reuse_atags();
> +
> +	dieid_num_r();
> +	print_cpuinfo();
> +
> +	/*
> +	 * Cortex-A8(r1p0..r1p2) errata 430973 workaround
> +	 * Set IBE bit in Auxiliary Control Register
> +	 */
> +	omap3_update_aux_cr_secure_rx51(1 << 6, 0);
> +
> +	return 0;
> +}
> +
> +/*
> + * Routine: set_muxconf_regs
> + * Description: Setting up the configuration Mux registers specific to the
> + *		hardware. Many pins need to be moved from protect to primary
> + *		mode.
> + */
> +void set_muxconf_regs(void)
> +{
> +	MUX_RX51();
> +}
> +
> +static unsigned long int twl_wd_time; /* last time of watchdog reset */
> +static unsigned long int twl_i2c_lock;

Are you sure you want to use global vars for these? These won't work before 
reloc!

[..]

> +
> +static u8 keys[8];
> +static u8 old_keys[8] = {0, 0, 0, 0, 0, 0, 0, 0};
> +#define KEYBUF_SIZE 32
> +static u8 keybuf[KEYBUF_SIZE];
> +static u8 keybuf_head;
> +static u8 keybuf_tail;

How much of this can be made const ?

> +/*
> + * Routine: rx51_kp_init
> + * Description: Initialize HW keyboard.
> + */
> +int rx51_kp_init(void)
> +{
> +	int ret = 0;
> +	u8 ctrl;
> +	ret = twl4030_i2c_read_u8(TWL4030_CHIP_KEYPAD, &ctrl,
> +		TWL4030_KEYPAD_KEYP_CTRL_REG);
> +

if (ret)
	return;

... do the rest here without additional indent ...

> +	if (!ret) {
> +		/* turn on keyboard and use hardware scanning */
> +		ctrl |= TWL4030_KEYPAD_CTRL_KBD_ON;
[...]

> +static void rx51_kp_fill(u8 k, u8 mods)
> +{

This magic needs at least _some_ documentation.

> +	if (!(mods & 2) && (k == 18 || k == 31 || k == 33 || k == 34)) {
> +		/* cursor keys, without fn */
> +		keybuf[keybuf_tail++] = '\e';

[...]

> +int rx51_kp_tstc(void)
> +{
> +	u8 c, r, dk, i;
> +	u8 intr;
> +	u8 mods;
> +
> +	/* localy lock twl4030 i2c bus */
> +	if (test_and_set_bit(0, &twl_i2c_lock))
> +		return 0;
> +
> +	/* twl4030 remembers up to 2 events */
> +	for (i = 0; i < 2; i++) {
> +
> +		/* check interrupt register for events */
> +		twl4030_i2c_read_u8(TWL4030_CHIP_KEYPAD, &intr,
> +				TWL4030_KEYPAD_KEYP_ISR1+(2*i));
> +
> +		if (intr&1) { /* got an event */

I will let you think about how to optimize the indent depth here ...

> +			/* read the key state */
> +			i2c_read(TWL4030_CHIP_KEYPAD,
> +				TWL4030_KEYPAD_FULL_CODE_7_0, 1, keys, 8);
> +
> +			/* cut out modifier keys from the keystate */
> +			mods = keys[4] >> 4;
> +			keys[4] &= 0x0f;
> +
> +			for (c = 0; c < 8; c++) {
> +
> +				/* get newly pressed keys only */
> +				dk = ((keys[c] ^ old_keys[c])&keys[c]);
> +				old_keys[c] = keys[c];
> +
> +				/* fill the keybuf */
> +				for (r = 0; r < 8; r++) {
> +					if (dk&1)

[...]


> +/*
> + * Routine: rx51_kp_getc
> + * Description: Get last pressed key (from buffer).
> + */
> +int rx51_kp_getc(void)
> +{
> +	keybuf_head %= KEYBUF_SIZE;
> +	while (!rx51_kp_tstc()) {
> +		udelay(1);
> +		hw_watchdog_reset();

WATCHDOG_RESET() and drop the udelay();

> +	}
> +	return keybuf[keybuf_head++];
> +}
> +
> +/*
> + * Routine: board_mmc_init
> + * Description: Initialize mmc devices.
> + */
> +int board_mmc_init(bd_t *bis)
> +{
> +	omap_mmc_init(0, 0, 0);
> +	omap_mmc_init(1, 0, 0);
> +	return 0;
> +}

[...]


> +#define tostring(s)		#s
> +#define stringify(s)		tostring(s)

We do have __stringify(), use that!
[...]