From mboxrd@z Thu Jan 1 00:00:00 1970 From: Albert ARIBAUD Date: Wed, 31 Jul 2013 10:16:36 +0200 Subject: [U-Boot] [PATCH v3 8/9] tegra: i2c: Enable new CONFIG_SYS_I2C framework In-Reply-To: <51F8BE73.2010101@denx.de> References: <1367668903-29653-1-git-send-email-hs@denx.de> <1367668903-29653-9-git-send-email-hs@denx.de> <51F6946F.8010500@wwwdotorg.org> <51F740FD.4080505@denx.de> <51F81261.7020008@wwwdotorg.org> <51F81B51.6010305@wwwdotorg.org> <51F83104.5090508@wwwdotorg.org> <51F83570.1010909@wwwdotorg.org> <20130731000921.724f5c71@lilith> <20130731051821.DDA1C380480@gemini.denx.de> <20130731090644.2b4ce215@lilith> <51F8BE73.2010101@denx.de> Message-ID: <20130731101636.059ad6de@lilith> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Heiko, On Wed, 31 Jul 2013 09:36:19 +0200, Heiko Schocher wrote: > > On the other hand, it may be hard to immediately know what functions > > throughout U-boot are safe to call from within board_init_f(); maybe we > > should start thinking about checking and marking these, the simplest > > way being to suffix them with "_f" once we have made sure they are safe > > to call from within board_init_f(). > > Hmmm... Maybe instead we should think (also in thinking common bring > up for all boards) about: > > getting rid of board_init_f in u-boot code, instead use for all > boards spl code to init needed things and copy and relocate u-boot > to ram in spl code ... so we have in u-boot no longer such > restictions ... but thats just an idea which whirs in my head ... > without thinking to deep in it. > > But this approach would have some advantages ... Well, the original SPL was basically board_init_f() plus some code to copy U-Boot from wherever it was to DDR, so it was tightly linked to board_init_f(). But... first, SPL has evolved into a "U-Boot lite" where much can happen beyond board_init_f() -- think Falcon mode, for instance -- and second, there are boards which do not have SPL at all, and their board_init_f() can thus not be "moved to SPL". So no, I don't think we can move U-Boot's design from "_f/_r" to "SPL/U-Boot". > > But we should strictly limit the scope of board_init_f() or we'll find > > the board_init_f()/board_init_r() pair following a patch similar to the > > SPL/U-Boot pair, where SPL started out as a tiny helper piece of code > > and ending up a resizeable (and, I dare to say, sizeable as well) kind > > of U-boot. If we let too many features slip in board_init_f(), it'll > > blur into a board_init_r() like and before we know it, it'll *require* > > DDR, and write access to it too... > > > > So, board_init_() should *strictly* be limited to setting up a console > > (for information purposes) and giving access to DDR while in the same > > time never writing to it itself. Bonus points if it can limit itself to > > *enabling* and postpone any *optimizing*(I am thinking of DDR settings > > here and no, I don't have specific existing cases in mind; just sayin'). > > > > In the present instance, I'd rather we either: > > > > - removed dependency on DT etc. by using "hard-coded" low level I2C > > reads for those boards that need it (I assume that for each of these > > boards the I2C slave, offset, and length to read are constant) in _f > > phase, or > > But DT is used for initializing the i2c driver in tegra ... Alright, out goes this proposal. Anyway, I didn't like it best. :) > > - parsed the _f phase looking for offending functions or calls which > > write to .data or .bss and fix them, suffixing them with _f; in > > essence, that amounts to starting the implementation of my suggestion > > above alongside fixing the issue at hand. > > > > The first approach is rather "let's bring the thing back up first", so > > it does not have my preference, but I would understand the need to > > quickly fix things. > > Yes. > > > The second approach seems to be going in the same direction as Heiko's > > proposal of 07:52 +0200, which I thus second provided it is applicable > > to all the boards Wolfgang had in mind. > > Lets do us this step as fixup ;-) Alright too. :) > bye, > Heiko Amicalement, -- Albert.