Re: [U-Boot] [PATCH 2/8] APM82xxx: Add Common register definitions

2010-08-30 Thread Tirumala Marri
 -Original Message-
 From: Wolfgang Denk [mailto:w...@denx.de]
 Sent: Sunday, August 29, 2010 1:57 AM
 To: tma...@apm.com
 Cc: u-boot@lists.denx.de; open-source-rev...@apm.com
 Subject: Re: [U-Boot] [PATCH 2/8] APM82xxx: Add Common register
 definitions

 Dear tma...@apm.com,

 In message 1282856749-24425-1-git-send-email-tma...@apm.com you
 wrote:
  From: Tirumala Marri tma...@apm.com
 
  This patch adds APM82XXX specific definitions, which include
  clock and boot strap.
 
  Signed-off-by: Tirumala R Marri tma...@apm.com
  ---
   include/ppc440.h |   57
 -
   include/ppc4xx.h |7 +++--
   2 files changed, 59 insertions(+), 5 deletions(-)

 It makes no sense to split this into a spearate patch. Please squash
 into the commit where these defines are first referenced.

Mr Wolfgang,
  Sure will fix it.
Regards,
Marri
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 2/8] APM82xxx: Add Common register definitions

2010-08-29 Thread Wolfgang Denk
Dear tma...@apm.com,

In message 1282856749-24425-1-git-send-email-tma...@apm.com you wrote:
 From: Tirumala Marri tma...@apm.com
 
 This patch adds APM82XXX specific definitions, which include
 clock and boot strap.
 
 Signed-off-by: Tirumala R Marri tma...@apm.com
 ---
  include/ppc440.h |   57 -
  include/ppc4xx.h |7 +++--
  2 files changed, 59 insertions(+), 5 deletions(-)

It makes no sense to split this into a spearate patch. Please squash
into the commit where these defines are first referenced.

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
How does a project get to be a year late?  ... One day at a time.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 2/8] APM82xxx: Add Common register definitions

2010-08-27 Thread Stefan Roese
Hi Marri,

On Thursday 26 August 2010 23:05:49 tma...@apm.com wrote:
 From: Tirumala Marri tma...@apm.com
 
 This patch adds APM82XXX specific definitions, which include
 clock and boot strap.

Please find some comments below.
 
 Signed-off-by: Tirumala R Marri tma...@apm.com
 ---
  include/ppc440.h |   57
 - include/ppc4xx.h |  
  7 +++--
  2 files changed, 59 insertions(+), 5 deletions(-)
 
 diff --git a/include/ppc440.h b/include/ppc440.h
 index c807dda..3bd8e98 100644
 --- a/include/ppc440.h
 +++ b/include/ppc440.h
 @@ -58,6 +58,25 @@
 
   | Clocking Controller
 
  
 +-
 ---*/ /* values for clkcfga register - indirect addressing of these regs */
 +#if defined(CONFIG_APM82XXX)
 +#define CPR0_CLKUPD  0x0020
 +#define CPR0_PLLC0x0040
 +#define CPR0_PLLC_SEL(pllc)  (((pllc)  0x0100)  24)
 +#define CPR0_PLLD0x0060
 +#define CPR0_PLLD_FDV(plld) (((plld)  0xff00)  24)
 +#define CPR0_PLLD_FWDVA(plld)(((plld)  0x000f)  16)
 +#define CPR0_CPUD0x0080
 +#define CPR0_CPUD_CPUDV(cpud)(((cpud)  0x0700)  24)
 +#define CPR0_PLB2D   0x00a0
 +#define CPR0_PLB2D_PLB2DV(plb2d) (((plb2d)  0x0600)  25)
 +#define CPR0_OPBD0x00c0
 +#define CPR0_OPBD_OPBDV(opbd)(((opbd)  0x0300)  24)
 +#define CPR0_PERD   0x00e0
 +#define CPR0_PERD_PERDV(perd)(((perd)  0x0300)  24)
 +#define CPR0_DDR2D  0x0100
 +#define CPR0_DDR2D_DDR2DV(ddr2d) (((ddr2d)  0x0600)  25)
 +#define CLK_ICFG 0x0140
 +#else
  #define CPR0_PLLC0x0040
  #define CPR0_PLLD0x0060
  #define CPR0_PRIMAD0 0x0080
 @@ -67,6 +86,7 @@
  #define CPR0_MALD0x0100
  #define CPR0_SPCID   0x0120
  #define CPR0_ICFG0x0140
 +#endif /*if defined(CONFIG_APM82XXX) */
 
  /* 440EPX boot strap options */
  #define BOOT_STRAP_OPTION_A  0x
 @@ -1275,7 +1295,36 @@
  #define SDR0_AHB_CFG 0x370
  #define SDR0_USB2HOST_CFG0x371
  #endif /* CONFIG_460EX || CONFIG_460GT */
 +#if defined(CONFIG_APM82XXX)
 +
 +#define SDR0_DDR00x00E1
 +#define SDR0_DDR0_DDRM_ENCODE(n) unsigned long)(n))0x03)29)
 +#define SDR0_DDR0_DDRM_DECODE(n) unsigned long)(n))29)0x03)
 +#define SDR0_DDR0_TUNE_ENCODE(n) unsigned long)(n))0x2FF)0)
 +#define SDR0_DDR0_TUNE_DECODE(n) unsigned long)(n))0)0x2FF)

Add spaces around the operators. I suggest to replace unsigned int
with u32 to reduce the line-size resulting in this:

#define SDR0_DDR0_DDRM_ENCODE(n)u32)(n))  0x03)  29)
#define SDR0_DDR0_DDRM_DECODE(n)u32)(n))  29)  0x03)
#define SDR0_DDR0_TUNE_ENCODE(n)u32)(n))  0x2FF)  0)
#define SDR0_DDR0_TUNE_DECODE(n)u32)(n))  0)  0x2FF)

BTW: I couldn't find any reference to this SDR0_DDR0 register (0xe1) in the
APM8xxx users manual. You might want to add this description there.

 +#define SDR_SDSTP1_RL_DECODE(x) ((x  0x000C)  18)

Missing braces around x.

 +#define SDR_SDSTP1_RL_EBC   0x0
 +#define SDR_SDSTP1_RL_NDFC  0x2
 +
 +/* ECID */
 +#define SDR0_ECID0 0x0080
 +#define SDR0_ECID1 0x0081
 +#define SDR0_ECID2 0x0082
 +#define SDR0_ECID3 0x0083
 +
 +/* AHB config. */
 +#define AHB_TOP 0xA4
 +#define AHB_BOT 0xA5
 +#define SDR0_AHB_CFG0x370
 +
 +/* DDR SDRAM Controller clock (CPR register)*/
 +#define SDR0_DDRCE 0x00E0 /* SDR register */
 +#define CPR0_DDR2D 0x0100 /* CPR register */
 +#define CPR0_DDR2D_DDR2DV_ENCODE(n)unsigned long)(n))0x03)25)
 +#define CPR0_DDR2D_DDR2DV_DECODE(n)unsigned long)(n))25)0x03)

Again, please add spaces and change to u32.

Please change and resubmit. Thanks.

Cheers,
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: off...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot