Re: DM3730 vs 3630 DSS Cock dividers

[Tomi Valkeinen]

On 16/10/15 13:23, Tomi Valkeinen wrote:

> Ok, so the clock data in the DT and the data in the dss are in sync and
> according to the TRM. So the patch in this thread is not correct.
> 
> How about 3730? Are we sure the dpll clocks are the same for 3730 and 3630?
> 
> So both 3730 and 3630 are failing the same way?
> 
> Hmm, wait... Beagle-xm has DM3730, right? It was working fine for me
> (until the whole board died, and you can't get new ones anymore =().

Could someone with a failing board post the boot log and these from
debugfs for both with and without the patch applied:

clk/clk_summary
omapdss/clk
omapdss/dss
omapdss/dispc

 Tomi



signature.asc
Description: OpenPGP digital signature

Re: DM3730 vs 3630 DSS Cock dividers

[Tomi Valkeinen]

On 13/10/15 10:55, Tero Kristo wrote:

> TRMs are correct, 3630 does not have x2 multiplier after DPLL4.
> 
> In the clock data, dpll4_m4x2 path is an x1 multiplier on omap3630, it
> is easier to represent this in DT than completely remove the dpll4_*x2
> nodes for omap36xx. This is how it was already before the DT clocks.
> 
> Here is a copy paste from clock debug data on omap36xx where you see the
> rate for the whole dpll4_m4 path is 96MHz:
> 
>  dpll4_m4_ck  019600
>0 0
> dpll4_m4x2_mul_ck   019600
>  0 0
>dpll4_m4x2_ck   019600
> 0 0
>   dss1_alwon_fck_3430es2   04  
> 9600  0 0
> 
> And same on omap34xx (not sure why the clock rate is totally different
> here though, but you see the x2 applied):
> 
>  dpll4_m4_ck  01   21600
>0
> 0
> dpll4_m4x2_mul_ck   01   43200
> 0 0
>dpll4_m4x2_ck   01   43200
> 0
>  0

Ok, so the clock data in the DT and the data in the dss are in sync and
according to the TRM. So the patch in this thread is not correct.

How about 3730? Are we sure the dpll clocks are the same for 3730 and 3630?

So both 3730 and 3630 are failing the same way?

Hmm, wait... Beagle-xm has DM3730, right? It was working fine for me
(until the whole board died, and you can't get new ones anymore =().

 Tomi



signature.asc
Description: OpenPGP digital signature

Re: DM3730 vs 3630 DSS Cock dividers

[Tony Lindgren]

* Adam Ford  [151016 04:12]:
> I will do that this weekend.  I only have one DM3730 board but based on the
> patch and the dialog, I don't appear to be the only one.

OK great. My guess this same issue applies to all 36xx meaning 3630, 3703,
3715 and 3730. No idea where the regression crept in.

Regards,

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

Re: DM3730 vs 3630 DSS Cock dividers

[Tero Kristo]

On 10/12/2015 10:35 PM, Tony Lindgren wrote:

* Tomi Valkeinen  [151012 11:08]:


On 12.10.2015 19:00, Tony Lindgren wrote:

* Adam Ford  [151010 13:29]:

Tomi and Tony,

I am working on the LogicPD DM3730 Torpedo module.  If I try to use the
DSS, I get the same errors as mentioned in these previous messages found
here:

http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/255103.html

The patch is basicaly:


* >>  drivers/video/fbdev/omap2/dss/dss.c | 5 +++--

*>* >>  1 file changed, 3 insertions(+), 2 deletions(-)
*>* >>
*>* >> diff --git a/drivers/video/fbdev/omap2/dss/dss.c
b/drivers/video/fbdev/omap2/dss/dss.c
*>* >> index d55266c..ad6561f 100644
*>* >> --- a/drivers/video/fbdev/omap2/dss/dss.c
*>* >> +++ b/drivers/video/fbdev/omap2/dss/dss.c
*>* >> @@ -707,9 +707,10 @@ static const struct dss_features
omap34xx_dss_feats __initconst = {
*>* >>   .dpi_select_source  =   _dpi_select_source_omap2_omap3,
*>* >>  };
*>* >>
*>* >> +/* Supposedly 3630 can use div 32 mult 2, but that needs to be
rechecked */
*>* >>  static const struct dss_features omap3630_dss_feats __initconst = {
*>* >> - .fck_div_max=   32,
*>* >> - .dss_fck_multiplier =   1,
*>* >> + .fck_div_max=   16,
*>* >> + .dss_fck_multiplier =   2,
*>* >
*>* > These values tell about the clock hardware, they are not settings that
*>* > can be changed to change the clock. OMAP3630 has a fixed x2 multiplier
*>* > and a divider with maximum value of 16.
*>* >
*>* >  Tomi
*>* >
*>* >*


I don't see this mainstream yet, but the patch is from a while ago.
Do you guys know if this will make it into the kernel?  Without it, I
cannot the DM3730 to DSS to operate correctly.


AFAIK 37xx is same as 3630 and does not work properly without the patch
above as we've seen.


Well, the patch is definitely wrong for 3630, as 3630 has divider range from
1 to 32, as seen from the CM_CLKSEL_DSS register.


Yes something is wrong somewhere for sure.. What if it's .dss_fck_multiplier = 2
and .fck_div_max = 32?


I can't find the fixed x2 multiplier from the TRM, but looking at the .dts
files, 3630 DSS gets the clock from dpll4_m4x2_ck, so maybe it is there. Or
maybe the clocks in the .dts files are wrong, and the multplier in dss.c is
right.


Yes grepping for it we have it both for legacy and dts clocks:

$ git grep dpll4_m4x2_ck
Documentation/devicetree/bindings/clock/ti/gate.txt:clocks = 
<_m4x2_ck>;
arch/arm/boot/dts/omap3430es1-clocks.dtsi:  clocks = 
<_m4x2_ck>;
arch/arm/boot/dts/omap36xx-am35xx-omap3430es2plus-clocks.dtsi:  clocks = 
<_m4x2_ck>;
arch/arm/boot/dts/omap3xxx-clocks.dtsi: dpll4_m4x2_ck: dpll4_m4x2_ck {
drivers/clk/ti/clk-3xxx-legacy.c:static struct ti_clk_gate dpll4_m4x2_ck_data = 
{
drivers/clk/ti/clk-3xxx-legacy.c:static struct ti_clk dpll4_m4x2_ck = {
drivers/clk/ti/clk-3xxx-legacy.c:   .name = "dpll4_m4x2_ck",
drivers/clk/ti/clk-3xxx-legacy.c:   .data = _m4x2_ck_data,
drivers/clk/ti/clk-3xxx-legacy.c:   .parent = "dpll4_m4x2_ck",
drivers/clk/ti/clk-3xxx-legacy.c:   .parent = "dpll4_m4x2_ck",
drivers/clk/ti/clk-3xxx-legacy.c:   CLK(NULL, "dpll4_m4x2_ck", 
_m4x2_ck),
drivers/clk/ti/clk-3xxx.c:  DT_CLK(NULL, "dpll4_m4x2_ck", "dpll4_m4x2_ck"),


And looking at the TRM, "3.5.3.3.4 DPLL Clock Summary" hints strongly that
there is no x2 multiplier there, so it might be that the dts clock files are
not right.


Or maybe the TRM was copied from the 34xx and never updated?

Tero, any ideas?


TRMs are correct, 3630 does not have x2 multiplier after DPLL4.

In the clock data, dpll4_m4x2 path is an x1 multiplier on omap3630, it 
is easier to represent this in DT than completely remove the dpll4_*x2 
nodes for omap36xx. This is how it was already before the DT clocks.


Here is a copy paste from clock debug data on omap36xx where you see the 
rate for the whole dpll4_m4 path is 96MHz:


 dpll4_m4_ck  019600 
   0 0
dpll4_m4x2_mul_ck   019600 
 0 0
   dpll4_m4x2_ck   019600 
0 0
  dss1_alwon_fck_3430es2   04 
  9600  0 0


And same on omap34xx (not sure why the clock rate is totally different 
here though, but you see the x2 applied):


 dpll4_m4_ck  01   21600 
   0

0
dpll4_m4x2_mul_ck   01   43200 


0 0
   dpll4_m4x2_ck   01   43200 
0

 0


-Tero




Unfortunately I have no working omap3 devices to test this =(.


Should not cost you more than few tens of whatever units to get one :)

Anybody have a spare 37xx device with an LCD to donate for Tomi?

Regards,

Tony



--
To unsubscribe from this list: send the line "unsubscribe 

Re: DM3730 vs 3630 DSS Cock dividers

[Tony Lindgren]

* Adam Ford  [151010 13:29]:
> Tomi and Tony,
> 
> I am working on the LogicPD DM3730 Torpedo module.  If I try to use the
> DSS, I get the same errors as mentioned in these previous messages found
> here:
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/255103.html
> 
> The patch is basicaly:
> 
> >* >>  drivers/video/fbdev/omap2/dss/dss.c | 5 +++--
> *>* >>  1 file changed, 3 insertions(+), 2 deletions(-)
> *>* >>
> *>* >> diff --git a/drivers/video/fbdev/omap2/dss/dss.c
> b/drivers/video/fbdev/omap2/dss/dss.c
> *>* >> index d55266c..ad6561f 100644
> *>* >> --- a/drivers/video/fbdev/omap2/dss/dss.c
> *>* >> +++ b/drivers/video/fbdev/omap2/dss/dss.c
> *>* >> @@ -707,9 +707,10 @@ static const struct dss_features
> omap34xx_dss_feats __initconst = {
> *>* >>   .dpi_select_source  =   
> _dpi_select_source_omap2_omap3,
> *>* >>  };
> *>* >>
> *>* >> +/* Supposedly 3630 can use div 32 mult 2, but that needs to be
> rechecked */
> *>* >>  static const struct dss_features omap3630_dss_feats __initconst = {
> *>* >> - .fck_div_max=   32,
> *>* >> - .dss_fck_multiplier =   1,
> *>* >> + .fck_div_max=   16,
> *>* >> + .dss_fck_multiplier =   2,
> *>* >
> *>* > These values tell about the clock hardware, they are not settings that
> *>* > can be changed to change the clock. OMAP3630 has a fixed x2 multiplier
> *>* > and a divider with maximum value of 16.
> *>* >
> *>* >  Tomi
> *>* >
> *>* >*
> 
> 
> I don't see this mainstream yet, but the patch is from a while ago.
> Do you guys know if this will make it into the kernel?  Without it, I
> cannot the DM3730 to DSS to operate correctly.

AFAIK 37xx is same as 3630 and does not work properly without the patch
above as we've seen.

Regards,

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

Re: DM3730 vs 3630 DSS Cock dividers

[Tomi Valkeinen]

On 12.10.2015 19:00, Tony Lindgren wrote:

* Adam Ford  [151010 13:29]:

Tomi and Tony,

I am working on the LogicPD DM3730 Torpedo module.  If I try to use the
DSS, I get the same errors as mentioned in these previous messages found
here:

http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/255103.html

The patch is basicaly:


* >>  drivers/video/fbdev/omap2/dss/dss.c | 5 +++--

*>* >>  1 file changed, 3 insertions(+), 2 deletions(-)
*>* >>
*>* >> diff --git a/drivers/video/fbdev/omap2/dss/dss.c
b/drivers/video/fbdev/omap2/dss/dss.c
*>* >> index d55266c..ad6561f 100644
*>* >> --- a/drivers/video/fbdev/omap2/dss/dss.c
*>* >> +++ b/drivers/video/fbdev/omap2/dss/dss.c
*>* >> @@ -707,9 +707,10 @@ static const struct dss_features
omap34xx_dss_feats __initconst = {
*>* >>   .dpi_select_source  =   _dpi_select_source_omap2_omap3,
*>* >>  };
*>* >>
*>* >> +/* Supposedly 3630 can use div 32 mult 2, but that needs to be
rechecked */
*>* >>  static const struct dss_features omap3630_dss_feats __initconst = {
*>* >> - .fck_div_max=   32,
*>* >> - .dss_fck_multiplier =   1,
*>* >> + .fck_div_max=   16,
*>* >> + .dss_fck_multiplier =   2,
*>* >
*>* > These values tell about the clock hardware, they are not settings that
*>* > can be changed to change the clock. OMAP3630 has a fixed x2 multiplier
*>* > and a divider with maximum value of 16.
*>* >
*>* >  Tomi
*>* >
*>* >*


I don't see this mainstream yet, but the patch is from a while ago.
Do you guys know if this will make it into the kernel?  Without it, I
cannot the DM3730 to DSS to operate correctly.


AFAIK 37xx is same as 3630 and does not work properly without the patch
above as we've seen.


Well, the patch is definitely wrong for 3630, as 3630 has divider range 
from 1 to 32, as seen from the CM_CLKSEL_DSS register.


I can't find the fixed x2 multiplier from the TRM, but looking at the 
.dts files, 3630 DSS gets the clock from dpll4_m4x2_ck, so maybe it is 
there. Or maybe the clocks in the .dts files are wrong, and the 
multplier in dss.c is right.


And looking at the TRM, "3.5.3.3.4 DPLL Clock Summary" hints strongly 
that there is no x2 multiplier there, so it might be that the dts clock 
files are not right.


Unfortunately I have no working omap3 devices to test this =(.

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

Re: DM3730 vs 3630 DSS Cock dividers

[Tony Lindgren]

* Tomi Valkeinen  [151012 11:08]:
> 
> On 12.10.2015 19:00, Tony Lindgren wrote:
> >* Adam Ford  [151010 13:29]:
> >>Tomi and Tony,
> >>
> >>I am working on the LogicPD DM3730 Torpedo module.  If I try to use the
> >>DSS, I get the same errors as mentioned in these previous messages found
> >>here:
> >>
> >>http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/255103.html
> >>
> >>The patch is basicaly:
> >>
> >>>* >>  drivers/video/fbdev/omap2/dss/dss.c | 5 +++--
> >>*>* >>  1 file changed, 3 insertions(+), 2 deletions(-)
> >>*>* >>
> >>*>* >> diff --git a/drivers/video/fbdev/omap2/dss/dss.c
> >>b/drivers/video/fbdev/omap2/dss/dss.c
> >>*>* >> index d55266c..ad6561f 100644
> >>*>* >> --- a/drivers/video/fbdev/omap2/dss/dss.c
> >>*>* >> +++ b/drivers/video/fbdev/omap2/dss/dss.c
> >>*>* >> @@ -707,9 +707,10 @@ static const struct dss_features
> >>omap34xx_dss_feats __initconst = {
> >>*>* >>   .dpi_select_source  =   
> >>_dpi_select_source_omap2_omap3,
> >>*>* >>  };
> >>*>* >>
> >>*>* >> +/* Supposedly 3630 can use div 32 mult 2, but that needs to be
> >>rechecked */
> >>*>* >>  static const struct dss_features omap3630_dss_feats __initconst = {
> >>*>* >> - .fck_div_max=   32,
> >>*>* >> - .dss_fck_multiplier =   1,
> >>*>* >> + .fck_div_max=   16,
> >>*>* >> + .dss_fck_multiplier =   2,
> >>*>* >
> >>*>* > These values tell about the clock hardware, they are not settings that
> >>*>* > can be changed to change the clock. OMAP3630 has a fixed x2 multiplier
> >>*>* > and a divider with maximum value of 16.
> >>*>* >
> >>*>* >  Tomi
> >>*>* >
> >>*>* >*
> >>
> >>
> >>I don't see this mainstream yet, but the patch is from a while ago.
> >>Do you guys know if this will make it into the kernel?  Without it, I
> >>cannot the DM3730 to DSS to operate correctly.
> >
> >AFAIK 37xx is same as 3630 and does not work properly without the patch
> >above as we've seen.
>
> Well, the patch is definitely wrong for 3630, as 3630 has divider range from
> 1 to 32, as seen from the CM_CLKSEL_DSS register.

Yes something is wrong somewhere for sure.. What if it's .dss_fck_multiplier = 2
and .fck_div_max = 32?

> I can't find the fixed x2 multiplier from the TRM, but looking at the .dts
> files, 3630 DSS gets the clock from dpll4_m4x2_ck, so maybe it is there. Or
> maybe the clocks in the .dts files are wrong, and the multplier in dss.c is
> right.

Yes grepping for it we have it both for legacy and dts clocks:

$ git grep dpll4_m4x2_ck
Documentation/devicetree/bindings/clock/ti/gate.txt:clocks = 
<_m4x2_ck>;
arch/arm/boot/dts/omap3430es1-clocks.dtsi:  clocks = 
<_m4x2_ck>;
arch/arm/boot/dts/omap36xx-am35xx-omap3430es2plus-clocks.dtsi:  clocks 
= <_m4x2_ck>;
arch/arm/boot/dts/omap3xxx-clocks.dtsi: dpll4_m4x2_ck: dpll4_m4x2_ck {
drivers/clk/ti/clk-3xxx-legacy.c:static struct ti_clk_gate dpll4_m4x2_ck_data = 
{
drivers/clk/ti/clk-3xxx-legacy.c:static struct ti_clk dpll4_m4x2_ck = {
drivers/clk/ti/clk-3xxx-legacy.c:   .name = "dpll4_m4x2_ck",
drivers/clk/ti/clk-3xxx-legacy.c:   .data = _m4x2_ck_data,
drivers/clk/ti/clk-3xxx-legacy.c:   .parent = "dpll4_m4x2_ck",
drivers/clk/ti/clk-3xxx-legacy.c:   .parent = "dpll4_m4x2_ck",
drivers/clk/ti/clk-3xxx-legacy.c:   CLK(NULL, "dpll4_m4x2_ck", 
_m4x2_ck),
drivers/clk/ti/clk-3xxx.c:  DT_CLK(NULL, "dpll4_m4x2_ck", "dpll4_m4x2_ck"),

> And looking at the TRM, "3.5.3.3.4 DPLL Clock Summary" hints strongly that
> there is no x2 multiplier there, so it might be that the dts clock files are
> not right.

Or maybe the TRM was copied from the 34xx and never updated?

Tero, any ideas?

> Unfortunately I have no working omap3 devices to test this =(.

Should not cost you more than few tens of whatever units to get one :)

Anybody have a spare 37xx device with an LCD to donate for Tomi?

Regards,

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