diff mbox

OMAP: clock DT conversion issues with omap36xx

Message ID 1390994505.5023.32.camel@mars (mailing list archive)
State New, archived
Headers show

Commit Message

Christoph Fritz Jan. 29, 2014, 11:21 a.m. UTC
On Tue, 2014-01-28 at 18:02 +0100, Christoph Fritz wrote:
> On Tue, 2014-01-28 at 15:40 +0200, Tero Kristo wrote:
> > 
> > >> Due to a regression since next-20140122 the following errors are present:
> > >>
> > >>   - pin sys_clkout2, which gets configured to 24 Mhz by the fourth patch
> > >>     in this set, erroneously outputs only 12 Mhz.
> > >>     Just out of curiosity, configuring it to 48 Mhz puts out desired 24 Mhz.
> > >>
> > >>   - omap_dss, which gets configured by the third patch in this set, fails
> > >>     to do 'dss_set_fck_rate(fck);' in
> > >>     drivers/video/omap2/dss/dss.c:dss_setup_default_clock() which leads to:
> > >>
> > >>      | omapdss_dss: probe of omapdss_dss failed with error -22
> > >>      | omapdss CORE error: Failed to initialize DSS platform driver
> > >>      | panel-dpi panel-dpi.0: failed to find video source 'dpi.0
> > >>
> > >>    Both regressions seem to have something to do with the clock framework.
> > >>    Could this be related to the DT clock conversion patches?
> > >
> > 
> > Yea its definitely possible, as the clock DT conversion touches pretty 
> > much everything. Have you tried whether this works properly with legacy 
> > boot? Personally I don't have access to any omap3 devices that would 
> > have display and have no possibility to check this out myself. Anyway, 
> > my initial guess is that some clock divider setup might be wrong with 
> > omap3, or we are missing some ti,set-rate-parent flag for some clock 
> > node which prevents escalating clk_set_rate properly. However, it should 
> > be easy to debug this by looking at the clock node in question, and its 
> > parent nodes to see if there are any problems.
> 
> Currently I only analyzed sys_clkout2 (see attachments for full
> clk_summary files):
> 
> clk_summary__next-20140115__works_as_expected:
>              dpll4_m2_ck        1           1            96000000
>                 dpll4_m2x2_ck   1           1            96000000
>                    omap_192m_alwon_fck 1           1            96000000
>                       omap_96m_alwon_fck 1           2            96000000
>                          per_96m_fck 0           6            96000000
>                             mcbsp4_fck 0           1            96000000
>                             mcbsp3_fck 0           2            96000000
>                             mcbsp2_fck 0           2            96000000
>                          cm_96m_fck 2           3            96000000
>                             clkout2_src_ck 1           1            96000000
>                                sys_clkout2 1           1            24000000
> 
> For real, on pin sys_clkout2 are correctly 24 Mhz measured.
> 
> clk_summary__next-20140124__sysclkout2_dss_fails:
>              dpll4_m2_ck        1           1            96000000
>                 dpll4_m2x2_mul_ck 1           1            192000000
>                    dpll4_m2x2_ck 1           1            192000000
>                       omap_192m_alwon_fck 0           0            192000000
>                       omap_96m_alwon_fck 1           2            192000000
>                          per_96m_fck 0           6            192000000
>                             mcbsp4_fck 0           1            192000000
>                             mcbsp3_fck 0           2            192000000
>                             mcbsp2_fck 0           2            192000000
>                          cm_96m_fck 2           3            192000000
>                             clkout2_src_ck 1           1            192000000
>                                sys_clkout2 1           1            24000000
> 
> For real, on pin sys_clkout2 are only ~12 Mhz measured.
> 
> So I added this patch:
> 
> Subject: [PATCH] ARM: dts: fix omap3 clock multiplier for dpll4_m2x2_ck
> 
> Before DT clock conversion, there was no multiplier for dpll4_m2x2_ck.
> So to be compatible again, set dpll4_m2x2_mul_ck multiplier back to 1.
> ---
>  arch/arm/boot/dts/omap3xxx-clocks.dtsi |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/omap3xxx-clocks.dtsi b/arch/arm/boot/dts/omap3xxx-clocks.dtsi
> index cb04d4b..b594050 100644
> --- a/arch/arm/boot/dts/omap3xxx-clocks.dtsi
> +++ b/arch/arm/boot/dts/omap3xxx-clocks.dtsi
> @@ -212,7 +212,7 @@
>  		#clock-cells = <0>;
>  		compatible = "fixed-factor-clock";
>  		clocks = <&dpll4_m2_ck>;
> -		clock-mult = <2>;
> +		clock-mult = <1>;
>  		clock-div = <1>;
>  	};
>
> And it works again. But due to the fact that sys_clkout2 was at 12 Mhz
> instead of 24, shouldn't it have been at 48 caused by "clock-mult = 2 ?

Any ideas on that?

I reverted the patch above and added:

From: Tero Kristo <t-kristo@ti.com>
Date: Wed, 29 Jan 2014 11:03:46 +0200
Subject: [PATCH] ARM: dts: omap36xx: fix omap96m_alwon_fck

OMAP36xx has different hardware implementation for the omap96m_alwon_fck
compared to other OMAP3 variants. Reflect this properly in the dts file.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
  arch/arm/boot/dts/omap36xx-clocks.dtsi |    9 +++++++++
  1 file changed, 9 insertions(+)


But the output of sys_clkout2 is still wrong.

clk_summary__next-20140124__patch_tero_fix_omap96m_alwon_fck
             dpll4_m2_ck        1           1            96000000
                dpll4_m2x2_mul_ck 1           1            192000000
                   dpll4_m2x2_ck 1           1            192000000
                      omap_192m_alwon_fck 1           1            192000000
                         omap_96m_alwon_fck 1           2            192000000
                            per_96m_fck 0           6            192000000
                               mcbsp4_fck 0           1            192000000
                               mcbsp3_fck 0           2            192000000
                               mcbsp2_fck 0           2            192000000
                            cm_96m_fck 2           3            192000000
                               clkout2_src_ck 1           1            192000000
                                  sys_clkout2 1           1            24000000

Thanks
  -- Christoph

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Tero Kristo Jan. 29, 2014, 2:57 p.m. UTC | #1
On 01/29/2014 01:21 PM, Christoph Fritz wrote:
> On Tue, 2014-01-28 at 18:02 +0100, Christoph Fritz wrote:
>> On Tue, 2014-01-28 at 15:40 +0200, Tero Kristo wrote:
>>>
>>>>> Due to a regression since next-20140122 the following errors are present:
>>>>>
>>>>>    - pin sys_clkout2, which gets configured to 24 Mhz by the fourth patch
>>>>>      in this set, erroneously outputs only 12 Mhz.
>>>>>      Just out of curiosity, configuring it to 48 Mhz puts out desired 24 Mhz.
>>>>>
>>>>>    - omap_dss, which gets configured by the third patch in this set, fails
>>>>>      to do 'dss_set_fck_rate(fck);' in
>>>>>      drivers/video/omap2/dss/dss.c:dss_setup_default_clock() which leads to:
>>>>>
>>>>>       | omapdss_dss: probe of omapdss_dss failed with error -22
>>>>>       | omapdss CORE error: Failed to initialize DSS platform driver
>>>>>       | panel-dpi panel-dpi.0: failed to find video source 'dpi.0
>>>>>
>>>>>     Both regressions seem to have something to do with the clock framework.
>>>>>     Could this be related to the DT clock conversion patches?
>>>>
>>>
>>> Yea its definitely possible, as the clock DT conversion touches pretty
>>> much everything. Have you tried whether this works properly with legacy
>>> boot? Personally I don't have access to any omap3 devices that would
>>> have display and have no possibility to check this out myself. Anyway,
>>> my initial guess is that some clock divider setup might be wrong with
>>> omap3, or we are missing some ti,set-rate-parent flag for some clock
>>> node which prevents escalating clk_set_rate properly. However, it should
>>> be easy to debug this by looking at the clock node in question, and its
>>> parent nodes to see if there are any problems.
>>
>> Currently I only analyzed sys_clkout2 (see attachments for full
>> clk_summary files):
>>
>> clk_summary__next-20140115__works_as_expected:
>>               dpll4_m2_ck        1           1            96000000
>>                  dpll4_m2x2_ck   1           1            96000000
>>                     omap_192m_alwon_fck 1           1            96000000
>>                        omap_96m_alwon_fck 1           2            96000000
>>                           per_96m_fck 0           6            96000000
>>                              mcbsp4_fck 0           1            96000000
>>                              mcbsp3_fck 0           2            96000000
>>                              mcbsp2_fck 0           2            96000000
>>                           cm_96m_fck 2           3            96000000
>>                              clkout2_src_ck 1           1            96000000
>>                                 sys_clkout2 1           1            24000000
>>
>> For real, on pin sys_clkout2 are correctly 24 Mhz measured.
>>
>> clk_summary__next-20140124__sysclkout2_dss_fails:
>>               dpll4_m2_ck        1           1            96000000
>>                  dpll4_m2x2_mul_ck 1           1            192000000
>>                     dpll4_m2x2_ck 1           1            192000000
>>                        omap_192m_alwon_fck 0           0            192000000
>>                        omap_96m_alwon_fck 1           2            192000000
>>                           per_96m_fck 0           6            192000000
>>                              mcbsp4_fck 0           1            192000000
>>                              mcbsp3_fck 0           2            192000000
>>                              mcbsp2_fck 0           2            192000000
>>                           cm_96m_fck 2           3            192000000
>>                              clkout2_src_ck 1           1            192000000
>>                                 sys_clkout2 1           1            24000000
>>
>> For real, on pin sys_clkout2 are only ~12 Mhz measured.
>>
>> So I added this patch:
>>
>> Subject: [PATCH] ARM: dts: fix omap3 clock multiplier for dpll4_m2x2_ck
>>
>> Before DT clock conversion, there was no multiplier for dpll4_m2x2_ck.
>> So to be compatible again, set dpll4_m2x2_mul_ck multiplier back to 1.
>> ---
>>   arch/arm/boot/dts/omap3xxx-clocks.dtsi |    2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/omap3xxx-clocks.dtsi b/arch/arm/boot/dts/omap3xxx-clocks.dtsi
>> index cb04d4b..b594050 100644
>> --- a/arch/arm/boot/dts/omap3xxx-clocks.dtsi
>> +++ b/arch/arm/boot/dts/omap3xxx-clocks.dtsi
>> @@ -212,7 +212,7 @@
>>   		#clock-cells = <0>;
>>   		compatible = "fixed-factor-clock";
>>   		clocks = <&dpll4_m2_ck>;
>> -		clock-mult = <2>;
>> +		clock-mult = <1>;
>>   		clock-div = <1>;
>>   	};
>>
>> And it works again. But due to the fact that sys_clkout2 was at 12 Mhz
>> instead of 24, shouldn't it have been at 48 caused by "clock-mult = 2 ?
>
> Any ideas on that?
>
> I reverted the patch above and added:

Hmm, well, the omap96m_alwon_fck rate is still wrong. Try adding 
ti,min-div = <2>; and ti,max-div = <4>; to properties of the clock node.

-Tero

>
> From: Tero Kristo <t-kristo@ti.com>
> Date: Wed, 29 Jan 2014 11:03:46 +0200
> Subject: [PATCH] ARM: dts: omap36xx: fix omap96m_alwon_fck
>
> OMAP36xx has different hardware implementation for the omap96m_alwon_fck
> compared to other OMAP3 variants. Reflect this properly in the dts file.
>
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> ---
>    arch/arm/boot/dts/omap36xx-clocks.dtsi |    9 +++++++++
>    1 file changed, 9 insertions(+)
>
> diff --git a/arch/arm/boot/dts/omap36xx-clocks.dtsi
> b/arch/arm/boot/dts/omap36xx-clocks.dtsi
> index 2fcf253..24ddf3f 100644
> --- a/arch/arm/boot/dts/omap36xx-clocks.dtsi
> +++ b/arch/arm/boot/dts/omap36xx-clocks.dtsi
> @@ -88,3 +88,12 @@
>    			 <&mcbsp4_ick>, <&uart4_fck>;
>    	};
>    };
> +
> +&omap_96m_alwon_fck {
> +	compatible = "ti,divider-clock";
> +	clocks = <&omap_192m_alwon_fck>;
> +	ti,bit-shift = <12>;
> +	ti,max-div = <2>;
> +	reg = <0x0a40>;
> +	ti,index-starts-at-one;
> +};
>
> But the output of sys_clkout2 is still wrong.
>
> clk_summary__next-20140124__patch_tero_fix_omap96m_alwon_fck
>               dpll4_m2_ck        1           1            96000000
>                  dpll4_m2x2_mul_ck 1           1            192000000
>                     dpll4_m2x2_ck 1           1            192000000
>                        omap_192m_alwon_fck 1           1            192000000
>                           omap_96m_alwon_fck 1           2            192000000
>                              per_96m_fck 0           6            192000000
>                                 mcbsp4_fck 0           1            192000000
>                                 mcbsp3_fck 0           2            192000000
>                                 mcbsp2_fck 0           2            192000000
>                              cm_96m_fck 2           3            192000000
>                                 clkout2_src_ck 1           1            192000000
>                                    sys_clkout2 1           1            24000000
>
> Thanks
>    -- Christoph
>

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nishanth Menon Jan. 29, 2014, 7:03 p.m. UTC | #2
On 01/29/2014 05:21 AM, Christoph Fritz wrote:
> On Tue, 2014-01-28 at 18:02 +0100, Christoph Fritz wrote:
>> On Tue, 2014-01-28 at 15:40 +0200, Tero Kristo wrote:
>>>
>>>>> Due to a regression since next-20140122 the following errors are present:
>>>>>
>>>>>   - pin sys_clkout2, which gets configured to 24 Mhz by the fourth patch
>>>>>     in this set, erroneously outputs only 12 Mhz.
>>>>>     Just out of curiosity, configuring it to 48 Mhz puts out desired 24 Mhz.
>>>>>
>>>>>   - omap_dss, which gets configured by the third patch in this set, fails
>>>>>     to do 'dss_set_fck_rate(fck);' in
>>>>>     drivers/video/omap2/dss/dss.c:dss_setup_default_clock() which leads to:
>>>>>
>>>>>      | omapdss_dss: probe of omapdss_dss failed with error -22
>>>>>      | omapdss CORE error: Failed to initialize DSS platform driver
>>>>>      | panel-dpi panel-dpi.0: failed to find video source 'dpi.0
>>>>>
>>>>>    Both regressions seem to have something to do with the clock framework.
>>>>>    Could this be related to the DT clock conversion patches?
>>>>
>>>
>>> Yea its definitely possible, as the clock DT conversion touches pretty 
>>> much everything. Have you tried whether this works properly with legacy 
>>> boot? Personally I don't have access to any omap3 devices that would 
>>> have display and have no possibility to check this out myself. Anyway, 
>>> my initial guess is that some clock divider setup might be wrong with 
>>> omap3, or we are missing some ti,set-rate-parent flag for some clock 
>>> node which prevents escalating clk_set_rate properly. However, it should 
>>> be easy to debug this by looking at the clock node in question, and its 
>>> parent nodes to see if there are any problems.
>>
>> Currently I only analyzed sys_clkout2 (see attachments for full
>> clk_summary files):

To help us debug similar problems, I wrote a tool today:
https://github.com/nmenon/ctt-dump - it is a simple memory read utility,

Input file is CTT dump-out
For example: 3630 CTT is here:
http://www.ti.com/pdfs/wtbu/CTT-OMAP3630ES1.x-v1.6.0.4.zip

to give an idea - i posted a screen shot here:
https://plus.google.com/112464029509057661457/posts/hNdee4gNfob

After generating the the rd1 file from CTT,
we pick up the registers using ctt-dump -> any tool which can do
register reads could do, but it might be handy having this.
Example output on beagle-xm: http://slexy.org/view/s2YWmM1ium
importing it back into CTT and after setting up the correct sysclk, we
can compare clock frequencies Vs debugfs output - example:
http://slexy.org/view/s21iQyDTct


I mean, it is awesome having to debugfs data, but with nascent
systems, it is always good to compare to what the hardware is really
configured to - and CTT is the easy way to deal with it.
Christoph Fritz Feb. 1, 2014, 6:55 p.m. UTC | #3
On Wed, Jan 29, 2014 at 04:57:00PM +0200, Tero Kristo wrote:
> 
> Hmm, well, the omap96m_alwon_fck rate is still wrong. Try adding
> ti,min-div = <2>; and ti,max-div = <4>; to properties of the clock
> node.

Hmm, doesn't change anything here.

Thanks
  -- Christoph
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/boot/dts/omap36xx-clocks.dtsi 
b/arch/arm/boot/dts/omap36xx-clocks.dtsi
index 2fcf253..24ddf3f 100644
--- a/arch/arm/boot/dts/omap36xx-clocks.dtsi
+++ b/arch/arm/boot/dts/omap36xx-clocks.dtsi
@@ -88,3 +88,12 @@ 
  			 <&mcbsp4_ick>, <&uart4_fck>;
  	};
  };
+
+&omap_96m_alwon_fck {
+	compatible = "ti,divider-clock";
+	clocks = <&omap_192m_alwon_fck>;
+	ti,bit-shift = <12>;
+	ti,max-div = <2>;
+	reg = <0x0a40>;
+	ti,index-starts-at-one;
+};