| Message ID | 1341575417-7719-1-git-send-email-gerlando.falauto@keymile.com |
|---|---|
| State | Accepted |
| Delegated to: | Prafulla Wadaskar |
| Headers | show |
Dear Gerlando Falauto, > Some boards might be equipped with different SDRAM configurations. > When that is the case, CPU CS Window Size Register (CS[0]n Size) should > be set to the biggest value through board.cfg file; then its value > can be fixed at runtime according to the detected SDRAM size. > > Therefore, implement kw_sdram_bs_set(), to be called for instance within > board_early_init_f(). > > Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com> > Reviewed-by: Marek Vasut <marex@denx.de> > Cc: Prafulla Wadaskar <prafulla@marvell.com> > Cc: Wolfgang Denk <wd@denx.de> > Cc: Valentin Longchamp <valentin.longchamp@keymile.com> > Cc: Holger Brunck <holger.brunck@keymile.com> > --- > changes for v2: > added Reviewed-by: Marek Vasut <marex@denx.de> [...] You don't have to resend it, patchwork does this additions for us :) Best regards, Marek Vasut
> -----Original Message----- > From: Gerlando Falauto [mailto:gerlando.falauto@keymile.com] > Sent: 06 July 2012 17:20 > To: u-boot@lists.denx.de > Cc: Gerlando Falauto; Prafulla Wadaskar; Wolfgang Denk; Valentin > Longchamp; Holger Brunck > Subject: [PATCH v2 1/2] kirkwood: implement kw_sdram_bs_set() > > Some boards might be equipped with different SDRAM configurations. > When that is the case, CPU CS Window Size Register (CS[0]n Size) > should > be set to the biggest value through board.cfg file; then its value > can be fixed at runtime according to the detected SDRAM size. > > Therefore, implement kw_sdram_bs_set(), to be called for instance > within > board_early_init_f(). > > Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com> > Reviewed-by: Marek Vasut <marex@denx.de> > Cc: Prafulla Wadaskar <prafulla@marvell.com> > Cc: Wolfgang Denk <wd@denx.de> > Cc: Valentin Longchamp <valentin.longchamp@keymile.com> > Cc: Holger Brunck <holger.brunck@keymile.com> > --- > changes for v2: > added Reviewed-by: Marek Vasut <marex@denx.de> > > arch/arm/cpu/arm926ejs/kirkwood/dram.c | 23 > +++++++++++++++++++++++ > arch/arm/include/asm/arch-kirkwood/cpu.h | 2 ++ > 2 files changed, 25 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/cpu/arm926ejs/kirkwood/dram.c > b/arch/arm/cpu/arm926ejs/kirkwood/dram.c > index 181b3e7..d938578 100644 > --- a/arch/arm/cpu/arm926ejs/kirkwood/dram.c > +++ b/arch/arm/cpu/arm926ejs/kirkwood/dram.c > @@ -32,6 +32,12 @@ DECLARE_GLOBAL_DATA_PTR; > > #define KW_REG_CPUCS_WIN_BAR(x) (KW_REGISTER(0x1500) + (x * > 0x08)) > #define KW_REG_CPUCS_WIN_SZ(x) (KW_REGISTER(0x1504) + (x * > 0x08)) Hi Gerlando I have a full ack for this patch. But I would like to ask you- Since dram.c is going through this change and macros are being used in stead of c-struct for DRAM register variables. This is being one of the todo item pending from long time. Would you like to convert macros to c-struct definition in this file? > + > +#define KW_REG_CPUCS_WIN_ENABLE (1 << 0) > +#define KW_REG_CPUCS_WIN_WR_PROTECT (1 << 1) > +#define KW_REG_CPUCS_WIN_WIN0_CS(x) (((x) & 0x3) << 2) > +#define KW_REG_CPUCS_WIN_SIZE(x) (((x) & 0xff) << 24) > + > /* > * kw_sdram_bar - reads SDRAM Base Address Register > */ > @@ -62,6 +68,23 @@ u32 kw_sdram_bs(enum memory_bank bank) > return result; > } > > +/* > + * kw_sdram_bs_set - writes SDRAM Bank size > + */ > +void kw_sdram_bs_set(enum memory_bank bank, u32 size) > +{ > + /* Read current register value */ > + u32 reg = readl(KW_REG_CPUCS_WIN_SZ(bank)); > + > + /* Clear window size */ > + reg &= ~KW_REG_CPUCS_WIN_SIZE(0xFF); > + > + /* Set new window size */ > + reg |= KW_REG_CPUCS_WIN_SIZE((size - 1) >> 24); > + > + writel(reg, KW_REG_CPUCS_WIN_SZ(bank)); > +} > + > #ifndef CONFIG_SYS_BOARD_DRAM_INIT > int dram_init(void) > { > diff --git a/arch/arm/include/asm/arch-kirkwood/cpu.h > b/arch/arm/include/asm/arch-kirkwood/cpu.h > index d28c51a..807154e 100644 > --- a/arch/arm/include/asm/arch-kirkwood/cpu.h > +++ b/arch/arm/include/asm/arch-kirkwood/cpu.h > @@ -159,6 +159,8 @@ void reset_cpu(unsigned long ignored); > unsigned char get_random_hex(void); > unsigned int kw_sdram_bar(enum memory_bank bank); > unsigned int kw_sdram_bs(enum memory_bank bank); > +void kw_sdram_bs_set(enum memory_bank bank, u32 size); > + Please remove this additional line > int kw_config_adr_windows(void); > void kw_config_gpio(unsigned int gpp0_oe_val, unsigned int > gpp1_oe_val, > unsigned int gpp0_oe, unsigned int gpp1_oe); > -- Regards... Prafulla . . . > 1.7.1
Dear Prafulla Wadaskar, [...] > > diff --git a/arch/arm/cpu/arm926ejs/kirkwood/dram.c > > b/arch/arm/cpu/arm926ejs/kirkwood/dram.c > > index 181b3e7..d938578 100644 > > --- a/arch/arm/cpu/arm926ejs/kirkwood/dram.c > > +++ b/arch/arm/cpu/arm926ejs/kirkwood/dram.c > > @@ -32,6 +32,12 @@ DECLARE_GLOBAL_DATA_PTR; > > > > #define KW_REG_CPUCS_WIN_BAR(x) (KW_REGISTER(0x1500) + (x * > > > > 0x08)) > > > > #define KW_REG_CPUCS_WIN_SZ(x) (KW_REGISTER(0x1504) + (x * > > > > 0x08)) > > Hi Gerlando > I have a full ack for this patch. > But I would like to ask you- > Since dram.c is going through this change and macros are being used in > stead of c-struct for DRAM register variables. This is being one of the > todo item pending from long time. > > Would you like to convert macros to c-struct definition in this file? Certainly, but let's get this applied first. [...] > > diff --git a/arch/arm/include/asm/arch-kirkwood/cpu.h > > b/arch/arm/include/asm/arch-kirkwood/cpu.h > > index d28c51a..807154e 100644 > > --- a/arch/arm/include/asm/arch-kirkwood/cpu.h > > +++ b/arch/arm/include/asm/arch-kirkwood/cpu.h > > @@ -159,6 +159,8 @@ void reset_cpu(unsigned long ignored); > > > > unsigned char get_random_hex(void); > > unsigned int kw_sdram_bar(enum memory_bank bank); > > unsigned int kw_sdram_bs(enum memory_bank bank); > > > > +void kw_sdram_bs_set(enum memory_bank bank, u32 size); > > + > > Please remove this additional line Well, maybe you can remove it in the application process since it's really one small thing? > > int kw_config_adr_windows(void); > > void kw_config_gpio(unsigned int gpp0_oe_val, unsigned int > > > > gpp1_oe_val, > > > > unsigned int gpp0_oe, unsigned int gpp1_oe); > > > > -- > > Regards... > Prafulla . . . > > > 1.7.1 Best regards, Marek Vasut
Hi Marek, hi Prafulla, On 07/20/2012 01:40 PM, Marek Vasut wrote: > Dear Prafulla Wadaskar, > > [...] > >>> diff --git a/arch/arm/cpu/arm926ejs/kirkwood/dram.c >>> b/arch/arm/cpu/arm926ejs/kirkwood/dram.c >>> index 181b3e7..d938578 100644 >>> --- a/arch/arm/cpu/arm926ejs/kirkwood/dram.c >>> +++ b/arch/arm/cpu/arm926ejs/kirkwood/dram.c >>> @@ -32,6 +32,12 @@ DECLARE_GLOBAL_DATA_PTR; >>> >>> #define KW_REG_CPUCS_WIN_BAR(x) (KW_REGISTER(0x1500) + (x * >>> >>> 0x08)) >>> >>> #define KW_REG_CPUCS_WIN_SZ(x) (KW_REGISTER(0x1504) + (x * >>> >>> 0x08)) >> >> Hi Gerlando >> I have a full ack for this patch. >> But I would like to ask you- >> Since dram.c is going through this change and macros are being used in >> stead of c-struct for DRAM register variables. This is being one of the >> todo item pending from long time. >> >> Would you like to convert macros to c-struct definition in this file? > > Certainly, but let's get this applied first. > > [...] > I have done a v3 version of this patchserie with Prafullas inputs. Gerlando is not in these days. I'll send the updates soon... I have already tested them on km_kirkwood. Regards Holger
diff --git a/arch/arm/cpu/arm926ejs/kirkwood/dram.c b/arch/arm/cpu/arm926ejs/kirkwood/dram.c index 181b3e7..d938578 100644 --- a/arch/arm/cpu/arm926ejs/kirkwood/dram.c +++ b/arch/arm/cpu/arm926ejs/kirkwood/dram.c @@ -32,6 +32,12 @@ DECLARE_GLOBAL_DATA_PTR; #define KW_REG_CPUCS_WIN_BAR(x) (KW_REGISTER(0x1500) + (x * 0x08)) #define KW_REG_CPUCS_WIN_SZ(x) (KW_REGISTER(0x1504) + (x * 0x08)) + +#define KW_REG_CPUCS_WIN_ENABLE (1 << 0) +#define KW_REG_CPUCS_WIN_WR_PROTECT (1 << 1) +#define KW_REG_CPUCS_WIN_WIN0_CS(x) (((x) & 0x3) << 2) +#define KW_REG_CPUCS_WIN_SIZE(x) (((x) & 0xff) << 24) + /* * kw_sdram_bar - reads SDRAM Base Address Register */ @@ -62,6 +68,23 @@ u32 kw_sdram_bs(enum memory_bank bank) return result; } +/* + * kw_sdram_bs_set - writes SDRAM Bank size + */ +void kw_sdram_bs_set(enum memory_bank bank, u32 size) +{ + /* Read current register value */ + u32 reg = readl(KW_REG_CPUCS_WIN_SZ(bank)); + + /* Clear window size */ + reg &= ~KW_REG_CPUCS_WIN_SIZE(0xFF); + + /* Set new window size */ + reg |= KW_REG_CPUCS_WIN_SIZE((size - 1) >> 24); + + writel(reg, KW_REG_CPUCS_WIN_SZ(bank)); +} + #ifndef CONFIG_SYS_BOARD_DRAM_INIT int dram_init(void) { diff --git a/arch/arm/include/asm/arch-kirkwood/cpu.h b/arch/arm/include/asm/arch-kirkwood/cpu.h index d28c51a..807154e 100644 --- a/arch/arm/include/asm/arch-kirkwood/cpu.h +++ b/arch/arm/include/asm/arch-kirkwood/cpu.h @@ -159,6 +159,8 @@ void reset_cpu(unsigned long ignored); unsigned char get_random_hex(void); unsigned int kw_sdram_bar(enum memory_bank bank); unsigned int kw_sdram_bs(enum memory_bank bank); +void kw_sdram_bs_set(enum memory_bank bank, u32 size); + int kw_config_adr_windows(void); void kw_config_gpio(unsigned int gpp0_oe_val, unsigned int gpp1_oe_val, unsigned int gpp0_oe, unsigned int gpp1_oe);