| Message ID | 1324898123-13973-1-git-send-email-jason.chen@linaro.org |
|---|---|
| State | New |
| Headers | show |
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.
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.
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.
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.
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
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?
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
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
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
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.
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(); }