Skip to contentSkip to conversation

ARM: OMAP4: clock: fix mult and div mask for USB_DPLL

PATCH In review Ameya Palande 5 participants 10 messages Jan 18, 2012 – Jan 25, 2012

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ARM: OMAP4: clock: fix mult and div mask for USB_DPLL

---
 arch/arm/mach-omap2/clock44xx_data.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
arch/arm/mach-omap2/clock44xx_data.c +2 -2
index 08e86d7..053cc15 100644
@@ -953,8 +953,8 @@ static struct dpll_data dpll_usb_dd = {
 	.modes		= (1 << DPLL_LOW_POWER_BYPASS) | (1 << DPLL_LOCKED),
 	.autoidle_reg	= OMAP4430_CM_AUTOIDLE_DPLL_USB,
 	.idlest_reg	= OMAP4430_CM_IDLEST_DPLL_USB,
-	.mult_mask	= OMAP4430_DPLL_MULT_MASK,
-	.div1_mask	= OMAP4430_DPLL_DIV_MASK,
+	.mult_mask	= OMAP4430_DPLL_MULT_USB_MASK,
+	.div1_mask	= OMAP4430_DPLL_DIV_0_7_MASK,
 	.enable_mask	= OMAP4430_DPLL_EN_MASK,
 	.autoidle_mask	= OMAP4430_AUTO_DPLL_MODE_MASK,
 	.idlest_mask	= OMAP4430_ST_DPLL_CLK_MASK,
Signature · 1 line
1.7.4.1
#1 lore raw

Any update on this?

Quoted 22 lines · Ameya Palande

---
 arch/arm/mach-omap2/clock44xx_data.c |    4 ++--

 1 files changed, 2 insertions(+), 2 deletions(-)
arch/arm/mach-omap2/clock44xx_data.c +2 -3
index 08e86d7..053cc15 100644
@@ -953,8 +953,8 @@ static struct dpll_data dpll_usb_dd = {
        .modes          = (1 << DPLL_LOW_POWER_BYPASS) | (1 << DPLL_LOCKED),
        .autoidle_reg   = OMAP4430_CM_AUTOIDLE_DPLL_USB,
        .idlest_reg     = OMAP4430_CM_IDLEST_DPLL_USB,
-       .mult_mask      = OMAP4430_DPLL_MULT_MASK,
-       .div1_mask      = OMAP4430_DPLL_DIV_MASK,
+       .mult_mask      = OMAP4430_DPLL_MULT_USB_MASK,
+       .div1_mask      = OMAP4430_DPLL_DIV_0_7_MASK,
        .enable_mask    = OMAP4430_DPLL_EN_MASK,
        .autoidle_mask  = OMAP4430_AUTO_DPLL_MODE_MASK,
        .idlest_mask    = OMAP4430_ST_DPLL_CLK_MASK,
--
Quoted 2 lines

1.7.4.1

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

#2 lore raw
Quoted 1 line · Ameya

Any update on this?

A descriptive changelog would be helpful here, ideally with a TRM reference to the correct values such that a reviewer could verify this quickly.

Also, please Cc [email protected] for upstream patches.

Thanks,

Kevin

Quoted 21 lines · Ameya Palande
Quoted 20 lines

---
 arch/arm/mach-omap2/clock44xx_data.c |    4 ++--

 1 files changed, 2 insertions(+), 2 deletions(-)
arch/arm/mach-omap2/clock44xx_data.c +2 -2
index 08e86d7..053cc15 100644
@@ -953,8 +953,8 @@ static struct dpll_data dpll_usb_dd = {
        .modes          = (1 << DPLL_LOW_POWER_BYPASS) | (1 << DPLL_LOCKED),
        .autoidle_reg   = OMAP4430_CM_AUTOIDLE_DPLL_USB,
        .idlest_reg     = OMAP4430_CM_IDLEST_DPLL_USB,
-       .mult_mask      = OMAP4430_DPLL_MULT_MASK,
-       .div1_mask      = OMAP4430_DPLL_DIV_MASK,
+       .mult_mask      = OMAP4430_DPLL_MULT_USB_MASK,
+       .div1_mask      = OMAP4430_DPLL_DIV_0_7_MASK,
        .enable_mask    = OMAP4430_DPLL_EN_MASK,
        .autoidle_mask  = OMAP4430_AUTO_DPLL_MODE_MASK,
Quoted 8 lines
Quoted 4 lines
arch/arm/mach-omap2/clock44xx_data.c +0 -1
        .idlest_mask    = OMAP4430_ST_DPLL_CLK_MASK,
--

1.7.4.1

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

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

Acked-by on v1 1/1
#3 lore raw
In reply to Ameya Palande

+ Mike

Quoted 1 line · Ameya Palande

Assuming that you will re-send with Kevin's comment taken into account, the fix is indeed very valid.

Quoted 9 lines
---
  arch/arm/mach-omap2/clock44xx_data.c |    4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)
arch/arm/mach-omap2/clock44xx_data.c +0 -0
index 08e86d7..053cc15 100644

arch/arm/mach-omap2/clock44xx_data.c static struct dpll_data dpll_usb_dd = { · line 957

Quoted 7 lines
arch/arm/mach-omap2/clock44xx_data.c +2 -2
@@ -953,8 +953,8 @@ static struct dpll_data dpll_usb_dd = {
  	.modes		= (1<<  DPLL_LOW_POWER_BYPASS) | (1<<  DPLL_LOCKED),
  	.autoidle_reg	= OMAP4430_CM_AUTOIDLE_DPLL_USB,
  	.idlest_reg	= OMAP4430_CM_IDLEST_DPLL_USB,
-	.mult_mask	= OMAP4430_DPLL_MULT_MASK,
-	.div1_mask	= OMAP4430_DPLL_DIV_MASK,
+	.mult_mask	= OMAP4430_DPLL_MULT_USB_MASK,
+	.div1_mask	= OMAP4430_DPLL_DIV_0_7_MASK,

We were wrongly assuming that all DPLLs were using the same mask, which cannot be the case for the USB DPLL since that one has a higher max multiplier and max divider than the other ones (4095/256 instead of 2047/128).

For Mike and Paul,

It is indeed a bug in the clock generator script.

Regards,
Benoit

#4 lore raw

PATCH 1/1 ARM: OMAP4: clock: fix mult and div mask for USB_DPLL

According to OMAP4 TRM Table 3-1183, CM_CLKSEL_DPLL_USB register defines following fields for multiplication and division factors:

DPLL_MULT (bits 19:8) DPLL multiplier factor (2 to 4095) DPLL_DIV (bits 7:0) DPLL divider factor (0 to 255)

---
 arch/arm/mach-omap2/clock44xx_data.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
arch/arm/mach-omap2/clock44xx_data.c +2 -2
index 08e86d7..053cc15 100644
@@ -953,8 +953,8 @@ static struct dpll_data dpll_usb_dd = {
 	.modes		= (1 << DPLL_LOW_POWER_BYPASS) | (1 << DPLL_LOCKED),
 	.autoidle_reg	= OMAP4430_CM_AUTOIDLE_DPLL_USB,
 	.idlest_reg	= OMAP4430_CM_IDLEST_DPLL_USB,
-	.mult_mask	= OMAP4430_DPLL_MULT_MASK,
-	.div1_mask	= OMAP4430_DPLL_DIV_MASK,
+	.mult_mask	= OMAP4430_DPLL_MULT_USB_MASK,
+	.div1_mask	= OMAP4430_DPLL_DIV_0_7_MASK,
 	.enable_mask	= OMAP4430_DPLL_EN_MASK,
 	.autoidle_mask	= OMAP4430_AUTO_DPLL_MODE_MASK,
 	.idlest_mask	= OMAP4430_ST_DPLL_CLK_MASK,
Signature · 1 line
1.7.4.1
maintainer
#5 lore raw

PATCH 1/1 ARM: OMAP4: clock: fix mult and div mask for USB_DPLL

Quoted 8 lines · Ameya Palande

According to OMAP4 TRM Table 3-1183, CM_CLKSEL_DPLL_USB register defines following fields for multiplication and division factors:

DPLL_MULT (bits 19:8) DPLL multiplier factor (2 to 4095) DPLL_DIV (bits 7:0) DPLL divider factor (0 to 255)

Thanks, queued for 3.4.

- Paul

maintainer
#6 lore raw

PATCH 1/1 ARM: OMAP4: clock: fix mult and div mask for USB_DPLL

In reply to Ameya Palande

Hi

one important question first:

Quoted 8 lines · Ameya Palande

According to OMAP4 TRM Table 3-1183, CM_CLKSEL_DPLL_USB register defines following fields for multiplication and division factors:

DPLL_MULT (bits 19:8) DPLL multiplier factor (2 to 4095) DPLL_DIV (bits 7:0) DPLL divider factor (0 to 255)

Have the scripts been updated?

- Paul

#7 lore raw

PATCH 1/1 ARM: OMAP4: clock: fix mult and div mask for USB_DPLL

Hi Paul,

Quoted 9 lines · Paul Walmsley

Hi

one important question first:

Quoted 3 lines · Ameya Palande

According to OMAP4 TRM Table 3-1183, CM_CLKSEL_DPLL_USB register defines following fields for multiplication and division factors:

Quoted 7 lines
Quoted 5 lines

DPLL_MULT (bits 19:8) DPLL multiplier factor (2 to 4095) DPLL_DIV (bits 7:0) DPLL divider factor (0 to 255)

Have the scripts been updated?

No not yet. The easy fix I was thinking about is to replace this define by the hexa value of the mask. I can extract directly from the max mult and max div.

Regards,
Benoit

#8 lore raw
In reply to Cousson, Benoit
Quoted 37 lines · Benoit

+ Mike

Quoted 2 lines · Ameya Palande

Assuming that you will re-send with Kevin's comment taken into account, the fix is indeed very valid.

Quoted 10 lines

---
 arch/arm/mach-omap2/clock44xx_data.c |    4 ++--

 1 files changed, 2 insertions(+), 2 deletions(-)
arch/arm/mach-omap2/clock44xx_data.c +0 -0
index 08e86d7..053cc15 100644
@@ -953,8 +953,8 @@ static struct dpll_data dpll_usb_dd = {
Quoted 8 lines

       .modes          = (1<<  DPLL_LOW_POWER_BYPASS) | (1<< DPLL_LOCKED),
       .autoidle_reg   = OMAP4430_CM_AUTOIDLE_DPLL_USB, .idlest_reg     = OMAP4430_CM_IDLEST_DPLL_USB,
-       .mult_mask      = OMAP4430_DPLL_MULT_MASK,
-       .div1_mask      = OMAP4430_DPLL_DIV_MASK,
+       .mult_mask      = OMAP4430_DPLL_MULT_USB_MASK,
+       .div1_mask      = OMAP4430_DPLL_DIV_0_7_MASK,

We were wrongly assuming that all DPLLs were using the same mask, which cannot be the case for the USB DPLL since that one has a higher max multiplier and max divider than the other ones (4095/256 instead of 2047/128).

Quoted 3 lines

For Mike and Paul,

It is indeed a bug in the clock generator script.

This DPLL does have some quirks. Rajendra recently solved a bug where .clkdm was not set for this DPLL since it is in the L3INIT clkdm, not an always-on clkdm, so that must also be taken into account in the data generation.

Regards,
Mike

Quoted 3 lines

Regards,
Benoit

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

#9 lore raw

Hi Mike,

Quoted 38 lines · Mike
Quoted 37 lines · Benoit

+ Mike

Quoted 2 lines · Ameya Palande

Assuming that you will re-send with Kevin's comment taken into account, the fix is indeed very valid.

Quoted 10 lines

---
 arch/arm/mach-omap2/clock44xx_data.c |    4 ++--

 1 files changed, 2 insertions(+), 2 deletions(-)
arch/arm/mach-omap2/clock44xx_data.c +0 -0
index 08e86d7..053cc15 100644
@@ -953,8 +953,8 @@ static struct dpll_data dpll_usb_dd = {
Quoted 8 lines

       .modes          = (1<<  DPLL_LOW_POWER_BYPASS) | (1<< DPLL_LOCKED),
       .autoidle_reg   = OMAP4430_CM_AUTOIDLE_DPLL_USB, .idlest_reg     = OMAP4430_CM_IDLEST_DPLL_USB,
-       .mult_mask      = OMAP4430_DPLL_MULT_MASK,
-       .div1_mask      = OMAP4430_DPLL_DIV_MASK,
+       .mult_mask      = OMAP4430_DPLL_MULT_USB_MASK,
+       .div1_mask      = OMAP4430_DPLL_DIV_0_7_MASK,

We were wrongly assuming that all DPLLs were using the same mask, which cannot be the case for the USB DPLL since that one has a higher max multiplier and max divider than the other ones (4095/256 instead of 2047/128).

Quoted 8 lines
Quoted 3 lines

For Mike and Paul,

It is indeed a bug in the clock generator script.

This DPLL does have some quirks.  Rajendra recently solved a bug where .clkdm was not set for this DPLL since it is in the L3INIT clkdm, not an always-on clkdm, so that must also be taken into account in the data generation.

I already submitted that patch for review:
http://www.mail-archive.com/[email protected]/msg61444.html

Cheers,
Ameya.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html