[U-Boot] Help: U-Boot on Nokia RX-51 (aka N900)

["Pali Rohár" <pali.rohar@gmail.com>]

On Thursday 01 September 2011 13:53:42 you wrote:
> Dear Pali =?ISO-8859-1?Q?Roh=E1r?=,
> 
> In message <34603899.3xsChgV26D@pali-elitebook> you wrote:
> > Ok, I attached all patches which I rebased on top of master. But U-Boot
> > still not working, not booting, no output on device.
> 
> Please stick to standard patch submission rules - all patches must be
> submitted inline; MIME attachments are strongly discouraged.  Fpr
> details pleass see http://www.denx.de/wiki/U-Boot/Patches

I wrote that this patch series was incomplete and not correct. I will fix that 
and I will use git send-email.

> 
> > From: Alistair Buxton <a.j.buxton@gmail.com>
> > Date: Wed, 1 Sep 2010 23:07:20 +0100
> > Subject: [PATCH 01/15] Make bootm optionally use pre-existing atags for
> > Linux kernel boot.
> > 
> > This patch adapts the bootm command so that it can use an existing atags
> > command set up by a previous bootloader. If the environment variable
> > "atags" is unset, bootm behaves as normal. If "atags" is set, bootm
> > will skip all boot args setup entirely, and pass the address found in
> > "atags". For example, if a previous boot
> > loader already set up the atags struct at 0x80000100:
> This has zero chances for being mainlined.  Please stick with
> standard boot commands, and adapt this for a standard SPL approach.

what is problem with using bi_boot_params from enviromental variable atagaddr?
what is SPL?

> 
> > From: Alistair Buxton <a.j.buxton@gmail.com>
> > Date: Wed, 1 Sep 2010 23:04:03 +0100
> > Subject: [PATCH 02/15] Store existing atags at startup if chainloading.
> > 
> > This patch stores the values in r1 and r2 at startup. It also stores the
> > address which u-boot was originally loaded to. This is useful if you
> > feed some other bootloader a u-boot.bin instead of the linux kernel it
> > was expecting. It is rather ugly because it stores these values in an
> > arbitrary memory address.
> Ditto.  This should be adapted to fit into the regular SPL framework
> instead.

so, what is correct way how to store atag address (register r2) for board 
code?

> 
> ...
> 
> > From: Alistair Buxton <a.j.buxton@gmail.com>
> > Date: Mon, 6 Sep 2010 03:01:34 +0100
> > Subject: [PATCH 03/15] Nokia RX-51 aka N900 support
> > 
> > This board definition results in a u-boot.bin which can be chainloaded
> > from NOLO in qemu or on a real N900. It does very little hardware config
> > because NOLO has already configured the board.
> 
> As mentioned before, this is normal in a SPL context.
> 
> > --- /dev/null
> > +++ b/board/nokia/rx51/config.mk
> 
> Please get rid of board specific config.mk files.

file deleted

> 
> > +TEXT_BASE 0x80e80000
> 
> This is wrong and needs to be fixed anyway.
> 
> > +int board_init(void)
> > +{
> > +	DECLARE_GLOBAL_DATA_PTR;
> 
> This cannot work..  The compiler miscompiles this. This declaration
> MUST be done at file scope.

fixed

> 
> > +		/* turn on keyboard and use hardware scanning */
> > +		ctrl |TWL4030_KEYPAD_CTRL_KBD_ON;
> > +		ctrl |TWL4030_KEYPAD_CTRL_SOFT_NRST;
> > +		ctrl |TWL4030_KEYPAD_CTRL_SOFTMODEN;
> > +		ret |twl4030_i2c_write_u8(TWL4030_CHIP_KEYPAD, ctrl,
> > +					TWL4030_KEYPAD_KEYP_CTRL_REG);
> > +		/* enable key event status */
> > +		ret |twl4030_i2c_write_u8(TWL4030_CHIP_KEYPAD, 0xfe,
> > +					TWL4030_KEYPAD_KEYP_IMR1);
> > +		/* using the second interrupt event breaks meamo pr1.2 kernel */
> > +		/*ret |twl4030_i2c_write_u8(TWL4030_CHIP_KEYPAD, 0xfe,
> > +					TWL4030_KEYPAD_KEYP_IMR2);*/
> > +		/* enable missed event tracking */
> > +		/*ret |twl4030_i2c_write_u8(TWL4030_CHIP_KEYPAD, 0x20,
> > +					TWL4030_KEYPAD_KEYP_SMS);*/
> > +		/* enable interrupt generation on rising and falling */
> > +		/* this is a workaround for qemu twl4030 emulation */
> > +		ret |twl4030_i2c_write_u8(TWL4030_CHIP_KEYPAD, 0x57,
> > +					TWL4030_KEYPAD_KEYP_EDR);
> > +		/* enable ISR clear on read */
> > +		ret |twl4030_i2c_write_u8(TWL4030_CHIP_KEYPAD, 0x05,
> > +					TWL4030_KEYPAD_KEYP_SIH_CTRL);
> 
> I have no idea what you think this code is doing.  It will definitely
> not be accepted for mainline.

this enable HW keyboard

> 
> Also, please make sure to remove dead (commented out) code.

ok, commented code will be deleted

> 
> ...
> 
> > diff --git a/include/configs/nokia_rx51.h b/include/configs/nokia_rx51.h
> > new file mode 100644
> > index 0000000..45b942a
> > --- /dev/null
> > +++ b/include/configs/nokia_rx51.h
> 
> ...
> 
> > +#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> > +#define CONFIG_OMAP		1	/* in a TI OMAP core */
> > +#define CONFIG_OMAP34XX		1	/* which is a 34XX */
> > +#define CONFIG_OMAP3430		1	/* which is in a 3430 */
> > +#define CONFIG_OMAP3_RX51	1	/* working with RX51 */
> > +#define CONFIG_CHAINLOADER	1	/* Loaded by NOLO */
> 
> Please never set any values for #defines which select features only.
> Please fix globally.

fixed all defines

> 
> ...
> 
> > From c5c232a3669bed778c438db0280ea78273d17e25 Mon Sep 17 00:00:00 2001
> > From: Matan Ziv-Av <matan@svgalib.org>
> > Date: Tue, 7 Dec 2010 12:01:34 +0100
> > Subject: [PATCH 04/15] Only delay boot if keyboard open
> > 
> > ---
> > 
> >  board/nokia/rx51/rx51.c      |    7 +++++++
> >  include/configs/nokia_rx51.h |    1 +
> >  2 files changed, 8 insertions(+), 0 deletions(-)
> > 
> > diff --git a/board/nokia/rx51/rx51.c b/board/nokia/rx51/rx51.c
> > index 3149a79..347d08a 100644
> > --- a/board/nokia/rx51/rx51.c
> > +++ b/board/nokia/rx51/rx51.c
> > @@ -104,6 +104,13 @@ int misc_init_r(void)
> > 
> >  	setenv("nolo_atagaddr", buf);
> >  
> >  #endif
> > 
> > +	// set environment variable slide_sw
> > +	// if keyboard slide is open/close
> 
> C++ comments are not allowed.  Please fix globally.  Make sure to run
> your patches through checkpatch.

"//" comments changed to /* .. */

> 
> > From: Matan Ziv-Av <matan@svgalib.org>
> > Date: Tue, 7 Dec 2010 12:03:38 +0100
> > Subject: [PATCH 05/15] Change Wireless LAN mode from M4 to M0
> 
> Please squash into earlier patch that creates this file.

I will squash all board files/commits into one patch.

> 
> > ---
> > 
> >  board/nokia/rx51/rx51.h |   20 ++++++++++----------
> >  1 files changed, 10 insertions(+), 10 deletions(-)
> > 
> > From: Matan Ziv-Av <matan@svgalib.org>
> > Date: Tue, 7 Dec 2010 12:08:54 +0100
> > Subject: [PATCH 06/15] Look for boot.scr on 'mmc 0:3' instead 'mmc 0'
> > and add support for loading boot.scr from 'mmc 2:1'
> Subject too long.
> 
> Please squash into earlier patch that creates this file.
> 
> > From: ?UTF-8?q?Pali Roh??r?<pali.rohar@gmail.com>
> > Date: Thu, 1 Sep 2011 12:13:32 +0200
> > Subject: [PATCH 07/15] RX-51: Fixed compilation on top of master
> > (changes from Beagle Board)
> Please squash into earlier patch that creates these files.
> 
> > From: ?UTF-8?q?Pali Roh??r?<pali.rohar@gmail.com>
> > Date: Wed, 31 Aug 2011 10:58:35 +0200
> > Subject: [PATCH 08/15] RX-51: Add support for resetting twl4030 watchdog
> > 
> >  * use test_and_set_bit and __clear_bit to access twl4030 i2c bus only
> >  once at same time
> Mind line length.  Please fix globally.
> 
> > +	__clear_bit(0, &twl_chip_lock);
> 
> Don't invent yoru own macros when we have standard mecros in place
> that do the same (here: clrbits_*() from <asm/io.h>).

I think that these are standrad macros. test_and_set_bit and __clear_bit are 
defined in include/asm/bitops.h and include/linux/bitops.h

> 
> > @@ -256,7 +278,6 @@ int rx51_kp_getc(void)
> > 
> >  {
> >  
> >  	keybuf_head %KEYBUF_SIZE;
> >  	while (!rx51_kp_tstc())
> > 
> > -		;
> > +		udelay(10000);
> 
> Use a much shorter delay here, maybe even udelay(1).

Ok I'm using udelay(1) + hw_watchdog_reset()

> 
> > From: ?UTF-8?q?Pali Roh??r?<pali.rohar@gmail.com>
> > Date: Wed, 31 Aug 2011 11:02:08 +0200
> > Subject: [PATCH 09/15] RX-51: Fix keymap
> > 
> >  * make functions and variables static
> >  * add support for additional key combination with ctrl and fn
> >  * add support for keys: up, down, left, right, volume_up, volume_down
> 
> Squash into earlier patches.
> 
> > From: ?UTF-8?q?Pali Roh??r?<pali.rohar@gmail.com>
> > Date: Wed, 31 Aug 2011 15:40:58 +0200
> > Subject: [PATCH 10/15] include/common.h: Add some macros for ANSI escape
> > codes
> > 
> > ---
> > 
> >  include/common.h |   32 ++++++++++++++++++++++++++++++++
> >  1 files changed, 32 insertions(+), 0 deletions(-)
> 
> This does not belong into common.h.

So where it should be defined?

> 
> ...
> 
> > From: ?UTF-8?q?Pali Roh??r?<pali.rohar@gmail.com>
> > Date: Wed, 31 Aug 2011 15:41:24 +0200
> > Subject: [PATCH 11/15] drivers/video/cfb_console.c: Added support for
> > ANSI escape codes
> This has several issues:
> 
> - You add it unconditionally, thus blowing up the code soze for
>   everybody, whether they want this feature or not.

Ok, I create for this config macro (CONFIG_VIDEO_CFB_ANSI) which can 
enable/disable ANSI termal support in cfb video driver.

> - You add it only here, not to all console drivers, thus creating
>   incompatible behaviour.  This is not nice.

Serial console output and usbtty are using host terminal (on PC). And all 
modern terminal support ANSI escape chars. So only framebuffer devices are 
without ANSI support. And this patch implement support fot standard framebuffer 
driver.

> 
> > +static void console_set_text_color(int c)
> > +{
> > +	// TODO
> > +}
> > +
> > +static void console_set_background_color(int c)
> > +{
> > +	// TODO
> > +}
> 
> Either implement the code, or omit the functions alltogeter.  Do not
> add dead code.

background and text color is not used, functions deleted.

> 
> ...
> 
> > +				if (num1 0) //reset swapped colors
> > +				{
> > +					if (ansi_colors_need_revert)
> > +					{
> > +						console_swap_colors();
> > +						ansi_colors_need_revert 0;
> > +					}
> 
> Incorrect brace style. Please fix globally.

fixed

> 
> > From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali.rohar@gmail.com>
> > Date: Wed, 31 Aug 2011 12:07:50 +0200
> > Subject: [PATCH 12/15] New command bootmenu: ANSI terminal Boot Menu
> > support> 
> >  * Configuration is done via env variables bootmenu_delay and 
bootmenu_<num>:
> Please check if you can rather use Jason Hobbs' "Add generic, reusable
> menu code" patch series.

I'd like to use for rx51 my bootmenu. I rebase bootmenu patches on top of 
master so then bootmenu can be simple added/deleted by one patch.

> 
> ...
> 
> > From: ?UTF-8?q?Pali Roh??r?<pali.rohar@gmail.com>
> > Date: Wed, 31 Aug 2011 14:12:06 +0200
> > Subject: [PATCH 14/15] New config variable CONFIG_PREMONITOR
> > 
> >  * if defined run env "premonitor" before Main Loop for Monitor Command
> >  Processing
> What does this do that preboot cannot do?

before running monitor loop I'd like wrote some info to console (specific board 
info, board commands, board macros). but if these info will be written in 
preboot it can be deleted by menu section. after preboot is running menu where 
can be used for example my bootmenu (which clean console output). so section 
after menu and before preboot is needed.

> 
> 
> Best regards,
> 
> Wolfgang Denk

-- 
Pali Roh?r
pali.rohar at gmail.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111009/a96b9281/attachment.pgp