KW: Move the memory register definitions into kirkwood.h

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: 27 June 2012 17:33
To: u-boot at lists.denx.de
Cc: Marek Vasut; Prafulla Wadaskar; Wolfgang Denk
Subject: [PATCH 2/2 V2] Kirkwood: Add support for Ka-Ro TK71

Signed-off-by Marek Vasut <[email protected]>

Cc Prafulla Wadaskar <[email protected]>

Cc Wolfgang Denk <[email protected]>

---
 board/karo/tk71/Makefile     |   45 +++++++++++
 board/karo/tk71/kwbimage.cfg |  174
++++++++++++++++++++++++++++++++++++++++++
 board/karo/tk71/tk71.c       |  174
++++++++++++++++++++++++++++++++++++++++++
 boards.cfg                   |    1 +
 include/configs/tk71.h       |  130 +++++++++++++++++++++++++++++++
 5 files changed, 524 insertions(+)
 create mode 100644 board/karo/tk71/Makefile
 create mode 100644 board/karo/tk71/kwbimage.cfg
 create mode 100644 board/karo/tk71/tk71.c
 create mode 100644 include/configs/tk71.h

V2: Enable hush parser

-----Original Message-----
From: Prafulla Wadaskar
Sent: 03 July 2012 17:14
To: 'Marek Vasut'; u-boot at lists.denx.de
Cc: Wolfgang Denk
Subject: RE: [PATCH 2/2 V2] Kirkwood: Add support for Ka-Ro TK71

Quoted 24 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: 27 June 2012 17:33
To: u-boot at lists.denx.de
Cc: Marek Vasut; Prafulla Wadaskar; Wolfgang Denk
Subject: [PATCH 2/2 V2] Kirkwood: Add support for Ka-Ro TK71

Signed-off-by Marek Vasut <[email protected]>

Cc Prafulla Wadaskar <[email protected]>

Cc Wolfgang Denk <[email protected]>

---
 board/karo/tk71/Makefile     |   45 +++++++++++
 board/karo/tk71/kwbimage.cfg |  174
++++++++++++++++++++++++++++++++++++++++++
 board/karo/tk71/tk71.c       |  174
++++++++++++++++++++++++++++++++++++++++++
 boards.cfg                   |    1 +
 include/configs/tk71.h       |  130 +++++++++++++++++++++++++++++++
 5 files changed, 524 insertions(+)
 create mode 100644 board/karo/tk71/Makefile
 create mode 100644 board/karo/tk71/kwbimage.cfg
 create mode 100644 board/karo/tk71/tk71.c
 create mode 100644 include/configs/tk71.h

Quoted 2 lines

V2: Enable hush parser

Applied to u-boot-marvell.git master branch

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: 27 June 2012 17:33
To: u-boot at lists.denx.de
Cc: Marek Vasut; Prafulla Wadaskar; Wolfgang Denk
Subject: [PATCH 2/2 V2] Kirkwood: Add support for Ka-Ro TK71

Signed-off-by Marek Vasut <[email protected]>

Cc Prafulla Wadaskar <[email protected]>

Cc Wolfgang Denk <[email protected]>

---
 board/karo/tk71/Makefile     |   45 +++++++++++
 board/karo/tk71/kwbimage.cfg |  174
++++++++++++++++++++++++++++++++++++++++++
 board/karo/tk71/tk71.c       |  174
++++++++++++++++++++++++++++++++++++++++++
 boards.cfg                   |    1 +
 include/configs/tk71.h       |  130 +++++++++++++++++++++++++++++++
 5 files changed, 524 insertions(+)
 create mode 100644 board/karo/tk71/Makefile
 create mode 100644 board/karo/tk71/kwbimage.cfg
 create mode 100644 board/karo/tk71/tk71.c
 create mode 100644 include/configs/tk71.h

V2: Enable hush parser

Quoted 29 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: 27 June 2012 17:33
To: u-boot at lists.denx.de
Cc: Marek Vasut; Prafulla Wadaskar; Wolfgang Denk
Subject: [PATCH 2/2 V2] Kirkwood: Add support for Ka-Ro TK71

Signed-off-by Marek Vasut <[email protected]>

Cc Prafulla Wadaskar <[email protected]>

Cc Wolfgang Denk <[email protected]>

---

 board/karo/tk71/Makefile     |   45 +++++++++++
 board/karo/tk71/kwbimage.cfg |  174

++++++++++++++++++++++++++++++++++++++++++

 board/karo/tk71/tk71.c       |  174

++++++++++++++++++++++++++++++++++++++++++

 boards.cfg                   |    1 +
 include/configs/tk71.h       |  130 +++++++++++++++++++++++++++++++
 5 files changed, 524 insertions(+)
 create mode 100644 board/karo/tk71/Makefile
 create mode 100644 board/karo/tk71/kwbimage.cfg
 create mode 100644 board/karo/tk71/tk71.c
 create mode 100644 include/configs/tk71.h

Quoted 1 line

V2: Enable hush parser

Hi Marek
This patch give build error against u-boot-marvell.git master branch since kw_mpp_config() API is changes

Also please consider the feedback from
Gerlando Falauto - June 29, 2012, 10:27 a.m.
Which even I find it more cleaner approach

Ref : http://patchwork.ozlabs.org/patch/167436/

Pls kindly fix it and repost

Regards...
Prafulla . . .

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: 03 July 2012 17:58
To: u-boot at lists.denx.de
Cc: Marek Vasut; Prafulla Wadaskar; Wolfgang Denk
Subject: [PATCH V3 2/2] Kirkwood: Add support for Ka-Ro TK71

Signed-off-by Marek Vasut <[email protected]>

Cc Prafulla Wadaskar <[email protected]>

Cc Wolfgang Denk <[email protected]>

board/karo/tk71/tk71.c new file +40 -0

new file mode 100644
index 0000000..50b563e
@@ -0,0 +1,174 @@
+/*
+ * Copyright (C) 2012 Marek Vasut <[email protected]>
+ * on behalf of DENX Software Engineering GmbH
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301 USA
+ */
+
+#include <common.h>
+#include <miiphy.h>
+#include <asm/arch/cpu.h>
+#include <asm/arch/kirkwood.h>
+#include <asm/arch/mpp.h>
+#include <asm/io.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+#define TK71_OE_LOW			(~0)
+#define TK71_OE_HIGH			(~0)
+#define TK71_OE_VAL_LOW			(0)
+#define TK71_OE_VAL_HIGH		(0)
+
+int board_early_init_f(void)
+{
+	unsigned long size = get_ram_size(PHYS_SDRAM_1,

PHYS_SDRAM_1_SIZE);

Quoted 10 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: 03 July 2012 17:58
To: u-boot at lists.denx.de
Cc: Marek Vasut; Prafulla Wadaskar; Wolfgang Denk
Subject: [PATCH V3 2/2] Kirkwood: Add support for Ka-Ro TK71

Signed-off-by Marek Vasut <[email protected]>

Cc Prafulla Wadaskar <[email protected]>

Cc Wolfgang Denk <[email protected]>

...snip...

Quoted 49 lines

board/karo/tk71/tk71.c new file +40 -0

new file mode 100644
index 0000000..50b563e
@@ -0,0 +1,174 @@
+/*
+ * Copyright (C) 2012 Marek Vasut <[email protected]>
+ * on behalf of DENX Software Engineering GmbH
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301 USA
+ */
+
+#include <common.h>
+#include <miiphy.h>
+#include <asm/arch/cpu.h>
+#include <asm/arch/kirkwood.h>
+#include <asm/arch/mpp.h>
+#include <asm/io.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+#define TK71_OE_LOW			(~0)
+#define TK71_OE_HIGH			(~0)
+#define TK71_OE_VAL_LOW			(0)
+#define TK71_OE_VAL_HIGH		(0)
+
+int board_early_init_f(void)
+{
+	unsigned long size = get_ram_size(PHYS_SDRAM_1,

PHYS_SDRAM_1_SIZE);

board/karo/tk71/tk71.c new file +2 -0

+
+	/* 256MB module, adjust BAR register */

board/karo/tk71/tk71.c line 46

Quoted 4 lines

board/karo/tk71/tk71.c new file +4 -0

+	if (size == 256 * 1024 * 1024) {
+		writel(KW_REG_CPUCS_WIN_ENABLE | KW_REG_CPUCS_WIN_SIZE(0xf),
+			KW_REG_CPUCS_WIN_SZ(0));
+	}

Hi Marek
This is first patch to support this board series, so ideally you don't need further tuning of DRAM configuration, if you address it in kwbimage.cfg

You may think of this approach if you have similar board support in future.

Please post the patch w/o updating DRAM registers or

Let's do it in cleaner way.

macros are NOT encouraged to use for register definition.

Regards...
Prafulla . . .

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: 03 July 2012 18:21
To: Prafulla Wadaskar
Cc: u-boot at lists.denx.de; Wolfgang Denk
Subject: Re: [PATCH V3 2/2] Kirkwood: Add support for Ka-Ro TK71

Dear Prafulla Wadaskar,

Quoted 24 lines

Quoted 10 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: 03 July 2012 17:58
To: u-boot at lists.denx.de
Cc: Marek Vasut; Prafulla Wadaskar; Wolfgang Denk
Subject: [PATCH V3 2/2] Kirkwood: Add support for Ka-Ro TK71

Signed-off-by Marek Vasut <[email protected]>

Cc Prafulla Wadaskar <[email protected]>

Cc Wolfgang Denk <[email protected]>

...snip...

Quoted 11 lines

board/karo/tk71/tk71.c new file +5 -0

new file mode 100644
index 0000000..50b563e
@@ -0,0 +1,174 @@
+/*
+ * Copyright (C) 2012 Marek Vasut <[email protected]>
+ * on behalf of DENX Software Engineering GmbH
+ *
+ * See file CREDITS for list of people who contributed to this

Quoted 8 lines

board/karo/tk71/tk71.c

Quoted 8 lines

board/karo/tk71/tk71.c new file +8 -0

+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be

useful,

Quoted 5 lines

board/karo/tk71/tk71.c

Quoted 5 lines

board/karo/tk71/tk71.c new file +5 -0

+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public

License

Quoted 19 lines

Quoted 19 lines

board/karo/tk71/tk71.c new file +19 -0

+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301 USA
+ */
+
+#include <common.h>
+#include <miiphy.h>
+#include <asm/arch/cpu.h>
+#include <asm/arch/kirkwood.h>
+#include <asm/arch/mpp.h>
+#include <asm/io.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+#define TK71_OE_LOW			(~0)
+#define TK71_OE_HIGH			(~0)
+#define TK71_OE_VAL_LOW			(0)
+#define TK71_OE_VAL_HIGH		(0)
+

Quoted 8 lines

board/karo/tk71/tk71.c

Quoted 8 lines

board/karo/tk71/tk71.c new file +3 -0

+int board_early_init_f(void)
+{
+	unsigned long size = get_ram_size(PHYS_SDRAM_1,

PHYS_SDRAM_1_SIZE);

board/karo/tk71/tk71.c new file +4 -0

+
+	/* 256MB module, adjust BAR register */
+	if (size == 256 * 1024 * 1024) {
+		writel(KW_REG_CPUCS_WIN_ENABLE |

KW_REG_CPUCS_WIN_SIZE(0xf),

Quoted 5 lines

board/karo/tk71/tk71.c line 46

Quoted 2 lines

board/karo/tk71/tk71.c new file +2 -0

+			KW_REG_CPUCS_WIN_SZ(0));
+	}

Hi Marek
This is first patch to support this board series, so ideally you

don't need

Quoted 1 line

further tuning of DRAM configuration, if you address it in

kwbimage.cfg

What do you mean adress is in kwbimage.cfg ?

Quoted 1 line

You may think of this approach if you have similar board support in

future.

Quoted 2 lines

Please post the patch w/o updating DRAM registers or

How exactly would you support the 256MB variant?

Quoted 1 line

Let's do it in cleaner way.

I already asked -- did you get anywhere with the cleaner approach? It's not even
discussed properly yet, let alone implemented.

Quoted 2 lines

macros are NOT encouraged to use for register definition.

What do you mean?

Quoted 104 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: 03 July 2012 18:21
To: Prafulla Wadaskar
Cc: u-boot at lists.denx.de; Wolfgang Denk
Subject: Re: [PATCH V3 2/2] Kirkwood: Add support for Ka-Ro TK71

Dear Prafulla Wadaskar,

Quoted 24 lines

Quoted 10 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: 03 July 2012 17:58
To: u-boot at lists.denx.de
Cc: Marek Vasut; Prafulla Wadaskar; Wolfgang Denk
Subject: [PATCH V3 2/2] Kirkwood: Add support for Ka-Ro TK71

Signed-off-by Marek Vasut <[email protected]>

Cc Prafulla Wadaskar <[email protected]>

Cc Wolfgang Denk <[email protected]>

...snip...

Quoted 11 lines

board/karo/tk71/tk71.c new file +5 -0

new file mode 100644
index 0000000..50b563e
@@ -0,0 +1,174 @@
+/*
+ * Copyright (C) 2012 Marek Vasut <[email protected]>
+ * on behalf of DENX Software Engineering GmbH
+ *
+ * See file CREDITS for list of people who contributed to this

Quoted 8 lines

board/karo/tk71/tk71.c

Quoted 8 lines

board/karo/tk71/tk71.c new file +8 -0

+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be

useful,

Quoted 5 lines

board/karo/tk71/tk71.c

Quoted 5 lines

board/karo/tk71/tk71.c new file +5 -0

+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public

License

Quoted 19 lines

Quoted 19 lines

board/karo/tk71/tk71.c new file +19 -0

+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301 USA
+ */
+
+#include <common.h>
+#include <miiphy.h>
+#include <asm/arch/cpu.h>
+#include <asm/arch/kirkwood.h>
+#include <asm/arch/mpp.h>
+#include <asm/io.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+#define TK71_OE_LOW			(~0)
+#define TK71_OE_HIGH			(~0)
+#define TK71_OE_VAL_LOW			(0)
+#define TK71_OE_VAL_HIGH		(0)
+

Quoted 8 lines

board/karo/tk71/tk71.c

Quoted 8 lines

board/karo/tk71/tk71.c new file +3 -0

+int board_early_init_f(void)
+{
+	unsigned long size = get_ram_size(PHYS_SDRAM_1,

PHYS_SDRAM_1_SIZE);

board/karo/tk71/tk71.c new file +4 -0

+
+	/* 256MB module, adjust BAR register */
+	if (size == 256 * 1024 * 1024) {
+		writel(KW_REG_CPUCS_WIN_ENABLE |

KW_REG_CPUCS_WIN_SIZE(0xf),

Quoted 5 lines

board/karo/tk71/tk71.c line 46

Quoted 2 lines

board/karo/tk71/tk71.c new file +2 -0

+			KW_REG_CPUCS_WIN_SZ(0));
+	}

Hi Marek
This is first patch to support this board series, so ideally you

don't need

Quoted 1 line

further tuning of DRAM configuration, if you address it in

kwbimage.cfg

What do you mean adress is in kwbimage.cfg ?

Quoted 1 line

You may think of this approach if you have similar board support in

future.

Quoted 1 line

Please post the patch w/o updating DRAM registers or

How exactly would you support the 256MB variant?

Quoted 5 lines

Quoted 1 line

Let's do it in cleaner way.

I already asked -- did you get anywhere with the cleaner approach? It's not even
discussed properly yet, let alone implemented.

Let's discuss and address this first

Quoted 3 lines

Quoted 1 line

macros are NOT encouraged to use for register definition.

What do you mean?

KW_REGISTER(0x1504) + (x * 0x08) is not recommended, let's use c-struct for this.

Regards..
Prafulla . . .

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: 03 July 2012 18:32
To: u-boot at lists.denx.de
Cc: Marek Vasut; Prafulla Wadaskar; Wolfgang Denk
Subject: [PATCH 2/2 V4] Kirkwood: Add support for Ka-Ro TK71

Signed-off-by Marek Vasut <[email protected]>

Cc Prafulla Wadaskar <[email protected]>

Cc Wolfgang Denk <[email protected]>

---
 board/karo/tk71/Makefile     |   45 +++++++++++
 board/karo/tk71/kwbimage.cfg |  174
++++++++++++++++++++++++++++++++++++++++++
 board/karo/tk71/tk71.c       |  166
++++++++++++++++++++++++++++++++++++++++
 boards.cfg                   |    1 +
 include/configs/tk71.h       |  130 +++++++++++++++++++++++++++++++
 5 files changed, 516 insertions(+)
 create mode 100644 board/karo/tk71/Makefile
 create mode 100644 board/karo/tk71/kwbimage.cfg
 create mode 100644 board/karo/tk71/tk71.c
 create mode 100644 include/configs/tk71.h

V2: Enable hush parser
V3: Fixup kirkwood_mpp_conf() arguments
V4: Drop 256MB board variant support

Dear Maruk,

I am currently trying to address the exact same issue (SDRAM size detection and fixup).
My idea was however (as opposed to moving register definition as you did), to add a fixup function to dram.c, say:

/*
* kw_sdram_bs - 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));

	printf("Current value: %x\n", reg);
	/* Clear window size */
	reg &= ~KW_REG_CPUCS_WIN_SIZE(0xFF);

	/* Set new window size */
	reg |= KW_REG_CPUCS_WIN_SIZE((size - 1) >> 24);

	printf("Writing: %x\n", reg);
	writel(reg, KW_REG_CPUCS_WIN_SZ(bank));
}

which would then be called to fix the window size according to the total memory size as reported by get_ram_size().

What do you think?

Thank you,
Gerlando

Dear Gerlando Falauto,

Quoted 1 line

Dear Maruk,

Quoted 25 lines

I am currently trying to address the exact same issue (SDRAM size detection and fixup).
My idea was however (as opposed to moving register definition as you did), to add a fixup function to dram.c, say:

/*
* kw_sdram_bs - 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));

	printf("Current value: %x\n", reg);
	/* Clear window size */
	reg&= ~KW_REG_CPUCS_WIN_SIZE(0xFF);

	/* Set new window size */
	reg |= KW_REG_CPUCS_WIN_SIZE((size - 1)>>  24);

	printf("Writing: %x\n", reg);
	writel(reg, KW_REG_CPUCS_WIN_SZ(bank));
}

Quoted 2 lines

which would then be called to fix the window size according to the total memory size as reported by get_ram_size().

Read up at [1].

http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/133991

Dear Marek Vasut,

Quoted 3 lines · Marek Vasut

Dear Gerlando Falauto,

Quoted 1 line

Dear Maruk,

sorry about my misspelling!

Quoted 26 lines

Quoted 26 lines

I am currently trying to address the exact same issue (SDRAM size detection and fixup).
My idea was however (as opposed to moving register definition as you did), to add a fixup function to dram.c, say:

/*

* kw_sdram_bs - 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));
	
	printf("Current value: %x\n", reg);
	/* Clear window size */
	reg&= ~KW_REG_CPUCS_WIN_SIZE(0xFF);
	
	/* Set new window size */
	reg |= KW_REG_CPUCS_WIN_SIZE((size - 1)>>  24);
	
	printf("Writing: %x\n", reg);
	writel(reg, KW_REG_CPUCS_WIN_SZ(bank));

Quoted 8 lines

Quoted 4 lines

}

which would then be called to fix the window size according to the total memory size as reported by get_ram_size().

Read up at [1].

http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/133991

Did read up, not sure I understand.
If I get it right, *this* thread and patchset follows the feedback from the above thread.

What I am trying to say is: we have a function (available to all kirkwood boards) which reads the value of the window size register for a given memory bank and returns its size.
Now we want to do the opposite, write the window size register according to the detected SDRAM size.
Why should we make up our own (board-specific) register manipulation tweaks, as opposed to having a function do it for us in dram.c as its companion?
Reasons behind it, are board-specific, so in your case it would turn into:

(quoted diff) +7 -0

+int board_early_init_f(void)
+{
+	unsigned long size = get_ram_size(PHYS_SDRAM_1, PHYS_SDRAM_1_SIZE);
+
+	/* 256MB module, adjust BAR register */
+	if (size == 256 * 1024 * 1024)
+		kw_sdram_bs_set(0, size);

Which I find cleaner.
Even the if part could be also removed.
And would work as-is on any board needing it (ours, for instance).

What do you think?

Thank you for your patience,
Gerlando

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 <[email protected]>

Cc Marek Vasut <[email protected]>

Cc Prafulla Wadaskar <[email protected]>

Cc Wolfgang Denk <[email protected]>

Cc Valentin Longchamp <[email protected]>

Cc Holger Brunck <[email protected]>

---
 arch/arm/cpu/arm926ejs/kirkwood/dram.c   |   28

++++++++++++++++++++++++++-- arch/arm/include/asm/arch-kirkwood/cpu.h
| 2 ++ 2 files changed, 28 insertions(+), 2 deletions(-)

(quoted diff) +0 -0

b/arch/arm/cpu/arm926ejs/kirkwood/dram.c index 2441554..e5409f1 100644

arch/arm/cpu/arm926ejs/kirkwood/dram.c +1 -1

@@ -28,8 +28,15 @@
 
 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)) +/* Kirkwood memory registers */
+#define KW_REG_CPUCS_WIN_BAR(x) (KW_REGISTER(0x1500)
+ ((x) * 0x08)) +#define KW_REG_CPUCS_WIN_SZ(x) (KW_REGISTER(0x1504) + ((x) * 0x08)) +

arch/arm/cpu/arm926ejs/kirkwood/dram.c +22 -0

+#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
  */

@@ -60,6 +67,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)
 {

(quoted diff) +0 -0

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

Hi Gerlando,

Quoted 8 lines · 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().

I don't like isolated patches that seem to create dead code. I know here it is not the goal, of course; so why not submit a two-patch series, providing both the new code *and* a use for it?

Hi Albert,

Quoted 12 lines · Albert ARIBAUD

Hi Gerlando,

Quoted 8 lines · 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().

[...]

Quoted 1 line

Quoted 3 lines

I don't like isolated patches that seem to create dead code. I know here it is not the goal, of course; so why not submit a two-patch series, providing both the new code *and* a use for it?

You're absolutely right.
But mine was more of an interruption/suggestion throughout the discussion between Marek and Prafulla... I just didn't want to take credit for someone else's code, or create any conflicts with it.

I figured it might be quicker (and more fair) that way, perhaps with Marek resubmitting it within his own series, or SoB-ing it, because in fact mine is just a refactoring of his idea...
I can of course submit the two-patch series for our boards, if that's OK with Marek.

Thanks,
Gerlando

-----Original Message-----
From: Albert ARIBAUD [mailto:albert.u.boot at aribaud.net]
Sent: 05 July 2012 16:32
To: Gerlando Falauto
Cc: u-boot at lists.denx.de; Marek Vasut; Longchamp, Valentin; Brunck, Holger; Prafulla Wadaskar
Subject: Re: [U-Boot] [PATCH] kirkwood: implement kw_sdram_bs_set()

Hi Gerlando,

Quoted 8 lines · Gerlando Falauto

Hi Albert,

Quoted 5 lines · Albert ARIBAUD

Hi Gerlando,

Quoted 1 line · Gerlando Falauto

Some boards might be equipped with different SDRAM

configurations.

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

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

Quoted 1 line

Quoted 1 line

Quoted 1 line

value can be fixed at runtime according to the detected SDRAM

size.

Quoted 7 lines

Quoted 4 lines

Quoted 4 lines

Therefore, implement kw_sdram_bs_set(), to be called for instance within board_early_init_f().

[...]

Quoted 2 lines

I don't like isolated patches that seem to create dead code. I

know

Quoted 7 lines

Quoted 2 lines

here it is not the goal, of course; so why not submit a two-patch series, providing both the new code *and* a use for it?

You're absolutely right.
But mine was more of an interruption/suggestion throughout the discussion between Marek and Prafulla... I just didn't want to take credit for someone else's code, or create any conflicts with it.

Understood. Next time, please just add [RFC] in the subject, so that the patch is not considered for inclusion.

Quoted 4 lines

I figured it might be quicker (and more fair) that way, perhaps with Marek resubmitting it within his own series, or SoB-ing it, because in fact mine is just a refactoring of his idea...
I can of course submit the two-patch series for our boards, if

that's

Quoted 1 line

OK with Marek.

That's to be sorted between Marek and you. :)