Hi Wolfgang,
On Wednesday 15 July 2009 17:08, Wolfgang Denk wrote:
Dear Matthias Fuchs,
In message 200907151251.25985.matthias.fuchs@esd.eu you wrote:
Signed-off-by: Matthias Fuchs matthias.fuchs@esd.eu
...
diff --git a/board/esd/pmc405de/pmc405de.c b/board/esd/pmc405de/pmc405de.c new file mode 100644 index 0000000..ca26d5c --- /dev/null +++ b/board/esd/pmc405de/pmc405de.c
...
- if ((pllmr0 & PLLMR0_CPU_TO_PLB_MASK) == PLLMR0_CPU_PLB_DIV_3) {
/* fCPU=333MHz, fPLB=111MHz */if (pci_is_66mhz()) {if ((pllmr0 & PLLMR0_PCI_TO_PLB_MASK) !=PLLMR0_PCI_PLB_DIV_1) {pll_write((pllmr0 & ~PLLMR0_PCI_TO_PLB_MASK) |PLLMR0_PCI_PLB_DIV_1, pllmr1);}} else {if ((pllmr0 & PLLMR0_PCI_TO_PLB_MASK) !=PLLMR0_PCI_PLB_DIV_2) {pll_write((pllmr0 & ~PLLMR0_PCI_TO_PLB_MASK) |PLLMR0_PCI_PLB_DIV_2, pllmr1);}}- } else {
/* fCPU=133|266MHz, fPLB=133MHz */if (pci_is_66mhz()) {if ((pllmr0 & PLLMR0_PCI_TO_PLB_MASK) !=PLLMR0_PCI_PLB_DIV_2) {pll_write((pllmr0 & ~PLLMR0_PCI_TO_PLB_MASK) |PLLMR0_PCI_PLB_DIV_2, pllmr1);}} else {if ((pllmr0 & PLLMR0_PCI_TO_PLB_MASK) !=PLLMR0_PCI_PLB_DIV_3) {pll_write((pllmr0 & ~PLLMR0_PCI_TO_PLB_MASK) |PLLMR0_PCI_PLB_DIV_3, pllmr1);}}- }
Please try to factor out repeated code like here.
Ack.
+static int is_monarch(void) +{
- if (in_be32((void*)GPIO0_IR) & CONFIG_SYS_GPIO_MONARCH_N)
return 0;- return 1;
or: return (in_be32((void*)GPIO0_IR) & CONFIG_SYS_GPIO_MONARCH_N) == 0;
Ack.
Umm... and why do we need a cast here? This should be fixed.
The in/out_be16/32 functions expect a pointer. When you grep over the U-Boot sources you will find tons of places with this cast. So I will and cannot fix this with my patch.
We could put the cast into the GPIO_IR (and friends) definitions. Do you want me to remove the cast and life with a compiler warning until this has been globally fixed?
Because this is not a particular problem with this patch it should be addressed in a separate discussion. But you are rigth - this cast is a little bit nerving :-)
+}
+static int pci_is_66mhz(void) +{
- if (in_be32((void*)GPIO0_IR) & CONFIG_SYS_GPIO_M66EN)
return 1;- return 0;
Ditto.
+}
+static int board_revision(void) +{
- return ((in_be32((void*)GPIO0_IR) & CONFIG_SYS_GPIO_HWREV_MASK) >>
CONFIG_SYS_GPIO_HWREV_SHIFT);+}
and again.
+static int cpld_revision(void) +{
- return ((in_8((void*)CPLD_VERSION) & CPLD_VERSION_MASK));
+}
and again.
Best regards,
Wolfgang Denk