Patchwork [v6,08/10] ARM: dt: Add references to tegra_car clocks

login
register
mail settings
Submitter Peter De Schrijver
Date Feb. 11, 2013, 11:04 a.m.
Message ID <1360580700-10245-9-git-send-email-pdeschrijver@nvidia.com>
Download mbox | patch
Permalink /patch/219564/
State Superseded, archived
Headers show

Comments

Peter De Schrijver - Feb. 11, 2013, 11:04 a.m.
Add references to tegra_car clocks for the basic device nodes.

Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
---
 arch/arm/boot/dts/tegra114.dtsi |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)
Stephen Warren - Feb. 12, 2013, 5:24 p.m.
On 02/11/2013 04:04 AM, Peter De Schrijver wrote:
> Add references to tegra_car clocks for the basic device nodes.

In this patch, you also need to remove the "clock-frequency" properties
from tegra114-*.dts board files.
--
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
Peter De Schrijver - Feb. 13, 2013, 11:38 a.m.
On Tue, Feb 12, 2013 at 06:24:05PM +0100, Stephen Warren wrote:
> On 02/11/2013 04:04 AM, Peter De Schrijver wrote:
> > Add references to tegra_car clocks for the basic device nodes.
> 
> In this patch, you also need to remove the "clock-frequency" properties
> from tegra114-*.dts board files.

That would probably break bisectability no? I think this needs to be done
in a separate patch. Also, the UARTD enabling in init_table[] can then be
removed.

Cheers,

Peter.
--
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 - Feb. 13, 2013, 4:48 p.m.
On 02/13/2013 04:38 AM, Peter De Schrijver wrote:
> On Tue, Feb 12, 2013 at 06:24:05PM +0100, Stephen Warren wrote:
>> On 02/11/2013 04:04 AM, Peter De Schrijver wrote:
>>> Add references to tegra_car clocks for the basic device nodes.
>>
>> In this patch, you also need to remove the "clock-frequency" properties
>> from tegra114-*.dts board files.
> 
> That would probably break bisectability no?

Oh right, I read the patch order backwards. You can fix this by moving
this patch to the end of the series.

> I think this needs to be done in a separate patch.

To avoid bisect issues, the clock-frequency property should be removed
in the same patch that adds the clocks property, or after it.

> Also, the UARTD enabling in init_table[] can then be removed.

You still need to initialize all the UART clocks in init_table[]. This
is required to make sure the clocks have the correct parent. Without
this explicit initialization, on Tegra30 at least, it was found that
UART C didn't work correctly for Bluetooth since the default parent was
something that wasn't generating the expected frequency.

Before removing the clock-frequency property, you also need to make sure
that init_table[] forces the UART clock on, otherwise since nothing will
clk_get()/enable() the UART clock, clk_disable_unused() will disable it.
After removing clock-frequency property, of_serial.c will
clk_get()/enable() the UART clock based on the clocks property, and
hence init_table[] doesn't need to force the UART clock on.
--
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
Peter De Schrijver - Feb. 14, 2013, 10:01 a.m.
On Wed, Feb 13, 2013 at 05:48:03PM +0100, Stephen Warren wrote:
> On 02/13/2013 04:38 AM, Peter De Schrijver wrote:
> > On Tue, Feb 12, 2013 at 06:24:05PM +0100, Stephen Warren wrote:
> >> On 02/11/2013 04:04 AM, Peter De Schrijver wrote:
> >>> Add references to tegra_car clocks for the basic device nodes.
> >>
> >> In this patch, you also need to remove the "clock-frequency" properties
> >> from tegra114-*.dts board files.
> > 
> > That would probably break bisectability no?
> 
> Oh right, I read the patch order backwards. You can fix this by moving
> this patch to the end of the series.
> 

Probably. Need to check if this doesn't cause other problems.

> > I think this needs to be done in a separate patch.
> 
> To avoid bisect issues, the clock-frequency property should be removed
> in the same patch that adds the clocks property, or after it.
> 
> > Also, the UARTD enabling in init_table[] can then be removed.
> 
> You still need to initialize all the UART clocks in init_table[]. This
> is required to make sure the clocks have the correct parent. Without
> this explicit initialization, on Tegra30 at least, it was found that
> UART C didn't work correctly for Bluetooth since the default parent was
> something that wasn't generating the expected frequency.
> 

Yes. The parent relationships still need to be defined. But I think that's
the only thing we actually need to define still? Also the parent relationships
can be board specific I think in some cases, so maybe we want to move those to
DT as well at some point?

Cheers,

Peter.
--
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 - Feb. 14, 2013, 5:30 p.m.
On 02/14/2013 03:01 AM, Peter De Schrijver wrote:
> On Wed, Feb 13, 2013 at 05:48:03PM +0100, Stephen Warren wrote:
...
>> You still need to initialize all the UART clocks in init_table[]. This
...
> Yes. The parent relationships still need to be defined. But I think that's
> the only thing we actually need to define still?

You might want to explicitly set the rate too, if there is a divider in
the clk module that affects it. If not, then parenting is indeed all you
need.

> Also the parent relationships
> can be board specific I think in some cases, so maybe we want to move those to
> DT as well at some point?

Yes, that's the plan. I think/thought Prashant was planning to work on that.
--
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
Peter De Schrijver - Feb. 15, 2013, 12:22 p.m.
On Thu, Feb 14, 2013 at 06:30:39PM +0100, Stephen Warren wrote:
> On 02/14/2013 03:01 AM, Peter De Schrijver wrote:
> > On Wed, Feb 13, 2013 at 05:48:03PM +0100, Stephen Warren wrote:
> ...
> >> You still need to initialize all the UART clocks in init_table[]. This
> ...
> > Yes. The parent relationships still need to be defined. But I think that's
> > the only thing we actually need to define still?
> 
> You might want to explicitly set the rate too, if there is a divider in
> the clk module that affects it. If not, then parenting is indeed all you
> need.

Yes, for PLLs that might be useful. Device clock rates can also be set by the
driver and probably should be set by the driver.

Cheers,

Peter.
--
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/tegra114.dtsi b/arch/arm/boot/dts/tegra114.dtsi
index 96a8235..6b0a01e 100644
--- a/arch/arm/boot/dts/tegra114.dtsi
+++ b/arch/arm/boot/dts/tegra114.dtsi
@@ -24,10 +24,11 @@ 
 			      0 42 0x04
 			      0 121 0x04
 			      0 122 0x04>;
+		clocks = <&tegra_car 5>;
 	};
 
 	tegra_car: clock {
-		compatible = "nvidia,tegra114-car, nvidia,tegra30-car";
+		compatible = "nvidia,tegra114-car";
 		reg = <0x60006000 0x1000>;
 		#clock-cells = <1>;
 	};
@@ -43,6 +44,7 @@ 
 		reg-shift = <2>;
 		interrupts = <0 36 0x04>;
 		status = "disabled";
+		clocks = <&tegra_car 6>;
 	};
 
 	serial@70006040 {
@@ -51,6 +53,7 @@ 
 		reg-shift = <2>;
 		interrupts = <0 37 0x04>;
 		status = "disabled";
+		clocks = <&tegra_car 192>;
 	};
 
 	serial@70006200 {
@@ -59,6 +62,7 @@ 
 		reg-shift = <2>;
 		interrupts = <0 46 0x04>;
 		status = "disabled";
+		clocks = <&tegra_car 55>;
 	};
 
 	serial@70006300 {
@@ -67,12 +71,14 @@ 
 		reg-shift = <2>;
 		interrupts = <0 90 0x04>;
 		status = "disabled";
+		clocks = <&tegra_car 65>;
 	};
 
 	rtc {
 		compatible = "nvidia,tegra114-rtc", "nvidia,tegra20-rtc";
 		reg = <0x7000e000 0x100>;
 		interrupts = <0 2 0x04>;
+		clocks = <&tegra_car 4>;
 	};
 
 	pmc {