diff mbox

[v1,3/5] ARM: imx28evk: add mmc dt support

Message ID 1331628428-24017-4-git-send-email-b29396@freescale.com
State New
Headers show

Commit Message

Dong Aisheng March 13, 2012, 8:47 a.m. UTC 
From: Dong Aisheng <dong.aisheng@linaro.org>

Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
---
 arch/arm/boot/dts/imx28-evk.dts |   14 ++++++++++++++
 arch/arm/boot/dts/imx28.dtsi    |   33 +++++++++++++++++++++++++++++++++
 arch/arm/mach-mxs/imx28-dt.c    |    2 ++
 3 files changed, 49 insertions(+), 0 deletions(-)

Comments

Rob Herring March 13, 2012, 2:39 p.m. UTC | #1 
On 03/13/2012 03:47 AM, Dong Aisheng wrote:
> From: Dong Aisheng <dong.aisheng@linaro.org>
> 
> Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
> ---
>  arch/arm/boot/dts/imx28-evk.dts |   14 ++++++++++++++
>  arch/arm/boot/dts/imx28.dtsi    |   33 +++++++++++++++++++++++++++++++++
>  arch/arm/mach-mxs/imx28-dt.c    |    2 ++
>  3 files changed, 49 insertions(+), 0 deletions(-)

snip

> diff --git a/arch/arm/mach-mxs/imx28-dt.c b/arch/arm/mach-mxs/imx28-dt.c
> index 78d1129..429b88e 100644
> --- a/arch/arm/mach-mxs/imx28-dt.c
> +++ b/arch/arm/mach-mxs/imx28-dt.c
> @@ -23,6 +23,8 @@ static const struct of_dev_auxdata imx28_auxdata_lookup[] __initconst = {
>  	OF_DEV_AUXDATA("arm,pl011", MX28_DUART_BASE_ADDR, "duart", NULL),
>  	OF_DEV_AUXDATA("fsl,imx28-fec", MX28_ENET_MAC0_BASE_ADDR, "imx28-fec.0", NULL),
>  	OF_DEV_AUXDATA("fsl,imx28-fec", MX28_ENET_MAC1_BASE_ADDR, "imx28-fec.1", NULL),
> +	OF_DEV_AUXDATA("fsl,imx28-mmc", MX28_SSP0_BASE_ADDR, "mxs-mmc.0", NULL),
> +	OF_DEV_AUXDATA("fsl,imx28-mmc", MX28_SSP1_BASE_ADDR, "mxs-mmc.1", NULL),

Why is this needed?

>  	{ /* sentinel */ }
>  };
>
Sascha Hauer March 13, 2012, 4:52 p.m. UTC | #2 
On Tue, Mar 13, 2012 at 09:39:30AM -0500, Rob Herring wrote:
> On 03/13/2012 03:47 AM, Dong Aisheng wrote:
> > From: Dong Aisheng <dong.aisheng@linaro.org>
> > 
> > Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
> > ---
> >  arch/arm/boot/dts/imx28-evk.dts |   14 ++++++++++++++
> >  arch/arm/boot/dts/imx28.dtsi    |   33 +++++++++++++++++++++++++++++++++
> >  arch/arm/mach-mxs/imx28-dt.c    |    2 ++
> >  3 files changed, 49 insertions(+), 0 deletions(-)
> 
> snip
> 
> > diff --git a/arch/arm/mach-mxs/imx28-dt.c b/arch/arm/mach-mxs/imx28-dt.c
> > index 78d1129..429b88e 100644
> > --- a/arch/arm/mach-mxs/imx28-dt.c
> > +++ b/arch/arm/mach-mxs/imx28-dt.c
> > @@ -23,6 +23,8 @@ static const struct of_dev_auxdata imx28_auxdata_lookup[] __initconst = {
> >  	OF_DEV_AUXDATA("arm,pl011", MX28_DUART_BASE_ADDR, "duart", NULL),
> >  	OF_DEV_AUXDATA("fsl,imx28-fec", MX28_ENET_MAC0_BASE_ADDR, "imx28-fec.0", NULL),
> >  	OF_DEV_AUXDATA("fsl,imx28-fec", MX28_ENET_MAC1_BASE_ADDR, "imx28-fec.1", NULL),
> > +	OF_DEV_AUXDATA("fsl,imx28-mmc", MX28_SSP0_BASE_ADDR, "mxs-mmc.0", NULL),
> > +	OF_DEV_AUXDATA("fsl,imx28-mmc", MX28_SSP1_BASE_ADDR, "mxs-mmc.1", NULL),
> 
> Why is this needed?

These are needed for the drivers which have still the mxs-mmc.* names
to find their clocks. Alternatively we could also add the appropriate
clocks to the clock file. Don't know if that's better though.

Sascha
Rob Herring March 13, 2012, 5:45 p.m. UTC | #3 
On 03/13/2012 11:52 AM, Sascha Hauer wrote:
> On Tue, Mar 13, 2012 at 09:39:30AM -0500, Rob Herring wrote:
>> On 03/13/2012 03:47 AM, Dong Aisheng wrote:
>>> From: Dong Aisheng <dong.aisheng@linaro.org>
>>>
>>> Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
>>> ---
>>>  arch/arm/boot/dts/imx28-evk.dts |   14 ++++++++++++++
>>>  arch/arm/boot/dts/imx28.dtsi    |   33 +++++++++++++++++++++++++++++++++
>>>  arch/arm/mach-mxs/imx28-dt.c    |    2 ++
>>>  3 files changed, 49 insertions(+), 0 deletions(-)
>>
>> snip
>>
>>> diff --git a/arch/arm/mach-mxs/imx28-dt.c b/arch/arm/mach-mxs/imx28-dt.c
>>> index 78d1129..429b88e 100644
>>> --- a/arch/arm/mach-mxs/imx28-dt.c
>>> +++ b/arch/arm/mach-mxs/imx28-dt.c
>>> @@ -23,6 +23,8 @@ static const struct of_dev_auxdata imx28_auxdata_lookup[] __initconst = {
>>>  	OF_DEV_AUXDATA("arm,pl011", MX28_DUART_BASE_ADDR, "duart", NULL),
>>>  	OF_DEV_AUXDATA("fsl,imx28-fec", MX28_ENET_MAC0_BASE_ADDR, "imx28-fec.0", NULL),
>>>  	OF_DEV_AUXDATA("fsl,imx28-fec", MX28_ENET_MAC1_BASE_ADDR, "imx28-fec.1", NULL),
>>> +	OF_DEV_AUXDATA("fsl,imx28-mmc", MX28_SSP0_BASE_ADDR, "mxs-mmc.0", NULL),
>>> +	OF_DEV_AUXDATA("fsl,imx28-mmc", MX28_SSP1_BASE_ADDR, "mxs-mmc.1", NULL),
>>
>> Why is this needed?
> 
> These are needed for the drivers which have still the mxs-mmc.* names
> to find their clocks. Alternatively we could also add the appropriate
> clocks to the clock file. Don't know if that's better though.

Ah, yes I should have known that... If clk lookup is all that's needed,
I'd suggest adding the necessary clk lookups either here or in the clock
code. Not much difference, but at least it removes the use of
*_BASE_ADDR defines.

Rob
Jean-Christophe PLAGNIOL-VILLARD March 14, 2012, 7:28 a.m. UTC | #4 
On 16:47 Tue 13 Mar     , Dong Aisheng wrote:
> From: Dong Aisheng <dong.aisheng@linaro.org>
> 
> Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
> ---
>  arch/arm/boot/dts/imx28-evk.dts |   14 ++++++++++++++
>  arch/arm/boot/dts/imx28.dtsi    |   33 +++++++++++++++++++++++++++++++++
>  arch/arm/mach-mxs/imx28-dt.c    |    2 ++
>  3 files changed, 49 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx28-evk.dts b/arch/arm/boot/dts/imx28-evk.dts
> index 9758dc4..61350ab 100644
> --- a/arch/arm/boot/dts/imx28-evk.dts
> +++ b/arch/arm/boot/dts/imx28-evk.dts
> @@ -21,6 +21,20 @@
>  		reg = <0x40000000 0x08000000>;
>  	};
>  
> +        apb@80000000 {
> +                apbh@80000000 {
> +			mmc1: ssp@80010000 {
> +				slot-8bit;
> +				status = "okay";
> +			};
> +
> +			mmc2: ssp@80012000 {
> +				slot-8bit;
> +				status = "okay";
> +			};
> +		};
> +	};
> +
>  	ahb@80080000 {
>  		fec@800f0000 {
>  			phy-mode = "rmii";
> diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
> index acf0dab..71c7bfb 100644
> --- a/arch/arm/boot/dts/imx28.dtsi
> +++ b/arch/arm/boot/dts/imx28.dtsi
> @@ -46,6 +46,39 @@
>  				#interrupt-cells = <1>;
>  				reg = <0x80000000 0x2000>;
>  			};
> +
> +			ssp@80010000 {
> +				compatible = "fsl,imx28-mmc";
> +				reg = <0x80010000 2000>;
> +				/* <irq_err irq_dma> */
> +				interrupts = <96 82>;
> +				dma_channel = <0>;
> +				status = "disabled";
> +			};
> +
> +			ssp@80012000 {
> +				compatible = "fsl,imx28-mmc";
> +				reg = <0x80012000 2000>;
> +				interrupts = <97 83>;
> +				dma_channel = <1>;
> +				status = "disabled";
> +			};
> +
> +			ssp@80014000 {
> +				compatible = "fsl,imx28-mmc";
> +				reg = <0x80014000 2000>;
> +				interrupts = <98 84>;
> +				dma_channel = <2>;
> +				status = "disabled";
> +			};
> +
> +			ssp@80016000 {
> +				compatible = "fsl,imx28-mmc";
> +				reg = <0x80016000 2000>;
> +				interrupts = <99 85>;
> +				dma_channel = <3>;
> +				status = "disabled";
> +			};
>  		};
>  
>  		apbx@80040000 {
> diff --git a/arch/arm/mach-mxs/imx28-dt.c b/arch/arm/mach-mxs/imx28-dt.c
> index 78d1129..429b88e 100644
> --- a/arch/arm/mach-mxs/imx28-dt.c
> +++ b/arch/arm/mach-mxs/imx28-dt.c
> @@ -23,6 +23,8 @@ static const struct of_dev_auxdata imx28_auxdata_lookup[] __initconst = {
>  	OF_DEV_AUXDATA("arm,pl011", MX28_DUART_BASE_ADDR, "duart", NULL),
>  	OF_DEV_AUXDATA("fsl,imx28-fec", MX28_ENET_MAC0_BASE_ADDR, "imx28-fec.0", NULL),
>  	OF_DEV_AUXDATA("fsl,imx28-fec", MX28_ENET_MAC1_BASE_ADDR, "imx28-fec.1", NULL),
> +	OF_DEV_AUXDATA("fsl,imx28-mmc", MX28_SSP0_BASE_ADDR, "mxs-mmc.0", NULL),
> +	OF_DEV_AUXDATA("fsl,imx28-mmc", MX28_SSP1_BASE_ADDR, "mxs-mmc.1", NULL),
do we really need to continue to add OF_DEV_AUXDATA

instead add a clkdev entry

IIRC OF_DEV_AUXDATA was a temporary solution, we need to stop to use it more
and more

Best Reards,
J.
Jean-Christophe PLAGNIOL-VILLARD March 14, 2012, 7:30 a.m. UTC | #5 
On 12:45 Tue 13 Mar     , Rob Herring wrote:
> On 03/13/2012 11:52 AM, Sascha Hauer wrote:
> > On Tue, Mar 13, 2012 at 09:39:30AM -0500, Rob Herring wrote:
> >> On 03/13/2012 03:47 AM, Dong Aisheng wrote:
> >>> From: Dong Aisheng <dong.aisheng@linaro.org>
> >>>
> >>> Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
> >>> ---
> >>>  arch/arm/boot/dts/imx28-evk.dts |   14 ++++++++++++++
> >>>  arch/arm/boot/dts/imx28.dtsi    |   33 +++++++++++++++++++++++++++++++++
> >>>  arch/arm/mach-mxs/imx28-dt.c    |    2 ++
> >>>  3 files changed, 49 insertions(+), 0 deletions(-)
> >>
> >> snip
> >>
> >>> diff --git a/arch/arm/mach-mxs/imx28-dt.c b/arch/arm/mach-mxs/imx28-dt.c
> >>> index 78d1129..429b88e 100644
> >>> --- a/arch/arm/mach-mxs/imx28-dt.c
> >>> +++ b/arch/arm/mach-mxs/imx28-dt.c
> >>> @@ -23,6 +23,8 @@ static const struct of_dev_auxdata imx28_auxdata_lookup[] __initconst = {
> >>>  	OF_DEV_AUXDATA("arm,pl011", MX28_DUART_BASE_ADDR, "duart", NULL),
> >>>  	OF_DEV_AUXDATA("fsl,imx28-fec", MX28_ENET_MAC0_BASE_ADDR, "imx28-fec.0", NULL),
> >>>  	OF_DEV_AUXDATA("fsl,imx28-fec", MX28_ENET_MAC1_BASE_ADDR, "imx28-fec.1", NULL),
> >>> +	OF_DEV_AUXDATA("fsl,imx28-mmc", MX28_SSP0_BASE_ADDR, "mxs-mmc.0", NULL),
> >>> +	OF_DEV_AUXDATA("fsl,imx28-mmc", MX28_SSP1_BASE_ADDR, "mxs-mmc.1", NULL),
> >>
> >> Why is this needed?
> > 
> > These are needed for the drivers which have still the mxs-mmc.* names
> > to find their clocks. Alternatively we could also add the appropriate
> > clocks to the clock file. Don't know if that's better though.
> 
> Ah, yes I should have known that... If clk lookup is all that's needed,
> I'd suggest adding the necessary clk lookups either here or in the clock
> code. Not much difference, but at least it removes the use of
> *_BASE_ADDR defines.
Agreed we do this on AT91

Best Regards,
J.
Dong Aisheng March 14, 2012, 8:20 a.m. UTC | #6 
On Wed, Mar 14, 2012 at 03:30:35PM +0800, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 12:45 Tue 13 Mar     , Rob Herring wrote:
> > On 03/13/2012 11:52 AM, Sascha Hauer wrote:
> > > On Tue, Mar 13, 2012 at 09:39:30AM -0500, Rob Herring wrote:
> > >> On 03/13/2012 03:47 AM, Dong Aisheng wrote:
> > >>> From: Dong Aisheng <dong.aisheng@linaro.org>
> > >>>
> > >>> Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
> > >>> ---
> > >>>  arch/arm/boot/dts/imx28-evk.dts |   14 ++++++++++++++
> > >>>  arch/arm/boot/dts/imx28.dtsi    |   33 +++++++++++++++++++++++++++++++++
> > >>>  arch/arm/mach-mxs/imx28-dt.c    |    2 ++
> > >>>  3 files changed, 49 insertions(+), 0 deletions(-)
> > >>
> > >> snip
> > >>
> > >>> diff --git a/arch/arm/mach-mxs/imx28-dt.c b/arch/arm/mach-mxs/imx28-dt.c
> > >>> index 78d1129..429b88e 100644
> > >>> --- a/arch/arm/mach-mxs/imx28-dt.c
> > >>> +++ b/arch/arm/mach-mxs/imx28-dt.c
> > >>> @@ -23,6 +23,8 @@ static const struct of_dev_auxdata imx28_auxdata_lookup[] __initconst = {
> > >>>  	OF_DEV_AUXDATA("arm,pl011", MX28_DUART_BASE_ADDR, "duart", NULL),
> > >>>  	OF_DEV_AUXDATA("fsl,imx28-fec", MX28_ENET_MAC0_BASE_ADDR, "imx28-fec.0", NULL),
> > >>>  	OF_DEV_AUXDATA("fsl,imx28-fec", MX28_ENET_MAC1_BASE_ADDR, "imx28-fec.1", NULL),
> > >>> +	OF_DEV_AUXDATA("fsl,imx28-mmc", MX28_SSP0_BASE_ADDR, "mxs-mmc.0", NULL),
> > >>> +	OF_DEV_AUXDATA("fsl,imx28-mmc", MX28_SSP1_BASE_ADDR, "mxs-mmc.1", NULL),
> > >>
> > >> Why is this needed?
> > > 
> > > These are needed for the drivers which have still the mxs-mmc.* names
> > > to find their clocks. Alternatively we could also add the appropriate
> > > clocks to the clock file. Don't know if that's better though.
> > 
> > Ah, yes I should have known that... If clk lookup is all that's needed,
> > I'd suggest adding the necessary clk lookups either here or in the clock
> > code. Not much difference, but at least it removes the use of
> > *_BASE_ADDR defines.
> Agreed we do this on AT91
> 
I'm willing to do like that.
But i'm new to this, can you help point our where the code i can reference
in Rob suggested way?

Regards
Dong Aisheng
Jean-Christophe PLAGNIOL-VILLARD March 14, 2012, 8:54 a.m. UTC | #7 
On 16:20 Wed 14 Mar     , Dong Aisheng wrote:
> On Wed, Mar 14, 2012 at 03:30:35PM +0800, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 12:45 Tue 13 Mar     , Rob Herring wrote:
> > > On 03/13/2012 11:52 AM, Sascha Hauer wrote:
> > > > On Tue, Mar 13, 2012 at 09:39:30AM -0500, Rob Herring wrote:
> > > >> On 03/13/2012 03:47 AM, Dong Aisheng wrote:
> > > >>> From: Dong Aisheng <dong.aisheng@linaro.org>
> > > >>>
> > > >>> Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
> > > >>> ---
> > > >>>  arch/arm/boot/dts/imx28-evk.dts |   14 ++++++++++++++
> > > >>>  arch/arm/boot/dts/imx28.dtsi    |   33 +++++++++++++++++++++++++++++++++
> > > >>>  arch/arm/mach-mxs/imx28-dt.c    |    2 ++
> > > >>>  3 files changed, 49 insertions(+), 0 deletions(-)
> > > >>
> > > >> snip
> > > >>
> > > >>> diff --git a/arch/arm/mach-mxs/imx28-dt.c b/arch/arm/mach-mxs/imx28-dt.c
> > > >>> index 78d1129..429b88e 100644
> > > >>> --- a/arch/arm/mach-mxs/imx28-dt.c
> > > >>> +++ b/arch/arm/mach-mxs/imx28-dt.c
> > > >>> @@ -23,6 +23,8 @@ static const struct of_dev_auxdata imx28_auxdata_lookup[] __initconst = {
> > > >>>  	OF_DEV_AUXDATA("arm,pl011", MX28_DUART_BASE_ADDR, "duart", NULL),
> > > >>>  	OF_DEV_AUXDATA("fsl,imx28-fec", MX28_ENET_MAC0_BASE_ADDR, "imx28-fec.0", NULL),
> > > >>>  	OF_DEV_AUXDATA("fsl,imx28-fec", MX28_ENET_MAC1_BASE_ADDR, "imx28-fec.1", NULL),
> > > >>> +	OF_DEV_AUXDATA("fsl,imx28-mmc", MX28_SSP0_BASE_ADDR, "mxs-mmc.0", NULL),
> > > >>> +	OF_DEV_AUXDATA("fsl,imx28-mmc", MX28_SSP1_BASE_ADDR, "mxs-mmc.1", NULL),
> > > >>
> > > >> Why is this needed?
> > > > 
> > > > These are needed for the drivers which have still the mxs-mmc.* names
> > > > to find their clocks. Alternatively we could also add the appropriate
> > > > clocks to the clock file. Don't know if that's better though.
> > > 
> > > Ah, yes I should have known that... If clk lookup is all that's needed,
> > > I'd suggest adding the necessary clk lookups either here or in the clock
> > > code. Not much difference, but at least it removes the use of
> > > *_BASE_ADDR defines.
> > Agreed we do this on AT91
> > 
> I'm willing to do like that.
> But i'm new to this, can you help point our where the code i can reference
> in Rob suggested way?
mach-at91

Best Regards,
J.
diff mbox

Patch

diff --git a/arch/arm/boot/dts/imx28-evk.dts b/arch/arm/boot/dts/imx28-evk.dts
index 9758dc4..61350ab 100644
--- a/arch/arm/boot/dts/imx28-evk.dts
+++ b/arch/arm/boot/dts/imx28-evk.dts
@@ -21,6 +21,20 @@ 
 		reg = <0x40000000 0x08000000>;
 	};
 
+        apb@80000000 {
+                apbh@80000000 {
+			mmc1: ssp@80010000 {
+				slot-8bit;
+				status = "okay";
+			};
+
+			mmc2: ssp@80012000 {
+				slot-8bit;
+				status = "okay";
+			};
+		};
+	};
+
 	ahb@80080000 {
 		fec@800f0000 {
 			phy-mode = "rmii";
diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
index acf0dab..71c7bfb 100644
--- a/arch/arm/boot/dts/imx28.dtsi
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -46,6 +46,39 @@ 
 				#interrupt-cells = <1>;
 				reg = <0x80000000 0x2000>;
 			};
+
+			ssp@80010000 {
+				compatible = "fsl,imx28-mmc";
+				reg = <0x80010000 2000>;
+				/* <irq_err irq_dma> */
+				interrupts = <96 82>;
+				dma_channel = <0>;
+				status = "disabled";
+			};
+
+			ssp@80012000 {
+				compatible = "fsl,imx28-mmc";
+				reg = <0x80012000 2000>;
+				interrupts = <97 83>;
+				dma_channel = <1>;
+				status = "disabled";
+			};
+
+			ssp@80014000 {
+				compatible = "fsl,imx28-mmc";
+				reg = <0x80014000 2000>;
+				interrupts = <98 84>;
+				dma_channel = <2>;
+				status = "disabled";
+			};
+
+			ssp@80016000 {
+				compatible = "fsl,imx28-mmc";
+				reg = <0x80016000 2000>;
+				interrupts = <99 85>;
+				dma_channel = <3>;
+				status = "disabled";
+			};
 		};
 
 		apbx@80040000 {
diff --git a/arch/arm/mach-mxs/imx28-dt.c b/arch/arm/mach-mxs/imx28-dt.c
index 78d1129..429b88e 100644
--- a/arch/arm/mach-mxs/imx28-dt.c
+++ b/arch/arm/mach-mxs/imx28-dt.c
@@ -23,6 +23,8 @@  static const struct of_dev_auxdata imx28_auxdata_lookup[] __initconst = {
 	OF_DEV_AUXDATA("arm,pl011", MX28_DUART_BASE_ADDR, "duart", NULL),
 	OF_DEV_AUXDATA("fsl,imx28-fec", MX28_ENET_MAC0_BASE_ADDR, "imx28-fec.0", NULL),
 	OF_DEV_AUXDATA("fsl,imx28-fec", MX28_ENET_MAC1_BASE_ADDR, "imx28-fec.1", NULL),
+	OF_DEV_AUXDATA("fsl,imx28-mmc", MX28_SSP0_BASE_ADDR, "mxs-mmc.0", NULL),
+	OF_DEV_AUXDATA("fsl,imx28-mmc", MX28_SSP1_BASE_ADDR, "mxs-mmc.1", NULL),
 	{ /* sentinel */ }
 };