| Message ID | 1364901583-24867-2-git-send-email-josephl@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 04/02/2013 05:19 AM, Joseph Lo wrote: > Adding the bindings of the clock source of PMC in DT. And getting > the clock from DT. > .../bindings/arm/tegra/nvidia,tegra20-pmc.txt | 23 ++++++++++++++++++++++ > arch/arm/mach-tegra/common.c | 2 +- > arch/arm/mach-tegra/pmc.c | 4 ++++ I'd like to re-order and split up this series slightly differently. Patch (1) should include the binding updates (nvidia,tegra20-pmc.txt) from this patch 1/3 and the DT changes (*.dts) from patch 2/3. Patch (2) should contain be the current patch 3/3 plus the *.c changes from this patch 1/3. That ensures that: a) The DT binding changes happen before (or rather at the same time) as the first other change that relies on them. b) The DT changes happen before the code changes, which is required to avoid the code-changes causing WARN_ON(IS_ERR(tegra_pclk)) from firing before all the patches are applied. c) The new code to retrieve tegra_pclk isn't added without anything that uses the new variable; it's added only when something makes use of it. > diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt > +- clocks : the source clock of PMC > +- clock-names : the name of source clock of PMC "the" is wrong here, since there is more than one clock. Also, the binding document needs to define the set of clocks that those properties must provide. I suggest the following wording, which I cribbed from the Tegra audio bindings: - clocks : Must contain an entry for each entry in clock-names. - clock-names : Must include the following entries: "pclk" (The Tegra clock of that name), "clk32k_in" (The 32KHz clock input to Tegra). That text should also be added to the "Required properties" section, not the "Optional proeprties" section. > +/ SoC dts including file > pmc@7000f400 { > compatible = "nvidia,tegra20-pmc"; > reg = <0x7000e400 0x400>; > nvidia,invert-interrupt; > + clocks = <&tegra_car 110>, <&clk32k_in>; > + clock-names = "pclk", "clk32k_in"; In this example, I'd prefer the clocks properties to be before the custom nvidia,invert-interrupt property, so that all standard and required properties are first, then custom properties. so, compatible, reg, clock*, nvidia,invert-interrupt. > diff --git a/arch/arm/mach-tegra/common.c b/arch/arm/mach-tegra/common.c > void __init tegra_dt_init_irq(void) > { > tegra_clocks_init(); > + tegra_pmc_init(); > tegra_init_irq(); > irqchip_init(); > } I'll let this slide for now. Please be aware that I'd prefer all non-IRQ-related initialization to happen somewhere inside the machine's .init_machine() callback, not its .init_irq() callback. However, we can clean this up later; most likely a lot of the calls in tegra_init_early() should move too.
diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt index b5846e2..1cb8f6b 100644 --- a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt +++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt @@ -9,11 +9,34 @@ Properties: signal is fed into the PMC. This signal is optionally inverted, and then fed into the ARM GIC. The PMC is not involved in the detection or handling of this interrupt signal, merely its inversion. +- clocks : the source clock of PMC +- clock-names : the name of source clock of PMC Example: +/ SoC dts including file pmc@7000f400 { compatible = "nvidia,tegra20-pmc"; reg = <0x7000e400 0x400>; nvidia,invert-interrupt; + clocks = <&tegra_car 110>, <&clk32k_in>; + clock-names = "pclk", "clk32k_in"; }; + +/ Tegra board dts file +{ + ... + clocks { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <0>; + + clk32k_in: clock@0 { + compatible = "fixed-clock"; + reg=<0>; + #clock-cells = <0>; + clock-frequency = <32768>; + }; + }; + ... +} diff --git a/arch/arm/mach-tegra/common.c b/arch/arm/mach-tegra/common.c index f0315c9..b02ebe7 100644 --- a/arch/arm/mach-tegra/common.c +++ b/arch/arm/mach-tegra/common.c @@ -61,6 +61,7 @@ u32 tegra_uart_config[4] = { void __init tegra_dt_init_irq(void) { tegra_clocks_init(); + tegra_pmc_init(); tegra_init_irq(); irqchip_init(); } @@ -100,7 +101,6 @@ void __init tegra_init_early(void) tegra_apb_io_init(); tegra_init_fuse(); tegra_init_cache(); - tegra_pmc_init(); tegra_powergate_init(); tegra_hotplug_init(); } diff --git a/arch/arm/mach-tegra/pmc.c b/arch/arm/mach-tegra/pmc.c index b30e921..dbe2577 100644 --- a/arch/arm/mach-tegra/pmc.c +++ b/arch/arm/mach-tegra/pmc.c @@ -19,6 +19,7 @@ #include <linux/io.h> #include <linux/of.h> #include <linux/of_address.h> +#include <linux/clk.h> #define PMC_CTRL 0x0 #define PMC_CTRL_INTR_LOW (1 << 17) @@ -43,6 +44,7 @@ static DEFINE_SPINLOCK(tegra_powergate_lock); static void __iomem *tegra_pmc_base; static bool tegra_pmc_invert_interrupt; +static struct clk *tegra_pclk; static inline u32 tegra_pmc_readl(u32 reg) { @@ -151,6 +153,8 @@ static void tegra_pmc_parse_dt(void) tegra_pmc_invert_interrupt = of_property_read_bool(np, "nvidia,invert-interrupt"); + tegra_pclk = of_clk_get_by_name(np, "pclk"); + WARN_ON(IS_ERR(tegra_pclk)); } void __init tegra_pmc_init(void)
Adding the bindings of the clock source of PMC in DT. And getting the clock from DT. Signed-off-by: Joseph Lo <josephl@nvidia.com> --- V3: * add binding document for the clock source of PMC * get the clock from DT by of_get_clk_by_name() * fix WARN_ON_ONCE to WARN_ON when there is no clock define in DT V2: * new in this series --- .../bindings/arm/tegra/nvidia,tegra20-pmc.txt | 23 ++++++++++++++++++++++ arch/arm/mach-tegra/common.c | 2 +- arch/arm/mach-tegra/pmc.c | 4 ++++ 3 files changed, 28 insertions(+), 1 deletion(-)