OMAP4: Clock: Correct OTG clock to use otg_60m_gfclk.

PATCH New Ruslan Bilovol 3 participants 6 messages Jun 29, 2012 – Jul 4, 2012

Subsystems (4)

Arm (32-bit) Arm sub-architectures OMAP2+ support OMAP clock framework support

lore

Share & tools

lore

Ruslan Bilovol

author

2012-06-29 11:53 UTC #0 lore raw

PATCH 1/1 OMAP4: Clock: Correct OTG clock to use otg_60m_gfclk.

From: Wenbiao Wang <[email protected]>

OTG clock usb_otg_hs_ick used a incorrect parent l3_div_ck. Correct it to use the right colck otg_60m_gfclk as its parent.

Signed-off-by Wenbiao Wang <[email protected]>

Signed-off-by Ruslan Bilovol <[email protected]>

---
 arch/arm/mach-omap2/clock44xx_data.c |   15 ++++++++-------
 1 files changed, 8 insertions(+), 7 deletions(-)

arch/arm/mach-omap2/clock44xx_data.c +8 -7

index b825049..fd43214 100644
@@ -199,12 +199,6 @@ static struct clk tie_low_clock_ck = {
 	.ops		= &clkops_null,
 };
 
-static struct clk utmi_phy_clkout_ck = {
-	.name		= "utmi_phy_clkout_ck",
-	.rate		= 60000000,
-	.ops		= &clkops_null,
-};
-
 static struct clk xclk60mhsp1_ck = {
 	.name		= "xclk60mhsp1_ck",
 	.rate		= 60000000,

@@ -992,6 +986,13 @@ static struct clk dpll_usb_clkdcoldo_ck = {
 	.recalc		= &followparent_recalc,
 };
 
+static struct clk utmi_phy_clkout_ck = {
+	.name		= "utmi_phy_clkout_ck",
+	.ops		= &clkops_null,
+	.parent		= &dpll_usb_clkdcoldo_ck,
+	.recalc		= &followparent_recalc,
+};
+
 static const struct clksel dpll_usb_m2_div[] = {
 	{ .parent = &dpll_usb_ck, .rates = div31_1to31_rates },
 	{ .parent = NULL },

@@ -2685,7 +2686,7 @@ static struct clk usb_otg_hs_ick = {
 	.enable_reg	= OMAP4430_CM_L3INIT_USB_OTG_CLKCTRL,
 	.enable_bit	= OMAP4430_MODULEMODE_HWCTRL,
 	.clkdm_name	= "l3_init_clkdm",
-	.parent		= &l3_div_ck,
+	.parent		= &otg_60m_gfclk,
 	.recalc		= &followparent_recalc,
 };

Signature · 1 line

1.7.1

Paul Walmsley

maintainer

2012-06-29 20:35 UTC #1 lore raw

PATCH 1/1 OMAP4: Clock: Correct OTG clock to use otg_60m_gfclk.

+ Beno?t who is the maintainer of this file

+ the linux-arm-kernel mailing list, which should be cc'ed on all OMAP patches

Quoted 44 lines · Ruslan Bilovol

From: Wenbiao Wang <[email protected]>

OTG clock usb_otg_hs_ick used a incorrect parent l3_div_ck. Correct it to use the right colck otg_60m_gfclk as its parent.

Signed-off-by Wenbiao Wang <[email protected]>

Signed-off-by Ruslan Bilovol <[email protected]>

---
 arch/arm/mach-omap2/clock44xx_data.c |   15 ++++++++-------
 1 files changed, 8 insertions(+), 7 deletions(-)

arch/arm/mach-omap2/clock44xx_data.c +7 -6

index b825049..fd43214 100644
@@ -199,12 +199,6 @@ static struct clk tie_low_clock_ck = {
 	.ops		= &clkops_null,
 };
 
-static struct clk utmi_phy_clkout_ck = {
-	.name		= "utmi_phy_clkout_ck",
-	.rate		= 60000000,
-	.ops		= &clkops_null,
-};
-
 static struct clk xclk60mhsp1_ck = {
 	.name		= "xclk60mhsp1_ck",
 	.rate		= 60000000,

@@ -992,6 +986,13 @@ static struct clk dpll_usb_clkdcoldo_ck = {
 	.recalc		= &followparent_recalc,
 };
 
+static struct clk utmi_phy_clkout_ck = {
+	.name		= "utmi_phy_clkout_ck",
+	.ops		= &clkops_null,
+	.parent		= &dpll_usb_clkdcoldo_ck,
+	.recalc		= &followparent_recalc,
+};
+
 static const struct clksel dpll_usb_m2_div[] = {
 	{ .parent = &dpll_usb_ck, .rates = div31_1to31_rates },
 	{ .parent = NULL },

arch/arm/mach-omap2/clock44xx_data.c static struct clk usb_otg_hs_ick = { · line 2691

Quoted 7 lines

arch/arm/mach-omap2/clock44xx_data.c +1 -1

@@ -2685,7 +2686,7 @@ static struct clk usb_otg_hs_ick = {
 	.enable_reg	= OMAP4430_CM_L3INIT_USB_OTG_CLKCTRL,
 	.enable_bit	= OMAP4430_MODULEMODE_HWCTRL,
 	.clkdm_name	= "l3_init_clkdm",
-	.parent		= &l3_div_ck,
+	.parent		= &otg_60m_gfclk,
 	.recalc		= &followparent_recalc,
 };

Beno?t should have a look at this one, I think.

- Paul

Benoit Cousson

2012-07-02 12:43 UTC #2 lore raw

PATCH 1/1 OMAP4: Clock: Correct OTG clock to use otg_60m_gfclk.

Quoted 12 lines · Paul Walmsley

+ Beno?t who is the maintainer of this file
+ the linux-arm-kernel mailing list, which should be cc'ed on all OMAP patches
Quoted 5 lines · Ruslan Bilovol
From: Wenbiao Wang <[email protected]>
OTG clock usb_otg_hs_ick used a incorrect parent l3_div_ck. Correct it to use the right colck otg_60m_gfclk as its parent.

Mmm, that does not seems to be correct.

otg_60m_gfclk is an optional clock. The interface clock is the main clock of that module. That's why this is the parent of the fake MODULEMODE clock node.

Moreover you are changing as well the utmi_phy_clkout_ck. That's not mentioned at all in the changelog.

I know that there are some non standard stuff in this clock scheme. The main reason being the utmi_phy_clkout_ck source is generated from the usb_phy module. Unfortunately the clock fmwk cannot handle module as a clock node.

So, as of today, this only way to get the OTG_60M_FCLK clock available is to ensure that the usb_phy module is enabled before the usb_otg_hs module.

Regards,
Benoit

Quoted 52 lines

Quoted 39 lines

Signed-off-by Wenbiao Wang <[email protected]>

Signed-off-by Ruslan Bilovol <[email protected]>

---
  arch/arm/mach-omap2/clock44xx_data.c |   15 ++++++++-------
  1 files changed, 8 insertions(+), 7 deletions(-)

arch/arm/mach-omap2/clock44xx_data.c +7 -6

index b825049..fd43214 100644
@@ -199,12 +199,6 @@ static struct clk tie_low_clock_ck = {
  	.ops		= &clkops_null,
  };
 
-static struct clk utmi_phy_clkout_ck = {
-	.name		= "utmi_phy_clkout_ck",
-	.rate		= 60000000,
-	.ops		= &clkops_null,
-};
-
  static struct clk xclk60mhsp1_ck = {
  	.name		= "xclk60mhsp1_ck",
  	.rate		= 60000000,

@@ -992,6 +986,13 @@ static struct clk dpll_usb_clkdcoldo_ck = {
  	.recalc		= &followparent_recalc,
  };
 
+static struct clk utmi_phy_clkout_ck = {
+	.name		= "utmi_phy_clkout_ck",
+	.ops		= &clkops_null,
+	.parent		= &dpll_usb_clkdcoldo_ck,
+	.recalc		= &followparent_recalc,
+};
+
  static const struct clksel dpll_usb_m2_div[] = {
  	{ .parent = &dpll_usb_ck, .rates = div31_1to31_rates },
  	{ .parent = NULL },

arch/arm/mach-omap2/clock44xx_data.c static struct clk usb_otg_hs_ick = { · line 2691

Quoted 7 lines

arch/arm/mach-omap2/clock44xx_data.c +1 -1

@@ -2685,7 +2686,7 @@ static struct clk usb_otg_hs_ick = {
  	.enable_reg	= OMAP4430_CM_L3INIT_USB_OTG_CLKCTRL,
  	.enable_bit	= OMAP4430_MODULEMODE_HWCTRL,
  	.clkdm_name	= "l3_init_clkdm",
-	.parent		= &l3_div_ck,
+	.parent		= &otg_60m_gfclk,
  	.recalc		= &followparent_recalc,
  };

Beno?t should have a look at this one, I think.

- Paul

Ruslan Bilovol

author

2012-07-02 16:29 UTC #3 lore raw

PATCH 1/1 OMAP4: Clock: Correct OTG clock to use otg_60m_gfclk.

Thanks Benoit for the explanation. It seems you are right.
+ Wenbiao who is the original patch author that may have another point of view and some additional explanations.

--
Best regards,
Ruslan Bilovol

Quoted 93 lines · Benoit Cousson

Quoted 13 lines · Paul Walmsley
+ Beno?t who is the maintainer of this file
+ the linux-arm-kernel mailing list, which should be cc'ed on all OMAP patches
Quoted 5 lines · Ruslan Bilovol
From: Wenbiao Wang <[email protected]>
OTG clock usb_otg_hs_ick used a incorrect parent l3_div_ck. Correct it to use the right colck otg_60m_gfclk as its parent.
Mmm, that does not seems to be correct.
otg_60m_gfclk is an optional clock. The interface clock is the main clock of that module. That's why this is the parent of the fake MODULEMODE clock node.
Moreover you are changing as well the utmi_phy_clkout_ck. That's not mentioned at all in the changelog.
I know that there are some non standard stuff in this clock scheme. The main reason being the utmi_phy_clkout_ck source is generated from the usb_phy module. Unfortunately the clock fmwk cannot handle module as a clock node.
So, as of today, this only way to get the OTG_60M_FCLK clock available is to ensure that the usb_phy module is enabled before the usb_otg_hs module.
Regards,
Benoit
Quoted 54 lines
Quoted 40 lines
Signed-off-by Wenbiao Wang <[email protected]>
Signed-off-by Ruslan Bilovol <[email protected]>
---
  arch/arm/mach-omap2/clock44xx_data.c |   15 ++++++++-------
  1 files changed, 8 insertions(+), 7 deletions(-)
arch/arm/mach-omap2/clock44xx_data.c +7 -6
index b825049..fd43214 100644
@@ -199,12 +199,6 @@ static struct clk tie_low_clock_ck = {
        .ops            = &clkops_null,
  };
 
-static struct clk utmi_phy_clkout_ck = {
-       .name           = "utmi_phy_clkout_ck",
-       .rate           = 60000000,
-       .ops            = &clkops_null,
-};
-
  static struct clk xclk60mhsp1_ck = {
        .name           = "xclk60mhsp1_ck",
        .rate           = 60000000,
@@ -992,6 +986,13 @@ static struct clk dpll_usb_clkdcoldo_ck = {
        .recalc         = &followparent_recalc,
  };
 
+static struct clk utmi_phy_clkout_ck = {
+       .name           = "utmi_phy_clkout_ck",
+       .ops            = &clkops_null,
+       .parent         = &dpll_usb_clkdcoldo_ck,
+       .recalc         = &followparent_recalc,
+};
+
  static const struct clksel dpll_usb_m2_div[] = {
        { .parent = &dpll_usb_ck, .rates = div31_1to31_rates },
        { .parent = NULL },
arch/arm/mach-omap2/clock44xx_data.c static struct clk usb_otg_hs_ick = { · line 2691
Quoted 7 lines
arch/arm/mach-omap2/clock44xx_data.c +1 -1
@@ -2685,7 +2686,7 @@ static struct clk usb_otg_hs_ick = {
        .enable_reg     = OMAP4430_CM_L3INIT_USB_OTG_CLKCTRL,
        .enable_bit     = OMAP4430_MODULEMODE_HWCTRL,
        .clkdm_name     = "l3_init_clkdm",
-       .parent         = &l3_div_ck,
+       .parent         = &otg_60m_gfclk,
        .recalc         = &followparent_recalc,
  };
Beno?t should have a look at this one, I think.
- Paul

Paul Walmsley

maintainer

2012-07-04 09:10 UTC #4 lore raw

PATCH 1/1 OMAP4: Clock: Correct OTG clock to use otg_60m_gfclk.

In reply to Benoit Cousson

Quoted 1 line · Benoit Cousson

Unfortunately the clock fmwk cannot handle module as a clock node.

Hmm. We have to have similar support in the clockfw for the CLKDIV32K clock for AM33xx. That uses the modulemode bits to enable and disable the clock. Or does this require something more complicated?

- Paul

Benoit Cousson

2012-07-04 13:09 UTC #5 lore raw

PATCH 1/1 OMAP4: Clock: Correct OTG clock to use otg_60m_gfclk.

Hi Paul,

Quoted 7 lines · Paul Walmsley

Quoted 1 line · Benoit Cousson
Unfortunately the clock fmwk cannot handle module as a clock node.
Hmm. We have to have similar support in the clockfw for the CLKDIV32K clock for AM33xx. That uses the modulemode bits to enable and disable the clock. Or does this require something more complicated?

I don't think so, in that case, that should probably be enough.

But playing with the modulemode in the clock frmwk still looks like a hack to me.
Ideally it should be represented by a hwmod instead, but then it will not fit in the clock fmwk.

That being said, using an IP (internal or not) as a source clock should be supported.
That will allow us to handle the power dependency we have with Phoenix audio that is the source clock of the McPDM.

Regards,
Benoit