* [U-Boot] [PATCH 1/9] FDT: Add compatible string for DWMMC
2012-12-17 11:19 [U-Boot] [PATCH 0/9] EXYNOS5: Enable dwmmc Amar
@ 2012-12-17 11:19 ` Amar
2012-12-20 1:53 ` Simon Glass
2012-12-17 11:19 ` [U-Boot] [PATCH 2/9] EXYNOS5: FDT: Add DWMMC device node data Amar
` (6 subsequent siblings)
7 siblings, 1 reply; 31+ messages in thread
From: Amar @ 2012-12-17 11:19 UTC (permalink / raw)
To: u-boot
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)
--
1.7.0.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* [U-Boot] [PATCH 1/9] FDT: Add compatible string for DWMMC
2012-12-17 11:19 ` [U-Boot] [PATCH 1/9] FDT: Add compatible string for DWMMC Amar
@ 2012-12-20 1:53 ` Simon Glass
0 siblings, 0 replies; 31+ messages in thread
From: Simon Glass @ 2012-12-20 1:53 UTC (permalink / raw)
To: u-boot
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
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 2/9] EXYNOS5: FDT: Add DWMMC device node data
2012-12-17 11:19 [U-Boot] [PATCH 0/9] EXYNOS5: Enable dwmmc Amar
2012-12-17 11:19 ` [U-Boot] [PATCH 1/9] FDT: Add compatible string for DWMMC Amar
@ 2012-12-17 11:19 ` Amar
2012-12-20 1:55 ` Simon Glass
2012-12-17 11:19 ` [U-Boot] [PATCH 4/9] EXYNOS5: DWMMC: Added dt support for DWMMC Amar
` (5 subsequent siblings)
7 siblings, 1 reply; 31+ messages in thread
From: Amar @ 2012-12-17 11:19 UTC (permalink / raw)
To: u-boot
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 at 12200000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "samsung,exynos5250-dwmmc";
+ reg = <0x12200000 0x1000>;
+ interrupts = <0 75 0>;
+ };
+
+ dwmmc at 12210000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "samsung,exynos5250-dwmmc";
+ reg = <0x12210000 0x1000>;
+ interrupts = <0 76 0>;
+ };
+
+ dwmmc at 12220000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "samsung,exynos5250-dwmmc";
+ reg = <0x12220000 0x1000>;
+ interrupts = <0 77 0>;
+ };
+
+ dwmmc at 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 at 12d40000";
spi3 = "/spi at 131a0000";
spi4 = "/spi at 131b0000";
+ dwmmc0 = "/dwmmc at 12200000";
+ dwmmc1 = "/dwmmc at 12210000";
+ dwmmc2 = "/dwmmc at 12220000";
+ dwmmc3 = "/dwmmc at 12230000";
};
sromc at 12250000 {
@@ -59,4 +63,24 @@
compatible = "wolfson,wm8994-codec";
};
};
+
+ dwmmc at 12200000 {
+ index = <0>;
+ bus-width = <8>;
+ timing = <1 3 3>;
+ };
+
+ dwmmc at 12210000 {
+ status = "disabled";
+ };
+
+ dwmmc at 12220000 {
+ index = <2>;
+ bus-width = <4>;
+ timing = <1 2 3>;
+ };
+
+ dwmmc at 12230000 {
+ status = "disabled";
+ };
};
--
1.7.0.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* [U-Boot] [PATCH 2/9] EXYNOS5: FDT: Add DWMMC device node data
2012-12-17 11:19 ` [U-Boot] [PATCH 2/9] EXYNOS5: FDT: Add DWMMC device node data Amar
@ 2012-12-20 1:55 ` Simon Glass
2012-12-21 9:19 ` Jaehoon Chung
0 siblings, 1 reply; 31+ messages in thread
From: Simon Glass @ 2012-12-20 1:55 UTC (permalink / raw)
To: u-boot
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 at 12200000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "samsung,exynos5250-dwmmc";
> + reg = <0x12200000 0x1000>;
> + interrupts = <0 75 0>;
> + };
> +
> + dwmmc at 12210000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "samsung,exynos5250-dwmmc";
> + reg = <0x12210000 0x1000>;
> + interrupts = <0 76 0>;
> + };
> +
> + dwmmc at 12220000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "samsung,exynos5250-dwmmc";
> + reg = <0x12220000 0x1000>;
> + interrupts = <0 77 0>;
> + };
> +
> + dwmmc at 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 at 12d40000";
> spi3 = "/spi at 131a0000";
> spi4 = "/spi at 131b0000";
> + dwmmc0 = "/dwmmc at 12200000";
> + dwmmc1 = "/dwmmc at 12210000";
> + dwmmc2 = "/dwmmc at 12220000";
> + dwmmc3 = "/dwmmc at 12230000";
> };
>
> sromc at 12250000 {
> @@ -59,4 +63,24 @@
> compatible = "wolfson,wm8994-codec";
> };
> };
> +
> + dwmmc at 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 at 12210000 {
> + status = "disabled";
> + };
> +
> + dwmmc at 12220000 {
> + index = <2>;
> + bus-width = <4>;
> + timing = <1 2 3>;
> + };
> +
> + dwmmc at 12230000 {
> + status = "disabled";
> + };
> };
> --
> 1.7.0.4
>
Regards,
Simon
^ permalink raw reply [flat|nested] 31+ messages in thread* [U-Boot] [PATCH 2/9] EXYNOS5: FDT: Add DWMMC device node data
2012-12-20 1:55 ` Simon Glass
@ 2012-12-21 9:19 ` Jaehoon Chung
2012-12-21 21:24 ` Simon Glass
0 siblings, 1 reply; 31+ messages in thread
From: Jaehoon Chung @ 2012-12-21 9:19 UTC (permalink / raw)
To: u-boot
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 at 12200000 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "samsung,exynos5250-dwmmc";
>> + reg = <0x12200000 0x1000>;
>> + interrupts = <0 75 0>;
>> + };
>> +
>> + dwmmc at 12210000 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "samsung,exynos5250-dwmmc";
>> + reg = <0x12210000 0x1000>;
>> + interrupts = <0 76 0>;
>> + };
>> +
>> + dwmmc at 12220000 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "samsung,exynos5250-dwmmc";
>> + reg = <0x12220000 0x1000>;
>> + interrupts = <0 77 0>;
>> + };
>> +
>> + dwmmc at 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 at 12d40000";
>> spi3 = "/spi at 131a0000";
>> spi4 = "/spi at 131b0000";
>> + dwmmc0 = "/dwmmc at 12200000";
>> + dwmmc1 = "/dwmmc at 12210000";
>> + dwmmc2 = "/dwmmc at 12220000";
>> + dwmmc3 = "/dwmmc at 12230000";
>> };
>>
>> sromc at 12250000 {
>> @@ -59,4 +63,24 @@
>> compatible = "wolfson,wm8994-codec";
>> };
>> };
>> +
>> + dwmmc at 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 at 12210000 {
>> + status = "disabled";
>> + };
>> +
>> + dwmmc at 12220000 {
>> + index = <2>;
>> + bus-width = <4>;
>> + timing = <1 2 3>;
>> + };
>> +
>> + dwmmc at 12230000 {
>> + status = "disabled";
>> + };
>> };
>> --
>> 1.7.0.4
>>
>
> Regards,
> Simon
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
^ permalink raw reply [flat|nested] 31+ messages in thread* [U-Boot] [PATCH 2/9] EXYNOS5: FDT: Add DWMMC device node data
2012-12-21 9:19 ` Jaehoon Chung
@ 2012-12-21 21:24 ` Simon Glass
2012-12-24 6:54 ` Amarendra Reddy
0 siblings, 1 reply; 31+ messages in thread
From: Simon Glass @ 2012-12-21 21:24 UTC (permalink / raw)
To: u-boot
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 at 12200000 {
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> + compatible = "samsung,exynos5250-dwmmc";
> >> + reg = <0x12200000 0x1000>;
> >> + interrupts = <0 75 0>;
> >> + };
> >> +
> >> + dwmmc at 12210000 {
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> + compatible = "samsung,exynos5250-dwmmc";
> >> + reg = <0x12210000 0x1000>;
> >> + interrupts = <0 76 0>;
> >> + };
> >> +
> >> + dwmmc at 12220000 {
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> + compatible = "samsung,exynos5250-dwmmc";
> >> + reg = <0x12220000 0x1000>;
> >> + interrupts = <0 77 0>;
> >> + };
> >> +
> >> + dwmmc at 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 at 12d40000";
> >> spi3 = "/spi at 131a0000";
> >> spi4 = "/spi at 131b0000";
> >> + dwmmc0 = "/dwmmc at 12200000";
> >> + dwmmc1 = "/dwmmc at 12210000";
> >> + dwmmc2 = "/dwmmc at 12220000";
> >> + dwmmc3 = "/dwmmc at 12230000";
> >> };
> >>
> >> sromc at 12250000 {
> >> @@ -59,4 +63,24 @@
> >> compatible = "wolfson,wm8994-codec";
> >> };
> >> };
> >> +
> >> + dwmmc at 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 at 12210000 {
> >> + status = "disabled";
> >> + };
> >> +
> >> + dwmmc at 12220000 {
> >> + index = <2>;
> >> + bus-width = <4>;
> >> + timing = <1 2 3>;
> >> + };
> >> +
> >> + dwmmc at 12230000 {
> >> + status = "disabled";
> >> + };
> >> };
> >> --
> >> 1.7.0.4
> >>
> >
> > Regards,
> > Simon
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
> >
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread* [U-Boot] [PATCH 2/9] EXYNOS5: FDT: Add DWMMC device node data
2012-12-21 21:24 ` Simon Glass
@ 2012-12-24 6:54 ` Amarendra Reddy
0 siblings, 0 replies; 31+ messages in thread
From: Amarendra Reddy @ 2012-12-24 6:54 UTC (permalink / raw)
To: 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 at 12200000 {
> > >> + #address-cells = <1>;
> > >> + #size-cells = <0>;
> > >> + compatible = "samsung,exynos5250-dwmmc";
> > >> + reg = <0x12200000 0x1000>;
> > >> + interrupts = <0 75 0>;
> > >> + };
> > >> +
> > >> + dwmmc at 12210000 {
> > >> + #address-cells = <1>;
> > >> + #size-cells = <0>;
> > >> + compatible = "samsung,exynos5250-dwmmc";
> > >> + reg = <0x12210000 0x1000>;
> > >> + interrupts = <0 76 0>;
> > >> + };
> > >> +
> > >> + dwmmc at 12220000 {
> > >> + #address-cells = <1>;
> > >> + #size-cells = <0>;
> > >> + compatible = "samsung,exynos5250-dwmmc";
> > >> + reg = <0x12220000 0x1000>;
> > >> + interrupts = <0 77 0>;
> > >> + };
> > >> +
> > >> + dwmmc at 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 at 12d40000";
> > >> spi3 = "/spi at 131a0000";
> > >> spi4 = "/spi at 131b0000";
> > >> + dwmmc0 = "/dwmmc at 12200000";
> > >> + dwmmc1 = "/dwmmc at 12210000";
> > >> + dwmmc2 = "/dwmmc at 12220000";
> > >> + dwmmc3 = "/dwmmc at 12230000";
> > >> };
> > >>
> > >> sromc at 12250000 {
> > >> @@ -59,4 +63,24 @@
> > >> compatible = "wolfson,wm8994-codec";
> > >> };
> > >> };
> > >> +
> > >> + dwmmc at 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 at 12210000 {
> > >> + status = "disabled";
> > >> + };
> > >> +
> > >> + dwmmc at 12220000 {
> > >> + index = <2>;
> > >> + bus-width = <4>;
> > >> + timing = <1 2 3>;
> > >> + };
> > >> +
> > >> + dwmmc at 12230000 {
> > >> + status = "disabled";
> > >> + };
> > >> };
> > >> --
> > >> 1.7.0.4
> > >>
> > >
> > > Regards,
> > > Simon
> > > _______________________________________________
> > > U-Boot mailing list
> > > U-Boot at lists.denx.de
> > > http://lists.denx.de/mailman/listinfo/u-boot
> > >
> >
> >
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
>
Thanks & Regards
Amarendra Reddy
^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 4/9] EXYNOS5: DWMMC: Added dt support for DWMMC
2012-12-17 11:19 [U-Boot] [PATCH 0/9] EXYNOS5: Enable dwmmc Amar
2012-12-17 11:19 ` [U-Boot] [PATCH 1/9] FDT: Add compatible string for DWMMC Amar
2012-12-17 11:19 ` [U-Boot] [PATCH 2/9] EXYNOS5: FDT: Add DWMMC device node data Amar
@ 2012-12-17 11:19 ` Amar
2012-12-20 2:23 ` Simon Glass
2012-12-17 11:19 ` [U-Boot] [PATCH 5/9] EXYNOS5: DWMMC: API to set mmc clock divisor Amar
` (4 subsequent siblings)
7 siblings, 1 reply; 31+ messages in thread
From: Amar @ 2012-12-17 11:19 UTC (permalink / raw)
To: u-boot
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;
--
1.7.0.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* [U-Boot] [PATCH 4/9] EXYNOS5: DWMMC: Added dt support for DWMMC
2012-12-17 11:19 ` [U-Boot] [PATCH 4/9] EXYNOS5: DWMMC: Added dt support for DWMMC Amar
@ 2012-12-20 2:23 ` Simon Glass
2012-12-21 5:16 ` Amarendra Reddy
0 siblings, 1 reply; 31+ messages in thread
From: Simon Glass @ 2012-12-20 2:23 UTC (permalink / raw)
To: u-boot
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;
> + else
> + flag = 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
^ permalink raw reply [flat|nested] 31+ messages in thread* [U-Boot] [PATCH 4/9] EXYNOS5: DWMMC: Added dt support for DWMMC
2012-12-20 2:23 ` Simon Glass
@ 2012-12-21 5:16 ` Amarendra Reddy
2012-12-21 9:18 ` Jaehoon Chung
0 siblings, 1 reply; 31+ messages in thread
From: Amarendra Reddy @ 2012-12-21 5:16 UTC (permalink / raw)
To: u-boot
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;
> > + else
> > + flag = 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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread* [U-Boot] [PATCH 4/9] EXYNOS5: DWMMC: Added dt support for DWMMC
2012-12-21 5:16 ` Amarendra Reddy
@ 2012-12-21 9:18 ` Jaehoon Chung
2012-12-24 7:20 ` Amarendra Reddy
0 siblings, 1 reply; 31+ messages in thread
From: Jaehoon Chung @ 2012-12-21 9:18 UTC (permalink / raw)
To: 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;
>>> + else
>>> + flag = 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 at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
>>
>>
>
>
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
^ permalink raw reply [flat|nested] 31+ messages in thread* [U-Boot] [PATCH 4/9] EXYNOS5: DWMMC: Added dt support for DWMMC
2012-12-21 9:18 ` Jaehoon Chung
@ 2012-12-24 7:20 ` Amarendra Reddy
0 siblings, 0 replies; 31+ messages in thread
From: Amarendra Reddy @ 2012-12-24 7:20 UTC (permalink / raw)
To: 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;
> >>> + else
> >>> + flag = 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 at lists.denx.de
> >> http://lists.denx.de/mailman/listinfo/u-boot
> >>
> >>
> >
> >
> >
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
> >
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 5/9] EXYNOS5: DWMMC: API to set mmc clock divisor
2012-12-17 11:19 [U-Boot] [PATCH 0/9] EXYNOS5: Enable dwmmc Amar
` (2 preceding siblings ...)
2012-12-17 11:19 ` [U-Boot] [PATCH 4/9] EXYNOS5: DWMMC: Added dt support for DWMMC Amar
@ 2012-12-17 11:19 ` Amar
2012-12-20 2:24 ` Simon Glass
2012-12-17 11:19 ` [U-Boot] [PATCH 6/9] SMDK5250: Enable DWMMC Amar
` (3 subsequent siblings)
7 siblings, 1 reply; 31+ messages in thread
From: Amar @ 2012-12-17 11:19 UTC (permalink / raw)
To: 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);
--
1.7.0.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* [U-Boot] [PATCH 5/9] EXYNOS5: DWMMC: API to set mmc clock divisor
2012-12-17 11:19 ` [U-Boot] [PATCH 5/9] EXYNOS5: DWMMC: API to set mmc clock divisor Amar
@ 2012-12-20 2:24 ` Simon Glass
2012-12-21 5:18 ` Amarendra Reddy
0 siblings, 1 reply; 31+ messages in thread
From: Simon Glass @ 2012-12-20 2:24 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 31+ messages in thread* [U-Boot] [PATCH 5/9] EXYNOS5: DWMMC: API to set mmc clock divisor
2012-12-20 2:24 ` Simon Glass
@ 2012-12-21 5:18 ` Amarendra Reddy
0 siblings, 0 replies; 31+ messages in thread
From: Amarendra Reddy @ 2012-12-21 5:18 UTC (permalink / raw)
To: u-boot
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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 6/9] SMDK5250: Enable DWMMC
2012-12-17 11:19 [U-Boot] [PATCH 0/9] EXYNOS5: Enable dwmmc Amar
` (3 preceding siblings ...)
2012-12-17 11:19 ` [U-Boot] [PATCH 5/9] EXYNOS5: DWMMC: API to set mmc clock divisor Amar
@ 2012-12-17 11:19 ` Amar
2012-12-20 2:29 ` Simon Glass
2012-12-17 11:19 ` [U-Boot] [PATCH 7/9] MMC: APIs to support creation of boot partition Amar
` (2 subsequent siblings)
7 siblings, 1 reply; 31+ messages in thread
From: Amar @ 2012-12-17 11:19 UTC (permalink / raw)
To: 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
--
1.7.0.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* [U-Boot] [PATCH 6/9] SMDK5250: Enable DWMMC
2012-12-17 11:19 ` [U-Boot] [PATCH 6/9] SMDK5250: Enable DWMMC Amar
@ 2012-12-20 2:29 ` Simon Glass
2012-12-21 5:27 ` Amarendra Reddy
0 siblings, 1 reply; 31+ messages in thread
From: Simon Glass @ 2012-12-20 2:29 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 31+ messages in thread* [U-Boot] [PATCH 6/9] SMDK5250: Enable DWMMC
2012-12-20 2:29 ` Simon Glass
@ 2012-12-21 5:27 ` Amarendra Reddy
0 siblings, 0 replies; 31+ messages in thread
From: Amarendra Reddy @ 2012-12-21 5:27 UTC (permalink / raw)
To: u-boot
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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 7/9] MMC: APIs to support creation of boot partition
2012-12-17 11:19 [U-Boot] [PATCH 0/9] EXYNOS5: Enable dwmmc Amar
` (4 preceding siblings ...)
2012-12-17 11:19 ` [U-Boot] [PATCH 6/9] SMDK5250: Enable DWMMC Amar
@ 2012-12-17 11:19 ` Amar
2012-12-20 2:31 ` Simon Glass
2012-12-17 11:19 ` [U-Boot] [PATCH 8/9] SMDK5250: Enable eMMC booting Amar
2012-12-17 11:19 ` [U-Boot] [PATCH 9/9] COMMON: MMC: Command to support " Amar
7 siblings, 1 reply; 31+ messages in thread
From: Amar @ 2012-12-17 11:19 UTC (permalink / raw)
To: 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@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)
--
1.7.0.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* [U-Boot] [PATCH 7/9] MMC: APIs to support creation of boot partition
2012-12-17 11:19 ` [U-Boot] [PATCH 7/9] MMC: APIs to support creation of boot partition Amar
@ 2012-12-20 2:31 ` Simon Glass
2012-12-20 13:57 ` Amarendra Reddy
0 siblings, 1 reply; 31+ messages in thread
From: Simon Glass @ 2012-12-20 2:31 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 31+ messages in thread* [U-Boot] [PATCH 7/9] MMC: APIs to support creation of boot partition
2012-12-20 2:31 ` Simon Glass
@ 2012-12-20 13:57 ` Amarendra Reddy
0 siblings, 0 replies; 31+ messages in thread
From: Amarendra Reddy @ 2012-12-20 13:57 UTC (permalink / raw)
To: u-boot
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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 8/9] SMDK5250: Enable eMMC booting
2012-12-17 11:19 [U-Boot] [PATCH 0/9] EXYNOS5: Enable dwmmc Amar
` (5 preceding siblings ...)
2012-12-17 11:19 ` [U-Boot] [PATCH 7/9] MMC: APIs to support creation of boot partition Amar
@ 2012-12-17 11:19 ` Amar
2012-12-20 2:35 ` Simon Glass
2012-12-17 11:19 ` [U-Boot] [PATCH 9/9] COMMON: MMC: Command to support " Amar
7 siblings, 1 reply; 31+ messages in thread
From: Amar @ 2012-12-17 11:19 UTC (permalink / raw)
To: 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;
}
--
1.7.0.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* [U-Boot] [PATCH 8/9] SMDK5250: Enable eMMC booting
2012-12-17 11:19 ` [U-Boot] [PATCH 8/9] SMDK5250: Enable eMMC booting Amar
@ 2012-12-20 2:35 ` Simon Glass
2012-12-20 13:53 ` Amarendra Reddy
0 siblings, 1 reply; 31+ messages in thread
From: Simon Glass @ 2012-12-20 2:35 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 31+ messages in thread* [U-Boot] [PATCH 8/9] SMDK5250: Enable eMMC booting
2012-12-20 2:35 ` Simon Glass
@ 2012-12-20 13:53 ` Amarendra Reddy
2012-12-26 19:59 ` Simon Glass
0 siblings, 1 reply; 31+ messages in thread
From: Amarendra Reddy @ 2012-12-20 13:53 UTC (permalink / raw)
To: u-boot
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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread* [U-Boot] [PATCH 8/9] SMDK5250: Enable eMMC booting
2012-12-20 13:53 ` Amarendra Reddy
@ 2012-12-26 19:59 ` Simon Glass
2012-12-27 3:55 ` Amarendra Reddy
0 siblings, 1 reply; 31+ messages in thread
From: Simon Glass @ 2012-12-26 19:59 UTC (permalink / raw)
To: 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 at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
>>
>
Regards,
Simon
^ permalink raw reply [flat|nested] 31+ messages in thread* [U-Boot] [PATCH 8/9] SMDK5250: Enable eMMC booting
2012-12-26 19:59 ` Simon Glass
@ 2012-12-27 3:55 ` Amarendra Reddy
0 siblings, 0 replies; 31+ messages in thread
From: Amarendra Reddy @ 2012-12-27 3:55 UTC (permalink / raw)
To: u-boot
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, 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.
> 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 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.
> 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 at lists.denx.de
> >> http://lists.denx.de/mailman/listinfo/u-boot
> >>
> >
>
> Regards,
> Simon
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [U-Boot] [PATCH 9/9] COMMON: MMC: Command to support eMMC booting
2012-12-17 11:19 [U-Boot] [PATCH 0/9] EXYNOS5: Enable dwmmc Amar
` (6 preceding siblings ...)
2012-12-17 11:19 ` [U-Boot] [PATCH 8/9] SMDK5250: Enable eMMC booting Amar
@ 2012-12-17 11:19 ` Amar
2012-12-20 2:40 ` Simon Glass
7 siblings, 1 reply; 31+ messages in thread
From: Amar @ 2012-12-17 11:19 UTC (permalink / raw)
To: u-boot
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
--
1.7.0.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* [U-Boot] [PATCH 9/9] COMMON: MMC: Command to support eMMC booting
2012-12-17 11:19 ` [U-Boot] [PATCH 9/9] COMMON: MMC: Command to support " Amar
@ 2012-12-20 2:40 ` Simon Glass
2012-12-20 13:45 ` Amarendra Reddy
0 siblings, 1 reply; 31+ messages in thread
From: Simon Glass @ 2012-12-20 2:40 UTC (permalink / raw)
To: u-boot
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?
> +
> + 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");
>
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.
> + 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;
> + }
> +
>
Seems the open/close code is almost the same. Can you combine it?
> + 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 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 size
> MB>\n");
>
Can you add another line of help here explaining what this does?
> #endif
> --
> 1.7.0.4
>
> Regards,
Simon
^ permalink raw reply [flat|nested] 31+ messages in thread* [U-Boot] [PATCH 9/9] COMMON: MMC: Command to support eMMC booting
2012-12-20 2:40 ` Simon Glass
@ 2012-12-20 13:45 ` Amarendra Reddy
0 siblings, 0 replies; 31+ messages in thread
From: Amarendra Reddy @ 2012-12-20 13:45 UTC (permalink / raw)
To: u-boot
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.
> > +
> > + 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");
> >
>
> 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.
>
> > + 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;
> > + }
> > +
> >
>
> 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");
> > + 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 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 size
> > MB>\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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread