Re: [Qemu-devel] [PATCH for-2.5 v1 4/4] arm: xilinx_zynq: Add linux pre-boot

[Peter Crosthwaite]

On Tue, Oct 27, 2015 at 8:37 AM, Peter Maydell <address@hidden> wrote:

On 27 October 2015 at 15:21, Peter Crosthwaite

<address@hidden> wrote:
>
>
> On Tue, Oct 27, 2015 at 5:31 AM, Peter Maydell <address@hidden>
> wrote:
>>
>> On 25 October 2015 at 23:13, Peter Crosthwaite
>> <address@hidden> wrote:
>> > Add a Linux-specific pre-boot routine that matches the device
>> > specific bootloaders behaviour. This is needed for modern Linux that
>> > expects the ARM PLL in SLCR to be a more even value (not 26).
>> >
>> > Cc: Alistair Francis <address@hidden>
>> > Signed-off-by: Peter Crosthwaite <address@hidden>
>> > ---
>> > Changed since RFC:
>> > Use bootloader callback to load blob.
>> > Change "firmware" to "board-setup" for consistency.
>> >
>> >  hw/arm/xilinx_zynq.c | 40 +++++++++++++++++++++++++++++++++++++++-
>> >  1 file changed, 39 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
>> > index 9f89483..8c249e8 100644
>> > --- a/hw/arm/xilinx_zynq.c
>> > +++ b/hw/arm/xilinx_zynq.c
>> > @@ -43,6 +43,41 @@ static const int dma_irqs[8] = {
>> >      46, 47, 48, 49, 72, 73, 74, 75
>> >  };
>> >
>> > +#define BOARD_SETUP_ADDR        0x0
>> > +#define LOAD_ADDR               0x1000
>> > +
>> > +#define SLCR_LOCK_OFFSET        0x004
>> > +#define SLCR_UNLOCK_OFFSET      0x008
>> > +#define SLCR_ARM_PLL_OFFSET     0x100
>> > +
>> > +#define SLCR_XILINX_UNLOCK_KEY  0xdf0d
>> > +#define SLCR_XILINX_LOCK_KEY    0x767b
>> > +
>
>
> Something like:
>
> /* Write immediate val to address r0 + addr. r0 should contain base offset
> of SLCR block.
>  * Clobbers r1.
>  */
>
> #define SLCR_WRITE(addr, val) \
>         0xe3a01000 + extract32((val), 0, 8), /* mov r1, val & (FF << 0) */ \
>         0xe3811c00 + extract32((val), 8, 8), /* orr r1, r1, val & (FF << 8)
> */ \
>         0xe3811800 + extract32((val), 16, 8), /* orr r1, r1, val & (FF <<
> 16) */ \
>         0xe3811400 + extract32((val), 24, 8), /* orr r1, r1, val & (FF <<
> 24) */ \
>         0xe5801000 + (addr) /* str r0, r1, #addr */

This board is Cortex-A9, right? Why not use movw/movt and
save yourself two instructions?

Ok i'll use the two instruction form.
 

(in fact two out of three
of your constants are 16-bit and can be done with a single
movw insn.)

Id like to just be able to recycle this macro for all three writes so I don't see too much value in hand-optimising the 16-bit writes to save only two instructions. The full 32b flexibility makes the code easier to patch if the values have to change (or more-likely, add more writes).

Regards,

Peter

 

thanks
-- PMM