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 <lpo...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
