diff mbox

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

Message ID 1363594199-10974-2-git-send-email-josephl@nvidia.com (mailing list archive)
State New, archived
Headers show

Commit Message

Joseph Lo March 18, 2013, 8:09 a.m. UTC
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(+)

Comments

Stephen Warren March 19, 2013, 4:42 p.m. UTC | #1
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.
Joseph Lo March 20, 2013, 10:12 a.m. UTC | #2
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>;
		};
	};
};
Stephen Warren March 20, 2013, 3:54 p.m. UTC | #3
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.
diff mbox

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 {