Hi Simon,
Thanks for review comments. Please find my responses below.
Thanks & Regards Amarendra Reddy
On 10 January 2013 22:27, Simon Glass sjg@chromium.org wrote:
Hi Amar,
On Fri, Jan 4, 2013 at 1:34 AM, Amar amarendra.xt@samsung.com wrote:
This patch enables and initialises DWMMC for SMDK5250. Supports both FDT and non-FDT. This patch creates a new file 'exynos5-dt.c' meant for FDT support. exynos5-dt.c: This file shall contain all code which supports
FDT.
Any addition of FDT support for any module needsto be
added in this file. smdk5250.c: This file shall contain the code which supportsnon-FDT.
version. Any addition of non-FDT support for anymodule
needs to be added in this file. May be, the file smdk5250.c can be removed innear future
when non-FDT is not required.The Makefile is updated to compile only one of the files exynos5-dt.c / smdk5250.c based on FDT configuration.
NOTE: Please note that all additions corresponding to FDT need to be added
into the
file exynos5-dt.c. At same time if non-FDT support is required then add the corresponding updations into smdk5250.c.
Changes from V1: 1)A new file 'exynos5-dt.c' is created meant for FDT support 2)Makefile is updated to compile only one of the files exynos5-dt.c / smdk5250.c based on FDT configuration
Changes from V2: 1)Updation of commit message and resubmition of proper patch set.
Changes from V3: No change.
Signed-off-by: Amar amarendra.xt@samsung.com
board/samsung/smdk5250/Makefile | 4 + board/samsung/smdk5250/exynos5-dt.c | 242
++++++++++++++++++++++++++++++++++++
board/samsung/smdk5250/smdk5250.c | 97 +++++++-------- include/configs/exynos5250-dt.h | 2 + include/i2c.h | 2 + 5 files changed, 292 insertions(+), 55 deletions(-) create mode 100644 board/samsung/smdk5250/exynos5-dt.c
diff --git a/board/samsung/smdk5250/Makefile
b/board/samsung/smdk5250/Makefile
index 47c6a5a..ecca9f3 100644 --- a/board/samsung/smdk5250/Makefile +++ b/board/samsung/smdk5250/Makefile @@ -32,8 +32,12 @@ COBJS += tzpc_init.o COBJS += smdk5250_spl.o
ifndef CONFIG_SPL_BUILD +ifdef CONFIG_OF_CONTROL +COBJS += exynos5-dt.o +else COBJS += smdk5250.o endif +endif
ifdef CONFIG_SPL_BUILD COBJS += spl_boot.o diff --git a/board/samsung/smdk5250/exynos5-dt.c
b/board/samsung/smdk5250/exynos5-dt.c
new file mode 100644 index 0000000..da539ca --- /dev/null +++ b/board/samsung/smdk5250/exynos5-dt.c @@ -0,0 +1,242 @@ +/*
- Copyright (C) 2012 Samsung Electronics
- 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., 59 Temple Place, Suite 330, Boston,
- MA 02111-1307 USA
- */
+#include <common.h> +#include <fdtdec.h> +#include <asm/io.h> +#include <i2c.h> +#include <netdev.h> +#include <spi.h> +#include <asm/arch/cpu.h> +#include <asm/arch/dwmmc.h> +#include <asm/arch/gpio.h> +#include <asm/arch/mmc.h> +#include <asm/arch/pinmux.h> +#include <asm/arch/sromc.h> +#include <power/pmic.h>
+DECLARE_GLOBAL_DATA_PTR;
+int board_init(void) +{
gd->bd->bi_boot_params = (PHYS_SDRAM_1 + 0x100UL);+#ifdef CONFIG_EXYNOS_SPI
spi_init();+#endif
return 0;+}
+int dram_init(void) +{
gd->ram_size = get_ram_size((long *)PHYS_SDRAM_1,PHYS_SDRAM_1_SIZE)
+ get_ram_size((long *)PHYS_SDRAM_2,PHYS_SDRAM_2_SIZE)
+ get_ram_size((long *)PHYS_SDRAM_3,PHYS_SDRAM_3_SIZE)
+ get_ram_size((long *)PHYS_SDRAM_4,PHYS_SDRAM_4_SIZE)
+ get_ram_size((long *)PHYS_SDRAM_5,PHYS_SDRAM_7_SIZE)
+ get_ram_size((long *)PHYS_SDRAM_6,PHYS_SDRAM_7_SIZE)
+ get_ram_size((long *)PHYS_SDRAM_7,PHYS_SDRAM_7_SIZE)
+ get_ram_size((long *)PHYS_SDRAM_8,PHYS_SDRAM_8_SIZE);
This looks ugly - is there any other way of doing this? Also 7 appears in more than one line.
Since the banks are all SDRAM_BANK_SIZE apart, perhaps you could just use a for loop with a single base address?
If this function is common with the other file then perhaps it should go in a common file?
In fact, this file "exynos5-dt.c" has been created for FDT support. Existing code from "smdk5250.c" has been copied into "exynos5-dt.c". The above piece of code computing 'gd->ram_size = ' is also copied from smdk5250.c.
So, Is it required to do changes for existing code as well? Please comment.
return 0;+}
+#if defined(CONFIG_POWER) +int power_init_board(void) +{
if (pmic_init(I2C_PMIC))debug()
return -1;elsereturn 0;+} +#endif
+void dram_init_banksize(void) +{
gd->bd->bi_dram[0].start = PHYS_SDRAM_1;gd->bd->bi_dram[0].size = get_ram_size((long *)PHYS_SDRAM_1,PHYS_SDRAM_1_SIZE);
gd->bd->bi_dram[1].start = PHYS_SDRAM_2;gd->bd->bi_dram[1].size = get_ram_size((long *)PHYS_SDRAM_2,PHYS_SDRAM_2_SIZE);
gd->bd->bi_dram[2].start = PHYS_SDRAM_3;gd->bd->bi_dram[2].size = get_ram_size((long *)PHYS_SDRAM_3,PHYS_SDRAM_3_SIZE);
gd->bd->bi_dram[3].start = PHYS_SDRAM_4;gd->bd->bi_dram[3].size = get_ram_size((long *)PHYS_SDRAM_4,PHYS_SDRAM_4_SIZE);
gd->bd->bi_dram[4].start = PHYS_SDRAM_5;gd->bd->bi_dram[4].size = get_ram_size((long *)PHYS_SDRAM_5,PHYS_SDRAM_5_SIZE);
gd->bd->bi_dram[5].start = PHYS_SDRAM_6;gd->bd->bi_dram[5].size = get_ram_size((long *)PHYS_SDRAM_6,PHYS_SDRAM_6_SIZE);
gd->bd->bi_dram[6].start = PHYS_SDRAM_7;gd->bd->bi_dram[6].size = get_ram_size((long *)PHYS_SDRAM_7,PHYS_SDRAM_7_SIZE);
gd->bd->bi_dram[7].start = PHYS_SDRAM_8;gd->bd->bi_dram[7].size = get_ram_size((long *)PHYS_SDRAM_8,PHYS_SDRAM_8_SIZE);
and here
+}
+static int decode_sromc(const void *blob, struct fdt_sromc *config) +{
int err;int node;node = fdtdec_next_compatible(blob, 0,COMPAT_SAMSUNG_EXYNOS5_SROMC);
if (node < 0) {debug("Could not find SROMC node\n");return node;}config->bank = fdtdec_get_int(blob, node, "bank", 0);config->width = fdtdec_get_int(blob, node, "width", 2);err = fdtdec_get_int_array(blob, node, "srom-timing",config->timing,
FDT_SROM_TIMING_COUNT);if (err < 0) {debug("Could not decode SROMC configuration\n");Suggest:
debug("Could not decode SROMC configuration: %s\n", fdt_strerror(err));
return -FDT_ERR_NOTFOUND;return err? Or the caller might just want -1
}return 0;+}
+int board_eth_init(bd_t *bis) +{ +#ifdef CONFIG_SMC911X
u32 smc_bw_conf, smc_bc_conf;struct fdt_sromc config;fdt_addr_t base_addr;int node;node = decode_sromc(gd->fdt_blob, &config);if (node < 0) {debug("%s: Could not find sromc configuration\n",__func__);
return 0;}node = fdtdec_next_compatible(gd->fdt_blob, node,COMPAT_SMSC_LAN9215);
if (node < 0) {debug("%s: Could not find lan9215 configuration\n",__func__);
return 0;}/* We now have a node, so any problems from now on are errors */base_addr = fdtdec_get_addr(gd->fdt_blob, node, "reg");if (base_addr == FDT_ADDR_T_NONE) {debug("%s: Could not find lan9215 address\n", __func__);return -1;}/* Ethernet needs data bus width of 16 bits */if (config.width != 2) {debug("%s: Unsupported bus width %d\n", __func__,config.width);return -1;}smc_bw_conf = SROMC_DATA16_WIDTH(config.bank)| SROMC_BYTE_ENABLE(config.bank);smc_bc_conf = SROMC_BC_TACS(config.timing[FDT_SROM_TACS]) |\Can you remove the \ from each line?
SROMC_BC_TCOS(config.timing[FDT_SROM_TCOS]) |\SROMC_BC_TACC(config.timing[FDT_SROM_TACC]) |\SROMC_BC_TCOH(config.timing[FDT_SROM_TCOH]) |\SROMC_BC_TAH(config.timing[FDT_SROM_TAH]) |\SROMC_BC_TACP(config.timing[FDT_SROM_TACP]) |\SROMC_BC_PMC(config.timing[FDT_SROM_PMC]);/* Select and configure the SROMC bank */exynos_pinmux_config(PERIPH_ID_SROMC, config.bank);s5p_config_sromc(config.bank, smc_bw_conf, smc_bc_conf);return smc911x_initialize(0, base_addr);+#endif
return 0;+}
+#ifdef CONFIG_DISPLAY_BOARDINFO +int checkboard(void) +{
printf("\nBoard: SMDK5250\n");return 0;+} +#endif
+#ifdef CONFIG_GENERIC_MMC +int board_mmc_init(bd_t *bis) +{
int ret = 0;Remove =0
/* dwmmc initializattion for available channels */ret = exynos_dwmmc_init(gd->fdt_blob);if (ret)debug("dwmmc init failed\n");return ret;+} +#endif
+static int board_uart_init(void) +{
int err;err = exynos_pinmux_config(PERIPH_ID_UART0, PINMUX_FLAG_NONE);if (err) {debug("UART0 not configured\n");return err;}err = exynos_pinmux_config(PERIPH_ID_UART1, PINMUX_FLAG_NONE);if (err) {debug("UART1 not configured\n");return err;}err = exynos_pinmux_config(PERIPH_ID_UART2, PINMUX_FLAG_NONE);if (err) {debug("UART2 not configured\n");return err;}err = exynos_pinmux_config(PERIPH_ID_UART3, PINMUX_FLAG_NONE);if (err) {debug("UART3 not configured\n");return err;}Loop for this?
return 0;+}
+#ifdef CONFIG_BOARD_EARLY_INIT_F +int board_early_init_f(void) +{
int err;blank line
err = board_uart_init();if (err) {debug("UART init failed\n");return err;}+#ifdef CONFIG_SYS_I2C_INIT_BOARD
board_i2c_init(gd->fdt_blob);+#endif
return err;+} +#endif diff --git a/board/samsung/smdk5250/smdk5250.c
b/board/samsung/smdk5250/smdk5250.c
index 73c3ec0..e0fec11 100644 --- a/board/samsung/smdk5250/smdk5250.c +++ b/board/samsung/smdk5250/smdk5250.c @@ -27,6 +27,7 @@ #include <netdev.h> #include <spi.h> #include <asm/arch/cpu.h> +#include <asm/arch/dwmmc.h> #include <asm/arch/gpio.h> #include <asm/arch/mmc.h> #include <asm/arch/pinmux.h> @@ -95,59 +96,13 @@ void dram_init_banksize(void)
PHYS_SDRAM_8_SIZE);
}
-#ifdef CONFIG_OF_CONTROL -static int decode_sromc(const void *blob, struct fdt_sromc *config) -{
int err;int node;node = fdtdec_next_compatible(blob, 0,COMPAT_SAMSUNG_EXYNOS5_SROMC);
if (node < 0) {debug("Could not find SROMC node\n");return node;}config->bank = fdtdec_get_int(blob, node, "bank", 0);config->width = fdtdec_get_int(blob, node, "width", 2);err = fdtdec_get_int_array(blob, node, "srom-timing",config->timing,
FDT_SROM_TIMING_COUNT);if (err < 0) {debug("Could not decode SROMC configuration\n");return -FDT_ERR_NOTFOUND;}return 0;-} -#endif
int board_eth_init(bd_t *bis) { #ifdef CONFIG_SMC911X u32 smc_bw_conf, smc_bc_conf; struct fdt_sromc config; fdt_addr_t base_addr;
int node;-#ifdef CONFIG_OF_CONTROL
node = decode_sromc(gd->fdt_blob, &config);if (node < 0) {debug("%s: Could not find sromc configuration\n",__func__);
return 0;}node = fdtdec_next_compatible(gd->fdt_blob, node,COMPAT_SMSC_LAN9215);
if (node < 0) {debug("%s: Could not find lan9215 configuration\n",__func__);
return 0;}/* We now have a node, so any problems from now on are errors */base_addr = fdtdec_get_addr(gd->fdt_blob, node, "reg");if (base_addr == FDT_ADDR_T_NONE) {debug("%s: Could not find lan9215 address\n", __func__);return -1;}-#else /* Non-FDT configuration - bank number and timing parameters*/ config.bank = CONFIG_ENV_SROM_BANK; config.width = 2; @@ -160,7 +115,6 @@ int board_eth_init(bd_t *bis) config.timing[FDT_SROM_TACP] = 0x09; config.timing[FDT_SROM_PMC] = 0x01; base_addr = CONFIG_SMC911X_BASE; -#endif
/* Ethernet needs data bus width of 16 bits */ if (config.width != 2) {@@ -199,16 +153,31 @@ int checkboard(void) #ifdef CONFIG_GENERIC_MMC int board_mmc_init(bd_t *bis) {
int err;
int err = 0, ret = 0; err = exynos_pinmux_config(PERIPH_ID_SDMMC0,PINMUX_FLAG_8BIT_MODE);
if (err) {
if (err) debug("SDMMC0 not configured\n");
return err;}err = s5p_mmc_init(0, 8);return err;
ret |= err;/*EMMC: dwmmc Channel-0 with 8 bit bus width */err = exynos_dwmmc_init(0, 8);This is not really init of the whole dwmmc, only a port - suggest exynos_dwmmc_add_port() or similar
Instead of calling exynos_dwmmc_add_port() here, I shall call exynos_dwmmc_init(*NULL*) here, as this is a non-FDT case. Inside the function exynos_dwmmc_init( * blob) { #ifdef CONFIG_OF_CONTROL
/* Read data from FDT */
exynos_dwmmc_add_port(index, bus_width, ...)
#else
exynos_dwmmc_add_port(0,8...)
exynos_dwmmc_add_port(2,4...)
#endif }
Please comment on the above.
if (err)debug("dwmmc Channel-0 init failed\n");ret |= err;err = exynos_pinmux_config(PERIPH_ID_SDMMC2, PINMUX_FLAG_NONE);if (err)debug("SDMMC2 not configured\n");ret |= err;/*SD: dwmmc Channel-2 with 4 bit bus width */err = exynos_dwmmc_init(2, 4);if (err)debug("dwmmc Channel-2 init failed\n");ret |= err;return ret;} #endif
@@ -243,6 +212,24 @@ static int board_uart_init(void) return 0; }
+#ifdef CONFIG_SYS_I2C_INIT_BOARD +static int board_i2c_init(void) +{
int i, err;for (i = 0; i < CONFIG_MAX_I2C_NUM; i++) {err = exynos_pinmux_config((PERIPH_ID_I2C0 + i),PINMUX_FLAG_NONE);if (err) {debug("I2C%d not configured\n", (PERIPH_ID_I2C0
- i));
return err;}}i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);return 0;+} +#endif
#ifdef CONFIG_BOARD_EARLY_INIT_F int board_early_init_f(void) { @@ -253,7 +240,7 @@ int board_early_init_f(void) return err; } #ifdef CONFIG_SYS_I2C_INIT_BOARD
board_i2c_init(gd->fdt_blob);
board_i2c_init();#endif return err; } diff --git a/include/configs/exynos5250-dt.h
b/include/configs/exynos5250-dt.h
index 59182f4..6ce73dc 100644 --- a/include/configs/exynos5250-dt.h +++ b/include/configs/exynos5250-dt.h @@ -84,6 +84,8 @@ #define CONFIG_MMC #define CONFIG_SDHCI #define CONFIG_S5P_SDHCI +#define CONFIG_DWMMC +#define CONFIG_EXYNOS_DWMMC
#define CONFIG_BOARD_EARLY_INIT_F
diff --git a/include/i2c.h b/include/i2c.h index c60d075..0944141 100644 --- a/include/i2c.h +++ b/include/i2c.h @@ -263,6 +263,7 @@ extern int get_multi_sda_pin(void); extern int multi_i2c_init(void); #endif
+#ifdef CONFIG_OF_CONTROL /**
- Get FDT values for i2c bus.
@@ -270,6 +271,7 @@ extern int multi_i2c_init(void);
- @return the number of I2C bus
*/ void board_i2c_init(const void *blob); +#endif
Do you need this #ifdef? It would be better to avoid having the same function with a different signature.
OK. Shall take care in next patch set.
i) call board_i2c_init(NULL) in case of non-FDT. ii) call board_i2c_init(const void *blob) in case of FDT.
/**
- Find the I2C bus number by given a FDT I2C node.
-- 1.8.0
Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot