Patchwork ARM: imx6q: add support for IRAM

login
register
mail settings
Submitter Jason Chen
Date Dec. 26, 2011, 11:15 a.m.
Message ID <1324898123-13973-1-git-send-email-jason.chen@linaro.org>
Download mbox | patch
Permalink /patch/133216/
State New
Headers show

Comments

Jason Chen - Dec. 26, 2011, 11:15 a.m.
Signed-off-by: Jason Chen <jason.chen@linaro.org>
Signed-off-by: Eric Miao <eric.miao@linaro.org>
---
 arch/arm/boot/dts/imx6q.dtsi    |    7 +++++++
 arch/arm/mach-imx/Kconfig       |    1 +
 arch/arm/mach-imx/clock-imx6q.c |    3 ++-
 arch/arm/mach-imx/mach-imx6q.c  |   17 +++++++++++++++++
 4 files changed, 27 insertions(+), 1 deletions(-)
Shawn Guo - Dec. 26, 2011, 1:41 p.m.
On Mon, Dec 26, 2011 at 07:15:23PM +0800, Jason Chen wrote:
> Signed-off-by: Jason Chen <jason.chen@linaro.org>
> Signed-off-by: Eric Miao <eric.miao@linaro.org>
> ---
>  arch/arm/boot/dts/imx6q.dtsi    |    7 +++++++
>  arch/arm/mach-imx/Kconfig       |    1 +
>  arch/arm/mach-imx/clock-imx6q.c |    3 ++-
>  arch/arm/mach-imx/mach-imx6q.c  |   17 +++++++++++++++++
>  4 files changed, 27 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
> index 263e8f3..01646b8 100644
> --- a/arch/arm/boot/dts/imx6q.dtsi
> +++ b/arch/arm/boot/dts/imx6q.dtsi
> @@ -80,6 +80,13 @@
>  		};
>  	};
>  
> +	ocram@00900000 {
> +		compatible = "fsl,imx6q-iram";

It should have nothing specific to imx6q, and could be "fsl,iram".
Then we can have some common code across different SoCs to match it.

> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		reg = <0x00900000 0x40000>;
> +	};
> +

Would it be better to put this node inside node 'soc', since it's
the part of SoC?  The bonus point would be that we can save
#address-cells and #size-cells for iram node.

>  	soc {
>  		#address-cells = <1>;
>  		#size-cells = <1>;
> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
> index fa5c10c..88eaef6 100644
> --- a/arch/arm/mach-imx/Kconfig
> +++ b/arch/arm/mach-imx/Kconfig
> @@ -841,6 +841,7 @@ config SOC_IMX6Q
>  	select HAVE_IMX_GPC
>  	select HAVE_IMX_MMDC
>  	select HAVE_IMX_SRC
> +	select IRAM_ALLOC
>  	select USE_OF
>  
>  	help
> diff --git a/arch/arm/mach-imx/clock-imx6q.c b/arch/arm/mach-imx/clock-imx6q.c
> index 9273c2a..285fd06 100644
> --- a/arch/arm/mach-imx/clock-imx6q.c
> +++ b/arch/arm/mach-imx/clock-imx6q.c
> @@ -1777,6 +1777,7 @@ DEF_CLK(mmdc_ch0_axi_clk, CCGR3, CG10, &periph_clk,	  &mmdc_ch0_ipg_clk);
>  DEF_CLK(mmdc_ch1_ipg_clk, CCGR3, CG13, &ipg_clk,	  NULL);
>  DEF_CLK(mmdc_ch1_axi_clk, CCGR3, CG11, &periph2_clk,	  &mmdc_ch1_ipg_clk);
>  DEF_CLK(openvg_axi_clk,   CCGR3, CG13, &axi_clk,	  NULL);
> +DEF_CLK(ocram_clk,   	  CCGR3, CG14, &ahb_clk,	  NULL);
>  DEF_CLK(pwm1_clk,	  CCGR4, CG8,  &ipg_perclk,	  NULL);
>  DEF_CLK(pwm2_clk,	  CCGR4, CG9,  &ipg_perclk,	  NULL);
>  DEF_CLK(pwm3_clk,	  CCGR4, CG10, &ipg_perclk,	  NULL);
> @@ -1982,7 +1983,7 @@ int __init mx6q_clocks_init(void)
>  	/* only keep necessary clocks on */
>  	writel_relaxed(0x3 << CG0  | 0x3 << CG1  | 0x3 << CG2,	CCGR0);
>  	writel_relaxed(0x3 << CG8  | 0x3 << CG9  | 0x3 << CG10,	CCGR2);
> -	writel_relaxed(0x3 << CG10 | 0x3 << CG12,		CCGR3);
> +	writel_relaxed(0x3 << CG10 | 0x3 << CG12 | 0x1 << CG14,	CCGR3);
>  	writel_relaxed(0x3 << CG4  | 0x3 << CG6  | 0x3 << CG7,	CCGR4);
>  	writel_relaxed(0x3 << CG0,				CCGR5);
>  	writel_relaxed(0,					CCGR6);
> diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
> index c257281..7612ef5 100644
> --- a/arch/arm/mach-imx/mach-imx6q.c
> +++ b/arch/arm/mach-imx/mach-imx6q.c
> @@ -27,6 +27,7 @@
>  #include <asm/mach/time.h>
>  #include <mach/common.h>
>  #include <mach/hardware.h>
> +#include <mach/iram.h>
>  
>  void imx6q_restart(char mode, const char *cmd)
>  {
> @@ -73,6 +74,21 @@ static int ksz9021rn_phy_fixup(struct phy_device *phydev)
>  	return 0;
>  }
>  
> +static int imx6q_init_iram(void)
> +{
> +	struct device_node *np;
> +	struct resource res;
> +
> +	np = of_find_compatible_node(NULL, NULL, "fsl,imx6q-iram");
> +	if (np) {
> +		if (!of_address_to_resource(np, 0, &res))
> +			return iram_init(res.start, res.end - res.start + 1);
> +		else
> +			return 0;
> +	} else
> +		return 0;
> +}
> +
This piece of code should not be imx6q specific.  Instead, it could
become the function iram_of_init() in iram_alloc.c, so that the future
soc like imx6dl or whatever that has iram integrated can just can call
iram_of_init() to have iram allocator set up.
Russell King - ARM Linux - Jan. 3, 2012, 9:10 a.m.
On Mon, Dec 26, 2011 at 09:41:30PM +0800, Shawn Guo wrote:
> On Mon, Dec 26, 2011 at 07:15:23PM +0800, Jason Chen wrote:
> > Signed-off-by: Jason Chen <jason.chen@linaro.org>
> > Signed-off-by: Eric Miao <eric.miao@linaro.org>
> > ---
> >  arch/arm/boot/dts/imx6q.dtsi    |    7 +++++++
> >  arch/arm/mach-imx/Kconfig       |    1 +
> >  arch/arm/mach-imx/clock-imx6q.c |    3 ++-
> >  arch/arm/mach-imx/mach-imx6q.c  |   17 +++++++++++++++++
> >  4 files changed, 27 insertions(+), 1 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
> > index 263e8f3..01646b8 100644
> > --- a/arch/arm/boot/dts/imx6q.dtsi
> > +++ b/arch/arm/boot/dts/imx6q.dtsi
> > @@ -80,6 +80,13 @@
> >  		};
> >  	};
> >  
> > +	ocram@00900000 {
> > +		compatible = "fsl,imx6q-iram";
> 
> It should have nothing specific to imx6q, and could be "fsl,iram".
> Then we can have some common code across different SoCs to match it.

Why is this even specific to fsl?  Isn't it something that could be
specified in a totally generic way?

As I showed with my original set of sram patches, there is not much
specific about this on-board RAM: what is specific is how a SoC uses
it, and that's up to the rest of the SoC code.
Russell King - ARM Linux - Jan. 3, 2012, 12:41 p.m.
On Tue, Jan 03, 2012 at 08:43:56PM +0800, Shawn Guo wrote:
> On Tue, Jan 03, 2012 at 09:10:15AM +0000, Russell King - ARM Linux wrote:
> > Why is this even specific to fsl?  Isn't it something that could be
> > specified in a totally generic way?
> > 
> > As I showed with my original set of sram patches, there is not much
> > specific about this on-board RAM: what is specific is how a SoC uses
> > it, and that's up to the rest of the SoC code.
> > 
> As I have not seen any news/updates about the sram consolidation series
> since May, I did not bring up it here.  So what's the state of the
> series?  If it shows up on some stable branch, we would be happy to
> base the work here on it.

I have no idea; as I've already said, I lost total interest in it.

What I _am_ saying though is that rather than defining some platform
specific bindings and continuing that idiotic state of affairs, we
have the chance to do things properly now: define a standard set of
DT bindings to describe on-board SRAM.

That's something which can be done with or without the sram
consolidation stuff.
Shawn Guo - Jan. 3, 2012, 12:43 p.m.
On Tue, Jan 03, 2012 at 09:10:15AM +0000, Russell King - ARM Linux wrote:
> On Mon, Dec 26, 2011 at 09:41:30PM +0800, Shawn Guo wrote:
> > On Mon, Dec 26, 2011 at 07:15:23PM +0800, Jason Chen wrote:
> > > Signed-off-by: Jason Chen <jason.chen@linaro.org>
> > > Signed-off-by: Eric Miao <eric.miao@linaro.org>
> > > ---
> > >  arch/arm/boot/dts/imx6q.dtsi    |    7 +++++++
> > >  arch/arm/mach-imx/Kconfig       |    1 +
> > >  arch/arm/mach-imx/clock-imx6q.c |    3 ++-
> > >  arch/arm/mach-imx/mach-imx6q.c  |   17 +++++++++++++++++
> > >  4 files changed, 27 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
> > > index 263e8f3..01646b8 100644
> > > --- a/arch/arm/boot/dts/imx6q.dtsi
> > > +++ b/arch/arm/boot/dts/imx6q.dtsi
> > > @@ -80,6 +80,13 @@
> > >  		};
> > >  	};
> > >  
> > > +	ocram@00900000 {
> > > +		compatible = "fsl,imx6q-iram";
> > 
> > It should have nothing specific to imx6q, and could be "fsl,iram".
> > Then we can have some common code across different SoCs to match it.
> 
> Why is this even specific to fsl?  Isn't it something that could be
> specified in a totally generic way?
> 
> As I showed with my original set of sram patches, there is not much
> specific about this on-board RAM: what is specific is how a SoC uses
> it, and that's up to the rest of the SoC code.
> 
As I have not seen any news/updates about the sram consolidation series
since May, I did not bring up it here.  So what's the state of the
series?  If it shows up on some stable branch, we would be happy to
base the work here on it.
Shawn Guo - Jan. 3, 2012, 12:45 p.m.
On Mon, Dec 26, 2011 at 07:15:23PM +0800, Jason Chen wrote:
> +DEF_CLK(ocram_clk,   	  CCGR3, CG14, &ahb_clk,	  NULL);

  CC      arch/arm/mach-imx/clock-imx6q.o
arch/arm/mach-imx/clock-imx6q.c:1782:1: warning: ‘ocram_clk’ defined but not used
Shawn Guo - Jan. 3, 2012, 1:04 p.m.
On Tue, Jan 03, 2012 at 12:41:36PM +0000, Russell King - ARM Linux wrote:
> On Tue, Jan 03, 2012 at 08:43:56PM +0800, Shawn Guo wrote:
> > On Tue, Jan 03, 2012 at 09:10:15AM +0000, Russell King - ARM Linux wrote:
> > > Why is this even specific to fsl?  Isn't it something that could be
> > > specified in a totally generic way?
> > > 
> > > As I showed with my original set of sram patches, there is not much
> > > specific about this on-board RAM: what is specific is how a SoC uses
> > > it, and that's up to the rest of the SoC code.
> > > 
> > As I have not seen any news/updates about the sram consolidation series
> > since May, I did not bring up it here.  So what's the state of the
> > series?  If it shows up on some stable branch, we would be happy to
> > base the work here on it.
> 
> I have no idea; as I've already said, I lost total interest in it.
> 
> What I _am_ saying though is that rather than defining some platform
> specific bindings and continuing that idiotic state of affairs, we
> have the chance to do things properly now: define a standard set of
> DT bindings to describe on-board SRAM.
> 
> That's something which can be done with or without the sram
> consolidation stuff.
> 
I guess it makes more sense to have a concentrated piece of code
backing that DT binding.  This can be another reason we should continue
the sram consolidation effort?
Linus Walleij - Jan. 4, 2012, 11:12 a.m.
On Tue, Jan 3, 2012 at 1:41 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Tue, Jan 03, 2012 at 08:43:56PM +0800, Shawn Guo wrote:
>> On Tue, Jan 03, 2012 at 09:10:15AM +0000, Russell King - ARM Linux wrote:
>> > Why is this even specific to fsl?  Isn't it something that could be
>> > specified in a totally generic way?
>> >
>> > As I showed with my original set of sram patches, there is not much
>> > specific about this on-board RAM: what is specific is how a SoC uses
>> > it, and that's up to the rest of the SoC code.
>> >
>> As I have not seen any news/updates about the sram consolidation series
>> since May, I did not bring up it here.  So what's the state of the
>> series?  If it shows up on some stable branch, we would be happy to
>> base the work here on it.
>
> I have no idea; as I've already said, I lost total interest in it.
>
> What I _am_ saying though is that rather than defining some platform
> specific bindings and continuing that idiotic state of affairs, we
> have the chance to do things properly now: define a standard set of
> DT bindings to describe on-board SRAM.
>
> That's something which can be done with or without the sram
> consolidation stuff.

I second this. It's just a piece of RAM, it should be described
the same way irrespective of system or even OS.

On-chip RAM is yet another area where platforms and their
maintainers sometimes think they are very different - just like
everyone else.

(This connects to the other IRAM patch floating on the list.)

Yours,
Linus Walleij
jason.chen@freescale.com - Jan. 5, 2012, 5:10 a.m.
On Tue, Jan 03, 2012 at 08:45:02PM +0800, Shawn Guo wrote:
> On Mon, Dec 26, 2011 at 07:15:23PM +0800, Jason Chen wrote:
> > +DEF_CLK(ocram_clk,   	  CCGR3, CG14, &ahb_clk,	  NULL);
> 
>   CC      arch/arm/mach-imx/clock-imx6q.o
> arch/arm/mach-imx/clock-imx6q.c:1782:1: warning: ‘ocram_clk’ defined but not used
it should be a dependcy of vpu/sdma who using IRAM, they can be added.
> 
> -- 
> Regards,
> Shawn
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
jason.chen@freescale.com - Jan. 5, 2012, 5:40 a.m.
On Thu, Jan 05, 2012 at 01:47:54PM +0800, Shawn Guo wrote:
> On Thu, Jan 05, 2012 at 01:10:48PM +0800, Jason Chen wrote:
> > On Tue, Jan 03, 2012 at 08:45:02PM +0800, Shawn Guo wrote:
> > > On Mon, Dec 26, 2011 at 07:15:23PM +0800, Jason Chen wrote:
> > > > +DEF_CLK(ocram_clk,   	  CCGR3, CG14, &ahb_clk,	  NULL);
> > > 
> > >   CC      arch/arm/mach-imx/clock-imx6q.o
> > > arch/arm/mach-imx/clock-imx6q.c:1782:1: warning: ‘ocram_clk’ defined but not used
> > it should be a dependcy of vpu/sdma who using IRAM, they can be added.
> 
> My point is your patch should not introduce any warning at any time.
sure, will fix it.
> 
> -- 
> Regards,
> Shawn
Shawn Guo - Jan. 5, 2012, 5:47 a.m.
On Thu, Jan 05, 2012 at 01:10:48PM +0800, Jason Chen wrote:
> On Tue, Jan 03, 2012 at 08:45:02PM +0800, Shawn Guo wrote:
> > On Mon, Dec 26, 2011 at 07:15:23PM +0800, Jason Chen wrote:
> > > +DEF_CLK(ocram_clk,   	  CCGR3, CG14, &ahb_clk,	  NULL);
> > 
> >   CC      arch/arm/mach-imx/clock-imx6q.o
> > arch/arm/mach-imx/clock-imx6q.c:1782:1: warning: ‘ocram_clk’ defined but not used
> it should be a dependcy of vpu/sdma who using IRAM, they can be added.

My point is your patch should not introduce any warning at any time.

Patch

diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index 263e8f3..01646b8 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -80,6 +80,13 @@ 
 		};
 	};
 
+	ocram@00900000 {
+		compatible = "fsl,imx6q-iram";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		reg = <0x00900000 0x40000>;
+	};
+
 	soc {
 		#address-cells = <1>;
 		#size-cells = <1>;
diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index fa5c10c..88eaef6 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -841,6 +841,7 @@  config SOC_IMX6Q
 	select HAVE_IMX_GPC
 	select HAVE_IMX_MMDC
 	select HAVE_IMX_SRC
+	select IRAM_ALLOC
 	select USE_OF
 
 	help
diff --git a/arch/arm/mach-imx/clock-imx6q.c b/arch/arm/mach-imx/clock-imx6q.c
index 9273c2a..285fd06 100644
--- a/arch/arm/mach-imx/clock-imx6q.c
+++ b/arch/arm/mach-imx/clock-imx6q.c
@@ -1777,6 +1777,7 @@  DEF_CLK(mmdc_ch0_axi_clk, CCGR3, CG10, &periph_clk,	  &mmdc_ch0_ipg_clk);
 DEF_CLK(mmdc_ch1_ipg_clk, CCGR3, CG13, &ipg_clk,	  NULL);
 DEF_CLK(mmdc_ch1_axi_clk, CCGR3, CG11, &periph2_clk,	  &mmdc_ch1_ipg_clk);
 DEF_CLK(openvg_axi_clk,   CCGR3, CG13, &axi_clk,	  NULL);
+DEF_CLK(ocram_clk,   	  CCGR3, CG14, &ahb_clk,	  NULL);
 DEF_CLK(pwm1_clk,	  CCGR4, CG8,  &ipg_perclk,	  NULL);
 DEF_CLK(pwm2_clk,	  CCGR4, CG9,  &ipg_perclk,	  NULL);
 DEF_CLK(pwm3_clk,	  CCGR4, CG10, &ipg_perclk,	  NULL);
@@ -1982,7 +1983,7 @@  int __init mx6q_clocks_init(void)
 	/* only keep necessary clocks on */
 	writel_relaxed(0x3 << CG0  | 0x3 << CG1  | 0x3 << CG2,	CCGR0);
 	writel_relaxed(0x3 << CG8  | 0x3 << CG9  | 0x3 << CG10,	CCGR2);
-	writel_relaxed(0x3 << CG10 | 0x3 << CG12,		CCGR3);
+	writel_relaxed(0x3 << CG10 | 0x3 << CG12 | 0x1 << CG14,	CCGR3);
 	writel_relaxed(0x3 << CG4  | 0x3 << CG6  | 0x3 << CG7,	CCGR4);
 	writel_relaxed(0x3 << CG0,				CCGR5);
 	writel_relaxed(0,					CCGR6);
diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index c257281..7612ef5 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -27,6 +27,7 @@ 
 #include <asm/mach/time.h>
 #include <mach/common.h>
 #include <mach/hardware.h>
+#include <mach/iram.h>
 
 void imx6q_restart(char mode, const char *cmd)
 {
@@ -73,6 +74,21 @@  static int ksz9021rn_phy_fixup(struct phy_device *phydev)
 	return 0;
 }
 
+static int imx6q_init_iram(void)
+{
+	struct device_node *np;
+	struct resource res;
+
+	np = of_find_compatible_node(NULL, NULL, "fsl,imx6q-iram");
+	if (np) {
+		if (!of_address_to_resource(np, 0, &res))
+			return iram_init(res.start, res.end - res.start + 1);
+		else
+			return 0;
+	} else
+		return 0;
+}
+
 static void __init imx6q_init_machine(void)
 {
 	if (of_machine_is_compatible("fsl,imx6q-sabrelite"))
@@ -81,6 +97,7 @@  static void __init imx6q_init_machine(void)
 
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
 
+	imx6q_init_iram();
 	imx6q_pm_init();
 }