From mboxrd@z Thu Jan 1 00:00:00 1970 From: Greg Ungerer Date: Tue, 10 Jul 2012 22:19:43 +1000 Subject: [U-Boot] [PATCH v2] arm: cm4008, cm41xx: set gd->ram_size in dram_init In-Reply-To: <20120709235851.091000f4@lilith> References: <201207061015.12028.yann.vernier@orsoc.se> <4FF6A53C.8080309@gmail.com> <201207061114.59249.yann.vernier@orsoc.se> <4FF6BCFC.4030500@gmail.com> <20120709235851.091000f4@lilith> Message-ID: <4FFC1DDF.2060903@opengear.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 07/10/2012 07:58 AM, Albert ARIBAUD wrote: > Hi Andreas, > > On Fri, 06 Jul 2012 12:25:00 +0200, "Andreas Bie?mann" wrote: >> Dear Yann Vernier, >> >> On 06.07.2012 11:14, Yann Vernier wrote: >>> On Friday 06 July 2012 10:43:40 you wrote: >>>>>>> =((CONFIG_SYS_SDRAM_BASE+CONFIG_SYS_SDRAM_SIZE-0x10000)<<(22-16) | \ + >>>>>>> (CONFIG_SYS_SDRAM_BASE>>(16-12) | 0x00e)) >>>>>> >>>>>> ugh ... magic. Will it work for every possible setting? >>>>>> How about one setting CONFIG_SYS_SDRAM_BASE != 0x0? I the base address >>>>>> related to register content in any way? >>>>> >>>>> It's not a guarded secret, although it is far from obvious where to find >>>>> it. One way is micrel.com - Products - Ethernet ICs / ARM based SOC's - >>>>> HW Design Kit. Within that zip file, >>>>> KS8695X_EVAL_HW_RV4.0_DP/RegDescription/KS8695X Register Description >>>>> v1.1.pdf >>>> >>>> With this information I understand your equation. I think you should not >>>> do it that way. You may solve the current state (all devices have 8 >>>> column, 4 bank and 32 bit). But one adding (well, if that will ever come >>>> ;) another board with different setting will get in trouble here and >>>> need to find another solution. Maybe more sophisticated by doing another >>>> equation. >>>> >>>> I think a straight forward solution here would be to add another special >>>> define in the board config, write the magic number down there and maybe >>>> describe what the number stands for. You can then just use the >>>> previously defined value in start.S. >>>> You may have a look for at91 lowlevel_init, there it is done that way. >>> >>> I shall. In fact, I may already have this problem as I need to verify the >>> timing on the flash memory (I have the demo board, not a cm4xxx), and the >>> reason I work on this is that we may design another board soon. >>> >>> I'm a little hesitant about how to fit these changes together, though; for now >>> I have three barely separated patches, but overhauling all those magic numbers >>> changes them all yet again. These three patches are all necessary to make my >>> u-boot work in the first place, while fixing the magic values is a code cleanup >>> change - probably editing arch/arm/include/asm/arch-ks8695/platform.h which >>> currently contains a few meaningless constants like KS8695_SDRAM_START and a >>> consistent misspelling of definitions. >>> >>> Could I save the value decoding and corresponding configuration changes for a >>> fourth patch? >> >> I'm fine with this suggestion. >> So the next question is who would pull it in mainline? Since this is arm >> related I guess Albert is the one in question. > > Ah, board-related patches. :) > >> I think these three patches are all fixes to get a board already in >> mainline working. Fixes or improvements? > So I think we should try to get these in -rc1. >> Albert, can you please comment? > > If the cm4008/cm41xx board maintainer (as per MAINTAINERS, this is Greg Ungerer, > Cc:) green-lights it, then I'm ok for pulling it in. I have no problem with them. I have only done a visual inspection of the revised patches in the mail archives, and not tested them on the real hardware. I would suggest defining the proper meanings of the remaining bits in the SDRAM setup registers that are still magic numbers, but that can be a future change. Acked-by: Greg Ungerer greg.ungerer at opengear.com Regards Greg