[U-Boot] [RFC] powerpc/lib: unsafe register handling in wait_ticks

[Joakim Tjernlund]

> 
> Hi,

Hi Mats

I would appreciate if you CC me directly on stuff I have been involved in.
I don't read every mail on u-boot list(to many of them). It was just
plain luck I saw this one. 

> 
> If watchdog is enabled, the arch/powerpc/lib/ticks.S::wait_ticks() 
function
> calls the function specified by the WATCHDOG_RESET macro.
> The wait_ticks function depends on the registers r0, r6 and r7 being
> preserved however that is not guaranteed if the reset function is a
> C function.
> 
> The following patch changes to using r14+r15 instead of r6+r7 and
> saves all necessary registers on the stack.

This adds some extra churn to the code that my patch didn't do.
On the other hand your patch makes the function look more
like how gcc would have done so I am fine with that.
However, I am not sure r14 is a good fit, I cannot remember if there is 
some
special purpose for r14. Hopefully somebody on the list knows. 

> 
> As I'm quite fresh to PowerPC assembly language I would appreciate
> any feedback on the implementation.
> 
> On a side note, one could wonder why this function is not written in
> C language to start with.

History I guess, once upon a time this function didn't need(or could not 
use)
the stack. Now it would be quite possible to change it into C.

> 
> Best regards,
> Mats Kärrman
> 
> --- a/arch/powerpc/lib/ticks.S
> +++ b/arch/powerpc/lib/ticks.S
> @@ -50,19 +50,24 @@ wait_ticks:
>     stwu   r1, -16(r1)
>     mflr   r0      /* save link register */
>     stw   r0, 20(r1)   /* Use r0 or GDB will be unhappy */
> -   mr   r7, r3      /* save tick count */
> +   stw   r14, 12(r1)   /* save used registers */
> +   stw   r15, 8(r1)
> +   mr   r14, r3      /* save tick count */
>     bl   get_ticks   /* Get start time */
> 
>     /* Calculate end time */
> -   addc    r7, r4, r7   /* Compute end time lower */
> -   addze   r6, r3      /*     and end time upper */
> +   addc    r14, r4, r14   /* Compute end time lower */
> +   addze   r15, r3      /*     and end time upper */
> 
>     WATCHDOG_RESET      /* Trigger watchdog, if needed */
>  1:   bl   get_ticks   /* Get current time */
> -   subfc   r4, r4, r7   /* Subtract current time from end time */
> -   subfe.   r3, r3, r6
> +   subfc   r4, r4, r14   /* Subtract current time from end time */
> +   subfe.   r3, r3, r15
>     bge   1b      /* Loop until time expired */
> 
> -   mtlr   r0      /* restore link register */
> +   lwz     r15, 8(r1)   /* restore saved registers */
> +   lwz     r14, 12(r1)
> +   lwz     r0, 20(r1)
>     addi   r1,r1,16
> +   mtlr   r0
>     blr
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot