Re: [U-Boot] [v2 PATCH 10/12] arm, spl, at91: add at91sam9260 and at91sam9g45 spl support

2014-10-31 Thread Heiko Schocher 

Hello Bo,

Am 31.10.2014 02:50, schrieb Bo Shen:

Hi Heiko,

On 10/30/2014 04:15 PM, Heiko Schocher wrote:

diff --git a/arch/arm/cpu/at91-common/spl.c b/arch/arm/cpu/at91-common/spl.c
index 674a470..5c9a3ad 100644
--- a/arch/arm/cpu/at91-common/spl.c
+++ b/arch/arm/cpu/at91-common/spl.c


I am thinking, whether it be better to keep this file as two copy? This will 
remove #ifdef, although a little code duplication.

If this solution acceptable, some suggestion as following:
- for armv5 (arm926ejs, now at91 series), named it spl_at91.c,
- for armv7 (cortex-a5, now, sama5d3), named it spl_atmel.c?
(As for arm9 series, we have at91 prefix for SoC name, and for armv7 SoC, we 
don't have at91 prefix, and it now changed to Atmel Smart)


Ok, I look into this.

[...]

@@ -57,77 +91,134 @@ static void switch_to_main_crystal_osc(void)

[...]

- /* disable watchdog */
+void spl_board_init(void)
+{
+ struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC;
+
+ lowlevel_clock_init();
at91_disable_wdt();

- /* PMC configuration */
- at91_pmc_init();
+ /*
+ * At this stage the main oscillator is supposed to be enabled
+ * PCK = MCK = MOSC
+ */
+ writel(0x00, pmc-pllicpr);

- at91_clock_init(CONFIG_SYS_AT91_MAIN_CLOCK);
+ /* Configure PLLA = MOSC * (PLL_MULA + 1) / PLL_DIVA */
+ at91_plla_init(CONFIG_SYS_AT91_PLLA);

- timer_init();
+ /* PCK = PLLA = 2 * MCK */
+ at91_mck_init(CONFIG_SYS_MCKR);

- board_early_init_f();
+ /* Switch MCK on PLLA output */
+ at91_mck_init(CONFIG_SYS_MCKR_CSS);
+
+#if defined(CONFIG_SYS_AT91_PLLB)
+ /* Configure PLLB */
+ at91_pllb_init(CONFIG_SYS_AT91_PLLB);
+#endif
+
+ /* Enable External Reset */
+ enable_ext_reset();

+#if defined(CONFIG_ATMEL_MATRIX_INIT)
+ /* Initialize matrix */
+ matrix_init();
+#endif


Can this also be weak function? And put matrix_init() code to SoC/board related 
file.


Changed.

Thanks!

bye,
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [v2 PATCH 10/12] arm, spl, at91: add at91sam9260 and at91sam9g45 spl support

2014-10-31 Thread Wolfgang Denk 
Dear Bo Shen,

In message 5452ead4.7080...@atmel.com you wrote:
 
 I am thinking, whether it be better to keep this file as two copy? This 
 will remove #ifdef, although a little code duplication.

I agree that we should try and minimize #ifdef's, but code duplication
is even worse.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
The explanation requiring the fewest assumptions is the  most  likely
to be correct.-- William of Occam
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [v2 PATCH 10/12] arm, spl, at91: add at91sam9260 and at91sam9g45 spl support

2014-10-30 Thread Bo Shen 

Hi Heiko,

On 10/30/2014 04:15 PM, Heiko Schocher wrote:

diff --git a/arch/arm/include/asm/arch-at91/at91_pmc.h 
b/arch/arm/include/asm/arch-at91/at91_pmc.h
index 27331ff..5f64583 100644
--- a/arch/arm/include/asm/arch-at91/at91_pmc.h
+++ b/arch/arm/include/asm/arch-at91/at91_pmc.h
@@ -95,7 +95,7 @@ typedef struct at91_pmc {
  #define AT91_PMC_MCKR_CSS_MAIN0x0001
  #define AT91_PMC_MCKR_CSS_PLLA0x0002
  #define AT91_PMC_MCKR_CSS_PLLB0x0003
-#define AT91_PMC_MCKR_CSS_MASK 0x0003
+#define AT91_PMC_MCKR_CSS_MASK 0x0007


Where this come from, CSS only two bits.


  #ifdef CONFIG_SAMA5D3
  #define AT91_PMC_MCKR_PRES_1  0x
@@ -114,7 +114,7 @@ typedef struct at91_pmc {
  #define AT91_PMC_MCKR_PRES_16 0x0010
  #define AT91_PMC_MCKR_PRES_32 0x0014
  #define AT91_PMC_MCKR_PRES_64 0x0018
-#define AT91_PMC_MCKR_PRES_MASK0x001C
+#define AT91_PMC_MCKR_PRES_MASK0x003C


Ditto


  #endif


Best Regards,
Bo Shen
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [v2 PATCH 10/12] arm, spl, at91: add at91sam9260 and at91sam9g45 spl support

2014-10-30 Thread Heiko Schocher 

Hello Bo,

Am 30.10.2014 11:17, schrieb Bo Shen:

Hi Heiko,

On 10/30/2014 04:15 PM, Heiko Schocher wrote:

diff --git a/arch/arm/include/asm/arch-at91/at91_pmc.h 
b/arch/arm/include/asm/arch-at91/at91_pmc.h
index 27331ff..5f64583 100644
--- a/arch/arm/include/asm/arch-at91/at91_pmc.h
+++ b/arch/arm/include/asm/arch-at91/at91_pmc.h
@@ -95,7 +95,7 @@ typedef struct at91_pmc {
#define AT91_PMC_MCKR_CSS_MAIN 0x0001
#define AT91_PMC_MCKR_CSS_PLLA 0x0002
#define AT91_PMC_MCKR_CSS_PLLB 0x0003
-#define AT91_PMC_MCKR_CSS_MASK 0x0003
+#define AT91_PMC_MCKR_CSS_MASK 0x0007


Where this come from, CSS only two bits.


Good question ... looked again into the doc, it is only two bits.




#ifdef CONFIG_SAMA5D3
#define AT91_PMC_MCKR_PRES_1 0x
@@ -114,7 +114,7 @@ typedef struct at91_pmc {
#define AT91_PMC_MCKR_PRES_16 0x0010
#define AT91_PMC_MCKR_PRES_32 0x0014
#define AT91_PMC_MCKR_PRES_64 0x0018
-#define AT91_PMC_MCKR_PRES_MASK 0x001C
+#define AT91_PMC_MCKR_PRES_MASK 0x003C


Ditto


Hmm... no idea, why I changed this ... good catch!

Is the rest of the patch (and the patchserie OK) ?

Then I can send a v3 ...

bye,
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [v2 PATCH 10/12] arm, spl, at91: add at91sam9260 and at91sam9g45 spl support

2014-10-30 Thread Bo Shen 

Hi Heiko,

On 10/30/2014 04:15 PM, Heiko Schocher wrote:

diff --git a/arch/arm/cpu/at91-common/spl.c b/arch/arm/cpu/at91-common/spl.c
index 674a470..5c9a3ad 100644
--- a/arch/arm/cpu/at91-common/spl.c
+++ b/arch/arm/cpu/at91-common/spl.c


I am thinking, whether it be better to keep this file as two copy? This 
will remove #ifdef, although a little code duplication.


If this solution acceptable, some suggestion as following:
  - for armv5 (arm926ejs, now at91 series), named it spl_at91.c,
  - for armv7 (cortex-a5, now, sama5d3), named it spl_atmel.c?
(As for arm9 series, we have at91 prefix for SoC name, and for armv7 
SoC, we don't have at91 prefix, and it now changed to Atmel Smart)



@@ -8,11 +8,18 @@
  #include common.h
  #include asm/io.h
  #include asm/arch/at91_common.h
+#if !defined(CONFIG_SAMA5D3)
+#include asm/arch/at91sam9_matrix.h
+#endif
+#include asm/arch/at91_pit.h
  #include asm/arch/at91_pmc.h
+#include asm/arch/at91_rstc.h
  #include asm/arch/at91_wdt.h
  #include asm/arch/clk.h
  #include spl.h

+DECLARE_GLOBAL_DATA_PTR;
+
  static void at91_disable_wdt(void)
  {
struct at91_wdt *wdt = (struct at91_wdt *)ATMEL_BASE_WDT;
@@ -20,6 +27,33 @@ static void at91_disable_wdt(void)
writel(AT91_WDT_MR_WDDIS, wdt-mr);
  }

+u32 spl_boot_device(void)
+{
+#ifdef CONFIG_SYS_USE_MMC
+   return BOOT_DEVICE_MMC1;
+#elif CONFIG_SYS_USE_NANDFLASH
+   return BOOT_DEVICE_NAND;
+#elif CONFIG_SYS_USE_SERIALFLASH
+   return BOOT_DEVICE_SPI;
+#endif
+   return BOOT_DEVICE_NONE;
+}
+
+u32 spl_boot_mode(void)
+{
+   switch (spl_boot_device()) {
+#ifdef CONFIG_SYS_USE_MMC
+   case BOOT_DEVICE_MMC1:
+   return MMCSD_MODE_FS;
+   break;
+#endif
+   case BOOT_DEVICE_NONE:
+   default:
+   hang();
+   }
+}
+
+#if defined(CONFIG_SAMA5D3)
  static void switch_to_main_crystal_osc(void)
  {
struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC;
@@ -57,77 +91,134 @@ static void switch_to_main_crystal_osc(void)
writel(tmp, pmc-mor);
  }

-void at91_plla_init(u32 pllar)
+void s_init(void)
  {
-   struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC;
+   switch_to_main_crystal_osc();

-   writel(pllar, pmc-pllar);
-   while (!(readl(pmc-sr)  (AT91_PMC_LOCKA | AT91_PMC_MCKRDY)))
-   ;
-}
+   /* disable watchdog */
+   at91_disable_wdt();

-void at91_mck_init(u32 mckr)
-{
-   struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC;
-   u32 tmp;
+   /* PMC configuration */
+   at91_pmc_init();

-   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);
-   writel(tmp, pmc-mckr);
+   at91_clock_init(CONFIG_SYS_AT91_MAIN_CLOCK);

-   while (!(readl(pmc-sr)  AT91_PMC_MCKRDY))
-   ;
-}
+   timer_init();

+   board_early_init_f();

-u32 spl_boot_device(void)
+   preloader_console_init();
+
+   mem_init();
+}
+#else
+static void enable_ext_reset(void)
  {
-#ifdef CONFIG_SYS_USE_MMC
-   return BOOT_DEVICE_MMC1;
-#elif CONFIG_SYS_USE_NANDFLASH
-   return BOOT_DEVICE_NAND;
-#elif CONFIG_SYS_USE_SERIALFLASH
-   return BOOT_DEVICE_SPI;
-#endif
-   return BOOT_DEVICE_NONE;
+   struct at91_rstc *rstc = (struct at91_rstc *)ATMEL_BASE_RSTC;
+
+   writel(AT91_RSTC_KEY | AT91_RSTC_MR_URSTEN, rstc-mr);
  }

-u32 spl_boot_mode(void)
+#if defined(CONFIG_ATMEL_MATRIX_INIT)
+static void matrix_init(void)
  {
-   switch (spl_boot_device()) {
-#ifdef CONFIG_SYS_USE_MMC
-   case BOOT_DEVICE_MMC1:
-   return MMCSD_MODE_FS;
-   break;
+   struct at91_matrix *mat = (struct at91_matrix *)ATMEL_BASE_MATRIX;
+
+   writel((readl(mat-scfg[3])  (~AT91_MATRIX_SLOT_CYCLE))
+   | AT91_MATRIX_SLOT_CYCLE_(0x40),
+   mat-scfg[3]);
+}
  #endif
-   case BOOT_DEVICE_NONE:
-   default:
-   hang();
+
+void lowlevel_clock_init(void)
+{
+   struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC;
+
+   if (!(readl(pmc-sr)  AT91_PMC_MOSCS)) {
+   /* Enable Main Oscillator */
+   writel(AT91_PMC_MOSCS | (0x40  8), pmc-mor);
+
+   /* Wait until Main Oscillator is stable */
+   while (!(readl(pmc-sr)  AT91_PMC_MOSCS))
+   ;
}
+
+   /* After stabilization, switch to Main Oscillator */
+   if ((readl(pmc-mckr)  AT91_PMC_CSS) == AT91_PMC_CSS_SLOW) {
+   unsigned long tmp;
+
+   tmp = readl(pmc-mckr);
+   tmp = ~AT91_PMC_CSS;
+   tmp |= AT91_PMC_CSS_MAIN;
+   writel(tmp, pmc-mckr);
+   while (!(readl(pmc-sr)  AT91_PMC_MCKRDY))
+   ;
+

Re: [U-Boot] [v2 PATCH 10/12] arm, spl, at91: add at91sam9260 and at91sam9g45 spl support

2014-10-30 Thread Bo Shen 

Hi Heiko,

On 10/30/2014 07:41 PM, Heiko Schocher wrote:

Hello Bo,

Am 30.10.2014 11:17, schrieb Bo Shen:

Hi Heiko,

On 10/30/2014 04:15 PM, Heiko Schocher wrote:

diff --git a/arch/arm/include/asm/arch-at91/at91_pmc.h
b/arch/arm/include/asm/arch-at91/at91_pmc.h
index 27331ff..5f64583 100644
--- a/arch/arm/include/asm/arch-at91/at91_pmc.h
+++ b/arch/arm/include/asm/arch-at91/at91_pmc.h
@@ -95,7 +95,7 @@ typedef struct at91_pmc {
#define AT91_PMC_MCKR_CSS_MAIN 0x0001
#define AT91_PMC_MCKR_CSS_PLLA 0x0002
#define AT91_PMC_MCKR_CSS_PLLB 0x0003
-#define AT91_PMC_MCKR_CSS_MASK 0x0003
+#define AT91_PMC_MCKR_CSS_MASK 0x0007


Where this come from, CSS only two bits.


Good question ... looked again into the doc, it is only two bits.




#ifdef CONFIG_SAMA5D3
#define AT91_PMC_MCKR_PRES_1 0x
@@ -114,7 +114,7 @@ typedef struct at91_pmc {
#define AT91_PMC_MCKR_PRES_16 0x0010
#define AT91_PMC_MCKR_PRES_32 0x0014
#define AT91_PMC_MCKR_PRES_64 0x0018
-#define AT91_PMC_MCKR_PRES_MASK 0x001C
+#define AT91_PMC_MCKR_PRES_MASK 0x003C


Ditto


Hmm... no idea, why I changed this ... good catch!

Is the rest of the patch (and the patchserie OK) ?


Except one suggestion and nitpick sent just now, the rest patch seems good.
For this whole series:
Reviewed-by: Bo Shen voice.s...@atmel.com


Then I can send a v3 ...

bye,
Heiko


Best Regards,
Bo Shen
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot