* [U-Boot] [PATCH] ppc4xx: Read pipeline depth set to 4 for PPC440SP/SPE, PPC405EX, PPC460EX/GT/SX processors
@ 2008-08-13 4:43 fkan at amcc.com
2008-08-13 5:37 ` Wolfgang Denk
0 siblings, 1 reply; 17+ messages in thread
From: fkan at amcc.com @ 2008-08-13 4:43 UTC (permalink / raw)
To: u-boot
From: Prodyut Hazarika <phazarika@amcc.com>
Signed-off-by: Prodyut Hazarika <phazarika@amcc.com>
Acked-by: Feng Kan <fkan@amcc.com>
---
cpu/ppc4xx/44x_spd_ddr2.c | 27 ++++++++++++++++---
cpu/ppc4xx/cpu_init.c | 16 +++++++++++-
include/asm-ppc/ppc4xx-sdram.h | 48 +++++++++++++++++++++++-----------
include/ppc440.h | 47 ----------------------------------
include/ppc4xx.h | 55 ++++++++++++++++++++++++++++++++++++++++
5 files changed, 125 insertions(+), 68 deletions(-)
diff --git a/cpu/ppc4xx/44x_spd_ddr2.c b/cpu/ppc4xx/44x_spd_ddr2.c
index 1c36324..0431754 100644
--- a/cpu/ppc4xx/44x_spd_ddr2.c
+++ b/cpu/ppc4xx/44x_spd_ddr2.c
@@ -2172,6 +2172,11 @@ static void program_memory_queue(unsigned long *dimm_populated,
unsigned long i;
unsigned long bank_0_populated = 0;
phys_size_t total_size = 0;
+#if defined(CONFIG_440SP) || defined(CONFIG_440SPE) || \
+ defined(CONFIG_460EX) || defined(CONFIG_460GT) || \
+ defined(CONFIG_460SX)
+ unsigned long val;
+#endif
/*------------------------------------------------------------------
* Reset the rank_base_address.
@@ -2249,17 +2254,31 @@ static void program_memory_queue(unsigned long *dimm_populated,
}
}
-#if defined(CONFIG_460EX) || defined(CONFIG_460GT)
+#if defined(CONFIG_440SP) || defined(CONFIG_440SPE) || \
+ defined(CONFIG_460EX) || defined(CONFIG_460GT) || \
+ defined(CONFIG_460SX)
+
/*
- * Enable high bandwidth access on 460EX/GT.
- * This should/could probably be done on other
- * PPC's too, like 440SPe.
+ * Enable high bandwidth access
* This is currently not used, but with this setup
* it is possible to use it later on in e.g. the Linux
* EMAC driver for performance gain.
*/
mtdcr(SDRAM_PLBADDULL, 0x00000000); /* MQ0_BAUL */
mtdcr(SDRAM_PLBADDUHB, 0x00000008); /* MQ0_BAUH */
+
+ /*
+ * Set optimal value for Memory Queue HB/LL Configuration registers
+ */
+
+ val = (mfdcr(SDRAM_CONF1HB) | SDRAM_CONF1HB_AAFR | SDRAM_CONF1HB_RPEN | SDRAM_CONF1HB_RFTE);
+ mtdcr(SDRAM_CONF1HB, val);
+
+ val = (mfdcr(SDRAM_CONF1LL) | SDRAM_CONF1LL_AAFR | SDRAM_CONF1LL_RPEN | SDRAM_CONF1LL_RFTE);
+ mtdcr(SDRAM_CONF1LL, val);
+
+ val = (mfdcr(SDRAM_CONFPATHB) | SDRAM_CONFPATHB_TPEN);
+ mtdcr(SDRAM_CONFPATHB, val);
#endif
}
diff --git a/cpu/ppc4xx/cpu_init.c b/cpu/ppc4xx/cpu_init.c
index e2d0402..c7c429e 100644
--- a/cpu/ppc4xx/cpu_init.c
+++ b/cpu/ppc4xx/cpu_init.c
@@ -138,7 +138,9 @@ void reconfigure_pll(u32 new_cpu_freq)
void
cpu_init_f (void)
{
-#if defined(CONFIG_WATCHDOG) || defined(CONFIG_440GX) || defined(CONFIG_460EX)
+#if defined(CONFIG_WATCHDOG) || defined(CONFIG_440GX) || defined(CONFIG_460EX) || \
+ defined(CONFIG_440SP) || defined(CONFIG_440SPE) || defined(CONFIG_405EX) || \
+ defined(CONFIG_460GT) || defined(CONFIG_460SX)
u32 val;
#endif
@@ -301,6 +303,18 @@ cpu_init_f (void)
val |= 0x400;
mtsdr(SDR0_USB2HOST_CFG, val);
#endif /* CONFIG_460EX */
+
+#if defined(CONFIG_440SP) || defined(CONFIG_440SPE) || \
+ defined(CONFIG_460EX) || defined(CONFIG_460GT) || \
+ defined(CONFIG_405EX) || defined(CONFIG_460SX)
+ /*
+ * Set PLB4 arbiter (Segment 0 and 1) to 4 deep pipeline read
+ */
+ val = (mfdcr(plb0_acr) & ~plb0_acr_rdp_mask) | plb0_acr_rdp_4deep;
+ mtdcr(plb0_acr, val);
+ val = (mfdcr(plb1_acr) & ~plb1_acr_rdp_mask) | plb1_acr_rdp_4deep;
+ mtdcr(plb1_acr, val);
+#endif /* CONFIG_440SP/SPE || CONFIG_460EX/GT || CONFIG_405EX */
}
/*
diff --git a/include/asm-ppc/ppc4xx-sdram.h b/include/asm-ppc/ppc4xx-sdram.h
index df787b3..d2e3b42 100644
--- a/include/asm-ppc/ppc4xx-sdram.h
+++ b/include/asm-ppc/ppc4xx-sdram.h
@@ -259,23 +259,39 @@
/*
* Memory queue defines
*/
-#define SDRAMQ_DCR_BASE 0x040
-
-#define SDRAM_R0BAS (SDRAMQ_DCR_BASE+0x0) /* rank 0 base address & size */
-#define SDRAM_R1BAS (SDRAMQ_DCR_BASE+0x1) /* rank 1 base address & size */
-#define SDRAM_R2BAS (SDRAMQ_DCR_BASE+0x2) /* rank 2 base address & size */
-#define SDRAM_R3BAS (SDRAMQ_DCR_BASE+0x3) /* rank 3 base address & size */
-#define SDRAM_CONF1HB (SDRAMQ_DCR_BASE+0x5) /* configuration 1 HB */
-#define SDRAM_ERRSTATHB (SDRAMQ_DCR_BASE+0x7) /* error status HB */
-#define SDRAM_ERRADDUHB (SDRAMQ_DCR_BASE+0x8) /* error address upper 32 HB */
-#define SDRAM_ERRADDLHB (SDRAMQ_DCR_BASE+0x9) /* error address lower 32 HB */
-#define SDRAM_PLBADDULL (SDRAMQ_DCR_BASE+0xA) /* PLB base address upper 32 LL */
-#define SDRAM_CONF1LL (SDRAMQ_DCR_BASE+0xB) /* configuration 1 LL */
-#define SDRAM_ERRSTATLL (SDRAMQ_DCR_BASE+0xC) /* error status LL */
-#define SDRAM_ERRADDULL (SDRAMQ_DCR_BASE+0xD) /* error address upper 32 LL */
-#define SDRAM_ERRADDLLL (SDRAMQ_DCR_BASE+0xE) /* error address lower 32 LL */
-#define SDRAM_CONFPATHB (SDRAMQ_DCR_BASE+0xF) /* configuration between paths */
-#define SDRAM_PLBADDUHB (SDRAMQ_DCR_BASE+0x10) /* PLB base address upper 32 LL */
+#define SDRAMQ_DCR_BASE 0x040
+
+#define SDRAM_R0BAS (SDRAMQ_DCR_BASE+0x0) /* rank 0 base address & size */
+#define SDRAM_R1BAS (SDRAMQ_DCR_BASE+0x1) /* rank 1 base address & size */
+#define SDRAM_R2BAS (SDRAMQ_DCR_BASE+0x2) /* rank 2 base address & size */
+#define SDRAM_R3BAS (SDRAMQ_DCR_BASE+0x3) /* rank 3 base address & size */
+#define SDRAM_CONF1HB (SDRAMQ_DCR_BASE+0x5) /* configuration 1 HB */
+#define SDRAM_CONF1HB_AAFR 0x80000000 /* Address Ack on First Request - Bit 0 */
+#define SDRAM_CONF1HB_PRPD 0x00080000 /* PLB Read pipeline Disable - Bit 12 */
+#define SDRAM_CONF1HB_PWPD 0x00040000 /* PLB Write pipeline Disable - Bit 13 */
+#define SDRAM_CONF1HB_PRW 0x00020000 /* PLB Read Wait - Bit 14 */
+#define SDRAM_CONF1HB_RPEN 0x00000800 /* Read Passing Enable - Bit 20 */
+#define SDRAM_CONF1HB_RFTE 0x00000400 /* Read Flow Through Enable - Bit 21 */
+
+#define SDRAM_ERRSTATHB (SDRAMQ_DCR_BASE+0x7) /* error status HB */
+#define SDRAM_ERRADDUHB (SDRAMQ_DCR_BASE+0x8) /* error address upper 32 HB */
+#define SDRAM_ERRADDLHB (SDRAMQ_DCR_BASE+0x9) /* error address lower 32 HB */
+#define SDRAM_PLBADDULL (SDRAMQ_DCR_BASE+0xA) /* PLB base address upper 32 LL */
+#define SDRAM_CONF1LL (SDRAMQ_DCR_BASE+0xB) /* configuration 1 LL */
+#define SDRAM_CONF1LL_AAFR 0x80000000 /* Address Ack on First Request - Bit 0 */
+#define SDRAM_CONF1LL_PRPD 0x00080000 /* PLB Read pipeline Disable - Bit 12 */
+#define SDRAM_CONF1LL_PWPD 0x00040000 /* PLB Write pipeline Disable - Bit 13 */
+#define SDRAM_CONF1LL_PRW 0x00020000 /* PLB Read Wait - Bit 14 */
+#define SDRAM_CONF1LL_RPEN 0x00000800 /* Read Passing Enable - Bit 20 */
+#define SDRAM_CONF1LL_RFTE 0x00000400 /* Read Flow Through Enable - Bit 21 */
+
+#define SDRAM_ERRSTATLL (SDRAMQ_DCR_BASE+0xC) /* error status LL */
+#define SDRAM_ERRADDULL (SDRAMQ_DCR_BASE+0xD) /* error address upper 32 LL */
+#define SDRAM_ERRADDLLL (SDRAMQ_DCR_BASE+0xE) /* error address lower 32 LL */
+#define SDRAM_CONFPATHB (SDRAMQ_DCR_BASE+0xF) /* configuration between paths */
+#define SDRAM_CONFPATHB_TPEN 0x08000000 /* Transaction Passing Enable - Bit 4 */
+
+#define SDRAM_PLBADDUHB (SDRAMQ_DCR_BASE+0x10) /* PLB base address upper 32 LL */
#if !defined(CONFIG_405EX)
/*
diff --git a/include/ppc440.h b/include/ppc440.h
index 92db15f..3584fd2 100644
--- a/include/ppc440.h
+++ b/include/ppc440.h
@@ -341,53 +341,6 @@
#define PLB4_ACR_WRP (0x80000000 >> 7)
-/* Nebula PLB4 Arbiter - PowerPC440EP */
-#define PLB_ARBITER_BASE 0x80
-
-#define plb0_revid (PLB_ARBITER_BASE+ 0x00)
-#define plb0_acr (PLB_ARBITER_BASE+ 0x01)
-#define plb0_acr_ppm_mask 0xF0000000
-#define plb0_acr_ppm_fixed 0x00000000
-#define plb0_acr_ppm_fair 0xD0000000
-#define plb0_acr_hbu_mask 0x08000000
-#define plb0_acr_hbu_disabled 0x00000000
-#define plb0_acr_hbu_enabled 0x08000000
-#define plb0_acr_rdp_mask 0x06000000
-#define plb0_acr_rdp_disabled 0x00000000
-#define plb0_acr_rdp_2deep 0x02000000
-#define plb0_acr_rdp_3deep 0x04000000
-#define plb0_acr_rdp_4deep 0x06000000
-#define plb0_acr_wrp_mask 0x01000000
-#define plb0_acr_wrp_disabled 0x00000000
-#define plb0_acr_wrp_2deep 0x01000000
-
-#define plb0_besrl (PLB_ARBITER_BASE+ 0x02)
-#define plb0_besrh (PLB_ARBITER_BASE+ 0x03)
-#define plb0_bearl (PLB_ARBITER_BASE+ 0x04)
-#define plb0_bearh (PLB_ARBITER_BASE+ 0x05)
-#define plb0_ccr (PLB_ARBITER_BASE+ 0x08)
-
-#define plb1_acr (PLB_ARBITER_BASE+ 0x09)
-#define plb1_acr_ppm_mask 0xF0000000
-#define plb1_acr_ppm_fixed 0x00000000
-#define plb1_acr_ppm_fair 0xD0000000
-#define plb1_acr_hbu_mask 0x08000000
-#define plb1_acr_hbu_disabled 0x00000000
-#define plb1_acr_hbu_enabled 0x08000000
-#define plb1_acr_rdp_mask 0x06000000
-#define plb1_acr_rdp_disabled 0x00000000
-#define plb1_acr_rdp_2deep 0x02000000
-#define plb1_acr_rdp_3deep 0x04000000
-#define plb1_acr_rdp_4deep 0x06000000
-#define plb1_acr_wrp_mask 0x01000000
-#define plb1_acr_wrp_disabled 0x00000000
-#define plb1_acr_wrp_2deep 0x01000000
-
-#define plb1_besrl (PLB_ARBITER_BASE+ 0x0A)
-#define plb1_besrh (PLB_ARBITER_BASE+ 0x0B)
-#define plb1_bearl (PLB_ARBITER_BASE+ 0x0C)
-#define plb1_bearh (PLB_ARBITER_BASE+ 0x0D)
-
/* Pin Function Control Register 1 */
#define SDR0_PFC1 0x4101
#define SDR0_PFC1_U1ME_MASK 0x02000000 /* UART1 Mode Enable */
diff --git a/include/ppc4xx.h b/include/ppc4xx.h
index c71da60..154956e 100644
--- a/include/ppc4xx.h
+++ b/include/ppc4xx.h
@@ -46,6 +46,61 @@
#define CONFIG_SDRAM_PPC4xx_IBM_DDR2 /* IBM DDR(2) controller */
#endif
+/* PLB4 CrossBar Arbiter Core supported across PPC4xx families */
+#if defined(CONFIG_440EP) || defined(CONFIG_440EPX) || \
+ defined(CONFIG_440GR) || defined(CONFIG_440GRX) || \
+ defined(CONFIG_440SP) || defined(CONFIG_440SPE) || \
+ defined(CONFIG_460EX) || defined(CONFIG_460GT) || \
+ defined(CONFIG_460SX) || defined(CONFIG_405EX)
+
+#define PLB_ARBITER_BASE 0x80
+
+#define plb0_revid (PLB_ARBITER_BASE+ 0x00)
+#define plb0_acr (PLB_ARBITER_BASE+ 0x01)
+#define plb0_acr_ppm_mask 0xF0000000
+#define plb0_acr_ppm_fixed 0x00000000
+#define plb0_acr_ppm_fair 0xD0000000
+#define plb0_acr_hbu_mask 0x08000000
+#define plb0_acr_hbu_disabled 0x00000000
+#define plb0_acr_hbu_enabled 0x08000000
+#define plb0_acr_rdp_mask 0x06000000
+#define plb0_acr_rdp_disabled 0x00000000
+#define plb0_acr_rdp_2deep 0x02000000
+#define plb0_acr_rdp_3deep 0x04000000
+#define plb0_acr_rdp_4deep 0x06000000
+#define plb0_acr_wrp_mask 0x01000000
+#define plb0_acr_wrp_disabled 0x00000000
+#define plb0_acr_wrp_2deep 0x01000000
+
+#define plb0_besrl (PLB_ARBITER_BASE+ 0x02)
+#define plb0_besrh (PLB_ARBITER_BASE+ 0x03)
+#define plb0_bearl (PLB_ARBITER_BASE+ 0x04)
+#define plb0_bearh (PLB_ARBITER_BASE+ 0x05)
+#define plb0_ccr (PLB_ARBITER_BASE+ 0x08)
+
+#define plb1_acr (PLB_ARBITER_BASE+ 0x09)
+#define plb1_acr_ppm_mask 0xF0000000
+#define plb1_acr_ppm_fixed 0x00000000
+#define plb1_acr_ppm_fair 0xD0000000
+#define plb1_acr_hbu_mask 0x08000000
+#define plb1_acr_hbu_disabled 0x00000000
+#define plb1_acr_hbu_enabled 0x08000000
+#define plb1_acr_rdp_mask 0x06000000
+#define plb1_acr_rdp_disabled 0x00000000
+#define plb1_acr_rdp_2deep 0x02000000
+#define plb1_acr_rdp_3deep 0x04000000
+#define plb1_acr_rdp_4deep 0x06000000
+#define plb1_acr_wrp_mask 0x01000000
+#define plb1_acr_wrp_disabled 0x00000000
+#define plb1_acr_wrp_2deep 0x01000000
+
+#define plb1_besrl (PLB_ARBITER_BASE+ 0x0A)
+#define plb1_besrh (PLB_ARBITER_BASE+ 0x0B)
+#define plb1_bearl (PLB_ARBITER_BASE+ 0x0C)
+#define plb1_bearh (PLB_ARBITER_BASE+ 0x0D)
+
+#endif /* 440EP/EPX 440GR/GRX 440SP/SPE 460EX/GT/SX 405EX*/
+
#if defined(CONFIG_440)
/*
* Enable long long (%ll ...) printf format on 440 PPC's since most of
--
1.5.5
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH] ppc4xx: Read pipeline depth set to 4 for PPC440SP/SPE, PPC405EX, PPC460EX/GT/SX processors
2008-08-13 4:43 [U-Boot] [PATCH] ppc4xx: Read pipeline depth set to 4 for PPC440SP/SPE, PPC405EX, PPC460EX/GT/SX processors fkan at amcc.com
@ 2008-08-13 5:37 ` Wolfgang Denk
2008-08-13 6:26 ` Stefan Roese
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: Wolfgang Denk @ 2008-08-13 5:37 UTC (permalink / raw)
To: u-boot
Dear Feng Kan,
in message <1218602619-19293-1-git-send-email-fkan@amcc.com> you wrote:
> From: Prodyut Hazarika <phazarika@amcc.com>
>
> Signed-off-by: Prodyut Hazarika <phazarika@amcc.com>
> Acked-by: Feng Kan <fkan@amcc.com>
> ---
It would be nice if the commit messages contained at least a minimal
explanation of the reasons for the changes.
> diff --git a/include/asm-ppc/ppc4xx-sdram.h b/include/asm-ppc/ppc4xx-sdram.h
> index df787b3..d2e3b42 100644
> --- a/include/asm-ppc/ppc4xx-sdram.h
> +++ b/include/asm-ppc/ppc4xx-sdram.h
> @@ -259,23 +259,39 @@
> /*
> * Memory queue defines
> */
> -#define SDRAMQ_DCR_BASE 0x040
> -
> -#define SDRAM_R0BAS (SDRAMQ_DCR_BASE+0x0) /* rank 0 base address & size */
> -#define SDRAM_R1BAS (SDRAMQ_DCR_BASE+0x1) /* rank 1 base address & size */
> -#define SDRAM_R2BAS (SDRAMQ_DCR_BASE+0x2) /* rank 2 base address & size */
> -#define SDRAM_R3BAS (SDRAMQ_DCR_BASE+0x3) /* rank 3 base address & size */
> -#define SDRAM_CONF1HB (SDRAMQ_DCR_BASE+0x5) /* configuration 1 HB */
> -#define SDRAM_ERRSTATHB (SDRAMQ_DCR_BASE+0x7) /* error status HB */
> -#define SDRAM_ERRADDUHB (SDRAMQ_DCR_BASE+0x8) /* error address upper 32 HB */
> -#define SDRAM_ERRADDLHB (SDRAMQ_DCR_BASE+0x9) /* error address lower 32 HB */
> -#define SDRAM_PLBADDULL (SDRAMQ_DCR_BASE+0xA) /* PLB base address upper 32 LL */
> -#define SDRAM_CONF1LL (SDRAMQ_DCR_BASE+0xB) /* configuration 1 LL */
> -#define SDRAM_ERRSTATLL (SDRAMQ_DCR_BASE+0xC) /* error status LL */
> -#define SDRAM_ERRADDULL (SDRAMQ_DCR_BASE+0xD) /* error address upper 32 LL */
> -#define SDRAM_ERRADDLLL (SDRAMQ_DCR_BASE+0xE) /* error address lower 32 LL */
> -#define SDRAM_CONFPATHB (SDRAMQ_DCR_BASE+0xF) /* configuration between paths */
> -#define SDRAM_PLBADDUHB (SDRAMQ_DCR_BASE+0x10) /* PLB base address upper 32 LL */
> +#define SDRAMQ_DCR_BASE 0x040
> +
> +#define SDRAM_R0BAS (SDRAMQ_DCR_BASE+0x0) /* rank 0 base address & size */
> +#define SDRAM_R1BAS (SDRAMQ_DCR_BASE+0x1) /* rank 1 base address & size */
> +#define SDRAM_R2BAS (SDRAMQ_DCR_BASE+0x2) /* rank 2 base address & size */
> +#define SDRAM_R3BAS (SDRAMQ_DCR_BASE+0x3) /* rank 3 base address & size */
> +#define SDRAM_CONF1HB (SDRAMQ_DCR_BASE+0x5) /* configuration 1 HB */
> +#define SDRAM_CONF1HB_AAFR 0x80000000 /* Address Ack on First Request - Bit 0 */
> +#define SDRAM_CONF1HB_PRPD 0x00080000 /* PLB Read pipeline Disable - Bit 12 */
> +#define SDRAM_CONF1HB_PWPD 0x00040000 /* PLB Write pipeline Disable - Bit 13 */
> +#define SDRAM_CONF1HB_PRW 0x00020000 /* PLB Read Wait - Bit 14 */
> +#define SDRAM_CONF1HB_RPEN 0x00000800 /* Read Passing Enable - Bit 20 */
> +#define SDRAM_CONF1HB_RFTE 0x00000400 /* Read Flow Through Enable - Bit 21 */
> +
> +#define SDRAM_ERRSTATHB (SDRAMQ_DCR_BASE+0x7) /* error status HB */
> +#define SDRAM_ERRADDUHB (SDRAMQ_DCR_BASE+0x8) /* error address upper 32 HB */
> +#define SDRAM_ERRADDLHB (SDRAMQ_DCR_BASE+0x9) /* error address lower 32 HB */
> +#define SDRAM_PLBADDULL (SDRAMQ_DCR_BASE+0xA) /* PLB base address upper 32 LL */
> +#define SDRAM_CONF1LL (SDRAMQ_DCR_BASE+0xB) /* configuration 1 LL */
> +#define SDRAM_CONF1LL_AAFR 0x80000000 /* Address Ack on First Request - Bit 0 */
> +#define SDRAM_CONF1LL_PRPD 0x00080000 /* PLB Read pipeline Disable - Bit 12 */
> +#define SDRAM_CONF1LL_PWPD 0x00040000 /* PLB Write pipeline Disable - Bit 13 */
> +#define SDRAM_CONF1LL_PRW 0x00020000 /* PLB Read Wait - Bit 14 */
> +#define SDRAM_CONF1LL_RPEN 0x00000800 /* Read Passing Enable - Bit 20 */
> +#define SDRAM_CONF1LL_RFTE 0x00000400 /* Read Flow Through Enable - Bit 21 */
> +
> +#define SDRAM_ERRSTATLL (SDRAMQ_DCR_BASE+0xC) /* error status LL */
> +#define SDRAM_ERRADDULL (SDRAMQ_DCR_BASE+0xD) /* error address upper 32 LL */
> +#define SDRAM_ERRADDLLL (SDRAMQ_DCR_BASE+0xE) /* error address lower 32 LL */
> +#define SDRAM_CONFPATHB (SDRAMQ_DCR_BASE+0xF) /* configuration between paths */
> +#define SDRAM_CONFPATHB_TPEN 0x08000000 /* Transaction Passing Enable - Bit 4 */
> +
> +#define SDRAM_PLBADDUHB (SDRAMQ_DCR_BASE+0x10) /* PLB base address upper 32 LL */
This change seems to be completely unrelated to above described
changes, so if it was a valid modification, it would have to be split
off into a separate commit.
But then, you are changing good TAB chanracters that were used for
vertical alignment into spaces. This is incorrect - please read the
Coding Style requirements.
Please do not do this.
NAK.
> #if !defined(CONFIG_405EX)
> /*
> diff --git a/include/ppc440.h b/include/ppc440.h
> index 92db15f..3584fd2 100644
> --- a/include/ppc440.h
> +++ b/include/ppc440.h
> @@ -341,53 +341,6 @@
>
> #define PLB4_ACR_WRP (0x80000000 >> 7)
>
> -/* Nebula PLB4 Arbiter - PowerPC440EP */
> -#define PLB_ARBITER_BASE 0x80
> -
> -#define plb0_revid (PLB_ARBITER_BASE+ 0x00)
> -#define plb0_acr (PLB_ARBITER_BASE+ 0x01)
> -#define plb0_acr_ppm_mask 0xF0000000
> -#define plb0_acr_ppm_fixed 0x00000000
> -#define plb0_acr_ppm_fair 0xD0000000
> -#define plb0_acr_hbu_mask 0x08000000
> -#define plb0_acr_hbu_disabled 0x00000000
> -#define plb0_acr_hbu_enabled 0x08000000
> -#define plb0_acr_rdp_mask 0x06000000
> -#define plb0_acr_rdp_disabled 0x00000000
> -#define plb0_acr_rdp_2deep 0x02000000
> -#define plb0_acr_rdp_3deep 0x04000000
> -#define plb0_acr_rdp_4deep 0x06000000
> -#define plb0_acr_wrp_mask 0x01000000
> -#define plb0_acr_wrp_disabled 0x00000000
> -#define plb0_acr_wrp_2deep 0x01000000
> -
> -#define plb0_besrl (PLB_ARBITER_BASE+ 0x02)
> -#define plb0_besrh (PLB_ARBITER_BASE+ 0x03)
> -#define plb0_bearl (PLB_ARBITER_BASE+ 0x04)
> -#define plb0_bearh (PLB_ARBITER_BASE+ 0x05)
> -#define plb0_ccr (PLB_ARBITER_BASE+ 0x08)
> -
> -#define plb1_acr (PLB_ARBITER_BASE+ 0x09)
> -#define plb1_acr_ppm_mask 0xF0000000
> -#define plb1_acr_ppm_fixed 0x00000000
> -#define plb1_acr_ppm_fair 0xD0000000
> -#define plb1_acr_hbu_mask 0x08000000
> -#define plb1_acr_hbu_disabled 0x00000000
> -#define plb1_acr_hbu_enabled 0x08000000
> -#define plb1_acr_rdp_mask 0x06000000
> -#define plb1_acr_rdp_disabled 0x00000000
> -#define plb1_acr_rdp_2deep 0x02000000
> -#define plb1_acr_rdp_3deep 0x04000000
> -#define plb1_acr_rdp_4deep 0x06000000
> -#define plb1_acr_wrp_mask 0x01000000
> -#define plb1_acr_wrp_disabled 0x00000000
> -#define plb1_acr_wrp_2deep 0x01000000
> -
> -#define plb1_besrl (PLB_ARBITER_BASE+ 0x0A)
> -#define plb1_besrh (PLB_ARBITER_BASE+ 0x0B)
> -#define plb1_bearl (PLB_ARBITER_BASE+ 0x0C)
> -#define plb1_bearh (PLB_ARBITER_BASE+ 0x0D)
> -
> /* Pin Function Control Register 1 */
> #define SDR0_PFC1 0x4101
> #define SDR0_PFC1_U1ME_MASK 0x02000000 /* UART1 Mode Enable */
> diff --git a/include/ppc4xx.h b/include/ppc4xx.h
> index c71da60..154956e 100644
> --- a/include/ppc4xx.h
> +++ b/include/ppc4xx.h
> @@ -46,6 +46,61 @@
> #define CONFIG_SDRAM_PPC4xx_IBM_DDR2 /* IBM DDR(2) controller */
> #endif
>
> +/* PLB4 CrossBar Arbiter Core supported across PPC4xx families */
> +#if defined(CONFIG_440EP) || defined(CONFIG_440EPX) || \
> + defined(CONFIG_440GR) || defined(CONFIG_440GRX) || \
> + defined(CONFIG_440SP) || defined(CONFIG_440SPE) || \
> + defined(CONFIG_460EX) || defined(CONFIG_460GT) || \
> + defined(CONFIG_460SX) || defined(CONFIG_405EX)
> +
> +#define PLB_ARBITER_BASE 0x80
> +
> +#define plb0_revid (PLB_ARBITER_BASE+ 0x00)
> +#define plb0_acr (PLB_ARBITER_BASE+ 0x01)
> +#define plb0_acr_ppm_mask 0xF0000000
> +#define plb0_acr_ppm_fixed 0x00000000
> +#define plb0_acr_ppm_fair 0xD0000000
> +#define plb0_acr_hbu_mask 0x08000000
> +#define plb0_acr_hbu_disabled 0x00000000
> +#define plb0_acr_hbu_enabled 0x08000000
> +#define plb0_acr_rdp_mask 0x06000000
> +#define plb0_acr_rdp_disabled 0x00000000
> +#define plb0_acr_rdp_2deep 0x02000000
> +#define plb0_acr_rdp_3deep 0x04000000
> +#define plb0_acr_rdp_4deep 0x06000000
> +#define plb0_acr_wrp_mask 0x01000000
> +#define plb0_acr_wrp_disabled 0x00000000
> +#define plb0_acr_wrp_2deep 0x01000000
> +
> +#define plb0_besrl (PLB_ARBITER_BASE+ 0x02)
> +#define plb0_besrh (PLB_ARBITER_BASE+ 0x03)
> +#define plb0_bearl (PLB_ARBITER_BASE+ 0x04)
> +#define plb0_bearh (PLB_ARBITER_BASE+ 0x05)
> +#define plb0_ccr (PLB_ARBITER_BASE+ 0x08)
> +
> +#define plb1_acr (PLB_ARBITER_BASE+ 0x09)
> +#define plb1_acr_ppm_mask 0xF0000000
> +#define plb1_acr_ppm_fixed 0x00000000
> +#define plb1_acr_ppm_fair 0xD0000000
> +#define plb1_acr_hbu_mask 0x08000000
> +#define plb1_acr_hbu_disabled 0x00000000
> +#define plb1_acr_hbu_enabled 0x08000000
> +#define plb1_acr_rdp_mask 0x06000000
> +#define plb1_acr_rdp_disabled 0x00000000
> +#define plb1_acr_rdp_2deep 0x02000000
> +#define plb1_acr_rdp_3deep 0x04000000
> +#define plb1_acr_rdp_4deep 0x06000000
> +#define plb1_acr_wrp_mask 0x01000000
> +#define plb1_acr_wrp_disabled 0x00000000
> +#define plb1_acr_wrp_2deep 0x01000000
> +
> +#define plb1_besrl (PLB_ARBITER_BASE+ 0x0A)
> +#define plb1_besrh (PLB_ARBITER_BASE+ 0x0B)
> +#define plb1_bearl (PLB_ARBITER_BASE+ 0x0C)
> +#define plb1_bearh (PLB_ARBITER_BASE+ 0x0D)
> +
> +#endif /* 440EP/EPX 440GR/GRX 440SP/SPE 460EX/GT/SX 405EX*/
> +
Here you move 44x specific code from a 44x specific header file into a
4xx generic header file which requires you to add a 44x specific
#ifdef's.
That's a change to the worse. Please don't do that.
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: wd at denx.de
Man is the best computer we can put aboard a spacecraft ... and the
only one that can be mass produced with unskilled labor.
- Wernher von Braun
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH] ppc4xx: Read pipeline depth set to 4 for PPC440SP/SPE, PPC405EX, PPC460EX/GT/SX processors
2008-08-13 5:37 ` Wolfgang Denk
@ 2008-08-13 6:26 ` Stefan Roese
2008-08-13 6:44 ` Wolfgang Denk
2008-08-13 13:59 ` Prodyut Hazarika
2008-08-13 17:13 ` Scott Wood
2008-08-13 19:04 ` prodyut hazarika
2 siblings, 2 replies; 17+ messages in thread
From: Stefan Roese @ 2008-08-13 6:26 UTC (permalink / raw)
To: u-boot
On Wednesday 13 August 2008, Wolfgang Denk wrote:
> > From: Prodyut Hazarika <phazarika@amcc.com>
> >
> > Signed-off-by: Prodyut Hazarika <phazarika@amcc.com>
> > Acked-by: Feng Kan <fkan@amcc.com>
> > ---
>
> It would be nice if the commit messages contained at least a minimal
> explanation of the reasons for the changes.
Yes, the original comment from Prodyut is gone. Please add it again.
<snip>
> > */ diff --git a/include/ppc4xx.h b/include/ppc4xx.h
> > index c71da60..154956e 100644
> > --- a/include/ppc4xx.h
> > +++ b/include/ppc4xx.h
> > @@ -46,6 +46,61 @@
> > #define CONFIG_SDRAM_PPC4xx_IBM_DDR2 /* IBM DDR(2) controller */
> > #endif
> >
> > +/* PLB4 CrossBar Arbiter Core supported across PPC4xx families */
> > +#if defined(CONFIG_440EP) || defined(CONFIG_440EPX) || \
> > + defined(CONFIG_440GR) || defined(CONFIG_440GRX) || \
> > + defined(CONFIG_440SP) || defined(CONFIG_440SPE) || \
> > + defined(CONFIG_460EX) || defined(CONFIG_460GT) || \
> > + defined(CONFIG_460SX) || defined(CONFIG_405EX)
> > +
> > +#define PLB_ARBITER_BASE 0x80
> > +
> > +#define plb0_revid (PLB_ARBITER_BASE+ 0x00)
> > +#define plb0_acr (PLB_ARBITER_BASE+ 0x01)
> > +#define plb0_acr_ppm_mask 0xF0000000
> > +#define plb0_acr_ppm_fixed 0x00000000
> > +#define plb0_acr_ppm_fair 0xD0000000
> > +#define plb0_acr_hbu_mask 0x08000000
> > +#define plb0_acr_hbu_disabled 0x00000000
> > +#define plb0_acr_hbu_enabled 0x08000000
> > +#define plb0_acr_rdp_mask 0x06000000
> > +#define plb0_acr_rdp_disabled 0x00000000
> > +#define plb0_acr_rdp_2deep 0x02000000
> > +#define plb0_acr_rdp_3deep 0x04000000
> > +#define plb0_acr_rdp_4deep 0x06000000
> > +#define plb0_acr_wrp_mask 0x01000000
> > +#define plb0_acr_wrp_disabled 0x00000000
> > +#define plb0_acr_wrp_2deep 0x01000000
> > +
> > +#define plb0_besrl (PLB_ARBITER_BASE+ 0x02)
> > +#define plb0_besrh (PLB_ARBITER_BASE+ 0x03)
> > +#define plb0_bearl (PLB_ARBITER_BASE+ 0x04)
> > +#define plb0_bearh (PLB_ARBITER_BASE+ 0x05)
> > +#define plb0_ccr (PLB_ARBITER_BASE+ 0x08)
> > +
> > +#define plb1_acr (PLB_ARBITER_BASE+ 0x09)
> > +#define plb1_acr_ppm_mask 0xF0000000
> > +#define plb1_acr_ppm_fixed 0x00000000
> > +#define plb1_acr_ppm_fair 0xD0000000
> > +#define plb1_acr_hbu_mask 0x08000000
> > +#define plb1_acr_hbu_disabled 0x00000000
> > +#define plb1_acr_hbu_enabled 0x08000000
> > +#define plb1_acr_rdp_mask 0x06000000
> > +#define plb1_acr_rdp_disabled 0x00000000
> > +#define plb1_acr_rdp_2deep 0x02000000
> > +#define plb1_acr_rdp_3deep 0x04000000
> > +#define plb1_acr_rdp_4deep 0x06000000
> > +#define plb1_acr_wrp_mask 0x01000000
> > +#define plb1_acr_wrp_disabled 0x00000000
> > +#define plb1_acr_wrp_2deep 0x01000000
> > +
> > +#define plb1_besrl (PLB_ARBITER_BASE+ 0x0A)
> > +#define plb1_besrh (PLB_ARBITER_BASE+ 0x0B)
> > +#define plb1_bearl (PLB_ARBITER_BASE+ 0x0C)
> > +#define plb1_bearh (PLB_ARBITER_BASE+ 0x0D)
> > +
> > +#endif /* 440EP/EPX 440GR/GRX 440SP/SPE 460EX/GT/SX 405EX*/
> > +
>
> Here you move 44x specific code from a 44x specific header file into a
> 4xx generic header file which requires you to add a 44x specific
> #ifdef's.
These defines are also used on the 405EX (and possibly future 405 variants as
well). It makes sense to move them to ppc4xx.h so we don't have to duplicate
those defines in 2 headers. This has been a big problem in the past with the
ppc405.h and ppc440.h headers.
Best regards,
Stefan
=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
=====================================================================
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH] ppc4xx: Read pipeline depth set to 4 for PPC440SP/SPE, PPC405EX, PPC460EX/GT/SX processors
2008-08-13 6:26 ` Stefan Roese
@ 2008-08-13 6:44 ` Wolfgang Denk
2008-08-13 13:59 ` Prodyut Hazarika
1 sibling, 0 replies; 17+ messages in thread
From: Wolfgang Denk @ 2008-08-13 6:44 UTC (permalink / raw)
To: u-boot
Dear Stefan Roese,
In message <200808130826.57511.sr@denx.de> you wrote:
>
> > > +/* PLB4 CrossBar Arbiter Core supported across PPC4xx families */
> > > +#if defined(CONFIG_440EP) || defined(CONFIG_440EPX) || \
> > > + defined(CONFIG_440GR) || defined(CONFIG_440GRX) || \
> > > + defined(CONFIG_440SP) || defined(CONFIG_440SPE) || \
> > > + defined(CONFIG_460EX) || defined(CONFIG_460GT) || \
> > > + defined(CONFIG_460SX) || defined(CONFIG_405EX)
--------------------------------------------^^^^^^^^^^^^^
...
> > Here you move 44x specific code from a 44x specific header file into a
> > 4xx generic header file which requires you to add a 44x specific
> > #ifdef's.
>
> These defines are also used on the 405EX (and possibly future 405 variants as
> well). It makes sense to move them to ppc4xx.h so we don't have to duplicate
> those defines in 2 headers. This has been a big problem in the past with the
> ppc405.h and ppc440.h headers.
You are right, I missed that single 405 there.
May I please ask to sort the list, then?
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: wd at denx.de
Let's say the docs present a simplified view of reality... :-)
- Larry Wall in <6940@jpl-devvax.JPL.NASA.GOV>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH] ppc4xx: Read pipeline depth set to 4 for PPC440SP/SPE, PPC405EX, PPC460EX/GT/SX processors
2008-08-13 6:26 ` Stefan Roese
2008-08-13 6:44 ` Wolfgang Denk
@ 2008-08-13 13:59 ` Prodyut Hazarika
2008-08-13 14:24 ` Stefan Roese
1 sibling, 1 reply; 17+ messages in thread
From: Prodyut Hazarika @ 2008-08-13 13:59 UTC (permalink / raw)
To: u-boot
> >
> > It would be nice if the commit messages contained at least a minimal
> > explanation of the reasons for the changes.
>
>Yes, the original comment from Prodyut is gone. Please add it again.
The original comments got omitted by mistake. I will resubmit it again.
> > > +#define plb1_bearl (PLB_ARBITER_BASE+ 0x0C)
> > > +#define plb1_bearh (PLB_ARBITER_BASE+ 0x0D)
> > > +
> > > +#endif /* 440EP/EPX 440GR/GRX 440SP/SPE 460EX/GT/SX 405EX*/
> > > +
> >
> > Here you move 44x specific code from a 44x specific header file into a
> > 4xx generic header file which requires you to add a 44x specific
> > #ifdef's.
>
> These defines are also used on the 405EX (and possibly future 405 variants as
> well). It makes sense to move them to ppc4xx.h so we don't have to duplicate
> those defines in 2 headers. This has been a big problem in the past with the
> ppc405.h and ppc440.h headers.
I will move the PPC405EX to the beginning of the list and resubmit.
Best regards,
Prodyut Hazarika
=========================
Staff Software Engineer
AMCC
=========================
--------------------------------------------------------
CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is for the sole use of the intended recipient(s) and contains information that is confidential and proprietary to Applied Micro Circuits Corporation or its subsidiaries. It is to be used solely for the purpose of furthering the parties' business relationship. All unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply e-mail and destroy all copies of the original message.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH] ppc4xx: Read pipeline depth set to 4 for PPC440SP/SPE, PPC405EX, PPC460EX/GT/SX processors
2008-08-13 13:59 ` Prodyut Hazarika
@ 2008-08-13 14:24 ` Stefan Roese
0 siblings, 0 replies; 17+ messages in thread
From: Stefan Roese @ 2008-08-13 14:24 UTC (permalink / raw)
To: u-boot
On Wednesday 13 August 2008, Prodyut Hazarika wrote:
> > > Here you move 44x specific code from a 44x specific header file into a
> > > 4xx generic header file which requires you to add a 44x specific
> > > #ifdef's.
> >
> > These defines are also used on the 405EX (and possibly future 405
> > variants as well). It makes sense to move them to ppc4xx.h so we don't
> > have to duplicate those defines in 2 headers. This has been a big problem
> > in the past with the ppc405.h and ppc440.h headers.
>
> I will move the PPC405EX to the beginning of the list and resubmit.
Thanks. Please rebase your patch now against the master branch of my
u-boot-ppc4xx repository. I've merged the next branch in the master branch
already.
Best regards,
Stefan
=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
=====================================================================
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH] ppc4xx: Read pipeline depth set to 4 for PPC440SP/SPE, PPC405EX, PPC460EX/GT/SX processors
2008-08-13 5:37 ` Wolfgang Denk
2008-08-13 6:26 ` Stefan Roese
@ 2008-08-13 17:13 ` Scott Wood
2008-08-13 18:31 ` Wolfgang Denk
2008-08-13 19:04 ` prodyut hazarika
2 siblings, 1 reply; 17+ messages in thread
From: Scott Wood @ 2008-08-13 17:13 UTC (permalink / raw)
To: u-boot
On Wed, Aug 13, 2008 at 07:37:40AM +0200, Wolfgang Denk wrote:
> This change seems to be completely unrelated to above described
> changes, so if it was a valid modification, it would have to be split
> off into a separate commit.
>
> But then, you are changing good TAB chanracters that were used for
> vertical alignment into spaces. This is incorrect - please read the
> Coding Style requirements.
Where? I see no mention of alignment in the coding style documentation.
I know it's common practice, but could someone please explain *why* TABs
are mandated for alignment? It makes sense for indentation, as it allows
adjusting the tab size[1], requires fewer keystrokes to change
indentation level, and conveys meaning with respect to the structure of
the code.
The only thing that using TABs for vertical alignment gets you is some
slight compression of the source code when not compressed by other means,
an annoying 1/8 chance of the column shifting by 8 characters when the
code to the left is changed, the removal of flexibility with respect to
tab size, and the breakage of the alignment in patches when the tabs
start at the leftmost column (due to the prepended [+- ] characters).
-Scott
[1] Yes, I know that 8 characters is canonical -- but why go out of your
way to break other sizes? What "makes things easier to read" is a matter
of personal preference -- and if that preference can be accomodated
without hardcoding it into the source code, it should be, even if some
see it as a "heretic movement". :-)
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH] ppc4xx: Read pipeline depth set to 4 for PPC440SP/SPE, PPC405EX, PPC460EX/GT/SX processors
@ 2008-08-13 18:10 fkan at amcc.com
2008-08-13 20:10 ` Wolfgang Denk
0 siblings, 1 reply; 17+ messages in thread
From: fkan at amcc.com @ 2008-08-13 18:10 UTC (permalink / raw)
To: u-boot
From: Prodyut Hazarika <phazarika@amcc.com>
Moved PLB4 Arbiter register definitions to ppc4xx.h since it is shared across processors
Optimize Memory Queue settings for PPC440SP/SPE and PPC460EX/GT/SX processors
Signed-off-by: Prodyut Hazarika <phazarika@amcc.com>
Acked-by: Feng Kan <fkan@amcc.com>
---
cpu/ppc4xx/44x_spd_ddr2.c | 27 ++++++++++++++++---
cpu/ppc4xx/cpu_init.c | 16 ++++++++++-
include/asm-ppc/ppc4xx-sdram.h | 48 ++++++++++++++++++++++-----------
include/ppc440.h | 47 ---------------------------------
include/ppc4xx.h | 56 ++++++++++++++++++++++++++++++++++++++++
5 files changed, 126 insertions(+), 68 deletions(-)
diff --git a/cpu/ppc4xx/44x_spd_ddr2.c b/cpu/ppc4xx/44x_spd_ddr2.c
index 1c36324..0431754 100644
--- a/cpu/ppc4xx/44x_spd_ddr2.c
+++ b/cpu/ppc4xx/44x_spd_ddr2.c
@@ -2172,6 +2172,11 @@ static void program_memory_queue(unsigned long *dimm_populated,
unsigned long i;
unsigned long bank_0_populated = 0;
phys_size_t total_size = 0;
+#if defined(CONFIG_440SP) || defined(CONFIG_440SPE) || \
+ defined(CONFIG_460EX) || defined(CONFIG_460GT) || \
+ defined(CONFIG_460SX)
+ unsigned long val;
+#endif
/*------------------------------------------------------------------
* Reset the rank_base_address.
@@ -2249,17 +2254,31 @@ static void program_memory_queue(unsigned long *dimm_populated,
}
}
-#if defined(CONFIG_460EX) || defined(CONFIG_460GT)
+#if defined(CONFIG_440SP) || defined(CONFIG_440SPE) || \
+ defined(CONFIG_460EX) || defined(CONFIG_460GT) || \
+ defined(CONFIG_460SX)
+
/*
- * Enable high bandwidth access on 460EX/GT.
- * This should/could probably be done on other
- * PPC's too, like 440SPe.
+ * Enable high bandwidth access
* This is currently not used, but with this setup
* it is possible to use it later on in e.g. the Linux
* EMAC driver for performance gain.
*/
mtdcr(SDRAM_PLBADDULL, 0x00000000); /* MQ0_BAUL */
mtdcr(SDRAM_PLBADDUHB, 0x00000008); /* MQ0_BAUH */
+
+ /*
+ * Set optimal value for Memory Queue HB/LL Configuration registers
+ */
+
+ val = (mfdcr(SDRAM_CONF1HB) | SDRAM_CONF1HB_AAFR | SDRAM_CONF1HB_RPEN | SDRAM_CONF1HB_RFTE);
+ mtdcr(SDRAM_CONF1HB, val);
+
+ val = (mfdcr(SDRAM_CONF1LL) | SDRAM_CONF1LL_AAFR | SDRAM_CONF1LL_RPEN | SDRAM_CONF1LL_RFTE);
+ mtdcr(SDRAM_CONF1LL, val);
+
+ val = (mfdcr(SDRAM_CONFPATHB) | SDRAM_CONFPATHB_TPEN);
+ mtdcr(SDRAM_CONFPATHB, val);
#endif
}
diff --git a/cpu/ppc4xx/cpu_init.c b/cpu/ppc4xx/cpu_init.c
index e2d0402..c7c429e 100644
--- a/cpu/ppc4xx/cpu_init.c
+++ b/cpu/ppc4xx/cpu_init.c
@@ -138,7 +138,9 @@ void reconfigure_pll(u32 new_cpu_freq)
void
cpu_init_f (void)
{
-#if defined(CONFIG_WATCHDOG) || defined(CONFIG_440GX) || defined(CONFIG_460EX)
+#if defined(CONFIG_WATCHDOG) || defined(CONFIG_440GX) || defined(CONFIG_460EX) || \
+ defined(CONFIG_440SP) || defined(CONFIG_440SPE) || defined(CONFIG_405EX) || \
+ defined(CONFIG_460GT) || defined(CONFIG_460SX)
u32 val;
#endif
@@ -301,6 +303,18 @@ cpu_init_f (void)
val |= 0x400;
mtsdr(SDR0_USB2HOST_CFG, val);
#endif /* CONFIG_460EX */
+
+#if defined(CONFIG_440SP) || defined(CONFIG_440SPE) || \
+ defined(CONFIG_460EX) || defined(CONFIG_460GT) || \
+ defined(CONFIG_405EX) || defined(CONFIG_460SX)
+ /*
+ * Set PLB4 arbiter (Segment 0 and 1) to 4 deep pipeline read
+ */
+ val = (mfdcr(plb0_acr) & ~plb0_acr_rdp_mask) | plb0_acr_rdp_4deep;
+ mtdcr(plb0_acr, val);
+ val = (mfdcr(plb1_acr) & ~plb1_acr_rdp_mask) | plb1_acr_rdp_4deep;
+ mtdcr(plb1_acr, val);
+#endif /* CONFIG_440SP/SPE || CONFIG_460EX/GT || CONFIG_405EX */
}
/*
diff --git a/include/asm-ppc/ppc4xx-sdram.h b/include/asm-ppc/ppc4xx-sdram.h
index df787b3..d2e3b42 100644
--- a/include/asm-ppc/ppc4xx-sdram.h
+++ b/include/asm-ppc/ppc4xx-sdram.h
@@ -259,23 +259,39 @@
/*
* Memory queue defines
*/
-#define SDRAMQ_DCR_BASE 0x040
-
-#define SDRAM_R0BAS (SDRAMQ_DCR_BASE+0x0) /* rank 0 base address & size */
-#define SDRAM_R1BAS (SDRAMQ_DCR_BASE+0x1) /* rank 1 base address & size */
-#define SDRAM_R2BAS (SDRAMQ_DCR_BASE+0x2) /* rank 2 base address & size */
-#define SDRAM_R3BAS (SDRAMQ_DCR_BASE+0x3) /* rank 3 base address & size */
-#define SDRAM_CONF1HB (SDRAMQ_DCR_BASE+0x5) /* configuration 1 HB */
-#define SDRAM_ERRSTATHB (SDRAMQ_DCR_BASE+0x7) /* error status HB */
-#define SDRAM_ERRADDUHB (SDRAMQ_DCR_BASE+0x8) /* error address upper 32 HB */
-#define SDRAM_ERRADDLHB (SDRAMQ_DCR_BASE+0x9) /* error address lower 32 HB */
-#define SDRAM_PLBADDULL (SDRAMQ_DCR_BASE+0xA) /* PLB base address upper 32 LL */
-#define SDRAM_CONF1LL (SDRAMQ_DCR_BASE+0xB) /* configuration 1 LL */
-#define SDRAM_ERRSTATLL (SDRAMQ_DCR_BASE+0xC) /* error status LL */
-#define SDRAM_ERRADDULL (SDRAMQ_DCR_BASE+0xD) /* error address upper 32 LL */
-#define SDRAM_ERRADDLLL (SDRAMQ_DCR_BASE+0xE) /* error address lower 32 LL */
-#define SDRAM_CONFPATHB (SDRAMQ_DCR_BASE+0xF) /* configuration between paths */
-#define SDRAM_PLBADDUHB (SDRAMQ_DCR_BASE+0x10) /* PLB base address upper 32 LL */
+#define SDRAMQ_DCR_BASE 0x040
+
+#define SDRAM_R0BAS (SDRAMQ_DCR_BASE+0x0) /* rank 0 base address & size */
+#define SDRAM_R1BAS (SDRAMQ_DCR_BASE+0x1) /* rank 1 base address & size */
+#define SDRAM_R2BAS (SDRAMQ_DCR_BASE+0x2) /* rank 2 base address & size */
+#define SDRAM_R3BAS (SDRAMQ_DCR_BASE+0x3) /* rank 3 base address & size */
+#define SDRAM_CONF1HB (SDRAMQ_DCR_BASE+0x5) /* configuration 1 HB */
+#define SDRAM_CONF1HB_AAFR 0x80000000 /* Address Ack on First Request - Bit 0 */
+#define SDRAM_CONF1HB_PRPD 0x00080000 /* PLB Read pipeline Disable - Bit 12 */
+#define SDRAM_CONF1HB_PWPD 0x00040000 /* PLB Write pipeline Disable - Bit 13 */
+#define SDRAM_CONF1HB_PRW 0x00020000 /* PLB Read Wait - Bit 14 */
+#define SDRAM_CONF1HB_RPEN 0x00000800 /* Read Passing Enable - Bit 20 */
+#define SDRAM_CONF1HB_RFTE 0x00000400 /* Read Flow Through Enable - Bit 21 */
+
+#define SDRAM_ERRSTATHB (SDRAMQ_DCR_BASE+0x7) /* error status HB */
+#define SDRAM_ERRADDUHB (SDRAMQ_DCR_BASE+0x8) /* error address upper 32 HB */
+#define SDRAM_ERRADDLHB (SDRAMQ_DCR_BASE+0x9) /* error address lower 32 HB */
+#define SDRAM_PLBADDULL (SDRAMQ_DCR_BASE+0xA) /* PLB base address upper 32 LL */
+#define SDRAM_CONF1LL (SDRAMQ_DCR_BASE+0xB) /* configuration 1 LL */
+#define SDRAM_CONF1LL_AAFR 0x80000000 /* Address Ack on First Request - Bit 0 */
+#define SDRAM_CONF1LL_PRPD 0x00080000 /* PLB Read pipeline Disable - Bit 12 */
+#define SDRAM_CONF1LL_PWPD 0x00040000 /* PLB Write pipeline Disable - Bit 13 */
+#define SDRAM_CONF1LL_PRW 0x00020000 /* PLB Read Wait - Bit 14 */
+#define SDRAM_CONF1LL_RPEN 0x00000800 /* Read Passing Enable - Bit 20 */
+#define SDRAM_CONF1LL_RFTE 0x00000400 /* Read Flow Through Enable - Bit 21 */
+
+#define SDRAM_ERRSTATLL (SDRAMQ_DCR_BASE+0xC) /* error status LL */
+#define SDRAM_ERRADDULL (SDRAMQ_DCR_BASE+0xD) /* error address upper 32 LL */
+#define SDRAM_ERRADDLLL (SDRAMQ_DCR_BASE+0xE) /* error address lower 32 LL */
+#define SDRAM_CONFPATHB (SDRAMQ_DCR_BASE+0xF) /* configuration between paths */
+#define SDRAM_CONFPATHB_TPEN 0x08000000 /* Transaction Passing Enable - Bit 4 */
+
+#define SDRAM_PLBADDUHB (SDRAMQ_DCR_BASE+0x10) /* PLB base address upper 32 LL */
#if !defined(CONFIG_405EX)
/*
diff --git a/include/ppc440.h b/include/ppc440.h
index 92db15f..3584fd2 100644
--- a/include/ppc440.h
+++ b/include/ppc440.h
@@ -341,53 +341,6 @@
#define PLB4_ACR_WRP (0x80000000 >> 7)
-/* Nebula PLB4 Arbiter - PowerPC440EP */
-#define PLB_ARBITER_BASE 0x80
-
-#define plb0_revid (PLB_ARBITER_BASE+ 0x00)
-#define plb0_acr (PLB_ARBITER_BASE+ 0x01)
-#define plb0_acr_ppm_mask 0xF0000000
-#define plb0_acr_ppm_fixed 0x00000000
-#define plb0_acr_ppm_fair 0xD0000000
-#define plb0_acr_hbu_mask 0x08000000
-#define plb0_acr_hbu_disabled 0x00000000
-#define plb0_acr_hbu_enabled 0x08000000
-#define plb0_acr_rdp_mask 0x06000000
-#define plb0_acr_rdp_disabled 0x00000000
-#define plb0_acr_rdp_2deep 0x02000000
-#define plb0_acr_rdp_3deep 0x04000000
-#define plb0_acr_rdp_4deep 0x06000000
-#define plb0_acr_wrp_mask 0x01000000
-#define plb0_acr_wrp_disabled 0x00000000
-#define plb0_acr_wrp_2deep 0x01000000
-
-#define plb0_besrl (PLB_ARBITER_BASE+ 0x02)
-#define plb0_besrh (PLB_ARBITER_BASE+ 0x03)
-#define plb0_bearl (PLB_ARBITER_BASE+ 0x04)
-#define plb0_bearh (PLB_ARBITER_BASE+ 0x05)
-#define plb0_ccr (PLB_ARBITER_BASE+ 0x08)
-
-#define plb1_acr (PLB_ARBITER_BASE+ 0x09)
-#define plb1_acr_ppm_mask 0xF0000000
-#define plb1_acr_ppm_fixed 0x00000000
-#define plb1_acr_ppm_fair 0xD0000000
-#define plb1_acr_hbu_mask 0x08000000
-#define plb1_acr_hbu_disabled 0x00000000
-#define plb1_acr_hbu_enabled 0x08000000
-#define plb1_acr_rdp_mask 0x06000000
-#define plb1_acr_rdp_disabled 0x00000000
-#define plb1_acr_rdp_2deep 0x02000000
-#define plb1_acr_rdp_3deep 0x04000000
-#define plb1_acr_rdp_4deep 0x06000000
-#define plb1_acr_wrp_mask 0x01000000
-#define plb1_acr_wrp_disabled 0x00000000
-#define plb1_acr_wrp_2deep 0x01000000
-
-#define plb1_besrl (PLB_ARBITER_BASE+ 0x0A)
-#define plb1_besrh (PLB_ARBITER_BASE+ 0x0B)
-#define plb1_bearl (PLB_ARBITER_BASE+ 0x0C)
-#define plb1_bearh (PLB_ARBITER_BASE+ 0x0D)
-
/* Pin Function Control Register 1 */
#define SDR0_PFC1 0x4101
#define SDR0_PFC1_U1ME_MASK 0x02000000 /* UART1 Mode Enable */
diff --git a/include/ppc4xx.h b/include/ppc4xx.h
index c71da60..da7098f 100644
--- a/include/ppc4xx.h
+++ b/include/ppc4xx.h
@@ -46,6 +46,62 @@
#define CONFIG_SDRAM_PPC4xx_IBM_DDR2 /* IBM DDR(2) controller */
#endif
+/* PLB4 CrossBar Arbiter Core supported across PPC4xx families */
+#if defined(CONFIG_405EX) || \
+ defined(CONFIG_440EP) || defined(CONFIG_440EPX) || \
+ defined(CONFIG_440GR) || defined(CONFIG_440GRX) || \
+ defined(CONFIG_440SP) || defined(CONFIG_440SPE) || \
+ defined(CONFIG_460EX) || defined(CONFIG_460GT) || \
+ defined(CONFIG_460SX)
+
+#define PLB_ARBITER_BASE 0x80
+
+#define plb0_revid (PLB_ARBITER_BASE+ 0x00)
+#define plb0_acr (PLB_ARBITER_BASE+ 0x01)
+#define plb0_acr_ppm_mask 0xF0000000
+#define plb0_acr_ppm_fixed 0x00000000
+#define plb0_acr_ppm_fair 0xD0000000
+#define plb0_acr_hbu_mask 0x08000000
+#define plb0_acr_hbu_disabled 0x00000000
+#define plb0_acr_hbu_enabled 0x08000000
+#define plb0_acr_rdp_mask 0x06000000
+#define plb0_acr_rdp_disabled 0x00000000
+#define plb0_acr_rdp_2deep 0x02000000
+#define plb0_acr_rdp_3deep 0x04000000
+#define plb0_acr_rdp_4deep 0x06000000
+#define plb0_acr_wrp_mask 0x01000000
+#define plb0_acr_wrp_disabled 0x00000000
+#define plb0_acr_wrp_2deep 0x01000000
+
+#define plb0_besrl (PLB_ARBITER_BASE+ 0x02)
+#define plb0_besrh (PLB_ARBITER_BASE+ 0x03)
+#define plb0_bearl (PLB_ARBITER_BASE+ 0x04)
+#define plb0_bearh (PLB_ARBITER_BASE+ 0x05)
+#define plb0_ccr (PLB_ARBITER_BASE+ 0x08)
+
+#define plb1_acr (PLB_ARBITER_BASE+ 0x09)
+#define plb1_acr_ppm_mask 0xF0000000
+#define plb1_acr_ppm_fixed 0x00000000
+#define plb1_acr_ppm_fair 0xD0000000
+#define plb1_acr_hbu_mask 0x08000000
+#define plb1_acr_hbu_disabled 0x00000000
+#define plb1_acr_hbu_enabled 0x08000000
+#define plb1_acr_rdp_mask 0x06000000
+#define plb1_acr_rdp_disabled 0x00000000
+#define plb1_acr_rdp_2deep 0x02000000
+#define plb1_acr_rdp_3deep 0x04000000
+#define plb1_acr_rdp_4deep 0x06000000
+#define plb1_acr_wrp_mask 0x01000000
+#define plb1_acr_wrp_disabled 0x00000000
+#define plb1_acr_wrp_2deep 0x01000000
+
+#define plb1_besrl (PLB_ARBITER_BASE+ 0x0A)
+#define plb1_besrh (PLB_ARBITER_BASE+ 0x0B)
+#define plb1_bearl (PLB_ARBITER_BASE+ 0x0C)
+#define plb1_bearh (PLB_ARBITER_BASE+ 0x0D)
+
+#endif /* 440EP/EPX 440GR/GRX 440SP/SPE 460EX/GT/SX 405EX*/
+
#if defined(CONFIG_440)
/*
* Enable long long (%ll ...) printf format on 440 PPC's since most of
--
1.5.5
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH] ppc4xx: Read pipeline depth set to 4 for PPC440SP/SPE, PPC405EX, PPC460EX/GT/SX processors
2008-08-13 17:13 ` Scott Wood
@ 2008-08-13 18:31 ` Wolfgang Denk
2008-08-13 18:44 ` Scott Wood
0 siblings, 1 reply; 17+ messages in thread
From: Wolfgang Denk @ 2008-08-13 18:31 UTC (permalink / raw)
To: u-boot
Dear Scott Wood,
In message <20080813171332.GC24225@ld0162-tx32.am.freescale.net> you wrote:
> >
> > But then, you are changing good TAB chanracters that were used for
> > vertical alignment into spaces. This is incorrect - please read the
> > Coding Style requirements.
>
> Where? I see no mention of alignment in the coding style documentation.
See http://www.denx.de/wiki/U-Boot/CodingStyle, bullet 4.2:
Use TAB characters for indentation and vertical alignment, not
spaces
> I know it's common practice, but could someone please explain *why* TABs
> are mandated for alignment? It makes sense for indentation, as it allows
> adjusting the tab size[1], requires fewer keystrokes to change
> indentation level, and conveys meaning with respect to the structure of
> the code.
The very same reasons apply for vertical alignment.
> The only thing that using TABs for vertical alignment gets you is some
> slight compression of the source code when not compressed by other means,
> an annoying 1/8 chance of the column shifting by 8 characters when the
> code to the left is changed, the removal of flexibility with respect to
That's 8 times better than the 100% chance that it will shift when
using spaces for alignment.
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: wd at denx.de
To live is always desirable.
-- Eleen the Capellan, "Friday's Child", stardate 3498.9
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH] ppc4xx: Read pipeline depth set to 4 for PPC440SP/SPE, PPC405EX, PPC460EX/GT/SX processors
2008-08-13 18:31 ` Wolfgang Denk
@ 2008-08-13 18:44 ` Scott Wood
0 siblings, 0 replies; 17+ messages in thread
From: Scott Wood @ 2008-08-13 18:44 UTC (permalink / raw)
To: u-boot
Wolfgang Denk wrote:
> In message <20080813171332.GC24225@ld0162-tx32.am.freescale.net> you wrote:
>>> But then, you are changing good TAB chanracters that were used for
>>> vertical alignment into spaces. This is incorrect - please read the
>>> Coding Style requirements.
>> Where? I see no mention of alignment in the coding style documentation.
>
> See http://www.denx.de/wiki/U-Boot/CodingStyle, bullet 4.2:
>
> Use TAB characters for indentation and vertical alignment, not
> spaces
OK, I was looking at the Coding Standards section of the README.
>> I know it's common practice, but could someone please explain *why* TABs
>> are mandated for alignment? It makes sense for indentation, as it allows
>> adjusting the tab size[1], requires fewer keystrokes to change
>> indentation level, and conveys meaning with respect to the structure of
>> the code.
>
> The very same reasons apply for vertical alignment.
They do not. It specifically *disallows* changing the tab size, and has
no correlation to code structure.
>> The only thing that using TABs for vertical alignment gets you is some
>> slight compression of the source code when not compressed by other means,
>> an annoying 1/8 chance of the column shifting by 8 characters when the
>> code to the left is changed, the removal of flexibility with respect to
>
> That's 8 times better than the 100% chance that it will shift when
> using spaces for alignment.
I disagree. It's much easier to edit when I know how many spaces I'll
have to add/remove beforehand.
-Scott
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH] ppc4xx: Read pipeline depth set to 4 for PPC440SP/SPE, PPC405EX, PPC460EX/GT/SX processors
2008-08-13 5:37 ` Wolfgang Denk
2008-08-13 6:26 ` Stefan Roese
2008-08-13 17:13 ` Scott Wood
@ 2008-08-13 19:04 ` prodyut hazarika
2008-08-13 19:16 ` Prodyut Hazarika
2008-08-13 20:01 ` Wolfgang Denk
2 siblings, 2 replies; 17+ messages in thread
From: prodyut hazarika @ 2008-08-13 19:04 UTC (permalink / raw)
To: u-boot
Dear Wolfgang,
Please see comments below.
> > But then, you are changing good TAB chanracters that were used for
> > vertical alignment into spaces. This is incorrect - please read the
> > Coding Style requirements.
> >
> > Please do not do this.
The problem is that lot of existing code use spaces to align the
defines. You can see include/ppc4xx.h and lot of other header files. I
have seen that spaces are used only in defines. As an example in
ppc4xx.h. I can send thousands of other places.:
#if defined(CONFIG_405EX) || \$
defined(CONFIG_440SP) || defined(CONFIG_440SPE) || \$
defined(CONFIG_460EX) || defined(CONFIG_460GT)$
#define CONFIG_SDRAM_PPC4xx_IBM_DDR2^I/* IBM DDR(2) controller */$
#endif$
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH] ppc4xx: Read pipeline depth set to 4 for PPC440SP/SPE, PPC405EX, PPC460EX/GT/SX processors
2008-08-13 19:04 ` prodyut hazarika
@ 2008-08-13 19:16 ` Prodyut Hazarika
2008-08-13 20:08 ` Wolfgang Denk
2008-08-13 20:01 ` Wolfgang Denk
1 sibling, 1 reply; 17+ messages in thread
From: Prodyut Hazarika @ 2008-08-13 19:16 UTC (permalink / raw)
To: u-boot
Dear Wolfgang,
One more comment.
> > But then, you are changing good TAB chanracters that were used
> > for vertical alignment into spaces. This is incorrect - please
> > read the Coding Style requirements.
> >
> > Please do not do this.
I basically moved the defines from include/ppc440.h to include/ppc4xx.h.
If you go to include/ppc440.h, you will see that the original define was aligned
With spaces. I just followed whatever was it.
Here is the original code in ppc440.h
#if defined(CONFIG_440EP) || defined (CONFIG_440GR) || \
^^ space here ^^ space here
defined(CONFIG_440EPX) || defined(CONFIG_440GR)
^^^^^space here
Please advice what is the best way to proceed. As you can see, this is certainly not my invention.
Regards,
Prodyut Hazarika
=====================
Staff S/w Engineer
=====================
--------------------------------------------------------
CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is for the sole use of the intended recipient(s) and contains information that is confidential and proprietary to Applied Micro Circuits Corporation or its subsidiaries. It is to be used solely for the purpose of furthering the parties' business relationship. All unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply e-mail and destroy all copies of the original message.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH] ppc4xx: Read pipeline depth set to 4 for PPC440SP/SPE, PPC405EX, PPC460EX/GT/SX processors
2008-08-13 19:04 ` prodyut hazarika
2008-08-13 19:16 ` Prodyut Hazarika
@ 2008-08-13 20:01 ` Wolfgang Denk
1 sibling, 0 replies; 17+ messages in thread
From: Wolfgang Denk @ 2008-08-13 20:01 UTC (permalink / raw)
To: u-boot
Dear "prodyut hazarika",
In message <49c0ff980808131204hdc940cel1dcc28d607429f78@mail.gmail.com> you wrote:
>
> > > But then, you are changing good TAB chanracters that were used for
> > > vertical alignment into spaces. This is incorrect - please read the
> > > Coding Style requirements.
> > >
> > > Please do not do this.
>
> The problem is that lot of existing code use spaces to align the
> defines. You can see include/ppc4xx.h and lot of other header files. I
> have seen that spaces are used only in defines. As an example in
> ppc4xx.h. I can send thousands of other places.:
>
> #if defined(CONFIG_405EX) || \$
> defined(CONFIG_440SP) || defined(CONFIG_440SPE) || \$
> defined(CONFIG_460EX) || defined(CONFIG_460GT)$
> #define CONFIG_SDRAM_PPC4xx_IBM_DDR2^I/* IBM DDR(2) controller */$
> #endif$
Here it makes sense to align the 'define's vertically, and it seems
obvious that only spaces can be used here.
Yes, I am aware that there are lots of bad examples around, but please
take the good ones as a guide, not the bad ones.
Finally, no matter what any examples were that you might have followed
when writing new code. What I am complaining about is that you changed
good code and converted it into bad one.
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: wd at denx.de
Quotation, n. The act of repeating erroneously the words of another.
The words erroneously repeated.
- Ambrose Bierce _The Devil's Dictionary_
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH] ppc4xx: Read pipeline depth set to 4 for PPC440SP/SPE, PPC405EX, PPC460EX/GT/SX processors
2008-08-13 19:16 ` Prodyut Hazarika
@ 2008-08-13 20:08 ` Wolfgang Denk
2008-08-13 20:19 ` Prodyut Hazarika
0 siblings, 1 reply; 17+ messages in thread
From: Wolfgang Denk @ 2008-08-13 20:08 UTC (permalink / raw)
To: u-boot
Dear "Prodyut Hazarika",
In message <0CA0A16855646F4FA96D25A158E299D604DB40C1@SDCEXCHANGE01.ad.amcc.com> you wrote:
>
> I basically moved the defines from include/ppc440.h to include/ppc4xx.h.
> If you go to include/ppc440.h, you will see that the original define was aligned
> With spaces. I just followed whatever was it.
>
> Here is the original code in ppc440.h
>
> #if defined(CONFIG_440EP) || defined (CONFIG_440GR) || \
> ^^ space here ^^ space here
> defined(CONFIG_440EPX) || defined(CONFIG_440GR)
> ^^^^^space here
This is not what we are talking about. This part of the code is OK.
I was complaining about this part:
> diff --git a/include/asm-ppc/ppc4xx-sdram.h b/include/asm-ppc/ppc4xx-sdram.h
> index df787b3..d2e3b42 100644
> --- a/include/asm-ppc/ppc4xx-sdram.h
> +++ b/include/asm-ppc/ppc4xx-sdram.h
> @@ -259,23 +259,39 @@
> /*
> * Memory queue defines
> */
> -#define SDRAMQ_DCR_BASE<<<>>>0x040
> -
> -#define SDRAM_R0BAS<>SDRAMQ_DCR_BASE+0x0)<<>>/* rank 0 base address & size */
> -#define SDRAM_R1BAS<>(SDRAMQ_DCR_BASE+0x1)<>>/* rank 1 base address & size */
> -#define SDRAM_R2BAS<>(SDRAMQ_DCR_BASE+0x2)<>>/* rank 2 base address & size */
> -#define SDRAM_R3BAS<>(SDRAMQ_DCR_BASE+0x3)<>>/* rank 3 base address & size */
> -#define SDRAM_CONF1HB<<<<>>>>(SDRAMQ_DCR_BASE+0x5)<>>/* configuration 1 HB */
> -#define SDRAM_ERRSTATHB<<<>>>(SDRAMQ_DCR_BASE+0x7)<>>/* error status HB */
> -#define SDRAM_ERRADDUHB<<<>>>(SDRAMQ_DCR_BASE+0x8)<>>/* error address upper 32 HB */
> -#define SDRAM_ERRADDLHB<<<>>>(SDRAMQ_DCR_BASE+0x9)<>>/* error address lower 32 HB */
> -#define SDRAM_PLBADDULL<<<>>>(SDRAMQ_DCR_BASE+0xA)<>>/* PLB base address upper 32 LL */
> -#define SDRAM_CONF1LL<<<<>>>>(SDRAMQ_DCR_BASE+0xB)<>>/* configuration 1 LL */
> -#define SDRAM_ERRSTATLL<<<>>>(SDRAMQ_DCR_BASE+0xC)<>>/* error status LL */
> -#define SDRAM_ERRADDULL<<<>>>(SDRAMQ_DCR_BASE+0xD)<>>/* error address upper 32 LL */
> -#define SDRAM_ERRADDLLL<<<>>>(SDRAMQ_DCR_BASE+0xE)<>>/* error address lower 32 LL */
> -#define SDRAM_CONFPATHB<<<>>>(SDRAMQ_DCR_BASE+0xF)<>>/* configuration between paths */
> -#define SDRAM_PLBADDUHB<<<>>>(SDRAMQ_DCR_BASE+0x10)<>/* PLB base address upper 32 LL */
> +#define SDRAMQ_DCR_BASE 0x040
> +
> +#define SDRAM_R0BAS (SDRAMQ_DCR_BASE+0x0) /* rank 0 base address & size */
----------------------^^^^^^--------------------^^^ all spaces here
> +#define SDRAM_R1BAS (SDRAMQ_DCR_BASE+0x1) /* rank 1 base address & size */
> +#define SDRAM_R2BAS (SDRAMQ_DCR_BASE+0x2) /* rank 2 base address & size */
> +#define SDRAM_R3BAS (SDRAMQ_DCR_BASE+0x3) /* rank 3 base address & size */
> +#define SDRAM_CONF1HB (SDRAMQ_DCR_BASE+0x5) /* configuration 1 HB */
> +#define SDRAM_CONF1HB_AAFR 0x80000000 /* Address Ack on First Request - Bit 0 */
> +#define SDRAM_CONF1HB_PRPD 0x00080000 /* PLB Read pipeline Disable - Bit 12 */
> +#define SDRAM_CONF1HB_PWPD 0x00040000 /* PLB Write pipeline Disable - Bit 13 */
> +#define SDRAM_CONF1HB_PRW 0x00020000 /* PLB Read Wait - Bit 14 */
> +#define SDRAM_CONF1HB_RPEN 0x00000800 /* Read Passing Enable - Bit 20 */
> +#define SDRAM_CONF1HB_RFTE 0x00000400 /* Read Flow Through Enable - Bit 21 */
... -------------------------^^^----------^^^^^^^^^ allspeace here
I marked the places in the original part that were actually TAB
characters by sequences of "<<>>" so you can see what I mean.
[Actually, the comment-end markers "*/" should be aligned by TABs,
too.]
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: wd at denx.de
Making files is easy under the UNIX operating system. Therefore,
users tend to create numerous files using large amounts of file
space. It has been said that the only standard thing about all UNIX
systems is the message-of-the-day telling users to clean up their
files. - System V.2 administrator's guide
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH] ppc4xx: Read pipeline depth set to 4 for PPC440SP/SPE, PPC405EX, PPC460EX/GT/SX processors
2008-08-13 18:10 fkan at amcc.com
@ 2008-08-13 20:10 ` Wolfgang Denk
0 siblings, 0 replies; 17+ messages in thread
From: Wolfgang Denk @ 2008-08-13 20:10 UTC (permalink / raw)
To: u-boot
Dear fkan at amcc.com,
In message <1218651001-22102-1-git-send-email-fkan@amcc.com> you wrote:
> From: Prodyut Hazarika <phazarika@amcc.com>
>
> Moved PLB4 Arbiter register definitions to ppc4xx.h since it is shared across processors
> Optimize Memory Queue settings for PPC440SP/SPE and PPC460EX/GT/SX processors
>
> Signed-off-by: Prodyut Hazarika <phazarika@amcc.com>
> Acked-by: Feng Kan <fkan@amcc.com>
...
> diff --git a/include/asm-ppc/ppc4xx-sdram.h b/include/asm-ppc/ppc4xx-sdram.h
> index df787b3..d2e3b42 100644
> --- a/include/asm-ppc/ppc4xx-sdram.h
> +++ b/include/asm-ppc/ppc4xx-sdram.h
> @@ -259,23 +259,39 @@
> /*
> * Memory queue defines
> */
> -#define SDRAMQ_DCR_BASE 0x040
> -
> -#define SDRAM_R0BAS (SDRAMQ_DCR_BASE+0x0) /* rank 0 base address & size */
> -#define SDRAM_R1BAS (SDRAMQ_DCR_BASE+0x1) /* rank 1 base address & size */
> -#define SDRAM_R2BAS (SDRAMQ_DCR_BASE+0x2) /* rank 2 base address & size */
> -#define SDRAM_R3BAS (SDRAMQ_DCR_BASE+0x3) /* rank 3 base address & size */
> -#define SDRAM_CONF1HB (SDRAMQ_DCR_BASE+0x5) /* configuration 1 HB */
> -#define SDRAM_ERRSTATHB (SDRAMQ_DCR_BASE+0x7) /* error status HB */
> -#define SDRAM_ERRADDUHB (SDRAMQ_DCR_BASE+0x8) /* error address upper 32 HB */
> -#define SDRAM_ERRADDLHB (SDRAMQ_DCR_BASE+0x9) /* error address lower 32 HB */
> -#define SDRAM_PLBADDULL (SDRAMQ_DCR_BASE+0xA) /* PLB base address upper 32 LL */
> -#define SDRAM_CONF1LL (SDRAMQ_DCR_BASE+0xB) /* configuration 1 LL */
> -#define SDRAM_ERRSTATLL (SDRAMQ_DCR_BASE+0xC) /* error status LL */
> -#define SDRAM_ERRADDULL (SDRAMQ_DCR_BASE+0xD) /* error address upper 32 LL */
> -#define SDRAM_ERRADDLLL (SDRAMQ_DCR_BASE+0xE) /* error address lower 32 LL */
> -#define SDRAM_CONFPATHB (SDRAMQ_DCR_BASE+0xF) /* configuration between paths */
> -#define SDRAM_PLBADDUHB (SDRAMQ_DCR_BASE+0x10) /* PLB base address upper 32 LL */
> +#define SDRAMQ_DCR_BASE 0x040
> +
> +#define SDRAM_R0BAS (SDRAMQ_DCR_BASE+0x0) /* rank 0 base address & size */
> +#define SDRAM_R1BAS (SDRAMQ_DCR_BASE+0x1) /* rank 1 base address & size */
> +#define SDRAM_R2BAS (SDRAMQ_DCR_BASE+0x2) /* rank 2 base address & size */
> +#define SDRAM_R3BAS (SDRAMQ_DCR_BASE+0x3) /* rank 3 base address & size */
> +#define SDRAM_CONF1HB (SDRAMQ_DCR_BASE+0x5) /* configuration 1 HB */
> +#define SDRAM_CONF1HB_AAFR 0x80000000 /* Address Ack on First Request - Bit 0 */
> +#define SDRAM_CONF1HB_PRPD 0x00080000 /* PLB Read pipeline Disable - Bit 12 */
> +#define SDRAM_CONF1HB_PWPD 0x00040000 /* PLB Write pipeline Disable - Bit 13 */
> +#define SDRAM_CONF1HB_PRW 0x00020000 /* PLB Read Wait - Bit 14 */
> +#define SDRAM_CONF1HB_RPEN 0x00000800 /* Read Passing Enable - Bit 20 */
> +#define SDRAM_CONF1HB_RFTE 0x00000400 /* Read Flow Through Enable - Bit 21 */
NAK again. Please do not replace the TAB characters in the original
code by spaces. Use TAB for vertical alignment. At least do not make
existing code 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: wd at denx.de
As far as we know, our computer has never had an undetected error.
-- Weisert
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH] ppc4xx: Read pipeline depth set to 4 for PPC440SP/SPE, PPC405EX, PPC460EX/GT/SX processors
2008-08-13 20:08 ` Wolfgang Denk
@ 2008-08-13 20:19 ` Prodyut Hazarika
2008-08-13 20:56 ` Wolfgang Denk
0 siblings, 1 reply; 17+ messages in thread
From: Prodyut Hazarika @ 2008-08-13 20:19 UTC (permalink / raw)
To: u-boot
Dear Wolfgang,
Thanks for the clarification. I will change include/asm-ppc/ppc4xx-sdram.h to tabs and resubmit.
But one more question.
> >
> > #if defined(CONFIG_440EP) || defined (CONFIG_440GR) || \
> > ^^ space here ^^ space here
> > defined(CONFIG_440EPX) || defined(CONFIG_440GR)
> This is not what we are talking about. This part of the code is OK.
I want to understand why is space alignment okay here. The coding guidelines state that we must use tabs for vertical alignment. Does this rule not apply to defines? If so, the coding guidelines doc should be updated.
Best regards,
Prodyut Hazarika
=====================
Staff S/W Engineer
AMCC
=====================
--------------------------------------------------------
CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is for the sole use of the intended recipient(s) and contains information that is confidential and proprietary to Applied Micro Circuits Corporation or its subsidiaries. It is to be used solely for the purpose of furthering the parties' business relationship. All unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply e-mail and destroy all copies of the original message.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH] ppc4xx: Read pipeline depth set to 4 for PPC440SP/SPE, PPC405EX, PPC460EX/GT/SX processors
2008-08-13 20:19 ` Prodyut Hazarika
@ 2008-08-13 20:56 ` Wolfgang Denk
0 siblings, 0 replies; 17+ messages in thread
From: Wolfgang Denk @ 2008-08-13 20:56 UTC (permalink / raw)
To: u-boot
Dear "Prodyut Hazarika",
In message <0CA0A16855646F4FA96D25A158E299D604DB40F8@SDCEXCHANGE01.ad.amcc.com> you wrote:
>
> > > #if defined(CONFIG_440EP) || defined (CONFIG_440GR) || \
> > > ^^ space here ^^ space here
> > > defined(CONFIG_440EPX) || defined(CONFIG_440GR)
>
> > This is not what we are talking about. This part of the code is OK.
>
> I want to understand why is space alignment okay here. The coding
The intention of vertical alignment is to make code look "nice". Com-
puters don't care (at least to the best of my knowledge), but humans
seem to find it easier if code is neatly aligned in columns.
For example, compare this example (randomly selected - it's from
include/4xx_i2c.h):
(1)
#define IIC_OK 0
#define IIC_NOK 1
#define IIC_NOK_LA 2 /* Lost arbitration */
#define IIC_NOK_ICT 3 /* Incomplete transfer */
#define IIC_NOK_XFRA 4 /* Transfer aborted */
#define IIC_NOK_DATA 5 /* No data in buffer */
#define IIC_NOK_TOUT 6 /* Transfer timeout */
#define IIC_TIMEOUT 1 /* 1 second */
against:
(2)
#define IIC_OK 0
#define IIC_NOK 1
#define IIC_NOK_LA 2 /* Lost arbitration */
#define IIC_NOK_ICT 3 /* Incomplete transfer */
#define IIC_NOK_XFRA 4 /* Transfer aborted */
#define IIC_NOK_DATA 5 /* No data in buffer */
#define IIC_NOK_TOUT 6 /* Transfer timeout */
#define IIC_TIMEOUT 1 /* 1 second */
Which is nicer, and easier to read - (1) or (2)?
Now if you have something that starts "#if defined(..)" in the first
line, then it seems only natural to me that (assuming we have to add
another line, which actually is an indication of a coding problem in
the first place), it should be aligned such that the "defined()"
parts start in the same column. So the second line would follow like
this:
#if defined(foo) ... || \
defined(bar)
There is no law, no written rule for that, just common sense and a a
certain amount of esthetical sensation.
> guidelines state that we must use tabs for vertical alignment. Does this
> rule not apply to defines? If so, the coding guidelines doc should be
> updated.
I'm sorry, but it would exceed my capabilities as an author to put
that in writing. Feel free to add to the wiki and improve the
description there yourself if you feel it needs it.
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: wd at denx.de
Es sind ?berhaupt nur die Dummk?pfe, die sich den Befehlen der M?ch-
tigen widersetzen. Um sie zu ruinieren ist es genug, ihre Befehle
treu zu erf?llen. - Peter Hacks: "Die sch?ne Helena"
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2008-08-13 20:56 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-08-13 4:43 [U-Boot] [PATCH] ppc4xx: Read pipeline depth set to 4 for PPC440SP/SPE, PPC405EX, PPC460EX/GT/SX processors fkan at amcc.com
2008-08-13 5:37 ` Wolfgang Denk
2008-08-13 6:26 ` Stefan Roese
2008-08-13 6:44 ` Wolfgang Denk
2008-08-13 13:59 ` Prodyut Hazarika
2008-08-13 14:24 ` Stefan Roese
2008-08-13 17:13 ` Scott Wood
2008-08-13 18:31 ` Wolfgang Denk
2008-08-13 18:44 ` Scott Wood
2008-08-13 19:04 ` prodyut hazarika
2008-08-13 19:16 ` Prodyut Hazarika
2008-08-13 20:08 ` Wolfgang Denk
2008-08-13 20:19 ` Prodyut Hazarika
2008-08-13 20:56 ` Wolfgang Denk
2008-08-13 20:01 ` Wolfgang Denk
-- strict thread matches above, loose matches on Subject: below --
2008-08-13 18:10 fkan at amcc.com
2008-08-13 20:10 ` Wolfgang Denk
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox