[U-Boot-Users] [PATCH] 85xx: Round up frequency calculations to get reasonable output

eg. because of rounding error we can get 799Mhz instead of 800Mhz.
Signed-off-by: Dejan Minic minic@freescale.com Signed-off-by: Srikanth Srinivasan srikanth.srinivasan@freescale.com Signed-off-by: Kumar Gala galak@kernel.crashing.org --- cpu/mpc85xx/cpu.c | 18 +++++++++++------- 1 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/cpu/mpc85xx/cpu.c b/cpu/mpc85xx/cpu.c index dcd8817..6972bb1 100644 --- a/cpu/mpc85xx/cpu.c +++ b/cpu/mpc85xx/cpu.c @@ -65,6 +65,11 @@ struct cpu_type cpu_type_list [] = { CPU_TYPE_ENTRY(8572_E), };
+static inline unsigned long integer_round (unsigned long val, unsigned long div) +{ + return ((val + (div/2)) / div); +} + int checkcpu (void) { sys_info_t sysinfo; @@ -116,22 +121,21 @@ int checkcpu (void) get_sys_info(&sysinfo);
puts("Clock Configuration:\n"); - printf(" CPU:%4lu MHz, ", sysinfo.freqProcessor / 1000000); - printf("CCB:%4lu MHz,\n", sysinfo.freqSystemBus / 1000000); - + printf(" CPU:%4lu MHz, ", integer_round(sysinfo.freqProcessor,1000000)); + printf("CCB:%4lu MHz,\n", integer_round(sysinfo.freqSystemBus,1000000)); ddr_ratio = ((gur->porpllsr) & 0x00003e00) >> 9; switch (ddr_ratio) { case 0x0: printf(" DDR:%4lu MHz (%lu MT/s data rate), ", - sysinfo.freqDDRBus / 2000000, sysinfo.freqDDRBus / 1000000); + integer_round(sysinfo.freqDDRBus,2000000), integer_round(sysinfo.freqDDRBus,1000000)); break; case 0x7: printf(" DDR:%4lu MHz (%lu MT/s data rate) (Synchronous), ", - sysinfo.freqDDRBus / 2000000, sysinfo.freqDDRBus / 1000000); + integer_round(sysinfo.freqDDRBus, 2000000), integer_round(sysinfo.freqDDRBus, 1000000)); break; default: printf(" DDR:%4lu MHz (%lu MT/s data rate) (Asynchronous), ", - sysinfo.freqDDRBus / 2000000, sysinfo.freqDDRBus / 1000000); + integer_round(sysinfo.freqDDRBus, 2000000), integer_round(sysinfo.freqDDRBus,1000000)); break; }
@@ -154,7 +158,7 @@ int checkcpu (void) clkdiv *= 2; #endif printf("LBC:%4lu MHz\n", - sysinfo.freqSystemBus / 1000000 / clkdiv); + integer_round(sysinfo.freqSystemBus, 1000000) / clkdiv); } else { printf("LBC: unknown (lcrr: 0x%08x)\n", lcrr); }

On Fri, Apr 18, 2008 at 11:57 AM, Kumar Gala galak@kernel.crashing.org wrote:
eg. because of rounding error we can get 799Mhz instead of 800Mhz.
Signed-off-by: Dejan Minic minic@freescale.com Signed-off-by: Srikanth Srinivasan srikanth.srinivasan@freescale.com Signed-off-by: Kumar Gala galak@kernel.crashing.org
cpu/mpc85xx/cpu.c | 18 +++++++++++------- 1 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/cpu/mpc85xx/cpu.c b/cpu/mpc85xx/cpu.c index dcd8817..6972bb1 100644 --- a/cpu/mpc85xx/cpu.c +++ b/cpu/mpc85xx/cpu.c @@ -65,6 +65,11 @@ struct cpu_type cpu_type_list [] = { CPU_TYPE_ENTRY(8572_E), };
+static inline unsigned long integer_round (unsigned long val, unsigned long div) +{
return ((val + (div/2)) / div);
+}
Ok, but I don't think we can really call it integer_round(). Can we call it something that reflects what it does? Like, rounded_divide() or something?
Andy

In message Pine.LNX.4.64.0804181156410.4255@blarg.am.freescale.net you wrote:
eg. because of rounding error we can get 799Mhz instead of 800Mhz.
Signed-off-by: Dejan Minic minic@freescale.com Signed-off-by: Srikanth Srinivasan srikanth.srinivasan@freescale.com Signed-off-by: Kumar Gala galak@kernel.crashing.org
cpu/mpc85xx/cpu.c | 18 +++++++++++------- 1 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/cpu/mpc85xx/cpu.c b/cpu/mpc85xx/cpu.c index dcd8817..6972bb1 100644 --- a/cpu/mpc85xx/cpu.c +++ b/cpu/mpc85xx/cpu.c @@ -65,6 +65,11 @@ struct cpu_type cpu_type_list [] = { CPU_TYPE_ENTRY(8572_E), };
+static inline unsigned long integer_round (unsigned long val, unsigned long div) +{
- return ((val + (div/2)) / div);
+}
As already mentioned, "integer_round" is not a good name for this. Also, it seems this should not live in cpu/mpc85xx/cpu.c but in a more central place and be used by lots of other code as well. Actually you might want to make it a macro to allow for different data types as well.
Best regards,
Wolfgang Denk

On Apr 20, 2008, at 6:28 PM, Wolfgang Denk wrote:
In message Pine.LNX.4.64.0804181156410.4255@blarg.am.freescale.net you wrote:
eg. because of rounding error we can get 799Mhz instead of 800Mhz.
Signed-off-by: Dejan Minic minic@freescale.com Signed-off-by: Srikanth Srinivasan srikanth.srinivasan@freescale.com Signed-off-by: Kumar Gala galak@kernel.crashing.org
cpu/mpc85xx/cpu.c | 18 +++++++++++------- 1 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/cpu/mpc85xx/cpu.c b/cpu/mpc85xx/cpu.c index dcd8817..6972bb1 100644 --- a/cpu/mpc85xx/cpu.c +++ b/cpu/mpc85xx/cpu.c @@ -65,6 +65,11 @@ struct cpu_type cpu_type_list [] = { CPU_TYPE_ENTRY(8572_E), };
+static inline unsigned long integer_round (unsigned long val, unsigned long div) +{
- return ((val + (div/2)) / div);
+}
As already mentioned, "integer_round" is not a good name for this. Also, it seems this should not live in cpu/mpc85xx/cpu.c but in a more central place and be used by lots of other code as well. Actually you might want to make it a macro to allow for different data types as well.
Ok. I've posted v3 of the patch that takes DIV_ROUND_UP from the kernel. See if that works for you.
- k
participants (3)
-
Andy Fleming
-
Kumar Gala
-
Wolfgang Denk