From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eugeniu Rosca Date: Tue, 12 Mar 2019 21:29:21 +0100 Subject: [U-Boot] [PATCH 3/4] ARM: renesas: Save boot parameters passed in by ATF In-Reply-To: <20190312194224.GR4690@bill-the-cat> References: <20190305032557.19788-1-marek.vasut+renesas@gmail.com> <20190305032557.19788-3-marek.vasut+renesas@gmail.com> <20190312194224.GR4690@bill-the-cat> Message-ID: <20190312202921.GA3755@vmlxhi-102.adit-jv.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Tom, On Tue, Mar 12, 2019 at 03:42:24PM -0400, Tom Rini wrote: > On Tue, Mar 12, 2019 at 07:59:40PM +0100, Eugeniu Rosca wrote: > > Hi Marek, > > > > On Tue, Mar 5, 2019 at 4:41 AM Marek Vasut wrote: > > [..] > > > +.align 8 > > > +.globl rcar_atf_boot_args > > > +rcar_atf_boot_args: > > > + .dword 0 > > > + .dword 0 > > > + .dword 0 > > > + .dword 0 > > > + > > > +ENTRY(save_boot_params) > > > + adr x8, rcar_atf_boot_args > > > + stp x0, x1, [x8], #16 > > > + stp x2, x3, [x8], #16 > > > + b save_boot_params_ret > > > +ENDPROC(save_boot_params) > > > > What about relocating the function to C like in [1] and passing the 4 > > arguments to it from ASM like in [2]? > > This would allow to: > > - reduce asm operations to minimum (mov and bl) and make code transparent. > > - avoid custom globals and store the ATF information in some C-defined struct. > > > > [1] https://github.com/ARM-software/arm-trusted-firmware/blob/c48d02bade88b07fa7f43aa44e5217f68e5d047f/bl1/bl1_main.c#L263 > > [2] https://github.com/ARM-software/arm-trusted-firmware/blob/c48d02bade88b07fa7f43aa44e5217f68e5d047f/bl1/aarch32/bl1_exceptions.S#L133 > > This is super early in the boot process and I'm really not a fan in > general of writing and calling C before we have things setup for C calls > to actually work normally, even more so for small things that can be > commented to be obvious as to what's going on. I don't have a strong preference about that. It is just my experience that Renesas once rewrote the elaborate Bosch-contributed BL2 routine of enabling the cntfrq_el0 system counter from ASM [1] to C [2]. This is at the BL1-BL2 boundary, while we are still in EL3, so much earlier in the boot process. This implementation currently runs on the targets of our customers. I am fine with any working solution. Thanks for commenting! [1] https://github.com/renesas-rcar/arm-trusted-firmware/commit/e23524689abb637b14a2961a305f6cfb43909319 [2] https://github.com/renesas-rcar/arm-trusted-firmware/commit/867d84191319#diff-471251fb86bd634ba14891d950c1440eR177 > -- > Tom Best regards, Eugeniu.