| Message ID | 1360580700-10245-9-git-send-email-pdeschrijver@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.
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.
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.
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.
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.
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.
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 {
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(-)