From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Goldschmidt Date: Wed, 5 Dec 2018 17:11:05 +0100 Subject: [U-Boot] [PATCH 66/93] arm: Remove ot1200 board In-Reply-To: References: <20181119155413.158098-1-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: <306081f6-75a1-db29-f9c6-08227e94c1db@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Am 05.12.2018 um 16:55 schrieb Simon Glass: > 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. That's a good idea. I might try that as soon as I find the time to continue working on that boot-crc issue. Thanks, Simon > > 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 >>> >>