Stelian Pop a écrit :
> On Tue, Jun 23, 2009 at 12:46:50PM +0200, Sedji Gaouaou wrote:
> 
>> On the boards at91sam9260ek, at91sam9263ek and afed9260, the rstc register 
>> was
>> set to 0 after being set to 500 ms for the PHY reset.
>> Now if back up is enable it will be set to the saved value.
> 
> The changelog message is not very clear. What means "if backup is enabled" ?
> I don't see anything in the patch which can be enabled or disabled...
> 
To enable the back up you need to put a battery, so if there is no 
battery there is no back up...that is why I said if back up is enable :)
> I think you meant something like: "Do backup the old reset length and restore
> it after the MACB initialisation".
> 
But you comment is better, so I will change it.
>> +    rstc = at91_sys_read(AT91_RSTC_MR);
> [...]
>>      /* Restore NRST value */
>>      at91_sys_write(AT91_RSTC_MR, AT91_RSTC_KEY |
>> -                                 AT91_RSTC_ERSTL | (0x0 << 8) |
>> +                                 (rstc) |
>>                                   AT91_RSTC_URSTEN);
> 
> Also, I don't like this: you backup in 'rstc' the _whole_ contents
> of the MR register (including for example the URSTEN bit), but on
> restore you still construct a bit mask...
> 
> In order to be consistent, I would do either:
> 
> rstc = at91_sys_read(AT91_RSTC_MR);
> ...
> at91_sys_write(AT91_RSTC_MR, AT91_RSTC_KEY | rstc);
> 
> or
> 
> rstc = at91_sys_read(AT91_RSTC_MR) & AT91_RSTC_ERSTL;
> ...
> at91_sys_write(AT91_RSTC_MR, AT91_RSTC_KEY | rstc | AT91_RSTC_URSTEN);
> 
I will change that to then.

Regards,
Sedji


_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to