[U-Boot] [PATCH] ppc4xx: Read pipeline depth set to 4 for PPC440SP/SPE, PPC405EX, PPC460EX/GT/SX processors
Wolfgang Denk
wd at denx.de
Wed Aug 13 07:37:40 CEST 2008
Dear Feng Kan,
in message <1218602619-19293-1-git-send-email-fkan at amcc.com> you wrote:
> From: Prodyut Hazarika <phazarika at amcc.com>
>
> Signed-off-by: Prodyut Hazarika <phazarika at amcc.com>
> Acked-by: Feng Kan <fkan at 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
More information about the U-Boot
mailing list