| Message ID | 1361515491-16199-2-git-send-email-josephl@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Feb 22, 2013 at 07:44:48AM +0100, Joseph Lo wrote: > The Tegra kernel only support boot from DT now. Clean up the PMC driver > to support DT only, that includes: > > * remove the ifdef of CONFIG_OF > * replace the static mapping of PMC addr to map from DT > > Signed-off-by: Joseph Lo <josephl@nvidia.com> > --- > arch/arm/mach-tegra/pmc.c | 56 +++++++++++++++++++++++------------------------ > 1 file changed, 28 insertions(+), 28 deletions(-) > > diff --git a/arch/arm/mach-tegra/pmc.c b/arch/arm/mach-tegra/pmc.c > index d4fdb5f..2315e25 100644 > --- a/arch/arm/mach-tegra/pmc.c > +++ b/arch/arm/mach-tegra/pmc.c > @@ -1,5 +1,5 @@ > /* > - * Copyright (C) 2012 NVIDIA CORPORATION. All rights reserved. > + * Copyright (C) 2012,2013 NVIDIA CORPORATION. All rights reserved. > * > * This program is free software; you can redistribute it and/or modify it > * under the terms and conditions of the GNU General Public License, > @@ -16,59 +16,59 @@ > */ > > #include <linux/kernel.h> > +#include <linux/err.h> > #include <linux/io.h> > #include <linux/of.h> > - > -#include "iomap.h" > +#include <linux/of_address.h> > > #define PMC_CTRL 0x0 > #define PMC_CTRL_INTR_LOW (1 << 17) > > +static void __iomem *tegra_pmc_base; > +static bool tegra_pmc_invert_interrupt; > + > static inline u32 tegra_pmc_readl(u32 reg) > { > - return readl(IO_ADDRESS(TEGRA_PMC_BASE + reg)); > + return readl_relaxed(tegra_pmc_base + reg); > } > > static inline void tegra_pmc_writel(u32 val, u32 reg) > { > - writel(val, IO_ADDRESS(TEGRA_PMC_BASE + reg)); > + writel_relaxed(val, (tegra_pmc_base + reg)); > } > > -#ifdef CONFIG_OF > static const struct of_device_id matches[] __initconst = { > { .compatible = "nvidia,tegra20-pmc" }, > { } At least an extra entry for tegra114-pmc is necessary here. tegra114.dtsi only has: pmc { compatible = "nvidia,tegra114-pmc", "nvidia,tegra30-pmc"; reg = <0x7000e400 0x400>; }; otherwise the system will crash early during boot. Cheers, Peter.
On 02/21/2013 11:44 PM, Joseph Lo wrote: > The Tegra kernel only support boot from DT now. Clean up the PMC driver > to support DT only, that includes: > > * remove the ifdef of CONFIG_OF > * replace the static mapping of PMC addr to map from DT > diff --git a/arch/arm/mach-tegra/pmc.c b/arch/arm/mach-tegra/pmc.c > static inline u32 tegra_pmc_readl(u32 reg) > { > - return readl(IO_ADDRESS(TEGRA_PMC_BASE + reg)); > + return readl_relaxed(tegra_pmc_base + reg); The change from readl to readl_relaxed is definitely unrelated to cleaning up DT support. It needs to be a separate patch, and needs to be mentioned in the commit description. > +static void tegra_pmc_parse_dt(void) > +{ > + struct device_node *np; > + > + np = of_find_matching_node(NULL, matches); > + if (np) { Here, if (!np) BUG(); would be simpler, and allow the rest of the function not to be indented. > void __init tegra_pmc_init(void) > { ... > + if (!of_have_populated_dt()) > + return; That won't ever be true.
On Fri, 2013-02-22 at 21:05 +0800, Peter De Schrijver wrote: > On Fri, Feb 22, 2013 at 07:44:48AM +0100, Joseph Lo wrote: > > The Tegra kernel only support boot from DT now. Clean up the PMC driver > > to support DT only, that includes: > > > > * remove the ifdef of CONFIG_OF > > * replace the static mapping of PMC addr to map from DT > > > > -#ifdef CONFIG_OF > > static const struct of_device_id matches[] __initconst = { > > { .compatible = "nvidia,tegra20-pmc" }, > > { } > > At least an extra entry for tegra114-pmc is necessary here. tegra114.dtsi only > has: > > pmc { > compatible = "nvidia,tegra114-pmc", "nvidia,tegra30-pmc"; > reg = <0x7000e400 0x400>; > }; > I think it should be something like below, isn't it? compatible = "nvidia,tegra114-pmc", "nvidia,tegra30-pmc", "nvidia,tegra20-pmc"; or should we add tegra114 and tegra30 in the DT match table? Thanks, Joseph
On 02/22/2013 07:03 PM, Joseph Lo wrote: > On Fri, 2013-02-22 at 21:05 +0800, Peter De Schrijver wrote: >> On Fri, Feb 22, 2013 at 07:44:48AM +0100, Joseph Lo wrote: >>> The Tegra kernel only support boot from DT now. Clean up the PMC driver >>> to support DT only, that includes: >>> >>> * remove the ifdef of CONFIG_OF >>> * replace the static mapping of PMC addr to map from DT >>> >>> -#ifdef CONFIG_OF >>> static const struct of_device_id matches[] __initconst = { >>> { .compatible = "nvidia,tegra20-pmc" }, >>> { } >> >> At least an extra entry for tegra114-pmc is necessary here. tegra114.dtsi only >> has: >> >> pmc { >> compatible = "nvidia,tegra114-pmc", "nvidia,tegra30-pmc"; >> reg = <0x7000e400 0x400>; >> }; >> > I think it should be something like below, isn't it? > > compatible = "nvidia,tegra114-pmc", "nvidia,tegra30-pmc", > "nvidia,tegra20-pmc"; > > or should we add tegra114 and tegra30 in the DT match table? The Tegra114 PMC HW is probably not 100% backwards-compatible with previous SoCs' PMC, so the DT file should probably only list the specific SoC, and the driver should probably include all the compatible values it supports. Peter, can you confirm exactly which HW versions, if any, are 100% backwards-compatible?
On Sat, Feb 23, 2013 at 05:31:17AM +0100, Stephen Warren wrote: > On 02/22/2013 07:03 PM, Joseph Lo wrote: > > On Fri, 2013-02-22 at 21:05 +0800, Peter De Schrijver wrote: > >> On Fri, Feb 22, 2013 at 07:44:48AM +0100, Joseph Lo wrote: > >>> The Tegra kernel only support boot from DT now. Clean up the PMC driver > >>> to support DT only, that includes: > >>> > >>> * remove the ifdef of CONFIG_OF > >>> * replace the static mapping of PMC addr to map from DT > >>> > >>> -#ifdef CONFIG_OF > >>> static const struct of_device_id matches[] __initconst = { > >>> { .compatible = "nvidia,tegra20-pmc" }, > >>> { } > >> > >> At least an extra entry for tegra114-pmc is necessary here. tegra114.dtsi only > >> has: > >> > >> pmc { > >> compatible = "nvidia,tegra114-pmc", "nvidia,tegra30-pmc"; > >> reg = <0x7000e400 0x400>; > >> }; > >> > > I think it should be something like below, isn't it? > > > > compatible = "nvidia,tegra114-pmc", "nvidia,tegra30-pmc", > > "nvidia,tegra20-pmc"; > > > > or should we add tegra114 and tegra30 in the DT match table? > > The Tegra114 PMC HW is probably not 100% backwards-compatible with > previous SoCs' PMC, so the DT file should probably only list the > specific SoC, and the driver should probably include all the compatible > values it supports. > > Peter, can you confirm exactly which HW versions, if any, are 100% > backwards-compatible? > The major difference between the PMCs are the powerdomain IDs. The Tegra30 IDs are a strict superset of the Tegra20 IDs, but the Tegra114 IDs are not. There are differences in the CPU domains and 3D. The CPU domains aren't a problem I think because we won't powergate those domains directly using the PMC. We always program the flowcontroller to do the powergating on WFI. 3D is a different story however. We do powergate 3D under software control and the sequencing is different between Tegra30 and Tegra114. Also Tegra30 has 2 domains for 3D while Tegra114 has only 1. Then ofcourse Tegra114 lacks the SATA and PCIe domains because those features are missing compared to Tegra30. But the IDs haven't been reused. All in all I think it's best to explicitly require the driver to support the various SoCs. Cheers, Peter.
On 02/25/2013 07:28 AM, Peter De Schrijver wrote: > On Sat, Feb 23, 2013 at 05:31:17AM +0100, Stephen Warren wrote: ... >> Peter, can you confirm exactly which [PMC] HW versions, if any, are 100% >> backwards-compatible? >> > > The major difference between the PMCs are the powerdomain IDs. The Tegra30 IDs > are a strict superset of the Tegra20 IDs, but the Tegra114 IDs are not. There > are differences in the CPU domains and 3D. The CPU domains aren't a problem I > think because we won't powergate those domains directly using the PMC. We > always program the flowcontroller to do the powergating on WFI. 3D is a > different story however. We do powergate 3D under software control and the > sequencing is different between Tegra30 and Tegra114. Also Tegra30 has 2 > domains for 3D while Tegra114 has only 1. Then of course Tegra114 lacks the > SATA and PCIe domains because those features are missing compared to Tegra30. > But the IDs haven't been reused. All in all I think it's best to explicitly > require the driver to support the various SoCs. That does sound like a good idea; a separate compatible value for each SoC.
On Mon, 2013-02-25 at 23:43 +0800, Stephen Warren wrote: > On 02/25/2013 07:28 AM, Peter De Schrijver wrote: > > On Sat, Feb 23, 2013 at 05:31:17AM +0100, Stephen Warren wrote: > ... > >> Peter, can you confirm exactly which [PMC] HW versions, if any, are 100% > >> backwards-compatible? > >> > > > > The major difference between the PMCs are the powerdomain IDs. The Tegra30 IDs > > are a strict superset of the Tegra20 IDs, but the Tegra114 IDs are not. There > > are differences in the CPU domains and 3D. The CPU domains aren't a problem I > > think because we won't powergate those domains directly using the PMC. We > > always program the flowcontroller to do the powergating on WFI. 3D is a > > different story however. We do powergate 3D under software control and the > > sequencing is different between Tegra30 and Tegra114. Also Tegra30 has 2 > > domains for 3D while Tegra114 has only 1. Then of course Tegra114 lacks the > > SATA and PCIe domains because those features are missing compared to Tegra30. > > But the IDs haven't been reused. All in all I think it's best to explicitly > > require the driver to support the various SoCs. > > That does sound like a good idea; a separate compatible value for each SoC. OK. Will add another patch to fix the PMC compatibility in Tegra30 and Tegra114 dts file. And also in the DT match table of PMC driver. Thanks, Joseph
diff --git a/arch/arm/mach-tegra/pmc.c b/arch/arm/mach-tegra/pmc.c index d4fdb5f..2315e25 100644 --- a/arch/arm/mach-tegra/pmc.c +++ b/arch/arm/mach-tegra/pmc.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2012 NVIDIA CORPORATION. All rights reserved. + * Copyright (C) 2012,2013 NVIDIA CORPORATION. All rights reserved. * * This program is free software; you can redistribute it and/or modify it * under the terms and conditions of the GNU General Public License, @@ -16,59 +16,59 @@ */ #include <linux/kernel.h> +#include <linux/err.h> #include <linux/io.h> #include <linux/of.h> - -#include "iomap.h" +#include <linux/of_address.h> #define PMC_CTRL 0x0 #define PMC_CTRL_INTR_LOW (1 << 17) +static void __iomem *tegra_pmc_base; +static bool tegra_pmc_invert_interrupt; + static inline u32 tegra_pmc_readl(u32 reg) { - return readl(IO_ADDRESS(TEGRA_PMC_BASE + reg)); + return readl_relaxed(tegra_pmc_base + reg); } static inline void tegra_pmc_writel(u32 val, u32 reg) { - writel(val, IO_ADDRESS(TEGRA_PMC_BASE + reg)); + writel_relaxed(val, (tegra_pmc_base + reg)); } -#ifdef CONFIG_OF static const struct of_device_id matches[] __initconst = { { .compatible = "nvidia,tegra20-pmc" }, { } }; -#endif + +static void tegra_pmc_parse_dt(void) +{ + struct device_node *np; + + np = of_find_matching_node(NULL, matches); + if (np) { + tegra_pmc_base = of_iomap(np, 0); + + tegra_pmc_invert_interrupt = of_property_read_bool(np, + "nvidia,invert-interrupt"); + } else { + /* Should not be here, the PMC DT node should always exist. */ + BUG(); + } +} void __init tegra_pmc_init(void) { - /* - * For now, Harmony is the only board that uses the PMC, and it wants - * the signal inverted. Seaboard would too if it used the PMC. - * Hopefully by the time other boards want to use the PMC, everything - * will be device-tree, or they also want it inverted. - */ - bool invert_interrupt = true; u32 val; -#ifdef CONFIG_OF - if (of_have_populated_dt()) { - struct device_node *np; + if (!of_have_populated_dt()) + return; - invert_interrupt = false; - - np = of_find_matching_node(NULL, matches); - if (np) { - if (of_find_property(np, "nvidia,invert-interrupt", - NULL)) - invert_interrupt = true; - } - } -#endif + tegra_pmc_parse_dt(); val = tegra_pmc_readl(PMC_CTRL); - if (invert_interrupt) + if (tegra_pmc_invert_interrupt) val |= PMC_CTRL_INTR_LOW; else val &= ~PMC_CTRL_INTR_LOW;
The Tegra kernel only support boot from DT now. Clean up the PMC driver to support DT only, that includes: * remove the ifdef of CONFIG_OF * replace the static mapping of PMC addr to map from DT Signed-off-by: Joseph Lo <josephl@nvidia.com> --- arch/arm/mach-tegra/pmc.c | 56 +++++++++++++++++++++++------------------------ 1 file changed, 28 insertions(+), 28 deletions(-)