Patchwork [V2,1/3] ARM: dts: tegra: add clock source for PMC

login
register
mail settings
Submitter Joseph Lo
Date March 18, 2013, 8:09 a.m.
Message ID <1363594199-10974-2-git-send-email-josephl@nvidia.com>
Download mbox | patch
Permalink /patch/228382/
State Superseded, archived
Headers show

Comments

Joseph Lo - March 18, 2013, 8:09 a.m.
The clock source of PMC is PCLK. Adding it into DTS for Tegra20 and Tegra30.

Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
V2:
* new in this change
---
 arch/arm/boot/dts/tegra20.dtsi | 1 +
 arch/arm/boot/dts/tegra30.dtsi | 1 +
 2 files changed, 2 insertions(+)
Stephen Warren - March 19, 2013, 4:42 p.m.
On 03/18/2013 02:09 AM, Joseph Lo wrote:
> The clock source of PMC is PCLK. Adding it into DTS for Tegra20 and Tegra30.

> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi

>  	pmc {
>  		compatible = "nvidia,tegra20-pmc";
>  		reg = <0x7000e400 0x400>;
> +		clocks = <&tegra_car 110>;
>  	};

The DT binding documentation needs to list the set of clocks that must
be present.

Doesn't the PMC also receive a "clk32k_in" from the PMIC, or is that
routed into the CAR, and then into the PMC? Either way, the PMC module
receives that clock somehow. Since there are multiple clocks, that also
means that a clock-names property is required.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Joseph Lo - March 20, 2013, 10:12 a.m.
On Wed, 2013-03-20 at 00:42 +0800, Stephen Warren wrote:
> On 03/18/2013 02:09 AM, Joseph Lo wrote:
> > The clock source of PMC is PCLK. Adding it into DTS for Tegra20 and Tegra30.
> 
> > diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
> 
> >  	pmc {
> >  		compatible = "nvidia,tegra20-pmc";
> >  		reg = <0x7000e400 0x400>;
> > +		clocks = <&tegra_car 110>;
> >  	};
> 
> The DT binding documentation needs to list the set of clocks that must
> be present.
> 
> Doesn't the PMC also receive a "clk32k_in" from the PMIC, or is that
> routed into the CAR, and then into the PMC? Either way, the PMC module
> receives that clock somehow. Since there are multiple clocks, that also
> means that a clock-names property is required.

Do you mean the DTS below and add it into binding document?

/ SoC dts including file
pmc {
	compatible = "nvidia,tegra20-pmc";
	reg = <0x7000e400 0x400>;
	clocks = <&tegra_car 110>, <&clk32k_in>;
	clock-names= "pclk", "clk32k_in";
};

/ Tegra board dts file

pmic {
	...
	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>;
		};
	};
};

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren - March 20, 2013, 3:54 p.m.
On 03/20/2013 04:12 AM, Joseph Lo wrote:
> On Wed, 2013-03-20 at 00:42 +0800, Stephen Warren wrote:
>> On 03/18/2013 02:09 AM, Joseph Lo wrote:
>>> The clock source of PMC is PCLK. Adding it into DTS for Tegra20 and Tegra30.
>>
>>> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
>>
>>>  	pmc {
>>>  		compatible = "nvidia,tegra20-pmc";
>>>  		reg = <0x7000e400 0x400>;
>>> +		clocks = <&tegra_car 110>;
>>>  	};
>>
>> The DT binding documentation needs to list the set of clocks that must
>> be present.
>>
>> Doesn't the PMC also receive a "clk32k_in" from the PMIC, or is that
>> routed into the CAR, and then into the PMC? Either way, the PMC module
>> receives that clock somehow. Since there are multiple clocks, that also
>> means that a clock-names property is required.
> 
> Do you mean the DTS below and add it into binding document?
> 
> / SoC dts including file
> pmc {
> 	compatible = "nvidia,tegra20-pmc";
> 	reg = <0x7000e400 0x400>;
> 	clocks = <&tegra_car 110>, <&clk32k_in>;
> 	clock-names= "pclk", "clk32k_in";
> };

Yes, that's what should technically be present.

> / Tegra board dts file
> 
> pmic {
> 	...
> 	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>;
> 		};
> 	};
> };

That won't work; the PMIC drivers don't enumerate sub-nodes as busss, so
 that clocks node won't ever be processed. Also, the PMIC drivers aren't
clock providers in most cases. It's not quite a correct representation
of the HW, but I would suggest simply creating a top-level "clock" node
for that fixed clock. If we ever have more top-level clocks, we can move
them into a top-level "clocks" node at that time.

Note: If there aren't already any other top-level clock nodes (which I
think is the case), the node can be named just "clock" rather than
"clock@0", since there are no naming conflicts.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index 3183581..4ba9c7b 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -419,6 +419,7 @@ 
 	pmc {
 		compatible = "nvidia,tegra20-pmc";
 		reg = <0x7000e400 0x400>;
+		clocks = <&tegra_car 110>;
 	};
 
 	memory-controller@7000f000 {
diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
index 11d04fe..7299014 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -428,6 +428,7 @@ 
 	pmc {
 		compatible = "nvidia,tegra30-pmc";
 		reg = <0x7000e400 0x400>;
+		clocks = <&tegra_car 218>;
 	};
 
 	memory-controller {