From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sedji Gaouaou Date: Wed, 24 Jun 2009 08:24:42 +0200 Subject: [U-Boot] [PATCH] at91sam9260/9263: add back up fot the rst(reset controller). In-Reply-To: <20090623203727.GA4904@galileo> References: <1245754010-26548-1-git-send-email-sedji.gaouaou@atmel.com> <20090623203727.GA4904@galileo> Message-ID: <4A41C6AA.3040509@atmel.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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