Hi Sekhar, Thanks for the review. Please see my comments below...
/* Block 1 - UART phys and virt already configured locally */ [...]
+ /* Use davinci_uart_phys/virt if already configured */ +10: mrc p15, 0, \rx, c1, c0 + tst \rx, #1 @ MMU enabled? + ldreq \rx, =__virt_to_phys(davinci_uart_phys) + ldrne \rx, =davinci_uart_virt + ldr \rx, [\rx] + cmp \rx, #0 @ is port configured? + bne 99f @ already configured
/* Block 2 - local config missing, copy DAVINCI_UART_INFO */ [...]
+ /* Copy uart phys address from decompressor uart info */ mrc p15, 0, \rx, c1, c0 tst \rx, #1 @ MMU enabled? + ldreq \tmp, =__virt_to_phys(davinci_uart_phys) + ldrne \tmp, =davinci_uart_phys + ldreq \rx, =DAVINCI_UART_INFO + ldrne \rx, =__phys_to_virt(DAVINCI_UART_INFO) + ldr \rx, [\rx, #0] + str \rx, [\tmp]
/* Block 3 - local config missing, copy DAVINCI_UART_INFO + 4 */ [...]
+ mrc p15, 0, \rx, c1, c0 + tst \rx, #1 @ MMU enabled? + ldreq \tmp, =__virt_to_phys(davinci_uart_virt) + ldrne \tmp, =davinci_uart_virt + ldreq \rx, =DAVINCI_UART_INFO + ldrne \rx, =__phys_to_virt(DAVINCI_UART_INFO) + ldr \rx, [\rx, #4] + str \rx, [\tmp]You should be able to combine the code chunks into one by using ldr/str{cond} to configure either the virtual or physical base depending on whether MMU is enabled on not. No need to configure both at the same time.
The first time that the addruart macro is used, it copies the phys and virt base addresses from the DAVINCI_UART_INFO area. Since this operation could be pre-mmu-init, it will need to copy both the phys and virt addresses from DAVINCI_UART_INFO to davinci_uart_[virt/phys].
Subsequent runs of addruart directly load davinci_uart_[virt/phys] (Block 1 above) without added jugglery.
Also please consider using a temporary variable to store the result of the first mrc call.
Ouch. I wish we had a few more usable registers to play with here, because that would have helped consolidate blocks 2 and 3 (above) into a single piece of streamlined code.
Since the register allocation in early boot is pretty tight, I couldn't take the liberty of clobbering anything beyond the macro's parameter list. That said, if there is a register that we are free to clobber, I'd gladly use it to condense the code down a bit. Any thoughts?
Regards Cyril. _______________________________________________ Davinci-linux-open-source mailing list [email protected] http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
