diff mbox

[U-Boot] powerpc/85xx: Change timebase divisor to be defined per processor

Message ID 1303412373-11991-1-git-send-email-galak@kernel.crashing.org
State Changes Requested
Headers show

Commit Message

Kumar Gala April 21, 2011, 6:59 p.m. UTC 
Introduce new CONFIG_SYS_FSL_TBCLK_DIV on 85xx platforms because

Comments

Tabi Timur-B04825 April 22, 2011, 11:50 a.m. UTC | #1 
On Thu, Apr 21, 2011 at 1:59 PM, Kumar Gala <galak@kernel.crashing.org> wrote:
> Introduce new CONFIG_SYS_FSL_TBCLK_DIV on 85xx platforms because
> different SoCs have different divisor amounts.  All the PQ3 parts are
> /8, the P4080/P4080 is /16, and P2040/P3041/P5020 are /32.

Shouldn't there also be a README update to document this option?

> +       unsigned long tbclk_div = CONFIG_SYS_FSL_TBCLK_DIV;
> +
> +       return (gd->bus_clk + (tbclk_div >> 1)) / tbclk_div;

Why not just:

return (gd->bus_clk + (CONFIG_SYS_FSL_TBCLK_DIV / 2)) /
CONFIG_SYS_FSL_TBCLK_DIV;
Kumar Gala April 22, 2011, 1:26 p.m. UTC | #2 
On Apr 22, 2011, at 6:50 AM, Tabi Timur-B04825 wrote:

> On Thu, Apr 21, 2011 at 1:59 PM, Kumar Gala <galak@kernel.crashing.org> wrote:
>> Introduce new CONFIG_SYS_FSL_TBCLK_DIV on 85xx platforms because
>> different SoCs have different divisor amounts.  All the PQ3 parts are
>> /8, the P4080/P4080 is /16, and P2040/P3041/P5020 are /32.
> 
> Shouldn't there also be a README update to document this option?

I think its self evident what it is.

>> +       unsigned long tbclk_div = CONFIG_SYS_FSL_TBCLK_DIV;
>> +
>> +       return (gd->bus_clk + (tbclk_div >> 1)) / tbclk_div;
> 
> Why not just:
> 
> return (gd->bus_clk + (CONFIG_SYS_FSL_TBCLK_DIV / 2)) /
> CONFIG_SYS_FSL_TBCLK_DIV;

It was done this way to deal w/line wrapping.

- k
Scott Wood April 22, 2011, 3:42 p.m. UTC | #3 
On Fri, 22 Apr 2011 08:26:55 -0500
Kumar Gala <galak@kernel.crashing.org> wrote:

> 
> On Apr 22, 2011, at 6:50 AM, Tabi Timur-B04825 wrote:
> 
> > On Thu, Apr 21, 2011 at 1:59 PM, Kumar Gala <galak@kernel.crashing.org> wrote:
> >> Introduce new CONFIG_SYS_FSL_TBCLK_DIV on 85xx platforms because
> >> different SoCs have different divisor amounts.  All the PQ3 parts are
> >> /8, the P4080/P4080 is /16, and P2040/P3041/P5020 are /32.
> > 
> > Shouldn't there also be a README update to document this option?
> 
> I think its self evident what it is.

Without commenting on whether this one is or isn't self-explanatory, there
are enough CONFIG symbols that are used without documentation that are not
self-explanatory (sometimes the purpose is clear, but the exact semantics
aren't), that it's better to just document all of them.

-Scott
Tabi Timur-B04825 April 22, 2011, 7:46 p.m. UTC | #4 
On Fri, Apr 22, 2011 at 8:26 AM, Kumar Gala <galak@kernel.crashing.org> wrote:

>> Shouldn't there also be a README update to document this option?
>
> I think its self evident what it is.

I have to disagree.  Without reading the patch description, it's hard
to know what value to choose for this macro.  Besides, Wolfgang is
already annoyed with the number of CONFIG_ options that are not
documented, so I can't imagine he would accept another one.
Wolfgang Denk April 23, 2011, 6:45 p.m. UTC | #5 
Dear Kumar Gala,

In message <54CE1181-5449-4FBF-9402-DF5D4E99E4CE@kernel.crashing.org> you wrote:
> 
> On Apr 22, 2011, at 6:50 AM, Tabi Timur-B04825 wrote:
> 
> > On Thu, Apr 21, 2011 at 1:59 PM, Kumar Gala <galak@kernel.crashing.org> wrote:
> >> Introduce new CONFIG_SYS_FSL_TBCLK_DIV on 85xx platforms because
> >> different SoCs have different divisor amounts.  All the PQ3 parts are
> >> /8, the P4080/P4080 is /16, and P2040/P3041/P5020 are /32.
> > 
> > Shouldn't there also be a README update to document this option?
> 
> I think its self evident what it is.

Please fix it.

> >> +       unsigned long tbclk_div = CONFIG_SYS_FSL_TBCLK_DIV;
> >> +
> >> +       return (gd->bus_clk + (tbclk_div >> 1)) / tbclk_div;
> > 
> > Why not just:
> > 
> > return (gd->bus_clk + (CONFIG_SYS_FSL_TBCLK_DIV / 2)) /
> > CONFIG_SYS_FSL_TBCLK_DIV;
> 
> It was done this way to deal w/line wrapping.

Now this is really a _very_ strange argument.  Please fix as well.

Best regards,

Wolfgang Denk
Kumar Gala April 28, 2011, 3:38 a.m. UTC | #6 
On Apr 23, 2011, at 1:45 PM, Wolfgang Denk wrote:

> Dear Kumar Gala,
> 
> In message <54CE1181-5449-4FBF-9402-DF5D4E99E4CE@kernel.crashing.org> you wrote:
>> 
>> On Apr 22, 2011, at 6:50 AM, Tabi Timur-B04825 wrote:
>> 
>>> On Thu, Apr 21, 2011 at 1:59 PM, Kumar Gala <galak@kernel.crashing.org> wrote:
>>>> Introduce new CONFIG_SYS_FSL_TBCLK_DIV on 85xx platforms because
>>>> different SoCs have different divisor amounts.  All the PQ3 parts are
>>>> /8, the P4080/P4080 is /16, and P2040/P3041/P5020 are /32.
>>> 
>>> Shouldn't there also be a README update to document this option?
>> 
>> I think its self evident what it is.
> 
> Please fix it.
> 
>>>> +       unsigned long tbclk_div = CONFIG_SYS_FSL_TBCLK_DIV;
>>>> +
>>>> +       return (gd->bus_clk + (tbclk_div >> 1)) / tbclk_div;
>>> 
>>> Why not just:
>>> 
>>> return (gd->bus_clk + (CONFIG_SYS_FSL_TBCLK_DIV / 2)) /
>>> CONFIG_SYS_FSL_TBCLK_DIV;
>> 
>> It was done this way to deal w/line wrapping.
> 
> Now this is really a _very_ strange argument.  Please fix as well.

I consider it more readable as it stands.  I'm not changing this.

- k
diff mbox

Patch

different SoCs have different divisor amounts.  All the PQ3 parts are
/8, the P4080/P4080 is /16, and P2040/P3041/P5020 are /32.

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 arch/powerpc/cpu/mpc85xx/cpu.c            |   11 ++++++-----
 arch/powerpc/include/asm/config_mpc85xx.h |    6 ++++++
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/cpu/mpc85xx/cpu.c b/arch/powerpc/cpu/mpc85xx/cpu.c
index f5b39c0..f863f4a 100644
--- a/arch/powerpc/cpu/mpc85xx/cpu.c
+++ b/arch/powerpc/cpu/mpc85xx/cpu.c
@@ -234,13 +234,14 @@  int do_reset (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 /*
  * Get timebase clock frequency
  */
+#ifndef CONFIG_SYS_FSL_TBCLK_DIV
+#define CONFIG_SYS_FSL_TBCLK_DIV 8
+#endif
 unsigned long get_tbclk (void)
 {
-#ifdef CONFIG_FSL_CORENET
-	return (gd->bus_clk + 8) / 16;
-#else
-	return (gd->bus_clk + 4UL)/8UL;
-#endif
+	unsigned long tbclk_div = CONFIG_SYS_FSL_TBCLK_DIV;
+
+	return (gd->bus_clk + (tbclk_div >> 1)) / tbclk_div;
 }
 
 
diff --git a/arch/powerpc/include/asm/config_mpc85xx.h b/arch/powerpc/include/asm/config_mpc85xx.h
index da2e998..d71c3fc 100644
--- a/arch/powerpc/include/asm/config_mpc85xx.h
+++ b/arch/powerpc/include/asm/config_mpc85xx.h
@@ -264,6 +264,7 @@ 
 #define CONFIG_SYS_NUM_FM1_DTSEC	5
 #define CONFIG_NUM_DDR_CONTROLLERS	1
 #define CONFIG_SYS_FM_MURAM_SIZE	0x28000
+#define CONFIG_SYS_FSL_TBCLK_DIV	32
 #define CONFIG_SYS_FSL_USB1_PHY_ENABLE
 #define CONFIG_SYS_FSL_USB2_PHY_ENABLE
 #define CONFIG_SYS_FSL_ERRATUM_ESDHC111
@@ -278,6 +279,7 @@ 
 #define CONFIG_SYS_NUM_FM1_10GEC	1
 #define CONFIG_NUM_DDR_CONTROLLERS	1
 #define CONFIG_SYS_FM_MURAM_SIZE	0x28000
+#define CONFIG_SYS_FSL_TBCLK_DIV	32
 #define CONFIG_SYS_FSL_USB1_PHY_ENABLE
 #define CONFIG_SYS_FSL_USB2_PHY_ENABLE
 #define CONFIG_SYS_FSL_ERRATUM_ESDHC111
@@ -288,6 +290,7 @@ 
 #define CONFIG_SYS_FSL_NUM_LAWS		32
 #define CONFIG_SYS_FSL_SEC_COMPAT	4
 #define CONFIG_SYS_FM_MURAM_SIZE	0x28000
+#define CONFIG_SYS_FSL_TBCLK_DIV	16
 
 #elif defined(CONFIG_PPC_P4080)
 #define CONFIG_MAX_CPUS			8
@@ -301,6 +304,7 @@ 
 #define CONFIG_SYS_NUM_FM2_10GEC	1
 #define CONFIG_NUM_DDR_CONTROLLERS	2
 #define CONFIG_SYS_FM_MURAM_SIZE	0x28000
+#define CONFIG_SYS_FSL_TBCLK_DIV	16
 #define CONFIG_SYS_FSL_ERRATUM_CPC_A002
 #define CONFIG_SYS_FSL_ERRATUM_CPC_A003
 #define CONFIG_SYS_FSL_ERRATUM_DDR_A003
@@ -323,6 +327,7 @@ 
 #define CONFIG_SYS_NUM_FM1_10GEC	1
 #define CONFIG_NUM_DDR_CONTROLLERS	1
 #define CONFIG_SYS_FM_MURAM_SIZE	0x28000
+#define CONFIG_SYS_FSL_TBCLK_DIV	32
 #define CONFIG_SYS_FSL_USB1_PHY_ENABLE
 #define CONFIG_SYS_FSL_USB2_PHY_ENABLE
 #define CONFIG_SYS_FSL_ERRATUM_ESDHC111
@@ -337,6 +342,7 @@ 
 #define CONFIG_SYS_NUM_FM1_10GEC	1
 #define CONFIG_NUM_DDR_CONTROLLERS	2
 #define CONFIG_SYS_FM_MURAM_SIZE	0x28000
+#define CONFIG_SYS_FSL_TBCLK_DIV	32
 #define CONFIG_SYS_FSL_USB1_PHY_ENABLE
 #define CONFIG_SYS_FSL_USB2_PHY_ENABLE
 #define CONFIG_SYS_FSL_ERRATUM_ESDHC111