Re: [U-Boot] [RFC Patch 2/5] driver: ddr: fsl_mmdc: Pass board parameters through data structure
> -Original Message-
> From: U-Boot [mailto:u-boot-boun...@lists.denx.de] On Behalf Of York Sun
> Sent: Thursday, September 22, 2016 4:21 AM
> To: tr...@konsulko.com
> Cc: u-boot@lists.denx.de
> Subject: [U-Boot] [RFC Patch 2/5] driver: ddr: fsl_mmdc: Pass board parameters
> through data structure
>
> Instead of using multiple macros, a data structure is used to pass
> board-specific parameters to MMDC DDR driver.
>
> Signed-off-by: York Sun
> CC: Shengzhou Liu
I doubt if it is one of the reason for root-cause.
May not be part of this patch set
> ---
> board/freescale/ls1012afrdm/ls1012afrdm.c | 18 ++-
> board/freescale/ls1012aqds/ls1012aqds.c | 18 ++-
> board/freescale/ls1012ardb/ls1012ardb.c | 18 ++-
> drivers/ddr/fsl/fsl_mmdc.c| 38
> +++
> include/configs/ls1012afrdm.h | 16 -
> include/configs/ls1012aqds.h | 16 -
> include/configs/ls1012ardb.h | 15
> include/fsl_mmdc.h| 21 +
> 8 files changed, 87 insertions(+), 73 deletions(-)
>
> diff --git a/board/freescale/ls1012afrdm/ls1012afrdm.c
> b/board/freescale/ls1012afrdm/ls1012afrdm.c
> index d644e94..b03bdb8 100644
> --- a/board/freescale/ls1012afrdm/ls1012afrdm.c
> +++ b/board/freescale/ls1012afrdm/ls1012afrdm.c
> @@ -26,7 +26,23 @@ int checkboard(void)
>
> int dram_init(void)
> {
> - mmdc_init();
> + static const struct fsl_mmdc_info mparam = {
> + 0x0418, /* mdctl */
> + 0x00030035, /* mdpdc */
> + 0x12554000, /* mdotc */
> + 0xbabf7954, /* mdcfg0 */
> + 0xdb328f64, /* mdcfg1 */
> + 0x01ff00db, /* mdcfg2 */
> + 0x1680, /* mdmisc */
> + 0x0f3c8000, /* mdref */
> + 0x2000, /* mdrwd */
> + 0x00bf1023, /* mdor */
> + 0x003f, /* mdasp */
> + 0x022a, /* mpodtctrl */
> + 0xa1390003, /* mpzqhwctrl */
> + };
> +
> + mmdc_init(&mparam);
>
Why cannot #define directly be used in fsl_mmdc.c.
If objective is to remove #define from board file, they even be required to
avoid magic numbers.
-prabhakar
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC Patch 2/5] driver: ddr: fsl_mmdc: Pass board parameters through data structure
On Thu, Sep 22, 2016 at 03:04:31AM +, Prabhakar Kushwaha wrote:
>
> > -Original Message-
> > From: U-Boot [mailto:u-boot-boun...@lists.denx.de] On Behalf Of York Sun
> > Sent: Thursday, September 22, 2016 4:21 AM
> > To: tr...@konsulko.com
> > Cc: u-boot@lists.denx.de
> > Subject: [U-Boot] [RFC Patch 2/5] driver: ddr: fsl_mmdc: Pass board
> > parameters
> > through data structure
> >
> > Instead of using multiple macros, a data structure is used to pass
> > board-specific parameters to MMDC DDR driver.
> >
> > Signed-off-by: York Sun
> > CC: Shengzhou Liu
>
>
> I doubt if it is one of the reason for root-cause.
> May not be part of this patch set
>
>
> > ---
> > board/freescale/ls1012afrdm/ls1012afrdm.c | 18 ++-
> > board/freescale/ls1012aqds/ls1012aqds.c | 18 ++-
> > board/freescale/ls1012ardb/ls1012ardb.c | 18 ++-
> > drivers/ddr/fsl/fsl_mmdc.c| 38
> > +++
> > include/configs/ls1012afrdm.h | 16 -
> > include/configs/ls1012aqds.h | 16 -
> > include/configs/ls1012ardb.h | 15
> > include/fsl_mmdc.h| 21 +
> > 8 files changed, 87 insertions(+), 73 deletions(-)
> >
> > diff --git a/board/freescale/ls1012afrdm/ls1012afrdm.c
> > b/board/freescale/ls1012afrdm/ls1012afrdm.c
> > index d644e94..b03bdb8 100644
> > --- a/board/freescale/ls1012afrdm/ls1012afrdm.c
> > +++ b/board/freescale/ls1012afrdm/ls1012afrdm.c
> > @@ -26,7 +26,23 @@ int checkboard(void)
> >
> > int dram_init(void)
> > {
> > - mmdc_init();
> > + static const struct fsl_mmdc_info mparam = {
> > + 0x0418, /* mdctl */
> > + 0x00030035, /* mdpdc */
> > + 0x12554000, /* mdotc */
> > + 0xbabf7954, /* mdcfg0 */
> > + 0xdb328f64, /* mdcfg1 */
> > + 0x01ff00db, /* mdcfg2 */
> > + 0x1680, /* mdmisc */
> > + 0x0f3c8000, /* mdref */
> > + 0x2000, /* mdrwd */
> > + 0x00bf1023, /* mdor */
> > + 0x003f, /* mdasp */
> > + 0x022a, /* mpodtctrl */
> > + 0xa1390003, /* mpzqhwctrl */
> > + };
> > +
> > + mmdc_init(&mparam);
> >
>
> Why cannot #define directly be used in fsl_mmdc.c.
>
> If objective is to remove #define from board file, they even be required to
> avoid magic numbers.
Please see the previous thread about this between York and I. The end
goal is not to have no magic numbers, the end goal is to make it clear
what the magic numbers are doing. I still wish there was also a link to
a tech note / manual / whatever to decode each of these values and a
comment about what DDR part is being used on the board, but this is a
step in the right direction.
And as a side bonus, this moves things one step closer to being able to
have more than one board supported by a given binary.
--
Tom
signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [RFC Patch 2/5] driver: ddr: fsl_mmdc: Pass board parameters through data structure
Instead of using multiple macros, a data structure is used to pass
board-specific parameters to MMDC DDR driver.
Signed-off-by: York Sun
CC: Shengzhou Liu
---
board/freescale/ls1012afrdm/ls1012afrdm.c | 18 ++-
board/freescale/ls1012aqds/ls1012aqds.c | 18 ++-
board/freescale/ls1012ardb/ls1012ardb.c | 18 ++-
drivers/ddr/fsl/fsl_mmdc.c| 38 +++
include/configs/ls1012afrdm.h | 16 -
include/configs/ls1012aqds.h | 16 -
include/configs/ls1012ardb.h | 15
include/fsl_mmdc.h| 21 +
8 files changed, 87 insertions(+), 73 deletions(-)
diff --git a/board/freescale/ls1012afrdm/ls1012afrdm.c
b/board/freescale/ls1012afrdm/ls1012afrdm.c
index d644e94..b03bdb8 100644
--- a/board/freescale/ls1012afrdm/ls1012afrdm.c
+++ b/board/freescale/ls1012afrdm/ls1012afrdm.c
@@ -26,7 +26,23 @@ int checkboard(void)
int dram_init(void)
{
- mmdc_init();
+ static const struct fsl_mmdc_info mparam = {
+ 0x0418, /* mdctl */
+ 0x00030035, /* mdpdc */
+ 0x12554000, /* mdotc */
+ 0xbabf7954, /* mdcfg0 */
+ 0xdb328f64, /* mdcfg1 */
+ 0x01ff00db, /* mdcfg2 */
+ 0x1680, /* mdmisc */
+ 0x0f3c8000, /* mdref */
+ 0x2000, /* mdrwd */
+ 0x00bf1023, /* mdor */
+ 0x003f, /* mdasp */
+ 0x022a, /* mpodtctrl */
+ 0xa1390003, /* mpzqhwctrl */
+ };
+
+ mmdc_init(&mparam);
gd->ram_size = CONFIG_SYS_SDRAM_SIZE;
diff --git a/board/freescale/ls1012aqds/ls1012aqds.c
b/board/freescale/ls1012aqds/ls1012aqds.c
index 188b6bc..94440b3 100644
--- a/board/freescale/ls1012aqds/ls1012aqds.c
+++ b/board/freescale/ls1012aqds/ls1012aqds.c
@@ -54,7 +54,23 @@ int checkboard(void)
int dram_init(void)
{
- mmdc_init();
+ static const struct fsl_mmdc_info mparam = {
+ 0x0518, /* mdctl */
+ 0x00030035, /* mdpdc */
+ 0x12554000, /* mdotc */
+ 0xbabf7954, /* mdcfg0 */
+ 0xdb328f64, /* mdcfg1 */
+ 0x01ff00db, /* mdcfg2 */
+ 0x1680, /* mdmisc */
+ 0x0f3c8000, /* mdref */
+ 0x2000, /* mdrwd */
+ 0x00bf1023, /* mdor */
+ 0x003f, /* mdasp */
+ 0x022a, /* mpodtctrl */
+ 0xa1390003, /* mpzqhwctrl */
+ };
+
+ mmdc_init(&mparam);
gd->ram_size = CONFIG_SYS_SDRAM_SIZE;
diff --git a/board/freescale/ls1012ardb/ls1012ardb.c
b/board/freescale/ls1012ardb/ls1012ardb.c
index 50f9187..778434d 100644
--- a/board/freescale/ls1012ardb/ls1012ardb.c
+++ b/board/freescale/ls1012ardb/ls1012ardb.c
@@ -58,7 +58,23 @@ int checkboard(void)
int dram_init(void)
{
- mmdc_init();
+ static const struct fsl_mmdc_info mparam = {
+ 0x0518, /* mdctl */
+ 0x00030035, /* mdpdc */
+ 0x12554000, /* mdotc */
+ 0xbabf7954, /* mdcfg0 */
+ 0xdb328f64, /* mdcfg1 */
+ 0x01ff00db, /* mdcfg2 */
+ 0x1680, /* mdmisc */
+ 0x0f3c8000, /* mdref */
+ 0x2000, /* mdrwd */
+ 0x00bf1023, /* mdor */
+ 0x003f, /* mdasp */
+ 0x022a, /* mpodtctrl */
+ 0xa1390003, /* mpzqhwctrl */
+ };
+
+ mmdc_init(&mparam);
gd->ram_size = CONFIG_SYS_SDRAM_SIZE;
diff --git a/drivers/ddr/fsl/fsl_mmdc.c b/drivers/ddr/fsl/fsl_mmdc.c
index 1e35967..52eec0f 100644
--- a/drivers/ddr/fsl/fsl_mmdc.c
+++ b/drivers/ddr/fsl/fsl_mmdc.c
@@ -26,7 +26,7 @@ static void set_wait_for_bits_clear(void *ptr, u32 value, u32
bits)
printf("Error: %p wait for clear timeout.\n", ptr);
}
-void mmdc_init(void)
+void mmdc_init(const struct fsl_mmdc_info *priv)
{
struct mmdc_regs *mmdc = (struct mmdc_regs *)CONFIG_SYS_FSL_DDR_ADDR;
unsigned int tmp;
@@ -35,26 +35,26 @@ void mmdc_init(void)
out_be32(&mmdc->mdscr, MDSCR_ENABLE_CON_REQ);
/* 2. configure the desired timing parameters */
- out_be32(&mmdc->mdotc, CONFIG_MMDC_MDOTC);
- out_be32(&mmdc->mdcfg0, CONFIG_MMDC_MDCFG0);
- out_be32(&mmdc->mdcfg1, CONFIG_MMDC_MDCFG1);
- out_be32(&mmdc->mdcfg2, CONFIG_MMDC_MDCFG2);
+ out_be32(&mmdc->mdotc, priv->mdotc);
+ out_be32(&mmdc->mdcfg0, priv->mdcfg0);
+ out_be32(&mmdc->mdcfg1, priv->mdcfg1);
+ out_be32(&mmdc->mdcfg2, priv->mdcfg2);
/* 3. configure DDR type and other miscellaneous parameters */
- out_be32(&mmdc->md
Re: [U-Boot] [RFC Patch 2/5] driver: ddr: fsl_mmdc: Pass board parameters through data structure
On Wed, Sep 21, 2016 at 03:51:15PM -0700, York Sun wrote: > Instead of using multiple macros, a data structure is used to pass > board-specific parameters to MMDC DDR driver. > > Signed-off-by: York Sun > CC: Shengzhou Liu Thanks for doing this! Reviewed-by: Tom Rini -- Tom signature.asc Description: Digital signature ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
