From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Wed, 5 Dec 2018 08:55:19 -0700 Subject: [U-Boot] [PATCH 66/93] arm: Remove ot1200 board In-Reply-To: <62b6527a-3837-7e7e-a65f-0ef40ce74862@gmail.com> References: <20181119155413.158098-1-sjg@chromium.org> <20181119155413.158098-67-sjg@chromium.org> <20181122125249.GA11247@bill-the-cat> <20181122132839.GC11247@bill-the-cat> <20181122170123.GB11247@bill-the-cat> <62b6527a-3837-7e7e-a65f-0ef40ce74862@gmail.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Simon, On Wed, 5 Dec 2018 at 07:17, Simon Goldschmidt wrote: > > Am 05.12.2018 um 14:54 schrieb Simon Glass: > > Hi Simon, > > > > On Wed, 5 Dec 2018 at 06:38, Simon Goldschmidt > > wrote: > >> > >> On Wed, Dec 5, 2018 at 2:21 PM Simon Glass wrote: > >>> > >>> Hi Simon, > >>> > >>> On Sun, 25 Nov 2018 at 23:05, Simon Goldschmidt > >>> wrote: > >>>> > >>>> [I've cut down the CC list a bit due to some gmail warnings] > >>>> On Mon, Nov 26, 2018 at 4:00 AM Simon Glass wrote: > >>>>> > >>>>> Hi Simon, > >>>>> > >>>>> On Sun, 25 Nov 2018 at 14:09, Simon Goldschmidt > >>>>> wrote: > >>>>>> > >>>>>> On Thu, Nov 22, 2018 at 9:50 PM Simon Glass wrote: > >>>>>>> > >>>>>>> Hi, > >>>>>>> > >>>>>>> On Thu, 22 Nov 2018 at 10:02, Tom Rini wrote: > >>>>>>>> > >>>>>>>> On Thu, Nov 22, 2018 at 03:44:28PM +0100, Simon Goldschmidt wrote: > >>>>>>>>> Am Do., 22. Nov. 2018, 14:44 hat Tom Rini geschrieben: > >>>>>>>>> > >>>>>>>>>> On Thu, Nov 22, 2018 at 02:24:49PM +0100, Marek Vasut wrote: > >>>>>>>>>>> On 11/22/2018 01:52 PM, Tom Rini wrote: > >>>>>>>>>>>> On Thu, Nov 22, 2018 at 10:25:14AM +0100, Christian Gmeiner wrote: > >>>>>>>>>>>> > >>>>>>>>>>>>> Am Mo., 19. Nov. 2018 um 16:56 Uhr schrieb Simon Glass < > >>>>>>>>>> sjg at chromium.org>: > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> This board has not been converted to CONFIG_DM_BLK by the deadline. > >>>>>>>>>>>>>> Remove it. > >>>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> As the board is still mainted I will NAK it for the moment. Are there > >>>>>>>>>>>>> any hints want needs to be done > >>>>>>>>>>>>> to port thie board over to new DM stuff? > >>>>>>>>>>>> > >>>>>>>>>>>> Yes, as a start you need to switch over to using CONFIG_OF_CONTROL and > >>>>>>>>>>>> selecting/providing a dtb file. I see ot1200 is using DWC_AHSATA which > >>>>>>>>>>>> needs more work, but this is the board-level work that needs doing. > >>>>>>>>>>> > >>>>>>>>>>> Wasn't there a possibility to use platform data in board file instead of > >>>>>>>>>>> DT ? Or is DT mandatory now , including the libfdt overhead ? > >>>>>>>>>> > >>>>>>>>>> In short, DT for U-Boot and platform data for SPL is what's recommended, > >>>>>>>>>> yes. > >>>>>>>>>> > >>>>>>>>> > >>>>>>>>> This is a little confusing for me. Socfpga gen5 SPL doesn't do that. And it > >>>>>>>>> seems a little strange or outdated overall. > >>>>>>>>> > >>>>>>>>> Would there be some kind of reference architecture or mach to look at > >>>>>>>>> what's the suggested/up-to-date way to implement SPL? Also regarding code > >>>>>>>>> flow? > >>>>>>>> > >>>>>>>> So, SPL is where things get, ahem, fuzzy. While I don't want to > >>>>>>>> encourage boundless growth in U-Boot proper, we aren't exactly size > >>>>>>>> constrained (but rather, functional/logical constrained). But in SPL, > >>>>>>>> yes, we have many platforms with 32/64/128 kilobyte hard limits (and > >>>>>>>> some smaller) and we can't always shove in a "TPL" before SPL either. > >>>>>>>> So in SPL we do make use of platform data instead. While not the > >>>>>>>> smallest size constraint, am335x_hs_evm is a reasonable thing to look at > >>>>>>>> in this case. > >>>>>>> > >>>>>>> Also 'rock' uses CONFIG_OF_PLATDATA which provides a halfway house - > >>>>>>> still uses DT, but it gets converted into C structs so saves code > >>>>>>> space. > >>>>>>> > >>>>>>> firefly-rk3288 is a pretty good DM/DT example, including SPL. > >>>>>> > >>>>>> I've currently got an issue on socfpga gen5 that could be solved best > >>>>>> by enabling CONFIG_OF_EMBED (mixing const and non-const sections is a > >>>>>> problem for CRC calculation). However, it could probably also solve by > >>>>>> using platform data (but that doesn't work out of the box, yet). The > >>>>>> problem with CONFIG_OF_EMBED is that I think it's OK to enable this > >>>>>> for SPL but I don't like enabling it for U-Boot, so: > >>>>>> > >>>>>> Would it make sense to duplicate the whole "Provider of DTB for OF > >>>>>> control" choice so that it can be OF_EMBED for SPL but different for > >>>>>> U-Boot? Or does it make more sense to convert socfpga gen5 to use > >>>>>> OF_PLATDATA? > >>>>> > >>>>> We should not be using OF_EMBED in in-tree boards or production code. > >>>> > >>>> What's the reason for this? I can understand this for U-Boot, and I > >>>> can understand that it's at least theoretically a bit cleaner for SPL, > >>>> too. But there are some drawbacks when doing this in SPL where code is > >>>> not relocated: > >>>> - you lose the ability to check total size in linker file (which is > >>>> bad for size-constrained platforms: sometimes you notice failure only > >>>> when booting) > >>> > >>> You can add an SPL size check in Makefile.spl if you like. > >> > >> That might be required, yes. > >> > >>>> - you get an inconsistent memory layout regarding read/write: the > >>>> linker places bss at the end but then, DTB follows as const data > >>> > >>> This should be handled by the $(SPL-BIN)-pad.bin file (or by binman if > >>> you are using that). > >> > >> I don't understand that. How does the padding help? I have these > >> sections (roughly): > >> - text: readonly > >> - bss: writable > >> - DTB: readonly, added as post build step after linking > >> > >> How does $(SPL-BIN)-pad.bin help? > > > > It covers over the BSS section so that the image ends where the DTB > > starts, thus fixing the addressing issue you mentioned. It allows you > > to do this: > > > > cat u-boot-spl-nodtb.bin u-boot-spl.dtn >-u-boot-spl.bin > > > >> > >>>> - binary size "on disk" grows due to this inconsistent memory layout > >>>> (since the flat binary includes the DTB, it needs to include the > >>>> zeroed-out bss, too) > >>> > >>> Right, but this is a few bytes. Why does it matter? > >>> > >>>> - "spl/u-boot-spl.hex" created by the default Makefiles does not seem > >>>> to include the DTB > >>> > >>> That might just be a bug. > >> > >> It might, yes. The hex file is currently built from the elf file, so > >> there's no DTB in there. > > > > OK. Could be worth a patch. > > > >> > >>>> > >>>>> Can you please explain the issue a bit more? > >>>> > >>>> Of course: socfpga gen5 has a feature where the boot rom can jump to > >>>> SPL in SRAM on warm boot. To ensure SPL is still valid after a reboot, > >>>> the boot rom can check its consistency by calculating a CRC over one > >>>> specified range in SRAM. On first boot, SPL stores its start, length > >>>> and CRC value to special registers for the boot rom. Since the > >>>> contents of bss changes while SPL is running, bss cannot be included > >>>> in this CRC range. (Same goes for the '.data' region, but it's > >>>> possible to build SPL without actually using it.) > >>> > >>> How about calculating that checksum at build time instead? You could > >>> use binman to do that. > >>> > >>>> > >>>> So to ensure the DTB is untouched, I have to make sure it has a lower > >>>> address than the bss section. Using OF_EMBED does this for me. And I > >>>> expect using platform data would work too. Do you have another idea > >>>> how to achieve my goal of combining all write-only sections in SPL > >>>> into one block? > >>> > >>> Yes, do it at build time. Or calculate your CRC before you write any > >>> BSS variables. > >> > >> Creating the correct checksum is not the point. I can do that before using bss. > >> > >> The problem is that on the next boot, this checksum is not valid any > >> more because bss might have changed. > > > > Right, but just skip the BSS section when checksumming - i.e. checksum > > the code and then the DT but omit the BSS. > > Yeah, well, good idea but that's not possible. The boot rom checks this > checksum on warm restart and it can only check the CRC of one block. And > of course I canno change the boot rom. > > So the only option I have without using OF_EMBED is to drop the CRC of > the DT, which is not really an option... Really it sounds like you want to move the BSS after the DT. We don't have a way of doing that at present. But I suppose it would be easy enough to arrange, by putting a placeholder for the DT in the link script. We just need a variable which holds the DT size and then can use '. = . + DT_SIZE' to reserve the space. Regards, Simon > > Regards, > Simon > > >>>> Oh, and I currently count 109 defconfig files containing "OF_EMBED", > >>>> so I wasn't aware that this should not be used. Maybe these platforms > >>>> have similar reasons like I have and would enable OF_EMBED only for > >>>> SPL if they could. At least for socfpga_stratix10 that should work. > >>> > >>> That is very bad news. I'll see about adding a Makefile warning. > >> > >> OK. Looking forward to the discussion that starts then :-) > > > > Yes... > > > > Regards, > > Simon > > >