Re: imx7d dual core boot

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

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@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox