From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?ISO-8859-1?Q?Andreas_Bie=DFmann?= Date: Thu, 14 Nov 2013 08:49:22 +0100 Subject: [U-Boot] [PATCH v3 6/6] arm: atmel: sama5d3: spl boot from fat fs SD card In-Reply-To: <52847356.20607@gmail.com> References: <1383715757-20170-1-git-send-email-voice.shen@atmel.com> <1383715757-20170-7-git-send-email-voice.shen@atmel.com> <52837FEF.3070703@gmail.com> <5284651F.8060208@denx.de> <52846D72.3010108@googlemail.com> <52847356.20607@gmail.com> Message-ID: <52848082.2080506@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Bo, On 11/14/2013 07:53 AM, Bo Shen wrote: > On 11/14/2013 02:28 PM, Andreas Bie?mann wrote: >> On 14.11.13 06:52, Heiko Schocher wrote: >>> Am 13.11.2013 14:34, schrieb Andreas Bie?mann: >>>> Hi Bo, >>>> >>>> On 11/06/2013 06:29 AM, Bo Shen wrote: >>>>> Enable Atmel sama5d3xek boart spl boot support, which can load u-boot >>>>> from SD card with FAT file system. >>>>> >>>>> Signed-off-by: Bo Shen >>>>> >>>>> --- >> >> >> >>>>> +static void at91_disable_wdt(void) >>>> >>>> Why should we disable the WDT in SPL? I think it would be better to >>>> configure a working timer value than just disable it. >>> >>> This is currently done in the at91bootstrap code too... >> >> I know ... >>> >>>> Well it's easy and works, but for the future I think it would be >>>> good to >>>> let it run while in SPL and u-boot. >>> >>> We should have the option to enable/disable it ... > > The watchdog is one time configurable. If configurabled, can not change > anymore, so we disable it by default. But we should give an easy way to enable it. I swear industrial users require this, so we should at least add a possibility to enable it easily. Take it as a comment, no need to change it for that patch. >>>>> +void at91_mck_init(u32 mckr) >>>>> +{ >>>>> + struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC; >>>>> + u32 tmp; >>>>> + >>>>> + tmp = readl(&pmc->mckr); >>>>> + tmp&= ~(AT91_PMC_MCKR_PRES_MASK | >>>>> + AT91_PMC_MCKR_MDIV_MASK | >>>>> + AT91_PMC_MCKR_PLLADIV_2); >>>>> + tmp |= mckr& (AT91_PMC_MCKR_PRES_MASK | >>>>> + AT91_PMC_MCKR_MDIV_MASK | >>>>> + AT91_PMC_MCKR_PLLADIV_2); >>>> >>>> Why gets the at91_mck_init() just some parts of the MCK register (some >>>> fields are preserved here) while the at91_plla_init() just rewrites the >>>> PLLA register? >> >> Don't you see the error in my sentence here? ;) I thought some parts of >> the register content where preserved. The other way round is true, we >> need to clean up relevant parts (PRES, MDIV, and PLLADIV Bit(s)). Sorry, >> my fault here. > > So, keep it as is(?) For now yes ... we need to think about a better solution in the future. Best regards Andreas Bie?mann