RE: [PATCHV2 1/4] OMAP3: introduce DPLL4 Jtype
-Original Message- From: Paul Walmsley [mailto:p...@pwsan.com] Sent: Tuesday, December 15, 2009 12:38 PM To: Cousson, Benoit Cc: Sripathy, Vishwanath; Nayak, Rajendra; ext-ari.kau...@nokia.com; linux- o...@vger.kernel.org; Woodruff, Richard; Menon, Nishanth Subject: RE: [PATCHV2 1/4] OMAP3: introduce DPLL4 Jtype Hi Benoît, Vishwanath, Thanks for the info, Benoît. On Fri, 11 Dec 2009, Cousson, Benoit wrote: From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap- ow...@vger.kernel.org] On Behalf Of Paul Walmsley Sent: Thursday, December 10, 2009 10:24 PM Hi Vishwanath, On Tue, 1 Dec 2009, Sripathy, Vishwanath wrote: Texas Instruments France SA, 821 Avenue Jack Kilby, 06270 Villeneuve Loubet. 036 420 040 R.C.S Antibes. Capital de EUR 753.920 -Original Message- From: Paul Walmsley [mailto:p...@pwsan.com] Sent: Monday, November 30, 2009 3:00 PM To: Sripathy, Vishwanath Cc: ext-ari.kau...@nokia.com; linux-omap@vger.kernel.org; Woodruff, Richard; Menon, Nishanth Subject: Re: [PATCHV2 1/4] OMAP3: introduce DPLL4 Jtype Hello Vishwanath, a few comments: On Thu, 26 Nov 2009, Vishwanath BS wrote: From: Richard Woodruff r-woodru...@ti.com DPLL4 for 3630 introduces a changed block requiring special divisor bits and additional reg fields. To allow for silicons to use this, this is introduced as a omap3_has_jtype_dpll4() and is enabled for 3630 silicon Tested with 3630 ZOOM3 Could you please consider Ari Kauppi's suggestions that he posted on 30 and 31 October? They look good to me. Here is a link: Regarding comments from Ari Kauppi on addition of dco_sel_mask and sd_div_mask, yes I had a look at them. However I feel, it's better to have these masks in the structure variable. Reason being this approach will help us to support when dpll types for other dplls get changed in future. For example, if dpll3 block is changed, then having new mask will help us to support it. Is there a product coming out that will add more j-type DPLLs with different dco_sel_masks and sd_div_masks? If not, then let's go with Ari's suggested changes, since we don't really know if any more of these j-type DPLLs will be used. We can always add the fields back in when we need them. But if you know for sure that there is a chip variant coming out soon that will use more j-type DPLLs, let's talk about it. OMAP4 is using the same J-type for the DPLL_USB. The programming model is almost the same... sd_div is a the same location, but dco_sel is not there anymore because that DPLL will always be locked at 960MHz and thus will not require the mode for frequency 1GHz. At the DCO location, we have the bypass_clksel, so the code will have to take care of that. 31:24 DPLL_SD_DIV 23DPLL_BYP_CLKSEL 19:8 DPLL_MULT 7:0 DPLL_DIV Vishwanath, it sounds like it makes sense to hardcode the DPLL_SD_DIV and DCO_SEL shifts, since, so far, when present, they are always at the same offset. Instead of using a 'u8 jtype;' it seems to make sense to have a 'u8 flags' and have at least two flags there: DPLL_J_TYPE and DPLL_NO_DCO_SEL. (The latter flag would skip any DCO_SEL reads/writes for J-type DPLLs if it is set, so it will be usable for OMAP4. The code should also warn if DPLL_NO_DCO_SEL is set and DPLL_J_TYPE is not set.) Sound reasonable? Make sense. I will post new set of patches after incorporating all these. Thanks. - Paul -- 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: [PATCHV2 1/4] OMAP3: introduce DPLL4 Jtype
Hi Vishwanath, On Tue, 1 Dec 2009, Sripathy, Vishwanath wrote: -Original Message- From: Paul Walmsley [mailto:p...@pwsan.com] Sent: Monday, November 30, 2009 3:00 PM To: Sripathy, Vishwanath Cc: ext-ari.kau...@nokia.com; linux-omap@vger.kernel.org; Woodruff, Richard; Menon, Nishanth Subject: Re: [PATCHV2 1/4] OMAP3: introduce DPLL4 Jtype Hello Vishwanath, a few comments: On Thu, 26 Nov 2009, Vishwanath BS wrote: From: Richard Woodruff r-woodru...@ti.com DPLL4 for 3630 introduces a changed block requiring special divisor bits and additional reg fields. To allow for silicons to use this, this is introduced as a omap3_has_jtype_dpll4() and is enabled for 3630 silicon Tested with 3630 ZOOM3 Could you please consider Ari Kauppi's suggestions that he posted on 30 and 31 October? They look good to me. Here is a link: Regarding comments from Ari Kauppi on addition of dco_sel_mask and sd_div_mask, yes I had a look at them. However I feel, it's better to have these masks in the structure variable. Reason being this approach will help us to support when dpll types for other dplls get changed in future. For example, if dpll3 block is changed, then having new mask will help us to support it. Is there a product coming out that will add more j-type DPLLs with different dco_sel_masks and sd_div_masks? If not, then let's go with Ari's suggested changes, since we don't really know if any more of these j-type DPLLs will be used. We can always add the fields back in when we need them. But if you know for sure that there is a chip variant coming out soon that will use more j-type DPLLs, let's talk about it. - One thing that is confusing is that the 3630 Rev A TRM (SWPU176A) conflicts with the 34xx-36xx Delta TRM (SWPU204). For example, the delta TRM claims that the FREQSEL bits are all removed, but the 3630 TRM claims that they are still present. Could you find out which one is correct? If no FREQSEL bits are present, then the clock code shouldn't write to them. 3630 TRM (Rev A) shows that FREQSEL is no longer present for Per DPLL (CM_CLKEN_PLL [23:20] is reserved). So we should not write freqsel for PER dpll. Okay. Could you please update the patch to that effect? Also, the other part of the question is that the delta TRM claims that none of the other DPLLs have FREQSEL bits either. If that's true, then please update the patch to avoid programming FREQSEL bits for all DPLLs on 3630. - Table 3-13 in the delta TRM claims that the DPLL4 multiplier bitfield can now go to 4096. [This looks like an off-by-one error in the documentation, since only 12 bits are available, so the max is (2^12 - 1) = 4095.] But the important point for this patch is that the struct dpll_data.max_multiplier field for DPLL4 needs to be increased. I confirmed that DPLL4 Multiplier is 12 bits. So max value is 4095. OK. I just realized that you put the 12 bit value in a different patch... - Paul -- 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: [PATCHV2 1/4] OMAP3: introduce DPLL4 Jtype
Paul, From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap- ow...@vger.kernel.org] On Behalf Of Paul Walmsley Sent: Thursday, December 10, 2009 10:24 PM Hi Vishwanath, On Tue, 1 Dec 2009, Sripathy, Vishwanath wrote: Texas Instruments France SA, 821 Avenue Jack Kilby, 06270 Villeneuve Loubet. 036 420 040 R.C.S Antibes. Capital de EUR 753.920 -Original Message- From: Paul Walmsley [mailto:p...@pwsan.com] Sent: Monday, November 30, 2009 3:00 PM To: Sripathy, Vishwanath Cc: ext-ari.kau...@nokia.com; linux-omap@vger.kernel.org; Woodruff, Richard; Menon, Nishanth Subject: Re: [PATCHV2 1/4] OMAP3: introduce DPLL4 Jtype Hello Vishwanath, a few comments: On Thu, 26 Nov 2009, Vishwanath BS wrote: From: Richard Woodruff r-woodru...@ti.com DPLL4 for 3630 introduces a changed block requiring special divisor bits and additional reg fields. To allow for silicons to use this, this is introduced as a omap3_has_jtype_dpll4() and is enabled for 3630 silicon Tested with 3630 ZOOM3 Could you please consider Ari Kauppi's suggestions that he posted on 30 and 31 October? They look good to me. Here is a link: Regarding comments from Ari Kauppi on addition of dco_sel_mask and sd_div_mask, yes I had a look at them. However I feel, it's better to have these masks in the structure variable. Reason being this approach will help us to support when dpll types for other dplls get changed in future. For example, if dpll3 block is changed, then having new mask will help us to support it. Is there a product coming out that will add more j-type DPLLs with different dco_sel_masks and sd_div_masks? If not, then let's go with Ari's suggested changes, since we don't really know if any more of these j-type DPLLs will be used. We can always add the fields back in when we need them. But if you know for sure that there is a chip variant coming out soon that will use more j-type DPLLs, let's talk about it. OMAP4 is using the same J-type for the DPLL_USB. The programming model is almost the same... sd_div is a the same location, but dco_sel is not there anymore because that DPLL will always be locked at 960MHz and thus will not require the mode for frequency 1GHz. At the DCO location, we have the bypass_clksel, so the code will have to take care of that. 31:24 DPLL_SD_DIV 23DPLL_BYP_CLKSEL 19:8 DPLL_MULT 7:0 DPLL_DIV Regards, Benoit - One thing that is confusing is that the 3630 Rev A TRM (SWPU176A) conflicts with the 34xx-36xx Delta TRM (SWPU204). For example, the delta TRM claims that the FREQSEL bits are all removed, but the 3630 TRM claims that they are still present. Could you find out which one is correct? If no FREQSEL bits are present, then the clock code shouldn't write to them. 3630 TRM (Rev A) shows that FREQSEL is no longer present for Per DPLL (CM_CLKEN_PLL [23:20] is reserved). So we should not write freqsel for PER dpll. Okay. Could you please update the patch to that effect? Also, the other part of the question is that the delta TRM claims that none of the other DPLLs have FREQSEL bits either. If that's true, then please update the patch to avoid programming FREQSEL bits for all DPLLs on 3630. - Table 3-13 in the delta TRM claims that the DPLL4 multiplier bitfield can now go to 4096. [This looks like an off-by-one error in the documentation, since only 12 bits are available, so the max is (2^12 - 1) = 4095.] But the important point for this patch is that the struct dpll_data.max_multiplier field for DPLL4 needs to be increased. I confirmed that DPLL4 Multiplier is 12 bits. So max value is 4095. OK. I just realized that you put the 12 bit value in a different patch... - Paul -- 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 -- 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: [PATCHV2 1/4] OMAP3: introduce DPLL4 Jtype
Paul
-Original Message-
From: Paul Walmsley [mailto:p...@pwsan.com]
Sent: Monday, November 30, 2009 3:00 PM
To: Sripathy, Vishwanath
Cc: ext-ari.kau...@nokia.com; linux-omap@vger.kernel.org; Woodruff, Richard;
Menon, Nishanth
Subject: Re: [PATCHV2 1/4] OMAP3: introduce DPLL4 Jtype
Hello Vishwanath,
a few comments:
On Thu, 26 Nov 2009, Vishwanath BS wrote:
From: Richard Woodruff r-woodru...@ti.com
DPLL4 for 3630 introduces a changed block requiring special divisor bits and
additional reg fields. To allow for silicons to use this, this is introduced
as a omap3_has_jtype_dpll4() and is enabled for 3630 silicon
Tested with 3630 ZOOM3
Could you please consider Ari Kauppi's suggestions that he posted on
30 and 31 October? They look good to me. Here is a link:
Regarding comments from Ari Kauppi on addition of dco_sel_mask and sd_div_mask,
yes I had a look at them. However I feel, it's better to have these masks in
the structure variable. Reason being this approach will help us to support when
dpll types for other dplls get changed in future. For example, if dpll3 block
is changed, then having new mask will help us to support it.
http://patchwork.kernel.org/patch/55009/
Also, finally got the 3630 documents, so some more detailed comments are
now possible:
- One thing that is confusing is that the 3630 Rev A TRM (SWPU176A)
conflicts with the 34xx-36xx Delta TRM (SWPU204). For example, the delta
TRM claims that the FREQSEL bits are all removed, but the 3630 TRM claims
that they are still present. Could you find out which one is correct? If
no FREQSEL bits are present, then the clock code shouldn't write to them.
3630 TRM (Rev A) shows that FREQSEL is no longer present for Per DPLL
(CM_CLKEN_PLL [23:20] is reserved). So we should not write freqsel for PER
dpll.
- Table 3-13 in the delta TRM claims that the DPLL4 multiplier bitfield
can now go to 4096. [This looks like an off-by-one error in the
documentation, since only 12 bits are available, so the max is (2^12 - 1)
= 4095.] But the important point for this patch is that the struct
dpll_data.max_multiplier field for DPLL4 needs to be increased.
I confirmed that DPLL4 Multiplier is 12 bits. So max value is 4095.
A few more questions below:
Cc: Paul Walmsley p...@pwsan.com
Signed-off-by: Richard Woodruff r-woodru...@ti.com
Signed-off-by: Nishanth Menon n...@ti.com
Signed-off-by: Vishwanath BS vishwanath...@ti.com
---
arch/arm/mach-omap2/clock34xx.c | 51
++-
arch/arm/mach-omap2/cm-regbits-34xx.h |6 +++-
arch/arm/mach-omap2/id.c|4 ++-
arch/arm/plat-omap/include/plat/clock.h |3 ++
arch/arm/plat-omap/include/plat/cpu.h |3 +-
5 files changed, 63 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-omap2/clock34xx.c b/arch/arm/mach-
omap2/clock34xx.c
index da5bc1f..832ed0b 100644
--- a/arch/arm/mach-omap2/clock34xx.c
+++ b/arch/arm/mach-omap2/clock34xx.c
@@ -679,6 +679,41 @@ static void omap3_noncore_dpll_disable(struct clk *clk)
_omap3_noncore_dpll_stop(clk);
}
+/**
+ * lookup_dco_sddiv - Set j-type DPLL4 compensation variables
+ * @clk: pointer to a DPLL struct clk
+ * @dco: digital control oscillator selector
+ * @sd_div: target sigma-delta divider
+ * @m: DPLL multiplier to set
+ * @n: DPLL divider to set
+ */
+static void lookup_dco_sddiv(struct clk *clk, u8 *dco, u8 *sd_div, u16
+ m, u8 n)
+ {
+ unsigned long fint, clkinp, sd; /* watch out for overflow */
+ int mod1, mod2;
+
+ clkinp = clk-parent-rate;
+ fint = (clkinp / n) * m;
+
+ if (fint 10)
+ *dco = 2;
+ else
+ *dco = 4;
+ /*
+* target sigma-delta to near 250MHz
+* sd = ceil[(m/(n+1)) * (clkinp_MHz / 250)]
+*/
+ clkinp /= 10; /* shift from MHz to 10*Hz for 38.4 and 19.2*/
+ mod1 = (clkinp * m) % (250 * n);
+ sd = (clkinp * m) / (250 * n);
+ mod2 = sd % 10;
+ sd /= 10;
+
+ if (mod1 + mod2)
+ sd++;
+ *sd_div = sd;
+}
/* Non-CORE DPLL rate set code */
@@ -711,6 +746,13 @@ static int omap3_noncore_dpll_program(struct clk *clk,
u16 m, u8 n, u16 freqsel)
v = ~(dd-mult_mask | dd-div1_mask);
v |= m __ffs(dd-mult_mask);
v |= (n - 1) __ffs(dd-div1_mask);
+ if (dd-jtype) {
+ u8 dco, sd_div;
+ lookup_dco_sddiv(clk, dco, sd_div, m, n);
+ v = ~(dd-dco_sel_mask | dd-sd_div_mask);
+ v |= dco __ffs(dd-dco_sel_mask);
+ v |= sd_div __ffs(dd-sd_div_mask);
+ }
__raw_writel(v, dd-mult_div1_reg);
/* We let the clock framework set the other output dividers later */
@@ -1026,7 +1068,7 @@ static unsigned long omap3_clkoutx2_recalc(struct clk
*clk)
v = __raw_readl(dd-control_reg
Re: [PATCHV2 1/4] OMAP3: introduce DPLL4 Jtype
Hello Vishwanath,
a few comments:
On Thu, 26 Nov 2009, Vishwanath BS wrote:
From: Richard Woodruff r-woodru...@ti.com
DPLL4 for 3630 introduces a changed block requiring special divisor bits and
additional reg fields. To allow for silicons to use this, this is introduced
as a omap3_has_jtype_dpll4() and is enabled for 3630 silicon
Tested with 3630 ZOOM3
Could you please consider Ari Kauppi's suggestions that he posted on
30 and 31 October? They look good to me. Here is a link:
http://patchwork.kernel.org/patch/55009/
Also, finally got the 3630 documents, so some more detailed comments are
now possible:
- One thing that is confusing is that the 3630 Rev A TRM (SWPU176A)
conflicts with the 34xx-36xx Delta TRM (SWPU204). For example, the delta
TRM claims that the FREQSEL bits are all removed, but the 3630 TRM claims
that they are still present. Could you find out which one is correct? If
no FREQSEL bits are present, then the clock code shouldn't write to them.
- Table 3-13 in the delta TRM claims that the DPLL4 multiplier bitfield
can now go to 4096. [This looks like an off-by-one error in the
documentation, since only 12 bits are available, so the max is (2^12 - 1)
= 4095.] But the important point for this patch is that the struct
dpll_data.max_multiplier field for DPLL4 needs to be increased.
A few more questions below:
Cc: Paul Walmsley p...@pwsan.com
Signed-off-by: Richard Woodruff r-woodru...@ti.com
Signed-off-by: Nishanth Menon n...@ti.com
Signed-off-by: Vishwanath BS vishwanath...@ti.com
---
arch/arm/mach-omap2/clock34xx.c | 51
++-
arch/arm/mach-omap2/cm-regbits-34xx.h |6 +++-
arch/arm/mach-omap2/id.c|4 ++-
arch/arm/plat-omap/include/plat/clock.h |3 ++
arch/arm/plat-omap/include/plat/cpu.h |3 +-
5 files changed, 63 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-omap2/clock34xx.c b/arch/arm/mach-omap2/clock34xx.c
index da5bc1f..832ed0b 100644
--- a/arch/arm/mach-omap2/clock34xx.c
+++ b/arch/arm/mach-omap2/clock34xx.c
@@ -679,6 +679,41 @@ static void omap3_noncore_dpll_disable(struct clk *clk)
_omap3_noncore_dpll_stop(clk);
}
+/**
+ * lookup_dco_sddiv - Set j-type DPLL4 compensation variables
+ * @clk: pointer to a DPLL struct clk
+ * @dco: digital control oscillator selector
+ * @sd_div: target sigma-delta divider
+ * @m: DPLL multiplier to set
+ * @n: DPLL divider to set
+ */
+static void lookup_dco_sddiv(struct clk *clk, u8 *dco, u8 *sd_div, u16
+ m, u8 n)
+ {
+ unsigned long fint, clkinp, sd; /* watch out for overflow */
+ int mod1, mod2;
+
+ clkinp = clk-parent-rate;
+ fint = (clkinp / n) * m;
+
+ if (fint 10)
+ *dco = 2;
+ else
+ *dco = 4;
+ /*
+ * target sigma-delta to near 250MHz
+ * sd = ceil[(m/(n+1)) * (clkinp_MHz / 250)]
+ */
+ clkinp /= 10; /* shift from MHz to 10*Hz for 38.4 and 19.2*/
+ mod1 = (clkinp * m) % (250 * n);
+ sd = (clkinp * m) / (250 * n);
+ mod2 = sd % 10;
+ sd /= 10;
+
+ if (mod1 + mod2)
+ sd++;
+ *sd_div = sd;
+}
/* Non-CORE DPLL rate set code */
@@ -711,6 +746,13 @@ static int omap3_noncore_dpll_program(struct clk *clk,
u16 m, u8 n, u16 freqsel)
v = ~(dd-mult_mask | dd-div1_mask);
v |= m __ffs(dd-mult_mask);
v |= (n - 1) __ffs(dd-div1_mask);
+ if (dd-jtype) {
+ u8 dco, sd_div;
+ lookup_dco_sddiv(clk, dco, sd_div, m, n);
+ v = ~(dd-dco_sel_mask | dd-sd_div_mask);
+ v |= dco __ffs(dd-dco_sel_mask);
+ v |= sd_div __ffs(dd-sd_div_mask);
+ }
__raw_writel(v, dd-mult_div1_reg);
/* We let the clock framework set the other output dividers later */
@@ -1026,7 +1068,7 @@ static unsigned long omap3_clkoutx2_recalc(struct clk
*clk)
v = __raw_readl(dd-control_reg) dd-enable_mask;
v = __ffs(dd-enable_mask);
- if (v != OMAP3XXX_EN_DPLL_LOCKED)
+ if (v != OMAP3XXX_EN_DPLL_LOCKED (!dd-jtype))
rate = clk-parent-rate;
else
rate = clk-parent-rate * 2;
Could you provide more information, and hopefully a TRM cite, on the above
change? It seems that DPLL4's outputs are multiplied by two even if the
DPLL is unlocked?
@@ -1174,6 +1216,13 @@ int __init omap2_clk_init(void)
cpu_mask |= RATE_IN_3430ES2;
cpu_clkflg |= CK_3430ES2;
}
+ if (omap3_has_jtype_dpll4()) {
+ dpll4_ck.dpll_data-jtype = 1;
+ dpll4_ck.dpll_data-dco_sel_mask =
+ OMAP3630_PERIPH_DPLL_DCO_SEL_MASK;
+ dpll4_ck.dpll_data-sd_div_mask =
+
