[U-Boot] [PATCH 0/9] EXYNOS5: Enable dwmmc
This patch set enables and initialises dwmmc for Exynos5250 on SMDK5250. Adds driver changes required for dwmmc. Adds dt support for dwmmc. Adds eMMC booting feature for SMDK5250.
This patch set is based on: "EXYNOS: mmc: support DesignWare Controller for Samsung-SoC", which is merged in u-boot-mmc "Exynos: clock: support get_mmc_clk for exynos" "Add DT based ethernet driver for SMDK5250" "SMDK5250: Add FDT support"
Amar (9): FDT: Add compatible string for DWMMC EXYNOS5: FDT: Add DWMMC device node data DWMMC: Initialise dwmci and resolved the boot partition write issue EXYNOS5: DWMMC: Added dt support for DWMMC EXYNOS5: DWMMC: API to set mmc clock divisor SMDK5250: Enable DWMMC MMC: APIs to support creation of boot partition SMDK5250: Enable eMMC booting COMMON: MMC: Command to support eMMC booting
arch/arm/cpu/armv7/exynos/clock.c | 39 +++++++++- arch/arm/dts/exynos5250.dtsi | 32 ++++++++ arch/arm/include/asm/arch-exynos/clk.h | 1 + arch/arm/include/asm/arch-exynos/dwmmc.h | 4 + board/samsung/dts/exynos5250-smdk5250.dts | 24 ++++++ board/samsung/smdk5250/smdk5250.c | 36 ++++++++- board/samsung/smdk5250/spl_boot.c | 38 +++++++++- common/cmd_mmc.c | 101 ++++++++++++++++++++++++- drivers/mmc/dw_mmc.c | 12 +++- drivers/mmc/exynos_dw_mmc.c | 117 +++++++++++++++++++++++++++-- drivers/mmc/mmc.c | 118 +++++++++++++++++++++++++++++ include/configs/exynos5250-dt.h | 9 ++ include/dwmmc.h | 4 + include/fdtdec.h | 1 + include/mmc.h | 16 ++++ lib/fdtdec.c | 1 + 16 files changed, 538 insertions(+), 15 deletions(-)
Add required compatible information for DWMMC driver.
Signed-off-by: Amar amarendra.xt@samsung.com --- include/fdtdec.h | 1 + lib/fdtdec.c | 1 + 2 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/include/fdtdec.h b/include/fdtdec.h index 539bb1b..f09c281 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -74,6 +74,7 @@ enum fdt_compat_id { COMPAT_SAMSUNG_EXYNOS5_SOUND, /* Exynos Sound */ COMPAT_WOLFSON_WM8994_CODEC, /* Wolfson WM8994 Sound Codec */ COMPAT_SAMSUNG_EXYNOS_SPI, /* Exynos SPI */ + COMPAT_SAMSUNG_EXYNOS5_DWMMC, /* Exynos5 DWMMC controller */
COMPAT_COUNT, }; diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 44c249d..6597661 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -51,6 +51,7 @@ static const char * const compat_names[COMPAT_COUNT] = { COMPAT(SAMSUNG_EXYNOS5_SOUND, "samsung,exynos-sound"), COMPAT(WOLFSON_WM8994_CODEC, "wolfson,wm8994-codec"), COMPAT(SAMSUNG_EXYNOS_SPI, "samsung,exynos-spi"), + COMPAT(SAMSUNG_EXYNOS5_DWMMC, "samsung,exynos5250-dwmmc"), };
const char *fdtdec_get_compatible(enum fdt_compat_id id)
On Mon, Dec 17, 2012 at 3:19 AM, Amar amarendra.xt@samsung.com wrote:
Add required compatible information for DWMMC driver.
Signed-off-by: Amar amarendra.xt@samsung.com
Acked-by: Simon Glass sjg@chromium.org
include/fdtdec.h | 1 + lib/fdtdec.c | 1 + 2 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/include/fdtdec.h b/include/fdtdec.h index 539bb1b..f09c281 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -74,6 +74,7 @@ enum fdt_compat_id { COMPAT_SAMSUNG_EXYNOS5_SOUND, /* Exynos Sound */ COMPAT_WOLFSON_WM8994_CODEC, /* Wolfson WM8994 Sound Codec */ COMPAT_SAMSUNG_EXYNOS_SPI, /* Exynos SPI */
COMPAT_SAMSUNG_EXYNOS5_DWMMC, /* Exynos5 DWMMC controller */ COMPAT_COUNT,}; diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 44c249d..6597661 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -51,6 +51,7 @@ static const char * const compat_names[COMPAT_COUNT] = { COMPAT(SAMSUNG_EXYNOS5_SOUND, "samsung,exynos-sound"), COMPAT(WOLFSON_WM8994_CODEC, "wolfson,wm8994-codec"), COMPAT(SAMSUNG_EXYNOS_SPI, "samsung,exynos-spi"),
COMPAT(SAMSUNG_EXYNOS5_DWMMC, "samsung,exynos5250-dwmmc"),};
const char *fdtdec_get_compatible(enum fdt_compat_id id)
1.7.0.4
Add DWMMC device node data for exynos5
Signed-off-by: Amar amarendra.xt@samsung.com --- arch/arm/dts/exynos5250.dtsi | 32 +++++++++++++++++++++++++++++ board/samsung/dts/exynos5250-smdk5250.dts | 24 +++++++++++++++++++++ 2 files changed, 56 insertions(+), 0 deletions(-)
diff --git a/arch/arm/dts/exynos5250.dtsi b/arch/arm/dts/exynos5250.dtsi index 1008797..b701ae5 100644 --- a/arch/arm/dts/exynos5250.dtsi +++ b/arch/arm/dts/exynos5250.dtsi @@ -138,4 +138,36 @@ reg = <0x131b0000 0x30>; interrupts = <0 130 0>; }; + + dwmmc@12200000 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "samsung,exynos5250-dwmmc"; + reg = <0x12200000 0x1000>; + interrupts = <0 75 0>; + }; + + dwmmc@12210000 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "samsung,exynos5250-dwmmc"; + reg = <0x12210000 0x1000>; + interrupts = <0 76 0>; + }; + + dwmmc@12220000 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "samsung,exynos5250-dwmmc"; + reg = <0x12220000 0x1000>; + interrupts = <0 77 0>; + }; + + dwmmc@12230000 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "samsung,exynos5250-dwmmc"; + reg = <0x12230000 0x1000>; + interrupts = <0 78 0>; + }; }; diff --git a/board/samsung/dts/exynos5250-smdk5250.dts b/board/samsung/dts/exynos5250-smdk5250.dts index a8e62da..b1b8d71 100644 --- a/board/samsung/dts/exynos5250-smdk5250.dts +++ b/board/samsung/dts/exynos5250-smdk5250.dts @@ -30,6 +30,10 @@ spi2 = "/spi@12d40000"; spi3 = "/spi@131a0000"; spi4 = "/spi@131b0000"; + dwmmc0 = "/dwmmc@12200000"; + dwmmc1 = "/dwmmc@12210000"; + dwmmc2 = "/dwmmc@12220000"; + dwmmc3 = "/dwmmc@12230000"; };
sromc@12250000 { @@ -59,4 +63,24 @@ compatible = "wolfson,wm8994-codec"; }; }; + + dwmmc@12200000 { + index = <0>; + bus-width = <8>; + timing = <1 3 3>; + }; + + dwmmc@12210000 { + status = "disabled"; + }; + + dwmmc@12220000 { + index = <2>; + bus-width = <4>; + timing = <1 2 3>; + }; + + dwmmc@12230000 { + status = "disabled"; + }; };
Hi Amar,
On Mon, Dec 17, 2012 at 3:19 AM, Amar amarendra.xt@samsung.com wrote:
Add DWMMC device node data for exynos5
Signed-off-by: Amar amarendra.xt@samsung.com
arch/arm/dts/exynos5250.dtsi | 32 +++++++++++++++++++++++++++++ board/samsung/dts/exynos5250-smdk5250.dts | 24 +++++++++++++++++++++
Do you also have a binding file for this please?
2 files changed, 56 insertions(+), 0 deletions(-)
diff --git a/arch/arm/dts/exynos5250.dtsi b/arch/arm/dts/exynos5250.dtsi index 1008797..b701ae5 100644 --- a/arch/arm/dts/exynos5250.dtsi +++ b/arch/arm/dts/exynos5250.dtsi @@ -138,4 +138,36 @@ reg = <0x131b0000 0x30>; interrupts = <0 130 0>; };
dwmmc@12200000 {#address-cells = <1>;#size-cells = <0>;compatible = "samsung,exynos5250-dwmmc";reg = <0x12200000 0x1000>;interrupts = <0 75 0>;};dwmmc@12210000 {#address-cells = <1>;#size-cells = <0>;compatible = "samsung,exynos5250-dwmmc";reg = <0x12210000 0x1000>;interrupts = <0 76 0>;};dwmmc@12220000 {#address-cells = <1>;#size-cells = <0>;compatible = "samsung,exynos5250-dwmmc";reg = <0x12220000 0x1000>;interrupts = <0 77 0>;};dwmmc@12230000 {#address-cells = <1>;#size-cells = <0>;compatible = "samsung,exynos5250-dwmmc";reg = <0x12230000 0x1000>;interrupts = <0 78 0>;};}; diff --git a/board/samsung/dts/exynos5250-smdk5250.dts b/board/samsung/dts/exynos5250-smdk5250.dts index a8e62da..b1b8d71 100644 --- a/board/samsung/dts/exynos5250-smdk5250.dts +++ b/board/samsung/dts/exynos5250-smdk5250.dts @@ -30,6 +30,10 @@ spi2 = "/spi@12d40000"; spi3 = "/spi@131a0000"; spi4 = "/spi@131b0000";
dwmmc0 = "/dwmmc@12200000";dwmmc1 = "/dwmmc@12210000";dwmmc2 = "/dwmmc@12220000";dwmmc3 = "/dwmmc@12230000"; }; sromc@12250000 {@@ -59,4 +63,24 @@ compatible = "wolfson,wm8994-codec"; }; };
dwmmc@12200000 {index = <0>;
Do you really need the index? You have the numbering from the aliaes I think.
bus-width = <8>;timing = <1 3 3>;
Might need a "samsung," prefix on these?
};dwmmc@12210000 {status = "disabled";};dwmmc@12220000 {index = <2>;bus-width = <4>;timing = <1 2 3>;};dwmmc@12230000 {status = "disabled";};};
1.7.0.4
Regards, Simon
On 12/20/2012 10:55 AM, Simon Glass wrote:
Hi Amar,
On Mon, Dec 17, 2012 at 3:19 AM, Amar amarendra.xt@samsung.com wrote:
Add DWMMC device node data for exynos5
Signed-off-by: Amar amarendra.xt@samsung.com
arch/arm/dts/exynos5250.dtsi | 32 +++++++++++++++++++++++++++++ board/samsung/dts/exynos5250-smdk5250.dts | 24 +++++++++++++++++++++
Do you also have a binding file for this please?
2 files changed, 56 insertions(+), 0 deletions(-)
diff --git a/arch/arm/dts/exynos5250.dtsi b/arch/arm/dts/exynos5250.dtsi index 1008797..b701ae5 100644 --- a/arch/arm/dts/exynos5250.dtsi +++ b/arch/arm/dts/exynos5250.dtsi @@ -138,4 +138,36 @@ reg = <0x131b0000 0x30>; interrupts = <0 130 0>; };
dwmmc@12200000 {#address-cells = <1>;#size-cells = <0>;compatible = "samsung,exynos5250-dwmmc";reg = <0x12200000 0x1000>;interrupts = <0 75 0>;};dwmmc@12210000 {#address-cells = <1>;#size-cells = <0>;compatible = "samsung,exynos5250-dwmmc";reg = <0x12210000 0x1000>;interrupts = <0 76 0>;};dwmmc@12220000 {#address-cells = <1>;#size-cells = <0>;compatible = "samsung,exynos5250-dwmmc";reg = <0x12220000 0x1000>;interrupts = <0 77 0>;};dwmmc@12230000 {#address-cells = <1>;#size-cells = <0>;compatible = "samsung,exynos5250-dwmmc";reg = <0x12230000 0x1000>;interrupts = <0 78 0>;};}; diff --git a/board/samsung/dts/exynos5250-smdk5250.dts b/board/samsung/dts/exynos5250-smdk5250.dts index a8e62da..b1b8d71 100644 --- a/board/samsung/dts/exynos5250-smdk5250.dts +++ b/board/samsung/dts/exynos5250-smdk5250.dts @@ -30,6 +30,10 @@ spi2 = "/spi@12d40000"; spi3 = "/spi@131a0000"; spi4 = "/spi@131b0000";
dwmmc0 = "/dwmmc@12200000";dwmmc1 = "/dwmmc@12210000";dwmmc2 = "/dwmmc@12220000";dwmmc3 = "/dwmmc@12230000"; }; sromc@12250000 {@@ -59,4 +63,24 @@ compatible = "wolfson,wm8994-codec"; }; };
dwmmc@12200000 {index = <0>;Do you really need the index? You have the numbering from the aliaes I think.
bus-width = <8>;timing = <1 3 3>;Might need a "samsung," prefix on these?
I think better that use the "exynos" instead of "samsung". how about?
};dwmmc@12210000 {status = "disabled";};dwmmc@12220000 {index = <2>;bus-width = <4>;timing = <1 2 3>;};dwmmc@12230000 {status = "disabled";};};
1.7.0.4
Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi,
On Fri, Dec 21, 2012 at 1:19 AM, Jaehoon Chung jh80.chung@samsung.comwrote:
On 12/20/2012 10:55 AM, Simon Glass wrote:
Hi Amar,
On Mon, Dec 17, 2012 at 3:19 AM, Amar amarendra.xt@samsung.com wrote:
Add DWMMC device node data for exynos5
Signed-off-by: Amar amarendra.xt@samsung.com
arch/arm/dts/exynos5250.dtsi | 32
+++++++++++++++++++++++++++++
board/samsung/dts/exynos5250-smdk5250.dts | 24 +++++++++++++++++++++
Do you also have a binding file for this please?
2 files changed, 56 insertions(+), 0 deletions(-)
diff --git a/arch/arm/dts/exynos5250.dtsi b/arch/arm/dts/exynos5250.dtsi index 1008797..b701ae5 100644 --- a/arch/arm/dts/exynos5250.dtsi +++ b/arch/arm/dts/exynos5250.dtsi @@ -138,4 +138,36 @@ reg = <0x131b0000 0x30>; interrupts = <0 130 0>; };
dwmmc@12200000 {#address-cells = <1>;#size-cells = <0>;compatible = "samsung,exynos5250-dwmmc";reg = <0x12200000 0x1000>;interrupts = <0 75 0>;};dwmmc@12210000 {#address-cells = <1>;#size-cells = <0>;compatible = "samsung,exynos5250-dwmmc";reg = <0x12210000 0x1000>;interrupts = <0 76 0>;};dwmmc@12220000 {#address-cells = <1>;#size-cells = <0>;compatible = "samsung,exynos5250-dwmmc";reg = <0x12220000 0x1000>;interrupts = <0 77 0>;};dwmmc@12230000 {#address-cells = <1>;#size-cells = <0>;compatible = "samsung,exynos5250-dwmmc";reg = <0x12230000 0x1000>;interrupts = <0 78 0>;};}; diff --git a/board/samsung/dts/exynos5250-smdk5250.dts
b/board/samsung/dts/exynos5250-smdk5250.dts
index a8e62da..b1b8d71 100644 --- a/board/samsung/dts/exynos5250-smdk5250.dts +++ b/board/samsung/dts/exynos5250-smdk5250.dts @@ -30,6 +30,10 @@ spi2 = "/spi@12d40000"; spi3 = "/spi@131a0000"; spi4 = "/spi@131b0000";
dwmmc0 = "/dwmmc@12200000";dwmmc1 = "/dwmmc@12210000";dwmmc2 = "/dwmmc@12220000";dwmmc3 = "/dwmmc@12230000"; }; sromc@12250000 {@@ -59,4 +63,24 @@ compatible = "wolfson,wm8994-codec"; }; };
dwmmc@12200000 {index = <0>;Do you really need the index? You have the numbering from the aliaes I
think.
bus-width = <8>;timing = <1 3 3>;Might need a "samsung," prefix on these?
I think better that use the "exynos" instead of "samsung". how about?
Well I think you are supposed to use vendor,propname when you define your own private binding.
};dwmmc@12210000 {status = "disabled";};dwmmc@12220000 {index = <2>;bus-width = <4>;timing = <1 2 3>;};dwmmc@12230000 {status = "disabled";};};
1.7.0.4
Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi
On 22 December 2012 02:54, Simon Glass sjg@chromium.org wrote:
Hi,
On Fri, Dec 21, 2012 at 1:19 AM, Jaehoon Chung <jh80.chung@samsung.com
wrote:
On 12/20/2012 10:55 AM, Simon Glass wrote:
Hi Amar,
On Mon, Dec 17, 2012 at 3:19 AM, Amar amarendra.xt@samsung.com
wrote:
Add DWMMC device node data for exynos5
Signed-off-by: Amar amarendra.xt@samsung.com
arch/arm/dts/exynos5250.dtsi | 32
+++++++++++++++++++++++++++++
board/samsung/dts/exynos5250-smdk5250.dts | 24
+++++++++++++++++++++
Do you also have a binding file for this please?
2 files changed, 56 insertions(+), 0 deletions(-)
diff --git a/arch/arm/dts/exynos5250.dtsi
b/arch/arm/dts/exynos5250.dtsi
index 1008797..b701ae5 100644 --- a/arch/arm/dts/exynos5250.dtsi +++ b/arch/arm/dts/exynos5250.dtsi @@ -138,4 +138,36 @@ reg = <0x131b0000 0x30>; interrupts = <0 130 0>; };
dwmmc@12200000 {#address-cells = <1>;#size-cells = <0>;compatible = "samsung,exynos5250-dwmmc";reg = <0x12200000 0x1000>;interrupts = <0 75 0>;};dwmmc@12210000 {#address-cells = <1>;#size-cells = <0>;compatible = "samsung,exynos5250-dwmmc";reg = <0x12210000 0x1000>;interrupts = <0 76 0>;};dwmmc@12220000 {#address-cells = <1>;#size-cells = <0>;compatible = "samsung,exynos5250-dwmmc";reg = <0x12220000 0x1000>;interrupts = <0 77 0>;};dwmmc@12230000 {#address-cells = <1>;#size-cells = <0>;compatible = "samsung,exynos5250-dwmmc";reg = <0x12230000 0x1000>;interrupts = <0 78 0>;};}; diff --git a/board/samsung/dts/exynos5250-smdk5250.dts
b/board/samsung/dts/exynos5250-smdk5250.dts
index a8e62da..b1b8d71 100644 --- a/board/samsung/dts/exynos5250-smdk5250.dts +++ b/board/samsung/dts/exynos5250-smdk5250.dts @@ -30,6 +30,10 @@ spi2 = "/spi@12d40000"; spi3 = "/spi@131a0000"; spi4 = "/spi@131b0000";
dwmmc0 = "/dwmmc@12200000";dwmmc1 = "/dwmmc@12210000";dwmmc2 = "/dwmmc@12220000";dwmmc3 = "/dwmmc@12230000"; }; sromc@12250000 {@@ -59,4 +63,24 @@ compatible = "wolfson,wm8994-codec"; }; };
dwmmc@12200000 {index = <0>;Do you really need the index? You have the numbering from the aliaes I
think.
bus-width = <8>;timing = <1 3 3>;Might need a "samsung," prefix on these?
I think better that use the "exynos" instead of "samsung". how about?
Well I think you are supposed to use vendor,propname when you define your own private binding.
Ok, I shall use "samsung," prefix.
};dwmmc@12210000 {status = "disabled";};dwmmc@12220000 {index = <2>;bus-width = <4>;timing = <1 2 3>;};dwmmc@12230000 {status = "disabled";};};
1.7.0.4
Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Thanks & Regards Amarendra Reddy
Signed-off-by: Amar amarendra.xt@samsung.com --- arch/arm/include/asm/arch-exynos/dwmmc.h | 4 + drivers/mmc/exynos_dw_mmc.c | 117 ++++++++++++++++++++++++++++-- include/dwmmc.h | 4 + 3 files changed, 119 insertions(+), 6 deletions(-)
diff --git a/arch/arm/include/asm/arch-exynos/dwmmc.h b/arch/arm/include/asm/arch-exynos/dwmmc.h index 8acdf9b..92352df 100644 --- a/arch/arm/include/asm/arch-exynos/dwmmc.h +++ b/arch/arm/include/asm/arch-exynos/dwmmc.h @@ -27,6 +27,9 @@ #define DWMCI_SET_DRV_CLK(x) ((x) << 16) #define DWMCI_SET_DIV_RATIO(x) ((x) << 24)
+#ifdef CONFIG_OF_CONTROL +unsigned int exynos_dwmmc_init(const void *blob); +#else int exynos_dwmci_init(u32 regbase, int bus_width, int index);
static inline unsigned int exynos_dwmmc_init(int index, int bus_width) @@ -34,3 +37,4 @@ static inline unsigned int exynos_dwmmc_init(int index, int bus_width) unsigned int base = samsung_get_base_mmc() + (0x10000 * index); return exynos_dwmci_init(base, bus_width, index); } +#endif diff --git a/drivers/mmc/exynos_dw_mmc.c b/drivers/mmc/exynos_dw_mmc.c index 72a31b7..3b3e3e5 100644 --- a/drivers/mmc/exynos_dw_mmc.c +++ b/drivers/mmc/exynos_dw_mmc.c @@ -19,23 +19,38 @@ */
#include <common.h> -#include <malloc.h> #include <dwmmc.h> +#include <fdtdec.h> +#include <libfdt.h> +#include <malloc.h> #include <asm/arch/dwmmc.h> #include <asm/arch/clk.h> +#include <asm/arch/pinmux.h> + +#define DWMMC_MAX_CH_NUM 4 +#define DWMMC_MAX_FREQ 52000000 +#define DWMMC_MIN_FREQ 400000 +#define DWMMC_MMC0_CLKSEL_VAL 0x03030001 +#define DWMMC_MMC2_CLKSEL_VAL 0x03020001
static char *EXYNOS_NAME = "EXYNOS DWMMC";
static void exynos_dwmci_clksel(struct dwmci_host *host) { - u32 val; - val = DWMCI_SET_SAMPLE_CLK(DWMCI_SHIFT_0) | - DWMCI_SET_DRV_CLK(DWMCI_SHIFT_0) | DWMCI_SET_DIV_RATIO(0); + dwmci_writel(host, DWMCI_CLKSEL, host->clksel_val); +}
- dwmci_writel(host, DWMCI_CLKSEL, val); +unsigned int exynos_dwmci_get_clk(int dev_index) +{ + return get_mmc_clk(dev_index); }
+#ifdef CONFIG_OF_CONTROL +static int exynos_dwmci_init(u32 regbase, int bus_width, + int index, u32 *timing) +#else int exynos_dwmci_init(u32 regbase, int bus_width, int index) +#endif { struct dwmci_host *host = NULL; host = malloc(sizeof(struct dwmci_host)); @@ -44,14 +59,104 @@ int exynos_dwmci_init(u32 regbase, int bus_width, int index) return 1; }
+ /* set the clock divisor - clk_div_fsys for mmc */ + if (exynos5_mmc_set_clk_div(index)) { + debug("mmc clock div set failed\n"); + return -1; + } + host->name = EXYNOS_NAME; host->ioaddr = (void *)regbase; host->buswidth = bus_width; +#ifdef CONFIG_OF_CONTROL + host->clksel_val = (DWMCI_SET_SAMPLE_CLK(timing[0]) | + DWMCI_SET_DRV_CLK(timing[1]) | + DWMCI_SET_DIV_RATIO(timing[2])); +#else + if (0 == index) + host->clksel_val = DWMMC_MMC0_CLKSEL_VAL; + if (2 == index) + host->clksel_val = DWMMC_MMC2_CLKSEL_VAL; +#endif host->clksel = exynos_dwmci_clksel; host->dev_index = index; + host->mmc_clk = exynos_dwmci_get_clk;
- add_dwmci(host, 52000000, 400000); + add_dwmci(host, DWMMC_MAX_FREQ, DWMMC_MIN_FREQ);
return 0; }
+#ifdef CONFIG_OF_CONTROL +unsigned int exynos_dwmmc_init(const void *blob) +{ + u32 base; + int index, bus_width; + int node_list[DWMMC_MAX_CH_NUM]; + int err = 0; + int dev_id, flag; + u32 timing[3]; + int count, i; + + count = fdtdec_find_aliases_for_id(blob, "dwmmc", + COMPAT_SAMSUNG_EXYNOS5_DWMMC, node_list, + DWMMC_MAX_CH_NUM); + + for (i = 0; i < count; i++) { + int node = node_list[i]; + + if (node <= 0) + continue; + + /* config pinmux for each mmc channel */ + dev_id = pinmux_decode_periph_id(blob, node); + if (dev_id == PERIPH_ID_SDMMC0) + flag = PINMUX_FLAG_8BIT_MODE; + else + flag = PINMUX_FLAG_NONE; + + err = exynos_pinmux_config(dev_id, flag); + if (err) { + debug("DWMMC not configured\n"); + return err; + } + + /* Get the base address from the device node */ + base = fdtdec_get_addr(blob, node, "reg"); + if (!base) { + debug("DWMMC: Can't get base address\n"); + return -1; + } + + /* Get the channel index from the device node */ + index = fdtdec_get_int(blob, node, "index", 0); + if (index < 0) { + debug("DWMMC: Can't get channel index\n"); + return -1; + } + + /* Get the bus width from the device node */ + bus_width = fdtdec_get_int(blob, node, "bus-width", 0); + if (bus_width < 0) { + debug("DWMMC: Can't get bus-width\n"); + return -1; + } + + err = fdtdec_get_int_array(blob, node, "timing", + timing, 3); + if (err) { + debug("Can't get sdr-timings for divider\n"); + return -1; + } + + err = exynos_dwmci_init(base, bus_width, + index, timing); + if (err) { + debug("Can't do dwmci init\n"); + return -1; + } + } + + return 0; +} +#endif diff --git a/include/dwmmc.h b/include/dwmmc.h index c8b1d40..4a42849 100644 --- a/include/dwmmc.h +++ b/include/dwmmc.h @@ -123,6 +123,9 @@ #define MSIZE(x) ((x) << 28) #define RX_WMARK(x) ((x) << 16) #define TX_WMARK(x) (x) +#define RX_WMARK_SHIFT 16 +#define RX_WMARK_MASK (0xfff << RX_WMARK_SHIFT) +
#define DWMCI_IDMAC_OWN (1 << 31) #define DWMCI_IDMAC_CH (1 << 4) @@ -144,6 +147,7 @@ struct dwmci_host { unsigned int bus_hz; int dev_index; int buswidth; + u32 clksel_val; u32 fifoth_val; struct mmc *mmc;
Hi Amar,
On Mon, Dec 17, 2012 at 3:19 AM, Amar amarendra.xt@samsung.com wrote:
Signed-off-by: Amar amarendra.xt@samsung.com
Good to see this patch! Please can you add a short commit message?
arch/arm/include/asm/arch-exynos/dwmmc.h | 4 +
drivers/mmc/exynos_dw_mmc.c | 117 ++++++++++++++++++++++++++++-- include/dwmmc.h | 4 + 3 files changed, 119 insertions(+), 6 deletions(-)
diff --git a/arch/arm/include/asm/arch-exynos/dwmmc.h b/arch/arm/include/asm/arch-exynos/dwmmc.h index 8acdf9b..92352df 100644 --- a/arch/arm/include/asm/arch-exynos/dwmmc.h +++ b/arch/arm/include/asm/arch-exynos/dwmmc.h @@ -27,6 +27,9 @@ #define DWMCI_SET_DRV_CLK(x) ((x) << 16) #define DWMCI_SET_DIV_RATIO(x) ((x) << 24)
+#ifdef CONFIG_OF_CONTROL +unsigned int exynos_dwmmc_init(const void *blob); +#else int exynos_dwmci_init(u32 regbase, int bus_width, int index);
static inline unsigned int exynos_dwmmc_init(int index, int bus_width) @@ -34,3 +37,4 @@ static inline unsigned int exynos_dwmmc_init(int index, int bus_width) unsigned int base = samsung_get_base_mmc() + (0x10000 * index); return exynos_dwmci_init(base, bus_width, index); } +#endif diff --git a/drivers/mmc/exynos_dw_mmc.c b/drivers/mmc/exynos_dw_mmc.c index 72a31b7..3b3e3e5 100644 --- a/drivers/mmc/exynos_dw_mmc.c +++ b/drivers/mmc/exynos_dw_mmc.c @@ -19,23 +19,38 @@ */
#include <common.h> -#include <malloc.h> #include <dwmmc.h> +#include <fdtdec.h> +#include <libfdt.h> +#include <malloc.h> #include <asm/arch/dwmmc.h> #include <asm/arch/clk.h> +#include <asm/arch/pinmux.h>
+#define DWMMC_MAX_CH_NUM 4 +#define DWMMC_MAX_FREQ 52000000 +#define DWMMC_MIN_FREQ 400000 +#define DWMMC_MMC0_CLKSEL_VAL 0x03030001 +#define DWMMC_MMC2_CLKSEL_VAL 0x03020001
What do these last two values mean? I think clock setting should be done in clock.c, not here.
static char *EXYNOS_NAME = "EXYNOS DWMMC";
static void exynos_dwmci_clksel(struct dwmci_host *host) {
u32 val;val = DWMCI_SET_SAMPLE_CLK(DWMCI_SHIFT_0) |DWMCI_SET_DRV_CLK(DWMCI_SHIFT_0) | DWMCI_SET_DIV_RATIO(0);
dwmci_writel(host, DWMCI_CLKSEL, host->clksel_val);+}
dwmci_writel(host, DWMCI_CLKSEL, val);+unsigned int exynos_dwmci_get_clk(int dev_index) +{
return get_mmc_clk(dev_index);}
+#ifdef CONFIG_OF_CONTROL +static int exynos_dwmci_init(u32 regbase, int bus_width,
int index, u32 *timing)+#else int exynos_dwmci_init(u32 regbase, int bus_width, int index) +#endif
I'm really not keen on having the same function with different signatures. My first question is whether we need to support operation without CONFIG_OF_CONTROL. If so, I suggest having an init routine that takes an FDT blob as a parameter, and a separate add_port function which can be called by non-FDT-enabled board files. The init routine will call the add_port function for each port it finds in the FDT.
Also please can you briefly comment non-trivial functions?
{ struct dwmci_host *host = NULL; host = malloc(sizeof(struct dwmci_host)); @@ -44,14 +59,104 @@ int exynos_dwmci_init(u32 regbase, int bus_width, int index) return 1; }
/* set the clock divisor - clk_div_fsys for mmc */if (exynos5_mmc_set_clk_div(index)) {debug("mmc clock div set failed\n");return -1;}host->name = EXYNOS_NAME; host->ioaddr = (void *)regbase; host->buswidth = bus_width;+#ifdef CONFIG_OF_CONTROL
host->clksel_val = (DWMCI_SET_SAMPLE_CLK(timing[0]) |DWMCI_SET_DRV_CLK(timing[1]) |DWMCI_SET_DIV_RATIO(timing[2]));+#else
if (0 == index)host->clksel_val = DWMMC_MMC0_CLKSEL_VAL;if (2 == index)host->clksel_val = DWMMC_MMC2_CLKSEL_VAL;+#endif host->clksel = exynos_dwmci_clksel; host->dev_index = index;
host->mmc_clk = exynos_dwmci_get_clk;
add_dwmci(host, 52000000, 400000);
add_dwmci(host, DWMMC_MAX_FREQ, DWMMC_MIN_FREQ); return 0;}
+#ifdef CONFIG_OF_CONTROL +unsigned int exynos_dwmmc_init(const void *blob) +{
u32 base;int index, bus_width;int node_list[DWMMC_MAX_CH_NUM];int err = 0;int dev_id, flag;u32 timing[3];int count, i;count = fdtdec_find_aliases_for_id(blob, "dwmmc",COMPAT_SAMSUNG_EXYNOS5_DWMMC, node_list,DWMMC_MAX_CH_NUM);for (i = 0; i < count; i++) {int node = node_list[i];if (node <= 0)continue;/* config pinmux for each mmc channel */dev_id = pinmux_decode_periph_id(blob, node);if (dev_id == PERIPH_ID_SDMMC0)flag = PINMUX_FLAG_8BIT_MODE;elseflag = PINMUX_FLAG_NONE;
This should be decoded from the FDT - e.g. fdtdec_get_int(blob, node, "samsung,width") == 8.
err = exynos_pinmux_config(dev_id, flag);if (err) {debug("DWMMC not configured\n");return err;}/* Get the base address from the device node */base = fdtdec_get_addr(blob, node, "reg");if (!base) {debug("DWMMC: Can't get base address\n");return -1;}/* Get the channel index from the device node */index = fdtdec_get_int(blob, node, "index", 0);if (index < 0) {debug("DWMMC: Can't get channel index\n");return -1;}
What is this needed for? Doesn't the reg address tell you everything you need here?
/* Get the bus width from the device node */bus_width = fdtdec_get_int(blob, node, "bus-width", 0);if (bus_width < 0) {debug("DWMMC: Can't get bus-width\n");return -1;}
+
err = fdtdec_get_int_array(blob, node, "timing",timing, 3);if (err) {debug("Can't get sdr-timings for divider\n");return -1;}err = exynos_dwmci_init(base, bus_width,index, timing);if (err) {debug("Can't do dwmci init\n");return -1;}}return 0;+} +#endif diff --git a/include/dwmmc.h b/include/dwmmc.h index c8b1d40..4a42849 100644 --- a/include/dwmmc.h +++ b/include/dwmmc.h @@ -123,6 +123,9 @@ #define MSIZE(x) ((x) << 28) #define RX_WMARK(x) ((x) << 16) #define TX_WMARK(x) (x) +#define RX_WMARK_SHIFT 16 +#define RX_WMARK_MASK (0xfff << RX_WMARK_SHIFT)
Extra blank line here?
#define DWMCI_IDMAC_OWN (1 << 31) #define DWMCI_IDMAC_CH (1 << 4) @@ -144,6 +147,7 @@ struct dwmci_host { unsigned int bus_hz; int dev_index; int buswidth;
u32 clksel_val; u32 fifoth_val; struct mmc *mmc;-- 1.7.0.4
Regards, Simon
Hi Simon,
Thanks for your review comments. Please find the responses below.
Thanks & Regards Amarendra Reddy
On 20 December 2012 07:53, Simon Glass sjg@chromium.org wrote:
Hi Amar,
On Mon, Dec 17, 2012 at 3:19 AM, Amar amarendra.xt@samsung.com wrote:
Signed-off-by: Amar amarendra.xt@samsung.com
Good to see this patch! Please can you add a short commit message? Oops.. sorry. I will put the commit msg.
arch/arm/include/asm/arch-exynos/dwmmc.h | 4 +
drivers/mmc/exynos_dw_mmc.c | 117 ++++++++++++++++++++++++++++-- include/dwmmc.h | 4 + 3 files changed, 119 insertions(+), 6 deletions(-)
diff --git a/arch/arm/include/asm/arch-exynos/dwmmc.h b/arch/arm/include/asm/arch-exynos/dwmmc.h index 8acdf9b..92352df 100644 --- a/arch/arm/include/asm/arch-exynos/dwmmc.h +++ b/arch/arm/include/asm/arch-exynos/dwmmc.h @@ -27,6 +27,9 @@ #define DWMCI_SET_DRV_CLK(x) ((x) << 16) #define DWMCI_SET_DIV_RATIO(x) ((x) << 24)
+#ifdef CONFIG_OF_CONTROL +unsigned int exynos_dwmmc_init(const void *blob); +#else int exynos_dwmci_init(u32 regbase, int bus_width, int index);
static inline unsigned int exynos_dwmmc_init(int index, int bus_width) @@ -34,3 +37,4 @@ static inline unsigned int exynos_dwmmc_init(int index, int bus_width) unsigned int base = samsung_get_base_mmc() + (0x10000 * index); return exynos_dwmci_init(base, bus_width, index); } +#endif diff --git a/drivers/mmc/exynos_dw_mmc.c b/drivers/mmc/exynos_dw_mmc.c index 72a31b7..3b3e3e5 100644 --- a/drivers/mmc/exynos_dw_mmc.c +++ b/drivers/mmc/exynos_dw_mmc.c @@ -19,23 +19,38 @@ */
#include <common.h> -#include <malloc.h> #include <dwmmc.h> +#include <fdtdec.h> +#include <libfdt.h> +#include <malloc.h> #include <asm/arch/dwmmc.h> #include <asm/arch/clk.h> +#include <asm/arch/pinmux.h>
+#define DWMMC_MAX_CH_NUM 4 +#define DWMMC_MAX_FREQ 52000000 +#define DWMMC_MIN_FREQ 400000 +#define DWMMC_MMC0_CLKSEL_VAL 0x03030001 +#define DWMMC_MMC2_CLKSEL_VAL 0x03020001
What do these last two values mean? I think clock setting should be done in clock.c, not here.
In case of non-dt support, these two values are written into respective
clock selection register-CLKSEL, to select the clock for MMC-channel0 and MMC-channel2. Will move clock setting into clock.c and the #defines will be moved into include/asm/arch/clk.h.
static char *EXYNOS_NAME = "EXYNOS DWMMC";
static void exynos_dwmci_clksel(struct dwmci_host *host) {
u32 val;val = DWMCI_SET_SAMPLE_CLK(DWMCI_SHIFT_0) |DWMCI_SET_DRV_CLK(DWMCI_SHIFT_0) |DWMCI_SET_DIV_RATIO(0);
dwmci_writel(host, DWMCI_CLKSEL, host->clksel_val);+}
dwmci_writel(host, DWMCI_CLKSEL, val);+unsigned int exynos_dwmci_get_clk(int dev_index) +{
return get_mmc_clk(dev_index);}
+#ifdef CONFIG_OF_CONTROL +static int exynos_dwmci_init(u32 regbase, int bus_width,
int index, u32 *timing)+#else int exynos_dwmci_init(u32 regbase, int bus_width, int index) +#endif
I'm really not keen on having the same function with different signatures. My first question is whether we need to support operation without CONFIG_OF_CONTROL. If so, I suggest having an init routine that takes an FDT blob as a parameter, and a separate add_port function which can be called by non-FDT-enabled board files. The init routine will call the add_port function for each port it finds in the FDT.
I think we need support operation without CONFIG_OF_CONTROL atleast untill
the entire FDT is formed. Hence I will implement it as per your suggestion.
Also please can you briefly comment non-trivial functions? OK.
{ struct dwmci_host *host = NULL; host = malloc(sizeof(struct dwmci_host)); @@ -44,14 +59,104 @@ int exynos_dwmci_init(u32 regbase, int bus_width,
int
index) return 1; }
/* set the clock divisor - clk_div_fsys for mmc */if (exynos5_mmc_set_clk_div(index)) {debug("mmc clock div set failed\n");return -1;}host->name = EXYNOS_NAME; host->ioaddr = (void *)regbase; host->buswidth = bus_width;+#ifdef CONFIG_OF_CONTROL
host->clksel_val = (DWMCI_SET_SAMPLE_CLK(timing[0]) |DWMCI_SET_DRV_CLK(timing[1]) |DWMCI_SET_DIV_RATIO(timing[2]));+#else
if (0 == index)host->clksel_val = DWMMC_MMC0_CLKSEL_VAL;if (2 == index)host->clksel_val = DWMMC_MMC2_CLKSEL_VAL;+#endif host->clksel = exynos_dwmci_clksel; host->dev_index = index;
host->mmc_clk = exynos_dwmci_get_clk;
add_dwmci(host, 52000000, 400000);
add_dwmci(host, DWMMC_MAX_FREQ, DWMMC_MIN_FREQ); return 0;}
+#ifdef CONFIG_OF_CONTROL +unsigned int exynos_dwmmc_init(const void *blob) +{
u32 base;int index, bus_width;int node_list[DWMMC_MAX_CH_NUM];int err = 0;int dev_id, flag;u32 timing[3];int count, i;count = fdtdec_find_aliases_for_id(blob, "dwmmc",COMPAT_SAMSUNG_EXYNOS5_DWMMC, node_list,DWMMC_MAX_CH_NUM);for (i = 0; i < count; i++) {int node = node_list[i];if (node <= 0)continue;/* config pinmux for each mmc channel */dev_id = pinmux_decode_periph_id(blob, node);if (dev_id == PERIPH_ID_SDMMC0)flag = PINMUX_FLAG_8BIT_MODE;elseflag = PINMUX_FLAG_NONE;This should be decoded from the FDT - e.g. fdtdec_get_int(blob, node, "samsung,width") == 8.
OK.
err = exynos_pinmux_config(dev_id, flag);if (err) {debug("DWMMC not configured\n");return err;}/* Get the base address from the device node */base = fdtdec_get_addr(blob, node, "reg");if (!base) {debug("DWMMC: Can't get base address\n");return -1;}/* Get the channel index from the device node */index = fdtdec_get_int(blob, node, "index", 0);if (index < 0) {debug("DWMMC: Can't get channel index\n");return -1;}What is this needed for? Doesn't the reg address tell you everything you need here? Ok will change accordingly.
/* Get the bus width from the device node */bus_width = fdtdec_get_int(blob, node, "bus-width", 0);if (bus_width < 0) {debug("DWMMC: Can't get bus-width\n");return -1;}
err = fdtdec_get_int_array(blob, node, "timing",timing, 3);if (err) {debug("Can't get sdr-timings for divider\n");return -1;}err = exynos_dwmci_init(base, bus_width,index, timing);if (err) {debug("Can't do dwmci init\n");return -1;}}return 0;+} +#endif diff --git a/include/dwmmc.h b/include/dwmmc.h index c8b1d40..4a42849 100644 --- a/include/dwmmc.h +++ b/include/dwmmc.h @@ -123,6 +123,9 @@ #define MSIZE(x) ((x) << 28) #define RX_WMARK(x) ((x) << 16) #define TX_WMARK(x) (x) +#define RX_WMARK_SHIFT 16 +#define RX_WMARK_MASK (0xfff << RX_WMARK_SHIFT)
Extra blank line here? Will remove it.
#define DWMCI_IDMAC_OWN (1 << 31) #define DWMCI_IDMAC_CH (1 << 4) @@ -144,6 +147,7 @@ struct dwmci_host { unsigned int bus_hz; int dev_index; int buswidth;
u32 clksel_val; u32 fifoth_val; struct mmc *mmc;-- 1.7.0.4
Regards, Simon
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On 12/21/2012 02:16 PM, Amarendra Reddy wrote:
Hi Simon,
Thanks for your review comments. Please find the responses below.
Thanks & Regards Amarendra Reddy
On 20 December 2012 07:53, Simon Glass sjg@chromium.org wrote:
Hi Amar,
On Mon, Dec 17, 2012 at 3:19 AM, Amar amarendra.xt@samsung.com wrote:
Signed-off-by: Amar amarendra.xt@samsung.com
Good to see this patch! Please can you add a short commit message? Oops.. sorry. I will put the commit msg.
arch/arm/include/asm/arch-exynos/dwmmc.h | 4 +
drivers/mmc/exynos_dw_mmc.c | 117 ++++++++++++++++++++++++++++-- include/dwmmc.h | 4 + 3 files changed, 119 insertions(+), 6 deletions(-)
diff --git a/arch/arm/include/asm/arch-exynos/dwmmc.h b/arch/arm/include/asm/arch-exynos/dwmmc.h index 8acdf9b..92352df 100644 --- a/arch/arm/include/asm/arch-exynos/dwmmc.h +++ b/arch/arm/include/asm/arch-exynos/dwmmc.h @@ -27,6 +27,9 @@ #define DWMCI_SET_DRV_CLK(x) ((x) << 16) #define DWMCI_SET_DIV_RATIO(x) ((x) << 24)
+#ifdef CONFIG_OF_CONTROL +unsigned int exynos_dwmmc_init(const void *blob); +#else int exynos_dwmci_init(u32 regbase, int bus_width, int index);
static inline unsigned int exynos_dwmmc_init(int index, int bus_width) @@ -34,3 +37,4 @@ static inline unsigned int exynos_dwmmc_init(int index, int bus_width) unsigned int base = samsung_get_base_mmc() + (0x10000 * index); return exynos_dwmci_init(base, bus_width, index); } +#endif diff --git a/drivers/mmc/exynos_dw_mmc.c b/drivers/mmc/exynos_dw_mmc.c index 72a31b7..3b3e3e5 100644 --- a/drivers/mmc/exynos_dw_mmc.c +++ b/drivers/mmc/exynos_dw_mmc.c @@ -19,23 +19,38 @@ */
#include <common.h> -#include <malloc.h> #include <dwmmc.h> +#include <fdtdec.h> +#include <libfdt.h> +#include <malloc.h> #include <asm/arch/dwmmc.h> #include <asm/arch/clk.h> +#include <asm/arch/pinmux.h>
+#define DWMMC_MAX_CH_NUM 4 +#define DWMMC_MAX_FREQ 52000000 +#define DWMMC_MIN_FREQ 400000 +#define DWMMC_MMC0_CLKSEL_VAL 0x03030001 +#define DWMMC_MMC2_CLKSEL_VAL 0x03020001
What do these last two values mean? I think clock setting should be done in clock.c, not here.
In case of non-dt support, these two values are written into respective
clock selection register-CLKSEL, to select the clock for MMC-channel0 and MMC-channel2. Will move clock setting into clock.c and the #defines will be moved into include/asm/arch/clk.h.
I didn't want to move into clk.h. Why do you move there? It is related with CLKSEL register into DesignWare IP.
static char *EXYNOS_NAME = "EXYNOS DWMMC";
static void exynos_dwmci_clksel(struct dwmci_host *host) {
u32 val;val = DWMCI_SET_SAMPLE_CLK(DWMCI_SHIFT_0) |DWMCI_SET_DRV_CLK(DWMCI_SHIFT_0) |DWMCI_SET_DIV_RATIO(0);
dwmci_writel(host, DWMCI_CLKSEL, host->clksel_val);+}
dwmci_writel(host, DWMCI_CLKSEL, val);+unsigned int exynos_dwmci_get_clk(int dev_index) +{
return get_mmc_clk(dev_index);}
+#ifdef CONFIG_OF_CONTROL +static int exynos_dwmci_init(u32 regbase, int bus_width,
int index, u32 *timing)+#else int exynos_dwmci_init(u32 regbase, int bus_width, int index) +#endif
I'm really not keen on having the same function with different signatures. My first question is whether we need to support operation without CONFIG_OF_CONTROL. If so, I suggest having an init routine that takes an FDT blob as a parameter, and a separate add_port function which can be called by non-FDT-enabled board files. The init routine will call the add_port function for each port it finds in the FDT.
I think we need support operation without CONFIG_OF_CONTROL atleast untill
the entire FDT is formed. Hence I will implement it as per your suggestion.
Also please can you briefly comment non-trivial functions? OK.
{ struct dwmci_host *host = NULL; host = malloc(sizeof(struct dwmci_host)); @@ -44,14 +59,104 @@ int exynos_dwmci_init(u32 regbase, int bus_width,
int
index) return 1; }
/* set the clock divisor - clk_div_fsys for mmc */if (exynos5_mmc_set_clk_div(index)) {debug("mmc clock div set failed\n");return -1;}host->name = EXYNOS_NAME; host->ioaddr = (void *)regbase; host->buswidth = bus_width;+#ifdef CONFIG_OF_CONTROL
host->clksel_val = (DWMCI_SET_SAMPLE_CLK(timing[0]) |DWMCI_SET_DRV_CLK(timing[1]) |DWMCI_SET_DIV_RATIO(timing[2]));+#else
if (0 == index)host->clksel_val = DWMMC_MMC0_CLKSEL_VAL;if (2 == index)host->clksel_val = DWMMC_MMC2_CLKSEL_VAL;+#endif host->clksel = exynos_dwmci_clksel; host->dev_index = index;
host->mmc_clk = exynos_dwmci_get_clk;
add_dwmci(host, 52000000, 400000);
add_dwmci(host, DWMMC_MAX_FREQ, DWMMC_MIN_FREQ); return 0;}
+#ifdef CONFIG_OF_CONTROL +unsigned int exynos_dwmmc_init(const void *blob) +{
u32 base;int index, bus_width;int node_list[DWMMC_MAX_CH_NUM];int err = 0;int dev_id, flag;u32 timing[3];int count, i;count = fdtdec_find_aliases_for_id(blob, "dwmmc",COMPAT_SAMSUNG_EXYNOS5_DWMMC, node_list,DWMMC_MAX_CH_NUM);for (i = 0; i < count; i++) {int node = node_list[i];if (node <= 0)continue;/* config pinmux for each mmc channel */dev_id = pinmux_decode_periph_id(blob, node);if (dev_id == PERIPH_ID_SDMMC0)flag = PINMUX_FLAG_8BIT_MODE;elseflag = PINMUX_FLAG_NONE;This should be decoded from the FDT - e.g. fdtdec_get_int(blob, node, "samsung,width") == 8.
OK.
err = exynos_pinmux_config(dev_id, flag);if (err) {debug("DWMMC not configured\n");return err;}/* Get the base address from the device node */base = fdtdec_get_addr(blob, node, "reg");if (!base) {debug("DWMMC: Can't get base address\n");return -1;}/* Get the channel index from the device node */index = fdtdec_get_int(blob, node, "index", 0);if (index < 0) {debug("DWMMC: Can't get channel index\n");return -1;}What is this needed for? Doesn't the reg address tell you everything you need here? Ok will change accordingly.
/* Get the bus width from the device node */bus_width = fdtdec_get_int(blob, node, "bus-width", 0);if (bus_width < 0) {debug("DWMMC: Can't get bus-width\n");return -1;}
err = fdtdec_get_int_array(blob, node, "timing",timing, 3);if (err) {debug("Can't get sdr-timings for divider\n");return -1;}err = exynos_dwmci_init(base, bus_width,index, timing);if (err) {debug("Can't do dwmci init\n");return -1;}}return 0;+} +#endif diff --git a/include/dwmmc.h b/include/dwmmc.h index c8b1d40..4a42849 100644 --- a/include/dwmmc.h +++ b/include/dwmmc.h @@ -123,6 +123,9 @@ #define MSIZE(x) ((x) << 28) #define RX_WMARK(x) ((x) << 16) #define TX_WMARK(x) (x) +#define RX_WMARK_SHIFT 16 +#define RX_WMARK_MASK (0xfff << RX_WMARK_SHIFT)
Extra blank line here? Will remove it.
#define DWMCI_IDMAC_OWN (1 << 31) #define DWMCI_IDMAC_CH (1 << 4) @@ -144,6 +147,7 @@ struct dwmci_host { unsigned int bus_hz; int dev_index; int buswidth;
u32 clksel_val; u32 fifoth_val; struct mmc *mmc;-- 1.7.0.4
Regards, Simon
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi Jaehoon, Thanks for your comments. Please find the response below.
Thanks & Regards Amarendra
On 21 December 2012 14:48, Jaehoon Chung jh80.chung@samsung.com wrote:
On 12/21/2012 02:16 PM, Amarendra Reddy wrote:
Hi Simon,
Thanks for your review comments. Please find the responses below.
Thanks & Regards Amarendra Reddy
On 20 December 2012 07:53, Simon Glass sjg@chromium.org wrote:
Hi Amar,
On Mon, Dec 17, 2012 at 3:19 AM, Amar amarendra.xt@samsung.com wrote:
Signed-off-by: Amar amarendra.xt@samsung.com
Good to see this patch! Please can you add a short commit message? Oops.. sorry. I will put the commit msg.
arch/arm/include/asm/arch-exynos/dwmmc.h | 4 +
drivers/mmc/exynos_dw_mmc.c | 117 ++++++++++++++++++++++++++++-- include/dwmmc.h | 4 + 3 files changed, 119 insertions(+), 6 deletions(-)
diff --git a/arch/arm/include/asm/arch-exynos/dwmmc.h b/arch/arm/include/asm/arch-exynos/dwmmc.h index 8acdf9b..92352df 100644 --- a/arch/arm/include/asm/arch-exynos/dwmmc.h +++ b/arch/arm/include/asm/arch-exynos/dwmmc.h @@ -27,6 +27,9 @@ #define DWMCI_SET_DRV_CLK(x) ((x) << 16) #define DWMCI_SET_DIV_RATIO(x) ((x) << 24)
+#ifdef CONFIG_OF_CONTROL +unsigned int exynos_dwmmc_init(const void *blob); +#else int exynos_dwmci_init(u32 regbase, int bus_width, int index);
static inline unsigned int exynos_dwmmc_init(int index, int bus_width) @@ -34,3 +37,4 @@ static inline unsigned int exynos_dwmmc_init(int
index,
int bus_width) unsigned int base = samsung_get_base_mmc() + (0x10000 * index); return exynos_dwmci_init(base, bus_width, index); } +#endif diff --git a/drivers/mmc/exynos_dw_mmc.c b/drivers/mmc/exynos_dw_mmc.c index 72a31b7..3b3e3e5 100644 --- a/drivers/mmc/exynos_dw_mmc.c +++ b/drivers/mmc/exynos_dw_mmc.c @@ -19,23 +19,38 @@ */
#include <common.h> -#include <malloc.h> #include <dwmmc.h> +#include <fdtdec.h> +#include <libfdt.h> +#include <malloc.h> #include <asm/arch/dwmmc.h> #include <asm/arch/clk.h> +#include <asm/arch/pinmux.h>
+#define DWMMC_MAX_CH_NUM 4 +#define DWMMC_MAX_FREQ 52000000 +#define DWMMC_MIN_FREQ 400000 +#define DWMMC_MMC0_CLKSEL_VAL 0x03030001 +#define DWMMC_MMC2_CLKSEL_VAL 0x03020001
What do these last two values mean? I think clock setting should be
done in
clock.c, not here.
In case of non-dt support, these two values are written into respective
clock selection register-CLKSEL, to select the clock for MMC-channel0 and MMC-channel2. Will move clock setting into clock.c and the #defines will be moved into include/asm/arch/clk.h.
I didn't want to move into clk.h. Why do you move there? It is related with CLKSEL register into DesignWare IP.
Ok, now I understand that all register settings corresponding to
DesignWare IP should be present in exynos_dw_mmc.c / dw_mmc.c. Fine, I will not move CLKSEL register setting into clock.c / clk.h.
static char *EXYNOS_NAME = "EXYNOS DWMMC";
static void exynos_dwmci_clksel(struct dwmci_host *host) {
u32 val;val = DWMCI_SET_SAMPLE_CLK(DWMCI_SHIFT_0) |DWMCI_SET_DRV_CLK(DWMCI_SHIFT_0) |DWMCI_SET_DIV_RATIO(0);
dwmci_writel(host, DWMCI_CLKSEL, host->clksel_val);+}
dwmci_writel(host, DWMCI_CLKSEL, val);+unsigned int exynos_dwmci_get_clk(int dev_index) +{
return get_mmc_clk(dev_index);}
+#ifdef CONFIG_OF_CONTROL +static int exynos_dwmci_init(u32 regbase, int bus_width,
int index, u32 *timing)+#else int exynos_dwmci_init(u32 regbase, int bus_width, int index) +#endif
I'm really not keen on having the same function with different
signatures.
My first question is whether we need to support operation without CONFIG_OF_CONTROL. If so, I suggest having an init routine that takes an FDT blob as a parameter, and a separate add_port function which can be called by non-FDT-enabled board files. The init routine will call the add_port function for each port it finds in the FDT.
I think we need support operation without CONFIG_OF_CONTROL atleast
untill
the entire FDT is formed. Hence I will implement it as per your suggestion.
Also please can you briefly comment non-trivial functions? OK.
{ struct dwmci_host *host = NULL; host = malloc(sizeof(struct dwmci_host)); @@ -44,14 +59,104 @@ int exynos_dwmci_init(u32 regbase, int bus_width,
int
index) return 1; }
/* set the clock divisor - clk_div_fsys for mmc */if (exynos5_mmc_set_clk_div(index)) {debug("mmc clock div set failed\n");return -1;}host->name = EXYNOS_NAME; host->ioaddr = (void *)regbase; host->buswidth = bus_width;+#ifdef CONFIG_OF_CONTROL
host->clksel_val = (DWMCI_SET_SAMPLE_CLK(timing[0]) |DWMCI_SET_DRV_CLK(timing[1]) |DWMCI_SET_DIV_RATIO(timing[2]));+#else
if (0 == index)host->clksel_val = DWMMC_MMC0_CLKSEL_VAL;if (2 == index)host->clksel_val = DWMMC_MMC2_CLKSEL_VAL;+#endif host->clksel = exynos_dwmci_clksel; host->dev_index = index;
host->mmc_clk = exynos_dwmci_get_clk;
add_dwmci(host, 52000000, 400000);
add_dwmci(host, DWMMC_MAX_FREQ, DWMMC_MIN_FREQ); return 0;}
+#ifdef CONFIG_OF_CONTROL +unsigned int exynos_dwmmc_init(const void *blob) +{
u32 base;int index, bus_width;int node_list[DWMMC_MAX_CH_NUM];int err = 0;int dev_id, flag;u32 timing[3];int count, i;count = fdtdec_find_aliases_for_id(blob, "dwmmc",COMPAT_SAMSUNG_EXYNOS5_DWMMC,node_list,
DWMMC_MAX_CH_NUM);for (i = 0; i < count; i++) {int node = node_list[i];if (node <= 0)continue;/* config pinmux for each mmc channel */dev_id = pinmux_decode_periph_id(blob, node);if (dev_id == PERIPH_ID_SDMMC0)flag = PINMUX_FLAG_8BIT_MODE;elseflag = PINMUX_FLAG_NONE;This should be decoded from the FDT - e.g. fdtdec_get_int(blob, node, "samsung,width") == 8.
OK.
err = exynos_pinmux_config(dev_id, flag);if (err) {debug("DWMMC not configured\n");return err;}/* Get the base address from the device node */base = fdtdec_get_addr(blob, node, "reg");if (!base) {debug("DWMMC: Can't get base address\n");return -1;}/* Get the channel index from the device node */index = fdtdec_get_int(blob, node, "index", 0);if (index < 0) {debug("DWMMC: Can't get channel index\n");return -1;}What is this needed for? Doesn't the reg address tell you everything you need here? Ok will change accordingly.
/* Get the bus width from the device node */bus_width = fdtdec_get_int(blob, node, "bus-width", 0);if (bus_width < 0) {debug("DWMMC: Can't get bus-width\n");return -1;}
err = fdtdec_get_int_array(blob, node, "timing",timing, 3);if (err) {debug("Can't get sdr-timings for divider\n");return -1;}err = exynos_dwmci_init(base, bus_width,index, timing);if (err) {debug("Can't do dwmci init\n");return -1;}}return 0;+} +#endif diff --git a/include/dwmmc.h b/include/dwmmc.h index c8b1d40..4a42849 100644 --- a/include/dwmmc.h +++ b/include/dwmmc.h @@ -123,6 +123,9 @@ #define MSIZE(x) ((x) << 28) #define RX_WMARK(x) ((x) << 16) #define TX_WMARK(x) (x) +#define RX_WMARK_SHIFT 16 +#define RX_WMARK_MASK (0xfff << RX_WMARK_SHIFT)
Extra blank line here? Will remove it.
#define DWMCI_IDMAC_OWN (1 << 31) #define DWMCI_IDMAC_CH (1 << 4) @@ -144,6 +147,7 @@ struct dwmci_host { unsigned int bus_hz; int dev_index; int buswidth;
u32 clksel_val; u32 fifoth_val; struct mmc *mmc;-- 1.7.0.4
Regards, Simon
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
This API computes the divisor value based on MPLL clock and writes it into the FSYS1 register.
Signed-off-by: Amar amarendra.xt@samsung.com --- arch/arm/cpu/armv7/exynos/clock.c | 39 ++++++++++++++++++++++++++++++- arch/arm/include/asm/arch-exynos/clk.h | 1 + 2 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/arch/arm/cpu/armv7/exynos/clock.c b/arch/arm/cpu/armv7/exynos/clock.c index 731bbff..6517296 100644 --- a/arch/arm/cpu/armv7/exynos/clock.c +++ b/arch/arm/cpu/armv7/exynos/clock.c @@ -379,7 +379,7 @@ static unsigned long exynos4_get_mmc_clk(int dev_index) (struct exynos4_clock *)samsung_get_base_clock(); unsigned long uclk, sclk; unsigned int sel, ratio, pre_ratio; - int shift; + int shift = 0;
sel = readl(&clk->src_fsys); sel = (sel >> (dev_index << 2)) & 0xf; @@ -428,7 +428,7 @@ static unsigned long exynos5_get_mmc_clk(int dev_index) (struct exynos5_clock *)samsung_get_base_clock(); unsigned long uclk, sclk; unsigned int sel, ratio, pre_ratio; - int shift; + int shift = 0;
sel = readl(&clk->src_fsys); sel = (sel >> (dev_index << 2)) & 0xf; @@ -526,6 +526,41 @@ static void exynos5_set_mmc_clk(int dev_index, unsigned int div) writel(val, addr); }
+/* exynos5: set the mmc clock div ratio in fsys1 */ +int exynos5_mmc_set_clk_div(int dev_index) +{ + struct exynos5_clock *clk = + (struct exynos5_clock *)samsung_get_base_clock(); + unsigned int addr; + unsigned int clock; + unsigned int tmp; + unsigned int i; + + /* get mpll clock */ + clock = get_pll_clk(MPLL) / 1000000; + + /* + * CLK_DIV_FSYS1 + * MMC0_PRE_RATIO [15:8], MMC0_RATIO [3:0] + * CLK_DIV_FSYS2 + * MMC2_PRE_RATIO [15:8], MMC2_RATIO [3:0] + */ + if (dev_index < 2) { + addr = (unsigned int)&clk->div_fsys1; + } else { + addr = (unsigned int)&clk->div_fsys2; + } + + tmp = readl(addr) & ~FSYS1_MMC0_DIV_MASK; + for (i = 0; i <= 0xf; i++) { + if ((clock / (i + 1)) <= 400) { + writel(tmp | i << 0, addr); + break; + } + } + return 0; +} + /* get_lcd_clk: return lcd clock frequency */ static unsigned long exynos4_get_lcd_clk(void) { diff --git a/arch/arm/include/asm/arch-exynos/clk.h b/arch/arm/include/asm/arch-exynos/clk.h index ff155d8..b0ecdd4 100644 --- a/arch/arm/include/asm/arch-exynos/clk.h +++ b/arch/arm/include/asm/arch-exynos/clk.h @@ -36,6 +36,7 @@ unsigned long get_pwm_clk(void); unsigned long get_uart_clk(int dev_index); unsigned long get_mmc_clk(int deV_index); void set_mmc_clk(int dev_index, unsigned int div); +int exynos5_mmc_set_clk_div(int dev_index); unsigned long get_lcd_clk(void); void set_lcd_clk(void); void set_mipi_clk(void);
Hi Amar,
On Mon, Dec 17, 2012 at 3:19 AM, Amar amarendra.xt@samsung.com wrote:
This API computes the divisor value based on MPLL clock and writes it into the FSYS1 register.
Signed-off-by: Amar amarendra.xt@samsung.com
arch/arm/cpu/armv7/exynos/clock.c | 39 ++++++++++++++++++++++++++++++- arch/arm/include/asm/arch-exynos/clk.h | 1 + 2 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/arch/arm/cpu/armv7/exynos/clock.c b/arch/arm/cpu/armv7/exynos/clock.c index 731bbff..6517296 100644 --- a/arch/arm/cpu/armv7/exynos/clock.c +++ b/arch/arm/cpu/armv7/exynos/clock.c @@ -379,7 +379,7 @@ static unsigned long exynos4_get_mmc_clk(int dev_index) (struct exynos4_clock *)samsung_get_base_clock(); unsigned long uclk, sclk; unsigned int sel, ratio, pre_ratio;
int shift;
int shift = 0; sel = readl(&clk->src_fsys); sel = (sel >> (dev_index << 2)) & 0xf;@@ -428,7 +428,7 @@ static unsigned long exynos5_get_mmc_clk(int dev_index) (struct exynos5_clock *)samsung_get_base_clock(); unsigned long uclk, sclk; unsigned int sel, ratio, pre_ratio;
int shift;
int shift = 0; sel = readl(&clk->src_fsys); sel = (sel >> (dev_index << 2)) & 0xf;@@ -526,6 +526,41 @@ static void exynos5_set_mmc_clk(int dev_index, unsigned int div) writel(val, addr); }
+/* exynos5: set the mmc clock div ratio in fsys1 */ +int exynos5_mmc_set_clk_div(int dev_index)
Shouldn't this take a periph_id instead of a dev_index?
+{
struct exynos5_clock *clk =(struct exynos5_clock *)samsung_get_base_clock();unsigned int addr;unsigned int clock;unsigned int tmp;unsigned int i;/* get mpll clock */clock = get_pll_clk(MPLL) / 1000000;/** CLK_DIV_FSYS1* MMC0_PRE_RATIO [15:8], MMC0_RATIO [3:0]* CLK_DIV_FSYS2* MMC2_PRE_RATIO [15:8], MMC2_RATIO [3:0]*/if (dev_index < 2) {addr = (unsigned int)&clk->div_fsys1;} else {addr = (unsigned int)&clk->div_fsys2;}tmp = readl(addr) & ~FSYS1_MMC0_DIV_MASK;for (i = 0; i <= 0xf; i++) {if ((clock / (i + 1)) <= 400) {writel(tmp | i << 0, addr);break;}}return 0;+}
/* get_lcd_clk: return lcd clock frequency */ static unsigned long exynos4_get_lcd_clk(void) { diff --git a/arch/arm/include/asm/arch-exynos/clk.h b/arch/arm/include/asm/arch-exynos/clk.h index ff155d8..b0ecdd4 100644 --- a/arch/arm/include/asm/arch-exynos/clk.h +++ b/arch/arm/include/asm/arch-exynos/clk.h @@ -36,6 +36,7 @@ unsigned long get_pwm_clk(void); unsigned long get_uart_clk(int dev_index); unsigned long get_mmc_clk(int deV_index); void set_mmc_clk(int dev_index, unsigned int div); +int exynos5_mmc_set_clk_div(int dev_index); unsigned long get_lcd_clk(void); void set_lcd_clk(void); void set_mipi_clk(void); -- 1.7.0.4
Regards, Simon
Hi Simon,
Thanks for your review comments. Please find the responses below.
Thanks & Regards Amarendra Reddy
On 20 December 2012 07:54, Simon Glass sjg@chromium.org wrote:
Hi Amar,
On Mon, Dec 17, 2012 at 3:19 AM, Amar amarendra.xt@samsung.com wrote:
This API computes the divisor value based on MPLL clock and writes it into the FSYS1 register.
Signed-off-by: Amar amarendra.xt@samsung.com
arch/arm/cpu/armv7/exynos/clock.c | 39 ++++++++++++++++++++++++++++++- arch/arm/include/asm/arch-exynos/clk.h | 1 + 2 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/arch/arm/cpu/armv7/exynos/clock.c b/arch/arm/cpu/armv7/exynos/clock.c index 731bbff..6517296 100644 --- a/arch/arm/cpu/armv7/exynos/clock.c +++ b/arch/arm/cpu/armv7/exynos/clock.c @@ -379,7 +379,7 @@ static unsigned long exynos4_get_mmc_clk(int
dev_index)
(struct exynos4_clock *)samsung_get_base_clock(); unsigned long uclk, sclk; unsigned int sel, ratio, pre_ratio;
int shift;
int shift = 0; sel = readl(&clk->src_fsys); sel = (sel >> (dev_index << 2)) & 0xf;@@ -428,7 +428,7 @@ static unsigned long exynos5_get_mmc_clk(int
dev_index)
(struct exynos5_clock *)samsung_get_base_clock(); unsigned long uclk, sclk; unsigned int sel, ratio, pre_ratio;
int shift;
int shift = 0; sel = readl(&clk->src_fsys); sel = (sel >> (dev_index << 2)) & 0xf;@@ -526,6 +526,41 @@ static void exynos5_set_mmc_clk(int dev_index, unsigned int div) writel(val, addr); }
+/* exynos5: set the mmc clock div ratio in fsys1 */ +int exynos5_mmc_set_clk_div(int dev_index)
Shouldn't this take a periph_id instead of a dev_index?
Ok will convert index int to peripheral_id in the calling function.
+{
struct exynos5_clock *clk =(struct exynos5_clock *)samsung_get_base_clock();unsigned int addr;unsigned int clock;unsigned int tmp;unsigned int i;/* get mpll clock */clock = get_pll_clk(MPLL) / 1000000;/** CLK_DIV_FSYS1* MMC0_PRE_RATIO [15:8], MMC0_RATIO [3:0]* CLK_DIV_FSYS2* MMC2_PRE_RATIO [15:8], MMC2_RATIO [3:0]*/if (dev_index < 2) {addr = (unsigned int)&clk->div_fsys1;} else {addr = (unsigned int)&clk->div_fsys2;}tmp = readl(addr) & ~FSYS1_MMC0_DIV_MASK;for (i = 0; i <= 0xf; i++) {if ((clock / (i + 1)) <= 400) {writel(tmp | i << 0, addr);break;}}return 0;+}
/* get_lcd_clk: return lcd clock frequency */ static unsigned long exynos4_get_lcd_clk(void) { diff --git a/arch/arm/include/asm/arch-exynos/clk.h b/arch/arm/include/asm/arch-exynos/clk.h index ff155d8..b0ecdd4 100644 --- a/arch/arm/include/asm/arch-exynos/clk.h +++ b/arch/arm/include/asm/arch-exynos/clk.h @@ -36,6 +36,7 @@ unsigned long get_pwm_clk(void); unsigned long get_uart_clk(int dev_index); unsigned long get_mmc_clk(int deV_index); void set_mmc_clk(int dev_index, unsigned int div); +int exynos5_mmc_set_clk_div(int dev_index); unsigned long get_lcd_clk(void); void set_lcd_clk(void); void set_mipi_clk(void); -- 1.7.0.4
Regards, Simon
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
This patch enables DWMMC for SMDK5250. Support both dt and non-dt versions.
Signed-off-by: Amar amarendra.xt@samsung.com --- board/samsung/smdk5250/smdk5250.c | 36 ++++++++++++++++++++++++++++++++---- include/configs/exynos5250-dt.h | 9 +++++++++ 2 files changed, 41 insertions(+), 4 deletions(-)
diff --git a/board/samsung/smdk5250/smdk5250.c b/board/samsung/smdk5250/smdk5250.c index 4d24978..7a9c8f6 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> @@ -192,16 +193,43 @@ int checkboard(void) #ifdef CONFIG_GENERIC_MMC int board_mmc_init(bd_t *bis) { - int err; + int err = 0, ret = 0;
+#ifdef CONFIG_OF_CONTROL + /* dwmmc initializattion for available channels */ + err = exynos_dwmmc_init(gd->fdt_blob); + if (err) { + debug("dwmmc init failed\n"); + } + ret |= err; +#else err = exynos_pinmux_config(PERIPH_ID_SDMMC0, PINMUX_FLAG_8BIT_MODE); if (err) { debug("SDMMC0 not configured\n"); - return err; } + ret |= err;
- err = s5p_mmc_init(0, 8); - return err; + /*eMMC: dwmmc Channel-0 with 8 bit bus width */ + err = exynos_dwmmc_init(0, 8); + 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; +#endif + return ret; } #endif
diff --git a/include/configs/exynos5250-dt.h b/include/configs/exynos5250-dt.h index 12f555c..3b89e20 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
@@ -116,6 +118,13 @@ #define CONFIG_SPL #define COPY_BL2_FNPTR_ADDR 0x02020030
+/* eMMC4.4 SPL */ +#define EMMC44_COPY_BL2_FNPTR_ADDR 0x02020044 +#define EMMC44_END_BOOTOP_FNPTR_ADDR 0x02020048 + +#define FSYS1_MMC0_DIV_MASK 0xff0f + + /* specific .lds file */ #define CONFIG_SPL_LDSCRIPT "board/samsung/smdk5250/smdk5250-uboot-spl.lds" #define CONFIG_SPL_TEXT_BASE 0x02023400
Hi Amar,
On Mon, Dec 17, 2012 at 3:19 AM, Amar amarendra.xt@samsung.com wrote:
This patch enables DWMMC for SMDK5250. Support both dt and non-dt versions.
Signed-off-by: Amar amarendra.xt@samsung.com
board/samsung/smdk5250/smdk5250.c | 36 ++++++++++++++++++++++++++++++++---- include/configs/exynos5250-dt.h | 9 +++++++++ 2 files changed, 41 insertions(+), 4 deletions(-)
diff --git a/board/samsung/smdk5250/smdk5250.c b/board/samsung/smdk5250/smdk5250.c index 4d24978..7a9c8f6 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> @@ -192,16 +193,43 @@ int checkboard(void) #ifdef CONFIG_GENERIC_MMC int board_mmc_init(bd_t *bis) {
int err;
int err = 0, ret = 0;+#ifdef CONFIG_OF_CONTROL
/* dwmmc initializattion for available channels */err = exynos_dwmmc_init(gd->fdt_blob);if (err) {debug("dwmmc init failed\n");}ret |= err;+#else err = exynos_pinmux_config(PERIPH_ID_SDMMC0, PINMUX_FLAG_8BIT_MODE); if (err) { debug("SDMMC0 not configured\n");
return err; }
ret |= err;
Perhaps we need an exynos5-dt.c board file instead of using smdk5250? It seems like you will end up with lots of #ifdefs otherwise.
err = s5p_mmc_init(0, 8);return err;
/*eMMC: dwmmc Channel-0 with 8 bit bus width */err = exynos_dwmmc_init(0, 8);if (err) {debug("dwmmc Channel-0 init failed\n");
Don't need {} here.
}ret |= err;err = exynos_pinmux_config(PERIPH_ID_SDMMC2, PINMUX_FLAG_NONE);if (err) {debug("SDMMC2 not configured\n");
and here
}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;+#endif
return ret;} #endif
diff --git a/include/configs/exynos5250-dt.h b/include/configs/exynos5250-dt.h index 12f555c..3b89e20 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
What does this config enable? Is it in a README somewhere?
+#define CONFIG_EXYNOS_DWMMC
#define CONFIG_BOARD_EARLY_INIT_F
@@ -116,6 +118,13 @@ #define CONFIG_SPL #define COPY_BL2_FNPTR_ADDR 0x02020030
+/* eMMC4.4 SPL */ +#define EMMC44_COPY_BL2_FNPTR_ADDR 0x02020044 +#define EMMC44_END_BOOTOP_FNPTR_ADDR 0x02020048
What are these for?
+#define FSYS1_MMC0_DIV_MASK 0xff0f
This seems like something for the SOC headers, not the board header.
/* specific .lds file */ #define CONFIG_SPL_LDSCRIPT "board/samsung/smdk5250/smdk5250-uboot-spl.lds"
#define CONFIG_SPL_TEXT_BASE 0x02023400
1.7.0.4
Regards, Simon
Hi Simon,
Thanks for the review comments. Please find the responses below.
Thanks & Regards Amarendra Reddy
On 20 December 2012 07:59, Simon Glass sjg@chromium.org wrote:
Hi Amar,
On Mon, Dec 17, 2012 at 3:19 AM, Amar amarendra.xt@samsung.com wrote:
This patch enables DWMMC for SMDK5250. Support both dt and non-dt versions.
Signed-off-by: Amar amarendra.xt@samsung.com
board/samsung/smdk5250/smdk5250.c | 36 ++++++++++++++++++++++++++++++++---- include/configs/exynos5250-dt.h | 9 +++++++++ 2 files changed, 41 insertions(+), 4 deletions(-)
diff --git a/board/samsung/smdk5250/smdk5250.c b/board/samsung/smdk5250/smdk5250.c index 4d24978..7a9c8f6 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> @@ -192,16 +193,43 @@ int checkboard(void) #ifdef CONFIG_GENERIC_MMC int board_mmc_init(bd_t *bis) {
int err;
int err = 0, ret = 0;+#ifdef CONFIG_OF_CONTROL
/* dwmmc initializattion for available channels */err = exynos_dwmmc_init(gd->fdt_blob);if (err) {debug("dwmmc init failed\n");}ret |= err;+#else err = exynos_pinmux_config(PERIPH_ID_SDMMC0, PINMUX_FLAG_8BIT_MODE); if (err) { debug("SDMMC0 not configured\n");
return err; }
ret |= err;Perhaps we need an exynos5-dt.c board file instead of using smdk5250? It seems like you will end up with lots of #ifdefs otherwise.
OK.
err = s5p_mmc_init(0, 8);return err;
/*eMMC: dwmmc Channel-0 with 8 bit bus width */err = exynos_dwmmc_init(0, 8);if (err) {debug("dwmmc Channel-0 init failed\n");Don't need {} here. OK.
}ret |= err;err = exynos_pinmux_config(PERIPH_ID_SDMMC2, PINMUX_FLAG_NONE);if (err) {debug("SDMMC2 not configured\n");and here OK.
}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;+#endif
return ret;} #endif
diff --git a/include/configs/exynos5250-dt.h b/include/configs/exynos5250-dt.h index 12f555c..3b89e20 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
What does this config enable? Is it in a README somewhere? CONFIG_DWMMC is for enaling DWMMC. It has to be added into README.
+#define CONFIG_EXYNOS_DWMMC
#define CONFIG_BOARD_EARLY_INIT_F
@@ -116,6 +118,13 @@ #define CONFIG_SPL #define COPY_BL2_FNPTR_ADDR 0x02020030
+/* eMMC4.4 SPL */ +#define EMMC44_COPY_BL2_FNPTR_ADDR 0x02020044 +#define EMMC44_END_BOOTOP_FNPTR_ADDR 0x02020048
What are these for?
These are the IROM function pointers used druing SPL boot. Any how these #defines wii be removed, as a table for all iROM function pointers will be maintained in spl_boot.c.
+#define FSYS1_MMC0_DIV_MASK 0xff0f
This seems like something for the SOC headers, not the board header.
FSYS1_MMC0_DIV_MASK will be moved into include/asm/arch/clk.h
/* specific .lds file */ #define CONFIG_SPL_LDSCRIPT "board/samsung/smdk5250/smdk5250-uboot-spl.lds"
#define CONFIG_SPL_TEXT_BASE 0x02023400
1.7.0.4
Regards, Simon
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
This pathc adds APIs to open, close and to create boot partiton for eMMC.
Signed-off-by: Amar amarendra.xt@samsung.com --- drivers/mmc/mmc.c | 118 +++++++++++++++++++++++++++++++++++++++++++++++++++++ include/mmc.h | 16 +++++++ 2 files changed, 134 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index 5ffd8c5..88b0435 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -1302,3 +1302,121 @@ int mmc_initialize(bd_t *bis)
return 0; } + +int mmc_boot_partition_size_change(struct mmc *mmc, unsigned long bootsize, + unsigned long rpmbsize) +{ + int err; + struct mmc_cmd cmd; + + /* Only use this command for raw eMMC moviNAND */ + /* Enter backdoor mode */ + cmd.cmdidx = MMC_CMD_RES_MAN; + cmd.resp_type = MMC_RSP_R1b; + cmd.cmdarg = MMC_CMD62_ARG1; + + err = mmc_send_cmd(mmc, &cmd, NULL); + if (err) { + debug("mmc_boot_partition_size_change: Error1 = %d\n", err); + return err; + } + + /* Boot partition changing mode */ + cmd.cmdidx = MMC_CMD_RES_MAN; + cmd.resp_type = MMC_RSP_R1b; + cmd.cmdarg = MMC_CMD62_ARG2; + + err = mmc_send_cmd(mmc, &cmd, NULL); + if (err) { + debug("mmc_boot_partition_size_change: Error2 = %d\n", err); + return err; + } + /* boot partition size is multiple of 128KB */ + bootsize = ((bootsize*1024)/128); + + /* Arg: boot partition size */ + cmd.cmdidx = MMC_CMD_RES_MAN; + cmd.resp_type = MMC_RSP_R1b; + cmd.cmdarg = bootsize; + + err = mmc_send_cmd(mmc, &cmd, NULL); + if (err) { + debug("mmc_boot_partition_size_change: Error3 = %d\n", err); + return err; + } + /* RPMB partition size is multiple of 128KB */ + rpmbsize = ((rpmbsize*1024)/128); + /* Arg: RPMB partition size */ + cmd.cmdidx = MMC_CMD_RES_MAN; + cmd.resp_type = MMC_RSP_R1b; + cmd.cmdarg = rpmbsize; + + err = mmc_send_cmd(mmc, &cmd, NULL); + if (err) { + debug("mmc_boot_partition_size_change: Error4 = %d\n", err); + return err; + } + return 0; +} + +int mmc_boot_open(struct mmc *mmc) +{ + int err; + struct mmc_cmd cmd; + + /* Boot ack enable, boot partition enable , boot partition access */ + cmd.cmdidx = MMC_CMD_SWITCH; + cmd.resp_type = MMC_RSP_R1b; + + cmd.cmdarg = (MMC_SWITCH_MODE_WRITE_BYTE << 24 | + EXT_CSD_PART_CONF << 16 | + (EXT_CSD_BOOT_ACK_ENABLE | + EXT_CSD_BOOT_PARTITION_ENABLE | + EXT_CSD_PARTITION_ACCESS_ENABLE) << 8); + + err = mmc_send_cmd(mmc, &cmd, NULL); + if (err) { + debug("mmc_boot_open: Error1 = %d\n", err); + return err; + } + + /* 4bit transfer mode at booting time. */ + cmd.cmdidx = MMC_CMD_SWITCH; + cmd.resp_type = MMC_RSP_R1b; + + cmd.cmdarg = (MMC_SWITCH_MODE_WRITE_BYTE << 24| + EXT_CSD_BOOT_BUS_WIDTH << 16| + ((1<<0) << 8)); + + err = mmc_send_cmd(mmc, &cmd, NULL); + if (err) { + debug("mmc_boot_open: Error2 = %d\n", err); + return err; + } + + return 0; +} + +int mmc_boot_close(struct mmc *mmc) +{ + int err; + struct mmc_cmd cmd; + + /* Boot ack enable, boot partition enable , boot partition access */ + cmd.cmdidx = MMC_CMD_SWITCH; + cmd.resp_type = MMC_RSP_R1b; + + cmd.cmdarg = (MMC_SWITCH_MODE_WRITE_BYTE << 24| + EXT_CSD_PART_CONF << 16| + (EXT_CSD_BOOT_ACK_ENABLE | + EXT_CSD_BOOT_PARTITION_ENABLE | + EXT_CSD_PARTITION_ACCESS_DISABLE) << 8); + + err = mmc_send_cmd(mmc, &cmd, NULL); + if (err) { + debug("mmc_boot_close: Error = %d\n", err); + return err; + } + + return 0; +} diff --git a/include/mmc.h b/include/mmc.h index a13e2bd..999f0a3 100644 --- a/include/mmc.h +++ b/include/mmc.h @@ -86,6 +86,11 @@ #define MMC_CMD_APP_CMD 55 #define MMC_CMD_SPI_READ_OCR 58 #define MMC_CMD_SPI_CRC_ON_OFF 59 +#define MMC_CMD_RES_MAN 62 + +#define MMC_CMD62_ARG1 0xefac62ec +#define MMC_CMD62_ARG2 0xcbaea7 +
#define SD_CMD_SEND_RELATIVE_ADDR 3 #define SD_CMD_SWITCH_FUNC 6 @@ -153,6 +158,7 @@ */ #define EXT_CSD_PARTITIONING_SUPPORT 160 /* RO */ #define EXT_CSD_ERASE_GROUP_DEF 175 /* R/W */ +#define EXT_CSD_BOOT_BUS_WIDTH 177 #define EXT_CSD_PART_CONF 179 /* R/W */ #define EXT_CSD_BUS_WIDTH 183 /* R/W */ #define EXT_CSD_HS_TIMING 185 /* R/W */ @@ -177,6 +183,12 @@ #define EXT_CSD_BUS_WIDTH_4 1 /* Card is in 4 bit mode */ #define EXT_CSD_BUS_WIDTH_8 2 /* Card is in 8 bit mode */
+#define EXT_CSD_BOOT_ACK_ENABLE (1<<6) +#define EXT_CSD_BOOT_PARTITION_ENABLE (1<<3) +#define EXT_CSD_PARTITION_ACCESS_ENABLE (1<<0) +#define EXT_CSD_PARTITION_ACCESS_DISABLE (0<<0) + + #define R1_ILLEGAL_COMMAND (1 << 22) #define R1_APP_CMD (1 << 5)
@@ -275,6 +287,10 @@ int board_mmc_getcd(struct mmc *mmc); int mmc_switch_part(int dev_num, unsigned int part_num); int mmc_getcd(struct mmc *mmc); void spl_mmc_load(void) __noreturn; +int mmc_boot_partition_size_change(struct mmc *mmc, unsigned long bootsize, + unsigned long rpmbsize); +int mmc_boot_open(struct mmc *mmc); +int mmc_boot_close(struct mmc *mmc);
#ifdef CONFIG_GENERIC_MMC #define mmc_host_is_spi(mmc) ((mmc)->host_caps & MMC_MODE_SPI)
Hi Amar,
On Mon, Dec 17, 2012 at 3:19 AM, Amar amarendra.xt@samsung.com wrote:
This pathc adds APIs to open, close and to create boot partiton for eMMC.
Signed-off-by: Amar amarendra.xt@samsung.com
I think you should run checkpatch (or patman!) on your patches to get rid of little errors. Or maybe you need to upgrade your checkpatch.
drivers/mmc/mmc.c | 118 +++++++++++++++++++++++++++++++++++++++++++++++++++++ include/mmc.h | 16 +++++++ 2 files changed, 134 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index 5ffd8c5..88b0435 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -1302,3 +1302,121 @@ int mmc_initialize(bd_t *bis)
return 0;}
+int mmc_boot_partition_size_change(struct mmc *mmc, unsigned long bootsize,
unsigned long rpmbsize)+{
int err;struct mmc_cmd cmd;/* Only use this command for raw eMMC moviNAND *//* Enter backdoor mode */
/* * line 1 * line 2 */
cmd.cmdidx = MMC_CMD_RES_MAN;cmd.resp_type = MMC_RSP_R1b;cmd.cmdarg = MMC_CMD62_ARG1;
[snip]
Regards, Simon
Hi Simon,
Ok. I will ensure to get rid of these little errors.
Thanks & Regards Amarendra Reddy
On 20 December 2012 08:01, Simon Glass sjg@chromium.org wrote:
Hi Amar,
On Mon, Dec 17, 2012 at 3:19 AM, Amar amarendra.xt@samsung.com wrote:
This pathc adds APIs to open, close and to create boot partiton for eMMC.
Signed-off-by: Amar amarendra.xt@samsung.com
I think you should run checkpatch (or patman!) on your patches to get rid of little errors. Or maybe you need to upgrade your checkpatch.
drivers/mmc/mmc.c | 118 +++++++++++++++++++++++++++++++++++++++++++++++++++++ include/mmc.h | 16 +++++++ 2 files changed, 134 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index 5ffd8c5..88b0435 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -1302,3 +1302,121 @@ int mmc_initialize(bd_t *bis)
return 0;}
+int mmc_boot_partition_size_change(struct mmc *mmc, unsigned long bootsize,
unsigned long rpmbsize)+{
int err;struct mmc_cmd cmd;/* Only use this command for raw eMMC moviNAND *//* Enter backdoor mode *//*
- line 1
- line 2
*/
cmd.cmdidx = MMC_CMD_RES_MAN;cmd.resp_type = MMC_RSP_R1b;cmd.cmdarg = MMC_CMD62_ARG1;[snip]
Regards, Simon
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
This patch adds support for eMMC booting on SMDK5250
Signed-off-by: Amar amarendra.xt@samsung.com --- board/samsung/smdk5250/spl_boot.c | 38 ++++++++++++++++++++++++++++++++++++- 1 files changed, 37 insertions(+), 1 deletions(-)
diff --git a/board/samsung/smdk5250/spl_boot.c b/board/samsung/smdk5250/spl_boot.c index d8f3c1e..2648b4e 100644 --- a/board/samsung/smdk5250/spl_boot.c +++ b/board/samsung/smdk5250/spl_boot.c @@ -23,15 +23,40 @@ #include<common.h> #include<config.h>
+#include <asm/arch/clock.h> +#include <asm/arch/clk.h> + +#define FSYS1_MMC0_DIV_VAL 0x0701 + enum boot_mode { BOOT_MODE_MMC = 4, + BOOT_MODE_eMMC = 8, /* eMMC44 */ BOOT_MODE_SERIAL = 20, /* Boot based on Operating Mode pin settings */ BOOT_MODE_OM = 32, BOOT_MODE_USB, /* Boot using USB download */ };
- typedef u32 (*spi_copy_func_t)(u32 offset, u32 nblock, u32 dst); +typedef u32 (*spi_copy_func_t)(u32 offset, u32 nblock, u32 dst); +static void set_emmc_clk(void); + +/* +* Set MMC0 clock divisor value. +* So that the mmc0 device operating freq is 50MHz. +*/ +static void set_emmc_clk(void) +{ + struct exynos5_clock *clk = (struct exynos5_clock *)EXYNOS5_CLOCK_BASE; + unsigned int addr; + unsigned int div_mmc; + + addr = (unsigned int) &clk->div_fsys1; + + div_mmc = readl(addr) & ~FSYS1_MMC0_DIV_MASK; + div_mmc |= FSYS1_MMC0_DIV_VAL; + writel(div_mmc, addr); +} +
/* * Copy U-boot from mmc to RAM: @@ -43,6 +68,8 @@ void copy_uboot_to_ram(void) spi_copy_func_t spi_copy; enum boot_mode bootmode; u32 (*copy_bl2)(u32, u32, u32); + u32 (*copy_bl2_emmc)(u32, u32); + void (*end_bootop_emmc)(void);
bootmode = readl(EXYNOS5_POWER_BASE) & OM_STAT;
@@ -57,6 +84,15 @@ void copy_uboot_to_ram(void) copy_bl2(BL2_START_OFFSET, BL2_SIZE_BLOC_COUNT, CONFIG_SYS_TEXT_BASE); break; + case BOOT_MODE_eMMC: + set_emmc_clk(); + end_bootop_emmc = (void *) *(u32 *)EMMC44_END_BOOTOP_FNPTR_ADDR; + copy_bl2_emmc = (void *) *(u32 *)EMMC44_COPY_BL2_FNPTR_ADDR; + + copy_bl2_emmc(BL2_SIZE_BLOC_COUNT, CONFIG_SYS_TEXT_BASE); + end_bootop_emmc(); + break; + default: break; }
Hi Amar,
On Mon, Dec 17, 2012 at 3:19 AM, Amar amarendra.xt@samsung.com wrote:
This patch adds support for eMMC booting on SMDK5250
Signed-off-by: Amar amarendra.xt@samsung.com
board/samsung/smdk5250/spl_boot.c | 38 ++++++++++++++++++++++++++++++++++++- 1 files changed, 37 insertions(+), 1 deletions(-)
diff --git a/board/samsung/smdk5250/spl_boot.c b/board/samsung/smdk5250/spl_boot.c index d8f3c1e..2648b4e 100644 --- a/board/samsung/smdk5250/spl_boot.c +++ b/board/samsung/smdk5250/spl_boot.c @@ -23,15 +23,40 @@ #include<common.h> #include<config.h>
+#include <asm/arch/clock.h> +#include <asm/arch/clk.h>
+#define FSYS1_MMC0_DIV_VAL 0x0701
Should go in arch/arm/include/... ?
enum boot_mode { BOOT_MODE_MMC = 4,
BOOT_MODE_eMMC = 8, /* eMMC44 */
I think should you use upper case E, although I'm not completely sure.
BOOT_MODE_SERIAL = 20, /* Boot based on Operating Mode pin settings */ BOOT_MODE_OM = 32, BOOT_MODE_USB, /* Boot using USB download */};
typedef u32 (*spi_copy_func_t)(u32 offset, u32 nblock, u32 dst);+typedef u32 (*spi_copy_func_t)(u32 offset, u32 nblock, u32 dst);
Should avoid adding typedefs.
+static void set_emmc_clk(void);
+/* +* Set MMC0 clock divisor value. +* So that the mmc0 device operating freq is 50MHz.
Do we only support booting from mmc0? That's fine, but I suggest adding a little comment.
+*/ +static void set_emmc_clk(void) +{
struct exynos5_clock *clk = (struct exynos5_clock*)EXYNOS5_CLOCK_BASE;
unsigned int addr;unsigned int div_mmc;addr = (unsigned int) &clk->div_fsys1;div_mmc = readl(addr) & ~FSYS1_MMC0_DIV_MASK;div_mmc |= FSYS1_MMC0_DIV_VAL;writel(div_mmc, addr);
Can this function go in clock.c?
+}
/*
- Copy U-boot from mmc to RAM:
@@ -43,6 +68,8 @@ void copy_uboot_to_ram(void) spi_copy_func_t spi_copy; enum boot_mode bootmode; u32 (*copy_bl2)(u32, u32, u32);
u32 (*copy_bl2_emmc)(u32, u32);void (*end_bootop_emmc)(void); bootmode = readl(EXYNOS5_POWER_BASE) & OM_STAT;@@ -57,6 +84,15 @@ void copy_uboot_to_ram(void) copy_bl2(BL2_START_OFFSET, BL2_SIZE_BLOC_COUNT, CONFIG_SYS_TEXT_BASE); break;
case BOOT_MODE_eMMC:set_emmc_clk();end_bootop_emmc = (void *) *(u32*)EMMC44_END_BOOTOP_FNPTR_ADDR;
copy_bl2_emmc = (void *) *(u32*)EMMC44_COPY_BL2_FNPTR_ADDR;
I think these are pointers to functions in the IROM. Do they have the same signature? Is it possible to create a table of them somewhere instead of using defines?
copy_bl2_emmc(BL2_SIZE_BLOC_COUNT, CONFIG_SYS_TEXT_BASE);end_bootop_emmc();break;default: break; }-- 1.7.0.4
Regards, Simon
Hi SImon,
Thanks for the review comments. Please find below the responses for your comments.
Thanks & Regards Amarendra
On 20 December 2012 08:05, Simon Glass sjg@chromium.org wrote:
Hi Amar,
On Mon, Dec 17, 2012 at 3:19 AM, Amar amarendra.xt@samsung.com wrote:
This patch adds support for eMMC booting on SMDK5250
Signed-off-by: Amar amarendra.xt@samsung.com
board/samsung/smdk5250/spl_boot.c | 38 ++++++++++++++++++++++++++++++++++++- 1 files changed, 37 insertions(+), 1 deletions(-)
diff --git a/board/samsung/smdk5250/spl_boot.c b/board/samsung/smdk5250/spl_boot.c index d8f3c1e..2648b4e 100644 --- a/board/samsung/smdk5250/spl_boot.c +++ b/board/samsung/smdk5250/spl_boot.c @@ -23,15 +23,40 @@ #include<common.h> #include<config.h>
+#include <asm/arch/clock.h> +#include <asm/arch/clk.h>
+#define FSYS1_MMC0_DIV_VAL 0x0701
Should go in arch/arm/include/... ?
OK. shall move it.
enum boot_mode { BOOT_MODE_MMC = 4,
BOOT_MODE_eMMC = 8, /* eMMC44 */I think should you use upper case E, although I'm not completely sure. OK. will make it upper case to be consistent every where.
BOOT_MODE_SERIAL = 20, /* Boot based on Operating Mode pin settings */ BOOT_MODE_OM = 32, BOOT_MODE_USB, /* Boot using USB download */};
typedef u32 (*spi_copy_func_t)(u32 offset, u32 nblock, u32 dst);+typedef u32 (*spi_copy_func_t)(u32 offset, u32 nblock, u32 dst);
Should avoid adding typedefs. Ok.
+static void set_emmc_clk(void);
+/* +* Set MMC0 clock divisor value. +* So that the mmc0 device operating freq is 50MHz.
Do we only support booting from mmc0? That's fine, but I suggest adding a little comment. OK.
+*/ +static void set_emmc_clk(void) +{
struct exynos5_clock *clk = (struct exynos5_clock*)EXYNOS5_CLOCK_BASE;
unsigned int addr;unsigned int div_mmc;addr = (unsigned int) &clk->div_fsys1;div_mmc = readl(addr) & ~FSYS1_MMC0_DIV_MASK;div_mmc |= FSYS1_MMC0_DIV_VAL;writel(div_mmc, addr);Can this function go in clock.c? This function is used by SPL, only during EMMC boot. Hence can be moved into board/samsung/smdk5250/clock_init.c.
Please comment on this.
+}
/*
- Copy U-boot from mmc to RAM:
@@ -43,6 +68,8 @@ void copy_uboot_to_ram(void) spi_copy_func_t spi_copy; enum boot_mode bootmode; u32 (*copy_bl2)(u32, u32, u32);
u32 (*copy_bl2_emmc)(u32, u32);void (*end_bootop_emmc)(void); bootmode = readl(EXYNOS5_POWER_BASE) & OM_STAT;@@ -57,6 +84,15 @@ void copy_uboot_to_ram(void) copy_bl2(BL2_START_OFFSET, BL2_SIZE_BLOC_COUNT, CONFIG_SYS_TEXT_BASE); break;
case BOOT_MODE_eMMC:set_emmc_clk();end_bootop_emmc = (void *) *(u32*)EMMC44_END_BOOTOP_FNPTR_ADDR;
copy_bl2_emmc = (void *) *(u32*)EMMC44_COPY_BL2_FNPTR_ADDR;
I think these are pointers to functions in the IROM. Do they have the same signature? Is it possible to create a table of them somewhere instead of using defines? OK.
The signatures are different for different booting devices. More over, SDMMC / SPI / USB booting requires only one function pointer. Where as EMMC 4.3/4.4 requires two of those function pointers.
May be something like this can be used to create table void (*ptr_table[])(void) = { (void *)0x02020030, /* iROM Function Pointer - SDMMC boot */ (void *)0x0202003C, /* iROM Function Pointer - EMMC 4.3 boot */ (void *)0x02020040, /* iROM Function Pointer - EMMC 4.3 end boot op */ (void *)0x02020044, /* iROM Function Pointer - EMMC 4.4 boot */ (void *)0x02020048, /* iROM Function Pointer - EMMC 4.4 end boot op */ (void *)0x02020050, /* iROM Function Pointer - EFNAND boot */ (void *)0x02020058, /* iROM Function Pointer - SPI boot */ (void *)0x02020070 /* iROM Function Pointer - USB boot */ };
Usage: copy_bl2_from_emmc = (void *) *(u32 *)ptr_table[3];
end_bootop_from_emmc = (void *) *(u32 *)ptr_table[4];
copy_bl2_emmc(BL2_SIZE_BLOC_COUNT, CONFIG_SYS_TEXT_BASE);end_bootop_emmc();break;default: break; }-- 1.7.0.4
Regards, Simon
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi Amarendra,
On Thu, Dec 20, 2012 at 5:53 AM, Amarendra Reddy amar.lavanuru@gmail.com wrote:
Hi SImon,
Thanks for the review comments. Please find below the responses for your comments.
Thanks & Regards Amarendra
On 20 December 2012 08:05, Simon Glass sjg@chromium.org wrote:
Hi Amar,
On Mon, Dec 17, 2012 at 3:19 AM, Amar amarendra.xt@samsung.com wrote:
This patch adds support for eMMC booting on SMDK5250
Signed-off-by: Amar amarendra.xt@samsung.com
board/samsung/smdk5250/spl_boot.c | 38 ++++++++++++++++++++++++++++++++++++- 1 files changed, 37 insertions(+), 1 deletions(-)
diff --git a/board/samsung/smdk5250/spl_boot.c b/board/samsung/smdk5250/spl_boot.c index d8f3c1e..2648b4e 100644 --- a/board/samsung/smdk5250/spl_boot.c +++ b/board/samsung/smdk5250/spl_boot.c @@ -23,15 +23,40 @@ #include<common.h> #include<config.h>
+#include <asm/arch/clock.h> +#include <asm/arch/clk.h>
+#define FSYS1_MMC0_DIV_VAL 0x0701
Should go in arch/arm/include/... ?
OK. shall move it.
enum boot_mode { BOOT_MODE_MMC = 4,
BOOT_MODE_eMMC = 8, /* eMMC44 */I think should you use upper case E, although I'm not completely sure. OK. will make it upper case to be consistent every where.
BOOT_MODE_SERIAL = 20, /* Boot based on Operating Mode pin settings */ BOOT_MODE_OM = 32, BOOT_MODE_USB, /* Boot using USB download */};
typedef u32 (*spi_copy_func_t)(u32 offset, u32 nblock, u32 dst);+typedef u32 (*spi_copy_func_t)(u32 offset, u32 nblock, u32 dst);
Should avoid adding typedefs. Ok.
+static void set_emmc_clk(void);
+/* +* Set MMC0 clock divisor value. +* So that the mmc0 device operating freq is 50MHz.
Do we only support booting from mmc0? That's fine, but I suggest adding a little comment. OK.
+*/ +static void set_emmc_clk(void) +{
struct exynos5_clock *clk = (struct exynos5_clock*)EXYNOS5_CLOCK_BASE;
unsigned int addr;unsigned int div_mmc;addr = (unsigned int) &clk->div_fsys1;div_mmc = readl(addr) & ~FSYS1_MMC0_DIV_MASK;div_mmc |= FSYS1_MMC0_DIV_VAL;writel(div_mmc, addr);Can this function go in clock.c? This function is used by SPL, only during EMMC boot. Hence can be moved into board/samsung/smdk5250/clock_init.c.
Please comment on this.
Yes that seems reasonable. There is a lot of code in clock_init that should eventually move to U-Boot proper (i.e. we should only init clocks in SPL that are actually needed in SPL). This can be addressed after Hatim (copied) has finished creating the upstream patches for clock_init / lowlevel_init refactor.
+}
/*
- Copy U-boot from mmc to RAM:
@@ -43,6 +68,8 @@ void copy_uboot_to_ram(void) spi_copy_func_t spi_copy; enum boot_mode bootmode; u32 (*copy_bl2)(u32, u32, u32);
u32 (*copy_bl2_emmc)(u32, u32);void (*end_bootop_emmc)(void); bootmode = readl(EXYNOS5_POWER_BASE) & OM_STAT;@@ -57,6 +84,15 @@ void copy_uboot_to_ram(void) copy_bl2(BL2_START_OFFSET, BL2_SIZE_BLOC_COUNT, CONFIG_SYS_TEXT_BASE); break;
case BOOT_MODE_eMMC:set_emmc_clk();end_bootop_emmc = (void *) *(u32*)EMMC44_END_BOOTOP_FNPTR_ADDR;
copy_bl2_emmc = (void *) *(u32*)EMMC44_COPY_BL2_FNPTR_ADDR;
I think these are pointers to functions in the IROM. Do they have the same signature? Is it possible to create a table of them somewhere instead of using defines? OK.
The signatures are different for different booting devices. More over, SDMMC / SPI / USB booting requires only one function pointer. Where as EMMC 4.3/4.4 requires two of those function pointers.
May be something like this can be used to create table void (*ptr_table[])(void) = { (void *)0x02020030, /* iROM Function Pointer - SDMMC boot */ (void *)0x0202003C, /* iROM Function Pointer - EMMC 4.3 boot */ (void *)0x02020040, /* iROM Function Pointer - EMMC 4.3 end boot op */ (void *)0x02020044, /* iROM Function Pointer - EMMC 4.4 boot */ (void *)0x02020048, /* iROM Function Pointer - EMMC 4.4 end boot op */ (void *)0x02020050, /* iROM Function Pointer - EFNAND boot */ (void *)0x02020058, /* iROM Function Pointer - SPI boot */ (void *)0x02020070 /* iROM Function Pointer - USB boot */ };
Well I suggest just having addresses in the table (ulong) instead of pointers if you can, so there are fewer casts.
Usage: copy_bl2_from_emmc = (void *) *(u32 *)ptr_table[3];
end_bootop_from_emmc = (void *) *(u32 *)ptr_table[4];
Seems reasonable, but please do create an enum for the available function numbers rather than just using 3 or 4.
copy_bl2_emmc(BL2_SIZE_BLOC_COUNT,CONFIG_SYS_TEXT_BASE);
end_bootop_emmc();break;default: break; }-- 1.7.0.4
Regards, Simon
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Regards, Simon
Hi Simon, Thanks for review comments. Please find my responses below.
Thanks & Regards Amarendra
On 27 December 2012 01:29, Simon Glass sjg@chromium.org wrote:
Hi Amarendra,
On Thu, Dec 20, 2012 at 5:53 AM, Amarendra Reddy amar.lavanuru@gmail.com wrote:
Hi SImon,
Thanks for the review comments. Please find below the responses for your comments.
Thanks & Regards Amarendra
On 20 December 2012 08:05, Simon Glass sjg@chromium.org wrote:
Hi Amar,
On Mon, Dec 17, 2012 at 3:19 AM, Amar amarendra.xt@samsung.com wrote:
This patch adds support for eMMC booting on SMDK5250
Signed-off-by: Amar amarendra.xt@samsung.com
board/samsung/smdk5250/spl_boot.c | 38 ++++++++++++++++++++++++++++++++++++- 1 files changed, 37 insertions(+), 1 deletions(-)
diff --git a/board/samsung/smdk5250/spl_boot.c b/board/samsung/smdk5250/spl_boot.c index d8f3c1e..2648b4e 100644 --- a/board/samsung/smdk5250/spl_boot.c +++ b/board/samsung/smdk5250/spl_boot.c @@ -23,15 +23,40 @@ #include<common.h> #include<config.h>
+#include <asm/arch/clock.h> +#include <asm/arch/clk.h>
+#define FSYS1_MMC0_DIV_VAL 0x0701
Should go in arch/arm/include/... ?
OK. shall move it.
enum boot_mode { BOOT_MODE_MMC = 4,
BOOT_MODE_eMMC = 8, /* eMMC44 */I think should you use upper case E, although I'm not completely sure. OK. will make it upper case to be consistent every where.
BOOT_MODE_SERIAL = 20, /* Boot based on Operating Mode pin settings */ BOOT_MODE_OM = 32, BOOT_MODE_USB, /* Boot using USB download */};
typedef u32 (*spi_copy_func_t)(u32 offset, u32 nblock, u32dst);
+typedef u32 (*spi_copy_func_t)(u32 offset, u32 nblock, u32 dst);
Should avoid adding typedefs. Ok.
+static void set_emmc_clk(void);
+/* +* Set MMC0 clock divisor value. +* So that the mmc0 device operating freq is 50MHz.
Do we only support booting from mmc0? That's fine, but I suggest adding
a
little comment. OK.
+*/ +static void set_emmc_clk(void) +{
struct exynos5_clock *clk = (struct exynos5_clock*)EXYNOS5_CLOCK_BASE;
unsigned int addr;unsigned int div_mmc;addr = (unsigned int) &clk->div_fsys1;div_mmc = readl(addr) & ~FSYS1_MMC0_DIV_MASK;div_mmc |= FSYS1_MMC0_DIV_VAL;writel(div_mmc, addr);Can this function go in clock.c? This function is used by SPL, only during EMMC boot. Hence can be moved into board/samsung/smdk5250/clock_init.c.
Please comment on this.
Yes that seems reasonable. There is a lot of code in clock_init that should eventually move to U-Boot proper (i.e. we should only init clocks in SPL that are actually needed in SPL). This can be addressed after Hatim (copied) has finished creating the upstream patches for clock_init / lowlevel_init refactor. Ok.
+}
/*
- Copy U-boot from mmc to RAM:
@@ -43,6 +68,8 @@ void copy_uboot_to_ram(void) spi_copy_func_t spi_copy; enum boot_mode bootmode; u32 (*copy_bl2)(u32, u32, u32);
u32 (*copy_bl2_emmc)(u32, u32);void (*end_bootop_emmc)(void); bootmode = readl(EXYNOS5_POWER_BASE) & OM_STAT;@@ -57,6 +84,15 @@ void copy_uboot_to_ram(void) copy_bl2(BL2_START_OFFSET, BL2_SIZE_BLOC_COUNT, CONFIG_SYS_TEXT_BASE); break;
case BOOT_MODE_eMMC:set_emmc_clk();end_bootop_emmc = (void *) *(u32*)EMMC44_END_BOOTOP_FNPTR_ADDR;
copy_bl2_emmc = (void *) *(u32*)EMMC44_COPY_BL2_FNPTR_ADDR;
I think these are pointers to functions in the IROM. Do they have the
same
signature? Is it possible to create a table of them somewhere instead of using defines? OK.
The signatures are different for different booting devices. More over, SDMMC / SPI / USB booting requires only one function pointer. Where as EMMC 4.3/4.4 requires two of those function pointers.
May be something like this can be used to create table void (*ptr_table[])(void) = { (void *)0x02020030, /* iROM Function Pointer - SDMMC boot
*/
(void *)0x0202003C, /* iROM Function Pointer - EMMC 4.3 boot*/
(void *)0x02020040, /* iROM Function Pointer - EMMC 4.3 end bootop
*/ (void *)0x02020044, /* iROM Function Pointer - EMMC 4.4 boot
*/
(void *)0x02020048, /* iROM Function Pointer - EMMC 4.4 end bootop
*/ (void *)0x02020050, /* iROM Function Pointer - EFNAND boot
*/
(void *)0x02020058, /* iROM Function Pointer - SPI boot*/
(void *)0x02020070 /* iROM Function Pointer - USB boot*/
};Well I suggest just having addresses in the table (ulong) instead of pointers if you can, so there are fewer casts. Ok.
Usage: copy_bl2_from_emmc = (void *) *(u32 *)ptr_table[3];
end_bootop_from_emmc = (void *) *(u32 *)ptr_table[4];
Seems reasonable, but please do create an enum for the available function numbers rather than just using 3 or 4. Yes, I have used enum in actual implementation. The above was an example.
copy_bl2_emmc(BL2_SIZE_BLOC_COUNT,CONFIG_SYS_TEXT_BASE);
end_bootop_emmc();break;default: break; }-- 1.7.0.4
Regards, Simon
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Regards, Simon
This patch adds commands to open, close and create partitions on eMMC
Signed-off-by: Amar amarendra.xt@samsung.com --- common/cmd_mmc.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 100 insertions(+), 1 deletions(-)
diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c index 62a1c22..1fd6b94 100644 --- a/common/cmd_mmc.c +++ b/common/cmd_mmc.c @@ -248,6 +248,102 @@ static int do_mmcops(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) curr_device, mmc->part_num);
return 0; + } else if (strcmp(argv[1], "open") == 0) { + int dev; + struct mmc *mmc; + + if (argc == 2) + dev = curr_device; + else if (argc == 3) + dev = simple_strtoul(argv[2], NULL, 10); + else if (argc == 4) + return 1; + + else + return CMD_RET_USAGE; + + mmc = find_mmc_device(dev); + if (!mmc) { + printf("no mmc device at slot %x\n", dev); + return 1; + } + + if (IS_SD(mmc)) { + printf("SD device cannot be opened/closed\n"); + return 1; + } + + if (!(mmc_boot_open(mmc))) { + printf("eMMC OPEN Success.!!\n"); + printf("\t\t\t!!!Notice!!!\n"); + printf("!You must close eMMC boot Partition" + "after all image writing!\n"); + printf("!eMMC boot partition has continuity" + "at image writing time.!\n"); + printf("!So, Do not close boot partition, Before," + "all images is written.!\n"); + } else { + printf("eMMC OPEN Failed.!!\n"); + } + return 0; + + } else if (strcmp(argv[1], "close") == 0) { + int dev; + struct mmc *mmc; + + if (argc == 2) + dev = curr_device; + else if (argc == 3) + dev = simple_strtoul(argv[2], NULL, 10); + else if (argc == 4) + return 1; + else + return CMD_RET_USAGE; + + mmc = find_mmc_device(dev); + if (!mmc) { + printf("no mmc device at slot %x\n", dev); + return 1; + } + + if (IS_SD(mmc)) { + printf("SD device cannot be opened/closed\n"); + return 1; + } + + if (!(mmc_boot_close(mmc))) + printf("eMMC CLOSE Success.!!\n"); + else + printf("eMMC CLOSE Failed.!!\n"); + + return 0; + + } else if (strcmp(argv[1], "bootpart") == 0) { + int dev; + dev = simple_strtoul(argv[2], NULL, 10); + + struct mmc *mmc = find_mmc_device(dev); + u32 bootsize = simple_strtoul(argv[3], NULL, 10); + u32 rpmbsize = simple_strtoul(argv[4], NULL, 10); + + if (!mmc) { + printf("no mmc device at slot %x\n", dev); + return 1; + } + + if (IS_SD(mmc)) { + printf("It is not a eMMC device\n"); + return 1; + } + + if (0 == mmc_boot_partition_size_change(mmc, + bootsize, rpmbsize)) { + printf("eMMC boot partition Size %d MB!!\n", bootsize); + printf("eMMC RPMB partition Size %d MB!!\n", rpmbsize); + } else { + printf("eMMC boot partition Size change Failed.!!\n"); + } + return 0; }
if (strcmp(argv[1], "read") == 0) @@ -318,5 +414,8 @@ U_BOOT_CMD( "mmc rescan\n" "mmc part - lists available partition on current mmc device\n" "mmc dev [dev] [part] - show or set current mmc device [partition]\n" - "mmc list - lists available devices"); + "mmc list - lists available devices\n" + "mmc open <device num> - opens the specified device\n" + "mmc close <device num> - closes the specified device\n" + "mmc bootpart <device num> <boot part size MB> <RPMB part size MB>\n"); #endif
Hi Amar,
On Mon, Dec 17, 2012 at 3:19 AM, Amar amarendra.xt@samsung.com wrote:
This patch adds commands to open, close and create partitions on eMMC
Signed-off-by: Amar amarendra.xt@samsung.com
common/cmd_mmc.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 100 insertions(+), 1 deletions(-)
diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c index 62a1c22..1fd6b94 100644 --- a/common/cmd_mmc.c +++ b/common/cmd_mmc.c @@ -248,6 +248,102 @@ static int do_mmcops(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) curr_device, mmc->part_num);
return 0;
} else if (strcmp(argv[1], "open") == 0) {int dev;struct mmc *mmc;if (argc == 2)dev = curr_device;else if (argc == 3)dev = simple_strtoul(argv[2], NULL, 10);else if (argc == 4)return 1;
Should this be CMD_RET_USAGE? What is returning 1 for?
elsereturn CMD_RET_USAGE;mmc = find_mmc_device(dev);if (!mmc) {printf("no mmc device at slot %x\n", dev);return 1;}if (IS_SD(mmc)) {printf("SD device cannot be opened/closed\n");return 1;}if (!(mmc_boot_open(mmc))) {printf("eMMC OPEN Success.!!\n");printf("\t\t\t!!!Notice!!!\n");printf("!You must close eMMC boot Partition""after all imagewriting!\n");
printf("!eMMC boot partition has continuity""at image writingtime.!\n");
printf("!So, Do not close boot partition, Before,""all images iswritten.!\n");
Maybe:
So, do not close boot partition before all images are written
+ } else {
printf("eMMC OPEN Failed.!!\n");
Is it eMMC or MMC? Lower case or upper case? Your messages should be consistent. And you don't need the !!! I think.
}return 0;} else if (strcmp(argv[1], "close") == 0) {int dev;struct mmc *mmc;if (argc == 2)dev = curr_device;else if (argc == 3)dev = simple_strtoul(argv[2], NULL, 10);else if (argc == 4)return 1;
Same Q here as above.
elsereturn CMD_RET_USAGE;mmc = find_mmc_device(dev);if (!mmc) {printf("no mmc device at slot %x\n", dev);return 1;}if (IS_SD(mmc)) {printf("SD device cannot be opened/closed\n");return 1;}
Seems the open/close code is almost the same. Can you combine it?
if (!(mmc_boot_close(mmc)))printf("eMMC CLOSE Success.!!\n");elseprintf("eMMC CLOSE Failed.!!\n");return 0;} else if (strcmp(argv[1], "bootpart") == 0) {int dev;dev = simple_strtoul(argv[2], NULL, 10);struct mmc *mmc = find_mmc_device(dev);u32 bootsize = simple_strtoul(argv[3], NULL, 10);u32 rpmbsize = simple_strtoul(argv[4], NULL, 10);if (!mmc) {printf("no mmc device at slot %x\n", dev);return 1;}if (IS_SD(mmc)) {printf("It is not a eMMC device\n");return 1;}if (0 == mmc_boot_partition_size_change(mmc,bootsize, rpmbsize)) {printf("eMMC boot partition Size %d MB!!\n",bootsize);
printf("eMMC RPMB partition Size %d MB!!\n",rpmbsize);
} else {printf("eMMC boot partition Size changeFailed.!!\n");
return 1 here I think
}return 0; } if (strcmp(argv[1], "read") == 0)@@ -318,5 +414,8 @@ U_BOOT_CMD( "mmc rescan\n" "mmc part - lists available partition on current mmc device\n" "mmc dev [dev] [part] - show or set current mmc device [partition]\n"
"mmc list - lists available devices");
"mmc list - lists available devices\n""mmc open <device num> - opens the specified device\n""mmc close <device num> - closes the specified device\n""mmc bootpart <device num> <boot part size MB> <RPMB part sizeMB>\n");
Can you add another line of help here explaining what this does?
#endif
1.7.0.4
Regards,
Simon
Hi SImon,
Thanks for the review comments. Please find below the responses for your comments.
Thanks & Regards Amarendra
On 20 December 2012 08:10, Simon Glass sjg@chromium.org wrote:
Hi Amar,
On Mon, Dec 17, 2012 at 3:19 AM, Amar amarendra.xt@samsung.com wrote:
This patch adds commands to open, close and create partitions on eMMC
Signed-off-by: Amar amarendra.xt@samsung.com
common/cmd_mmc.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 100 insertions(+), 1 deletions(-)
diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c index 62a1c22..1fd6b94 100644 --- a/common/cmd_mmc.c +++ b/common/cmd_mmc.c @@ -248,6 +248,102 @@ static int do_mmcops(cmd_tbl_t *cmdtp, int flag,
int
argc, char * const argv[]) curr_device, mmc->part_num);
return 0;
} else if (strcmp(argv[1], "open") == 0) {int dev;struct mmc *mmc;if (argc == 2)dev = curr_device;else if (argc == 3)dev = simple_strtoul(argv[2], NULL, 10);else if (argc == 4)return 1;Should this be CMD_RET_USAGE? What is returning 1 for? Yes. Shall correct it.
elsereturn CMD_RET_USAGE;mmc = find_mmc_device(dev);if (!mmc) {printf("no mmc device at slot %x\n", dev);return 1;}if (IS_SD(mmc)) {printf("SD device cannot be opened/closed\n");return 1;}if (!(mmc_boot_open(mmc))) {printf("eMMC OPEN Success.!!\n");printf("\t\t\t!!!Notice!!!\n");printf("!You must close eMMC boot Partition""after all imagewriting!\n");
printf("!eMMC boot partition has continuity""at image writingtime.!\n");
printf("!So, Do not close boot partition,Before,"
"all images iswritten.!\n");
Maybe:
So, do not close boot partition before all images are written OK.. will change the wordings.
+ } else {
printf("eMMC OPEN Failed.!!\n");Is it eMMC or MMC? Lower case or upper case? Your messages should be consistent. And you don't need the !!! I think.
OK. Will maintain EMMC consistently every where. Will remove "!!!".
}
return 0;} else if (strcmp(argv[1], "close") == 0) {int dev;struct mmc *mmc;if (argc == 2)dev = curr_device;else if (argc == 3)dev = simple_strtoul(argv[2], NULL, 10);else if (argc == 4)return 1;Same Q here as above.
Ok, will be addressed.
elsereturn CMD_RET_USAGE;mmc = find_mmc_device(dev);if (!mmc) {printf("no mmc device at slot %x\n", dev);return 1;}if (IS_SD(mmc)) {printf("SD device cannot be opened/closed\n");return 1;}Seems the open/close code is almost the same. Can you combine it?
Ok. Will combine open/close.
if (!(mmc_boot_close(mmc)))printf("eMMC CLOSE Success.!!\n");elseprintf("eMMC CLOSE Failed.!!\n");return 0;} else if (strcmp(argv[1], "bootpart") == 0) {int dev;dev = simple_strtoul(argv[2], NULL, 10);struct mmc *mmc = find_mmc_device(dev);u32 bootsize = simple_strtoul(argv[3], NULL, 10);u32 rpmbsize = simple_strtoul(argv[4], NULL, 10);if (!mmc) {printf("no mmc device at slot %x\n", dev);return 1;}if (IS_SD(mmc)) {printf("It is not a eMMC device\n");return 1;}if (0 == mmc_boot_partition_size_change(mmc,bootsize, rpmbsize)) {printf("eMMC boot partition Size %d MB!!\n",bootsize);
printf("eMMC RPMB partition Size %d MB!!\n",rpmbsize);
} else {printf("eMMC boot partition Size changeFailed.!!\n");
return 1 here I think
Yes. WIll be corrected.
}return 0; } if (strcmp(argv[1], "read") == 0)@@ -318,5 +414,8 @@ U_BOOT_CMD( "mmc rescan\n" "mmc part - lists available partition on current mmc device\n" "mmc dev [dev] [part] - show or set current mmc device [partition]\n"
"mmc list - lists available devices");
"mmc list - lists available devices\n""mmc open <device num> - opens the specified device\n""mmc close <device num> - closes the specified device\n""mmc bootpart <device num> <boot part size MB> <RPMB part sizeMB>\n");
Can you add another line of help here explaining what this does? OK. I will add another line of help for mmc bootpart.
#endif
1.7.0.4
Regards,
Simon
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
participants (4)
-
Amar -
Amarendra Reddy -
Jaehoon Chung -
Simon Glass