Hello, On 4/7/20 2:28 PM, Giorgio wrote: >> Great. Even better than hardcoding the CLIENT_DOMAIN. >> > OK. > > To read the current value of DACR, in secure mode, we need a > get_domain(). I would add it to mmu.h, beside the set_domain(). sounds good. >>> What do you mean with the 'other i.MX7 patches' ? >> >> Didn't you add support for some i.MX7 spi flash image format? >> > > That was an evil hack actually, just to verify why the normal barebox image > didn't worked. To really support booting barebox from the qspi flash > I think we need more *structural* changes to the way barebox starts. > For this I need *at least* some suggestions from someone that really knows > in detail how barebox works and how an image is built, like you or Sascha... scripts/imx/imx-image.c is what's building the i.MX images. It receives the imxcfg specific to a board as argument. If you have extra configuration for the QSPI, you'll probably need to extend that, either with a new option or maybe by adding new directives to the existing imxcfg format. What other changes do you think will be necessary? > Maybe what can be committed is the dts block in imx7s.dtsi for the qspi that > enables using the flash as normal runtime mass storage. This should be not > so dangerous because the qspi becomes only active when its status is also enabled. Funny that no one so far has been bothered to add it upstream ^^. You can add it to barebox arch/arm/dts/imx7s.dtsi, but as you might want to access it from Linux as well, you should consider posting a patch to add the qspi node to the upstream imx7s.dtsi. Shortly after landing in a Linux -rc, Sascha will import it along with other changes to barebox dts/ and we can drop the node then from arch/arm/dts again. >>> I've also noticed that some asm() statements in sm.c lack the 'volatile' >>> attribute. >> >> Are they strictly necessary? My understanding is that we need >> asm volatile on no output operands, but side effects. > I think for the current code in sm.c we don't strictly need the volatiles, > I noticed problems in debug code like this: > Here the compiler could think he can call read_dacr() only once, > cache the result in a register and use its value two times. I verified > that for the previous debug code the volatile attribute makes a > difference. Interesting. As arm_smccc_smc expands to an external function and barebox does no Link-Time Optimization, I'd have though arm_smccc_smc to be a barrier and everything would be reloaded afterwards. That it's not, sounds quite broken.. Cheers Ahmad -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ barebox mailing list barebox@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/barebox