On 02/17/2012 06:22 AM, Laurence Withers wrote:
On Mon, Feb 06, 2012 at 04:00:44PM +0530, Manjunath Hadli wrote:
add support for printing various clock frequency info found
in SOC such as ARM core frequency, DSP core frequency and DDR
frequency as part of bdinfo command.
Signed-off-by: Manjunath Hadli<[email protected]>
Cc: Tom Rini<[email protected]>
Sorry I am late to the party, especially as this already seems to have been
applied to u-boot master, but I have just tried out this bit of code and have
some feedback having looked at it.
Review is always welcome, thanks.
[snip]
diff --git a/arch/arm/include/asm/u-boot.h b/arch/arm/include/asm/u-boot.h
index f30b9fc..20e1653 100644
--- a/arch/arm/include/asm/u-boot.h
+++ b/arch/arm/include/asm/u-boot.h
@@ -41,6 +41,9 @@ typedef struct bd_info {
unsigned long bi_ip_addr; /* IP Address */
ulong bi_arch_number; /* unique id for this board */
ulong bi_boot_params; /* where this board expects params */
+ unsigned long bi_arm_freq; /* arm frequency */
+ unsigned long bi_dsp_freq; /* dsp core frequency */
+ unsigned long bi_ddr_freq; /* ddr frequency */
struct /* RAM configuration */
{
ulong start;
I'm not sure that this really belongs in every ARM board's struct bd_info, as
it implies the only three clocks we'll care about are ARM, DSP and DDR. I
doubt that maps nicely onto most of the SoCs out there.
Checking powerpc there is precedent for #if'ing this structure, so that
should be done. As for making it more generic, yes, my hope is that
once the next SoC says "I want to print clocks" we can start mapping
things out more. And as you noted above (and I snipped) we do need to
clean up and fix the current init sequence.
Whitespace doesn't match the rest of the file (even if the rest of the file
is wrong, and this patch has used tabs as it should have).
I was hoping checkpatch warned on that type of mismatch so I need to
rework my workflow a bit.
[snip]
diff --git a/common/cmd_bdinfo.c b/common/cmd_bdinfo.c
index 97f2945..5359a47 100644
--- a/common/cmd_bdinfo.c
+++ b/common/cmd_bdinfo.c
@@ -370,6 +370,15 @@ int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char *
const argv[])
print_num("irq_sp", gd->irq_sp); /* irq stack pointer */
print_num("sp start ", gd->start_addr_sp);
print_num("FB base ", gd->fb_base);
+ /*
+ * TODO: Currently only support for davinci SOC's is added.
+ * Remove this check once all the board implement this.
+ */
+#ifdef CONFIG_CLOCKS
+ printf("ARM frequency = %ld MHz\n", gd->bd->bi_arm_freq);
+ printf("DSP frequency = %ld MHz\n", gd->bd->bi_dsp_freq);
+ printf("DDR frequency = %ld MHz\n", gd->bd->bi_ddr_freq);
+#endif
return 0;
}
Again, here it is implied that the only clocks we'll ever want to print are
ARM, DSP and DDR. Seems way too specific for such common code.
Note that do_bdinfo is already per-arch. This is intended as a starting
point for generic work and the first step away from doing this in a
per-board fashion. Thanks!
--
Tom
_______________________________________________
U-Boot mailing list
[email protected]
http://lists.denx.de/mailman/listinfo/u-boot
