Hi Tom,
I'm currently busy with other work; on the other hand, careful rebasing shall require some time, especially the Palmas stuff. What would be the deadline for a V2 submission?
Meanwhile could you please have a look at the (already old) http://patchwork.ozlabs.org/patch/232743/? A simple patch, shall be needed if we enable USB (for the uEVM along with our board). In general, what are your plans regarding USB (.../patch/232742/)?
And again on I2C (.../patch/233823/): what is you final opinion? I'm confident that this patch is a major improvement for OMAP4/5 at least.
Please see some more comments inline below.
On 13/05/13 22:37, Tom Rini wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 04/26/2013 11:59 AM, Lubomir Popov wrote:
Hi Tom,
On 25/04/13 22:01, Tom Rini wrote:
On Mon, Apr 01, 2013 at 05:06:16PM +0300, Lubomir Popov wrote:
Signed-off-by: Lubomir Popov lpopov@mm-sol.com
Thought I had reviewed this already, sorry.
Thanks for your review. During the past month U-Boot has changed; I have tried to follow as well (although I'm engaged with other stuff) and some of your remarks have been already fixed. Please see my comments inline below.
Anyway, I guess that a V2 patch shall have to be referenced against the current master. Or against u-boot-ti/next?
OK, sorry for the late reply again. Now that u-boot-arm has resynced with master, I've also resynced and gotten a pull done. You can use either u-boot-arm/master or u-boot-ti/master.
Please note that for this board to work with the defined configuration, the following patches are also required (unfortunately some are already quite old and might cause conflicts):
- Power: http://patchwork.ozlabs.org/patch/232732/. Potential
conflict with Nishanth Menon's series of March 26, applied to u-boot-ti/next. - For I2C support: * The patch series of Apr. 8 that enables I2C4 and I2C5 (applied to u-boot-ti/next; affects all OMAP5 boards). * The modified i2c driver: http://patchwork.ozlabs.org/patch/233823/ (useful for all OMAP3/4/5 boards). - For USB support: * http://patchwork.ozlabs.org/patch/235684/ (affects all OMAP5 boards) * http://patchwork.ozlabs.org/patch/232742/ (affects all OMAP5 boards)
OK. Please make sure these still apply and if not update and re-send. I think I've already grabbed a few of these.
[snip]
+#define USB_HOST_HS_CLKCTRL_MASK 0x0100D7C0 /* CM_L3INIT_USB_HOST_HS_CLKCTRL */ +#define USB_TLL_HS_CLKCTRL_MASK 0x00000700 /* CM_L3INIT_USB_TLL_HS_CLKCTRL */
Some header please.
Currently moved to board header. I wondered if a common OMAP header wouldn't be more suitable, but having in mind that the utilized USB ports (and thus the clocks that should be enabled) vary from board to board, perhaps this (i.e. board header) is the best place.
The masks won't change tho, yes? Common header somewhere.
These are in fact not masks, but enable bits (sort of confusing terminology found throughout TI headers). Now renamed to ..._CLKCTRL_EN; staying in board header.
* TODO: Replace this ugly hardcoding with proper defines +*/ + writel(0x0100, 0x4ae0a310);
Again, do please.
This should be (*scrm)->auxclk0. The problem is that the omap5_scrm_regs struct (holding the auxclk0 member) has to be defined somewhere in the common OMAP5 headers. Sricharan? Or should I hack around?
Add it to the most likely struct in the headers.
The entire struct (I call it omap5_scrm_regs in theory, similar to the corresponding omap4_scrm_regs for OMAP4) is not defined anywhere. Of course I could define only the member that I need, but I guess it is a (responsible) TI job to define hardware descriptors. Or I'm wrong? Please advise. If I have time, I could do it myself - it's some 27 registers, almost identical to the OMAP4, and should go into arch/arm/include/asm/arch-omap5/clocks.h.
[snip]
Ah, since you do have this part already, just update the rest as I had said.
/* Machine type for Linux */ #ifndef MACH_TYPE_SOM5_EVB #define MACH_TYPE_SOM5_EVB 4545 #endif #define CONFIG_MACH_TYPE MACH_TYPE_SOM5_EVB
Is this OK?
I think we'd decided in general to not do ifndef and just always define it.
+/* Enable all clocks: */ +/*#define CONFIG_SYS_CLOCKS_ENABLE_ALL*/ + +#define CONFIG_SYS_ENABLE_PADS_ALL /* Enable all PADS for now */
Not allowed.
Shall see to it. What if one needs to test pins and connections during board bring-up, e.g. via gpio commands?
Then code in what you need at the time, drop later.
[snip]
This is a little un-clear. If MMC will be in mmc like the uEVM, just do so, and if no storage of env, leave it as NOWHERE.
Cerrently looks like this: /* MMC ENV related defines */ #undef CONFIG_ENV_IS_IN_MMC #undef CONFIG_SYS_MMC_ENV_DEV #undef CONFIG_ENV_OFFSET #define CONFIG_ENV_IS_NOWHERE
Shouldn't need that now, omap5_common is common and the env bits got moved to omap5_uevm.h
[snip]
+/* + * memtest setup + */ +#define CONFIG_SYS_MEMTEST_START 0xb8000000 +#define CONFIG_SYS_MEMTEST_END (CONFIG_SYS_MEMTEST_START + (256 << 20)) +/* Undef following two for simple mtest */ +#define CONFIG_SYS_ALT_MEMTEST +#define CONFIG_SYS_MEMTEST_SCRATCH 0x81000000
Please see doc/README.memory-test and update as mtest is no longer a default command.
Now the config tail looks like this:
/* Undef/remove after bring-up: */ #define CONFIG_CMD_MEMTEST
/* Disabled commands */ #undef CONFIG_CMD_SAVEENV
/* Prompt */ #define CONFIG_SYS_PROMPT "SOM5_EVB # "
#ifdef CONFIG_CMD_MEMTEST /* Undef following two for simple mtest */ #define CONFIG_SYS_ALT_MEMTEST #define CONFIG_SYS_MEMTEST_SCRATCH 0xb7fffff0 #ifdef CONFIG_SYS_ALT_MEMTEST #undef CONFIG_SYS_MEMTEST_START #define CONFIG_SYS_MEMTEST_START 0xb8000000 #undef CONFIG_SYS_MEMTEST_END #define CONFIG_SYS_MEMTEST_END (CONFIG_SYS_MEMTEST_START + (256 << 20)) #endif #endif /* CONFIG_CMD_MEMTEST */
OK.
Tom -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQIcBAEBAgAGBQJRkUEQAAoJENk4IS6UOR1WtscP/0RApAXEgttNem+eho1kFJMZ FSi3UgiO0+XqGbETTyYuG/r6RTz9grDcbtSs9np6/9H3wrc2FjCtlqXygJmQgbmr 2idPh4fyHerFvkZTZexJ3Lr+GC/1cJJyJliPvhj281OujbL/iDICkI/SLsnS3rtA hDC8MmrWWoDITtLzVNCbzkUk6gZVjFG/49dgjNMrRL9E8ctYp3NXtPK8VA4MpFnz WWM7qXQAHdBOuPRmixU4XuxHgG7/PAAzXH8Ac5dqFjnas+H8PEHqO90LPJm8tsFS 8dgb2ieQKlLl7yRLyRoQNvLy/B5EKUu+LKp6Yr3UI0oLm1iUnLdplEnKBbxpNHLS T+gQ7ScVCd/fFrx9oiF2tNurd6dhKTvAm8v7R9cfBAM/PjqZmoZGMAla8nvdw4XY g41Q1inBVn5w5/QbIzJCDZsWl9CHfoLMUHvGOKXV11NFbhjhY/9eDXwBzQmsKUXr 3dQYnzlCi3MxaZfsnDV9uKNSJ5sn84uBK4t9TanqGMsshen3CN5UU+bKPZKC3yty OQoxVTTAFnDlyJ+cQL77TmA/zkqDrL61qCrPBwZStX1f9lze1Ht7jcHwIthK3UzD wORqMKYFs/1DV5N7x7Un298qPGuCq9nPObl9JcCP5QWuhX6RTBn+g8ULYolQTSY0 xWeTauEupQWcZNDUldwK =D5d3 -----END PGP SIGNATURE-----
Best regards, Lubo