Dear David Jander, In message <00b6eb8822a36b67a5e6154fd256e51e605fe47a.1282213859.git.da...@protonic.nl> you wrote: > > board/Protonic/prtlvt2/Makefile | 48 ++++ > board/Protonic/prtlvt2/config.mk | 25 ++ > board/Protonic/prtlvt2/imximage.cfg | 171 ++++++++++++ > board/Protonic/prtlvt2/prtlvt2.c | 513 > +++++++++++++++++++++++++++++++++++ > board/Protonic/prtlvt2/prtlvt2.h | 50 ++++ > boards.cfg | 1 + > include/configs/prtlvt2.h | 203 ++++++++++++++ > 7 files changed, 1011 insertions(+), 0 deletions(-) > create mode 100644 board/Protonic/prtlvt2/Makefile > create mode 100644 board/Protonic/prtlvt2/config.mk > create mode 100644 board/Protonic/prtlvt2/imximage.cfg > create mode 100644 board/Protonic/prtlvt2/prtlvt2.c > create mode 100644 board/Protonic/prtlvt2/prtlvt2.h > create mode 100644 include/configs/prtlvt2.h
Entry to MAINTAINERS missing. ... > + /* SCL_2V8, SDA_2V8 on KEY_COL4 and KEY_COL5 */ > + {MX51_PIN_KEY_COL4, IOMUX_CONFIG_ALT3, PIO_OD, > MUX_IN_I2C2_IPP_SCL_IN_SELECT_INPUT, INPUT_CTL_PATH1}, > + {MX51_PIN_KEY_COL5, IOMUX_CONFIG_ALT3, PIO_OD, > MUX_IN_I2C2_IPP_SCL_IN_SELECT_INPUT, INPUT_CTL_PATH1}, Lines too long, please fix globally. ... > + /* Write needed to update Charger 0 */ > + /* Charge voltage=4.2V, Current=full-on, Plim=800mW, charger sw, > battfet off */ Incorrect multiline comment format, please fix globally. > + pmic_reg_write(REG_CHARGE, VCHRG0 | VCHRG1 /*| VCHRG2 */ | > + ICHRG0 | ICHRG1 | ICHRG2 | ICHRG3 | FETOVRD | > + PLIM0 /* | PLIM1 */ | > + CHRGLEDEN | CHGAUTOB | CYCLB); > + > + /* Configure Power Control, enable PWRON switches */ > + pmic_reg_write(REG_POWER_CTL2, RESTARTEN | > + PWRON1RSTEN | PWRON2RSTEN | PWRON3RSTEN | > + PWRON1DBNC0 | PWRON2DBNC0 | PWRON3DBNC0 | > + WDIRESET | STBYDLY0); > + > + /* power up the system first */ > + // FIXME: Is this bit still there in rev 2? > + // This bit was called PWUP?? No C++ comments, please! > + pmic_reg_write(REG_POWER_MISC, GPO4ADIN); It would really be great if someone cold clean up this mess in "include/fsl_pmic.h" Using an "enum" for register definitions is just horrible. I am well aware that you did not introduce this code, but reading this feels is if my nails are rolling up. > + /* Set core voltage to 1.1V */ > + val = pmic_reg_read(REG_SW_0); > + val = (val & (~0x80001F)) | 0x14; > + pmic_reg_write(REG_SW_0, val); > + > + /* Setup VCC (SW2) to 1.225 */ > + val = pmic_reg_read(REG_SW_1); > + val = (val & (~0x80001F)) | 0x19; > + pmic_reg_write(REG_SW_1, val); > + > + /* Setup 1V2_DIG1 (SW3) to 1.2 */ > + val = pmic_reg_read(REG_SW_2); > + val = (val & (~0x80001F)) | 0x18; > + pmic_reg_write(REG_SW_2, val); > + udelay(50); Don't you feel the need to intr=oduce some readable symbolic constants for all these magic numbers here? > + /* Raise the core frequency to 800MHz */ > + /* printf("Core at 400 MHz!\n"); */ > + /* writel(0x1, &mxc_ccm->cacrr); */ Please remove dead code. > +#if 0 /* FIXME: This shouldn't be changed for PRTLVT2 */ > + /* Set VDIG to 1.25V, VGEN3 to 1.8V, VCAM to 2.6V */ > + val = pmic_reg_read(REG_SETTING_0); > + val &= ~(VCAM_MASK | VGEN3_MASK | VDIG_MASK); > + val |= VDIG_1_25 | VGEN3_1_8 | VCAM_2_6; > + pmic_reg_write(REG_SETTING_0, val); > +#endif Please remove dead code. > + /* Turn on backlight */ > + val = readl(GPIO1_BASE_ADDR + 0x0); > + val |= 0x00000004; /* Make GPIO1_2 high (BLEN) */ > + writel(val, GPIO1_BASE_ADDR + 0x0); > + > + val = readl(GPIO1_BASE_ADDR + 0x4); > + val |= 0x00000004; /* configure GPIO line as output */ > + writel(val, GPIO1_BASE_ADDR + 0x4); We don't accept register base plus offset notation any more. Please use a C struct to describe the register layout. Please fix globally. ... > +#define CONFIG_EXTRA_ENV_SETTINGS > \ > + "netdev=eth0\0" \ > + "uboot_addr=0xa0000000\0" \ > + "uboot=u-boot.bin\0" \ > + "loadaddr=0x90800000\0" \ > + "bootargs_base=setenv bootargs console=tty "\ > + "console=ttymxc0,${baudrate}\0"\ > + "bootargs_nfs=setenv bootargs ${bootargs} root=/dev/nfs "\ > + "ip=${ipaddr} > nfsroot=${nfsserverip}:${nfsroot},v3,tcp\0"\ > + "bootcmd_net=run bootargs_base bootargs_nfs; " \ > + "tftpboot ${loadaddr} ${kernel}; bootm\0" \ > + "bootcmd_SD=run bootcmd_SD1 bootcmd_SD2\0" \ Indentation by TAB only, please. [Check & fix globally, please.] > + "ethaddr=00:00:00:00:00:00\0" \ > + "ipaddr=192.168.1.244\0" \ > + "serverip=192.168.1.132\0" \ > + "nfsserverip=192.168.1.160\0" \ NAK!! We do not allow network parameters in the default environment (and especially not broken/incorrect MAC addresses. > + "nfsroot=/srv/home/david/Devel/Sandboxes/LEL/XDH/nfsroot\0" \ > + "kernel=linux-2.6.31-prtlvt2.uImage\0" \ Do you think it makes any sense to your customers to encode your local settings like "/srv/home/david/Devel/Sandboxes/LEL/XDH/nfsroot" in the default environment? I doubt that. > +#define CONFIG_ARP_TIMEOUT 200UL Is this really needed on your hardware? > +#define CONFIG_ENV_SECT_SIZE (128 * 1024) > +#define CONFIG_ENV_SIZE CONFIG_ENV_SECT_SIZE Please use TABs for vertical alignment. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de If some day we are defeated, well, war has its fortunes, good and bad. -- Commander Kor, "Errand of Mercy", stardate 3201.7 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot