Dear David Jander,
In message 00b6eb8822a36b67a5e6154fd256e51e605fe47a.1282213859.git.david@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