Skip to contentSkip to conversation

Add support for PRTLVT2 boards

PATCH New David Jander 4 participants 16 messages Aug 19, 2010 – Aug 20, 2010

Subsystems (2)
Arm (32-bit) Arm i.MX and Layerscape
lore
Share & tools
lore

David Jander

author
#0 lore raw

cover Add support for PRTLVT2 boards

The following patches add support for PRTLVT2 based boards (i.MX515 SoC).

[PATCH 1/4] MX51: iomux: Added support for mxc_iomux_set_input()
[PATCH 2/4] MX51: Added missing pin definition
[PATCH 3/4] mc13982 driver: corrected/added some definitions according to latest user-manual [PATCH 4/4] Added initial support for PRTLVT2-based boards.

Signed-off-by David Jander <[email protected]>
author
#1 lore raw

PATCH 1/4 MX51: iomux: Added support for mxc_iomux_set_input()

Signed-off-by David Jander <[email protected]>
---
 arch/arm/cpu/armv7/mx51/iomux.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)
arch/arm/cpu/armv7/mx51/iomux.c +7 -1
index 62b2954..fb48f1c 100644
@@ -34,7 +34,7 @@ enum iomux_reg_addr {
 	IOMUXSW_MUX_CTL = IOMUXC_BASE_ADDR,
 	IOMUXSW_MUX_END = IOMUXC_BASE_ADDR + MUX_I_END,
 	IOMUXSW_PAD_CTL = IOMUXC_BASE_ADDR + PAD_I_START,
-	IOMUXSW_INPUT_CTL = IOMUXC_BASE_ADDR,
+	IOMUXSW_INPUT_CTL = IOMUXC_BASE_ADDR + INPUT_CTL_START,
 };
 
 #define MUX_PIN_NUM_MAX (((MUX_I_END - MUX_I_START) >> 2) + 1)

@@ -164,3 +164,9 @@ unsigned int mxc_iomux_get_pad(iomux_pin_name_t pin)
 	u32 pad_reg = get_pad_reg(pin);
 	return readl(pad_reg);
 }
+
+void mxc_iomux_set_input(iomux_input_select_t input, u32 config)
+{
+	u32 pad_reg = IOMUXSW_INPUT_CTL+(input*4);
+	writel(config, pad_reg);
+}
Signature · 1 line
1.6.3.3
author
#2 lore raw

PATCH 2/4 MX51: Added missing pin definition

Signed-off-by David Jander <[email protected]>
---
 arch/arm/include/asm/arch-mx51/mx51_pins.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
arch/arm/include/asm/arch-mx51/mx51_pins.h +1 -0
index ca26f41..c443f13 100644
@@ -320,6 +320,7 @@ enum iomux_pins {
 	MX51_PIN_DISP1_DAT22 = _MXC_BUILD_NON_GPIO_PIN(0x324, 0x724),
 	MX51_PIN_DISP1_DAT23 = _MXC_BUILD_NON_GPIO_PIN(0x328, 0x728),
 	MX51_PIN_DI1_PIN3 = _MXC_BUILD_NON_GPIO_PIN(0x32C, 0x72C),
+	MX51_PIN_DI1_DISP_CLK = _MXC_BUILD_NON_GPIO_PIN(0, 0x730), /* No MUX register!! */
 	MX51_PIN_DI1_PIN2 = _MXC_BUILD_NON_GPIO_PIN(0x330, 0x734),
 	MX51_PIN_DI_GP1 = _MXC_BUILD_NON_GPIO_PIN(0x334, 0x73C),
 	MX51_PIN_DI_GP2 = _MXC_BUILD_NON_GPIO_PIN(0x338, 0x740),
Signature · 1 line
1.6.3.3
author
#3 lore raw

PATCH 3/4 mc13982 driver: corrected/added some definitions according to latest user-manual

Signed-off-by David Jander <[email protected]>
---
 include/fsl_pmic.h |    2 +-
 include/mc13892.h  |   41 +++++++++++++++++++++++++++++++++--------
 2 files changed, 34 insertions(+), 9 deletions(-)
include/fsl_pmic.h +1 -1
index e3abde6..2f2aa7d 100644
@@ -112,7 +112,7 @@ enum {
 #define GPO4STBY	(1 << 13)
 #define PWGT1SPIEN	(1 << 15)
 #define PWGT2SPIEN	(1 << 16)
-#define PWUP		(1 << 21)
+#define GPO4ADIN	(1 << 21)
 
 /* Power Control 0 */
 #define COINCHEN	(1 << 23)
include/mc13892.h +33 -8

68 lines not on this page · raw message

Signature · 1 line
1.6.3.3
author
#4 lore raw

PATCH 4/4 Added initial support for PRTLVT2-based boards.

Signed-off-by David Jander <[email protected]>
---
 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
board/Protonic/prtlvt2/Makefile new file +48 -0
new file mode 100644
index 0000000..e1f1157
@@ -0,0 +1,48 @@
+#
+# Copyright (C) 2007, Guennadi Liakhovetski <[email protected]>
+#
+# (C) Copyright 2009 Freescale Semiconductor, Inc.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+# MA 02111-1307 USA
+#
+
+include $(TOPDIR)/config.mk
+
+LIB	= $(obj)lib$(BOARD).a
+
+COBJS	:= prtlvt2.o
+
+SRCS	:= $(SOBJS:.o=.S) $(COBJS:.o=.c)
+OBJS	:= $(addprefix $(obj),$(COBJS))
+SOBJS	:= $(addprefix $(obj),$(SOBJS))
+
+$(LIB):	$(obj).depend $(OBJS) $(SOBJS)
+	$(AR) $(ARFLAGS) $@ $(OBJS) $(SOBJS)
+
+clean:
+	rm -f $(SOBJS) $(OBJS)
+
+distclean:	clean
+	rm -f $(LIB) core *.bak .depend
+
+#########################################################################
+
+# defines $(obj).depend target
+include $(SRCTREE)/rules.mk
+
+sinclude $(obj).depend
+
+#########################################################################
board/Protonic/prtlvt2/config.mk new file +25 -0
new file mode 100644
index 0000000..af70ec2
@@ -0,0 +1,25 @@
+#
+# Copyright 2009 Freescale Semiconductor, Inc. All Rights Reserved.
+#
+# See file CREDITS for list of people who contributed to this
+# project.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+# MA 02111-1307 USA
+#
+
+LDSCRIPT = $(CPUDIR)/$(SOC)/u-boot.lds
+TEXT_BASE = 0x97800000
+IMX_CONFIG = $(SRCTREE)/board/$(BOARDDIR)/imximage.cfg
board/Protonic/prtlvt2/imximage.cfg new file +171 -0

171 lines not on this page · raw message

board/Protonic/prtlvt2/prtlvt2.c new file +513 -0

513 lines not on this page · raw message

board/Protonic/prtlvt2/prtlvt2.h new file +50 -0

50 lines not on this page · raw message

boards.cfg +1 -0
index b82f530..f3f9f02 100644
@@ -46,6 +46,7 @@ pm9263		arm	arm926ejs	-		ronetix		at91
 suen3		arm	arm926ejs	km_arm		keymile		kirkwood
 rd6281a		arm	arm926ejs	-		Marvell		kirkwood
 mx51evk		arm	armv7		mx51evk		freescale	mx51
+prtlvt2		arm	armv7		prtlvt2		Protonic	mx51
 actux1		arm	ixp
 actux2		arm	ixp
 actux3		arm	ixp
include/configs/prtlvt2.h new file +203 -0

203 lines not on this page · raw message

Signature · 1 line
1.6.3.3
NAK
#5 lore raw

PATCH 4/4 Added initial support for PRTLVT2-based boards.

Dear David Jander,

Quoted 15 lines · In message <00b6eb8822a36b67a5e6154fd256e51e605fe47a.1282213859.git.david@proto…
 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.

...

board/Protonic/prtlvt2/prtlvt2.c line 87

Quoted 3 lines
board/Protonic/prtlvt2/prtlvt2.c new file +3 -0
+	/* 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.

...

board/Protonic/prtlvt2/prtlvt2.c line 308

Quoted 2 lines
board/Protonic/prtlvt2/prtlvt2.c new file +2 -0
+	/* 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.

Quoted 4 lines
board/Protonic/prtlvt2/prtlvt2.c new file +4 -0
+	pmic_reg_write(REG_CHARGE, VCHRG0 | VCHRG1 /*| VCHRG2 */ |
+		ICHRG0 | ICHRG1 |  ICHRG2 | ICHRG3 | FETOVRD |
+		PLIM0 /* |  PLIM1 */ |
+	    CHRGLEDEN | CHGAUTOB | CYCLB);

board/Protonic/prtlvt2/prtlvt2.c line 322

Quoted 10 lines
board/Protonic/prtlvt2/prtlvt2.c new file +10 -0
+	
+	/* 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!

board/Protonic/prtlvt2/prtlvt2.c line 323

Quoted 1 line
board/Protonic/prtlvt2/prtlvt2.c new file +1 -0
+	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.

Quoted 5 lines
board/Protonic/prtlvt2/prtlvt2.c new file +5 -0
+	/* Set core voltage to 1.1V */
+	val = pmic_reg_read(REG_SW_0);
+	val = (val & (~0x80001F)) | 0x14;
+	pmic_reg_write(REG_SW_0, val);
+

board/Protonic/prtlvt2/prtlvt2.c line 339

Quoted 10 lines
board/Protonic/prtlvt2/prtlvt2.c new file +10 -0
+	/* 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?

board/Protonic/prtlvt2/prtlvt2.c line 343

Quoted 3 lines
board/Protonic/prtlvt2/prtlvt2.c new file +3 -0
+	/* Raise the core frequency to 800MHz */
+	/* printf("Core at 400 MHz!\n"); */
+	/* writel(0x1, &mxc_ccm->cacrr); */

Please remove dead code.

board/Protonic/prtlvt2/prtlvt2.c line 369

Quoted 7 lines
board/Protonic/prtlvt2/prtlvt2.c new file +7 -0
+#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.

board/Protonic/prtlvt2/prtlvt2.c line 391

Quoted 8 lines
board/Protonic/prtlvt2/prtlvt2.c new file +8 -0
+	/* 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.

...

include/configs/prtlvt2.h line 142

Quoted 12 lines
include/configs/prtlvt2.h new file +12 -0
+#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.]

include/configs/prtlvt2.h line 150

Quoted 4 lines
include/configs/prtlvt2.h new file +4 -0
+		"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.

include/configs/prtlvt2.h line 152

Quoted 2 lines
include/configs/prtlvt2.h new file +2 -0
+		"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.

Quoted 1 line

+#define CONFIG_ARP_TIMEOUT 200UL

Is this really needed on your hardware?

include/configs/prtlvt2.h line 200

Quoted 2 lines
include/configs/prtlvt2.h new file +2 -0
+#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

Signature · 6 lines
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: wd at denx.de
If some day we are defeated, well, war has  its  fortunes,  good  and
bad.
	-- Commander Kor, "Errand of Mercy", stardate 3201.7
#6 lore raw

PATCH 4/4 Added initial support for PRTLVT2-based boards.

Dear Wolfgang,

Quoted 9 lines · Wolfgang Denk

[...]

Quoted 6 lines
 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.

Oops. Thanks for pointing out. Will add maintainer info.

Quoted 7 lines
Quoted 5 lines
(quoted diff) +2 -0
+	/* 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},

(quoted diff) +1 -0
+	{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.

Ok. Will fix that.

Quoted 5 lines
Quoted 3 lines
(quoted diff) +2 -0
+	/* 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.

Ok.

Quoted 5 lines

[...]

board/Protonic/prtlvt2/prtlvt2.c line 323

Quoted 1 line
board/Protonic/prtlvt2/prtlvt2.c new file +1 -0
+	pmic_reg_write(REG_POWER_MISC, GPO4ADIN);

It would really be great if someone cold clean up this mess in "include/fsl_pmic.h"

Yes, but fixing it will probably break a lot of other code. Am I supposed to do that and fix everything that I can find breaking and hope for the rest (which I can't test myself)?

Quoted 1 line

Using an "enum" for register definitions is just horrible.

I agree.

Quoted 2 lines

I am well aware that you did not introduce this code, but reading this feels is if my nails are rolling up.

I agree. Should I propose a fix first (could be a lot of work)?

Quoted 10 lines
Quoted 10 lines
board/Protonic/prtlvt2/prtlvt2.c new file +10 -0
+	/* 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);
+
Quoted 8 lines

board/Protonic/prtlvt2/prtlvt2.c line 339

Quoted 5 lines
board/Protonic/prtlvt2/prtlvt2.c new file +5 -0
+	/* 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?

Yes I do. It was copied almost literally from the MX51EVK code, and somehow I think this should go elsewhere anyway....
I'll see what I can come up with.

Quoted 5 lines

board/Protonic/prtlvt2/prtlvt2.c line 343

Quoted 3 lines
board/Protonic/prtlvt2/prtlvt2.c new file +3 -0
+	/* Raise the core frequency to 800MHz */
+	/* printf("Core at 400 MHz!\n"); */
+	/* writel(0x1, &mxc_ccm->cacrr); */

Please remove dead code.

Ok.

Quoted 9 lines

board/Protonic/prtlvt2/prtlvt2.c line 369

Quoted 7 lines
board/Protonic/prtlvt2/prtlvt2.c new file +7 -0
+#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.

Ok.

Quoted 11 lines

board/Protonic/prtlvt2/prtlvt2.c line 391

Quoted 8 lines
board/Protonic/prtlvt2/prtlvt2.c new file +8 -0
+	/* 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.

Again, this is copy/pasted from MX51EVK code somewhere. Will try to improve this.

Quoted 16 lines

...

include/configs/prtlvt2.h line 142

Quoted 12 lines
include/configs/prtlvt2.h new file +12 -0
+#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.]

Ok.

Quoted 9 lines

include/configs/prtlvt2.h line 150

Quoted 4 lines
include/configs/prtlvt2.h new file +4 -0
+		"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.

Ooops. This wasn't meant to be here :-(
Anyway, I am currently working on implementing access to SPI-NOR-flash, and this was there only for convinience while developing on one board. It was meant to be removed, though.
Btw, I am also trying to fix the awfully broken mxc_spi driver. The spi_xfer() function assumes 32-bit word transfers (because the hardware does it like this, and FSL didn't bother adapting the driver to linux/u-boot spi conventions), so I need to introduce a second spi_xfer_fsl() function for all existing, but broken device drivers that assume fsl-byte-ordering (like the PMIC driver) and fix the original spi_xfer(). Is that acceptable?

Quoted 2 lines

include/configs/prtlvt2.h line 152

Quoted 2 lines
include/configs/prtlvt2.h new file +2 -0
+		"nfsroot=/srv/home/david/Devel/Sandboxes/LEL/XDH/nfsroot\0" \
+		"kernel=linux-2.6.31-prtlvt2.uImage\0" \

This also wasn't supposed to be here....

Quoted 3 lines
Quoted 1 line

+#define CONFIG_ARP_TIMEOUT 200UL

Is this really needed on your hardware?

Honestly I didn't even know what this is for. It's from MX51EVK.

Quoted 4 lines

include/configs/prtlvt2.h line 200

Quoted 2 lines
include/configs/prtlvt2.h new file +2 -0
+#define CONFIG_ENV_SECT_SIZE    (128 * 1024)
+#define CONFIG_ENV_SIZE		CONFIG_ENV_SECT_SIZE

Please use TABs for vertical alignment.

Excuse this dumb question about u-boot coding-style: Are tabs supposed to be used 8-spaces wide?

And I thought my version was much cleaner than the MX51EVK BSP which is already in u-boot.... :-(

Best regards,

Signature · 2 lines
David Jander
Protonic Holland.
#7 lore raw

PATCH 4/4 Added initial support for PRTLVT2-based boards.

In reply to Wolfgang Denk

Wolfgang Denk wrote:

Hi Wolfgang,

Quoted 9 lines

board/Protonic/prtlvt2/prtlvt2.c line 323

Quoted 1 line
board/Protonic/prtlvt2/prtlvt2.c new file +1 -0
+	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.

Well, that's me ! I am aware it sounds crazy. However, the registers on the pmic are not memory mapped and the access is done via SPI. The pmic manuals (MC13783, MC13892,) describe the SPI frame (ok, frame is probably a big word..it is only a 32 bit) and one of the field is the internal register number.

Using a structure looks like to me even stranger. We have to use some kind of offsetof() to get the index number back and put it inside the SPI data.

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#8 lore raw

PATCH 4/4 Added initial support for PRTLVT2-based boards.

In reply to David Jander
Quoted 2 lines · David Jander
Signed-off-by David Jander <[email protected]>

---

Hi David,

in addition to Wolfgang's comments:

board/Protonic/prtlvt2/prtlvt2.c line 46

Quoted 2 lines
board/Protonic/prtlvt2/prtlvt2.c new file +2 -0
+static u32 system_rev;
+struct io_board_ctrl *mx51_io_board;

Structure is not used, and probably does not match your board. You should drop it.

board/Protonic/prtlvt2/prtlvt2.c line 68

Quoted 7 lines
board/Protonic/prtlvt2/prtlvt2.c new file +7 -0
+#define POUT_HS (PAD_CTL_DRV_HIGH | PAD_CTL_SRE_FAST)
+#define POUT_MS (PAD_CTL_DRV_MAX | PAD_CTL_SRE_FAST)
+#define POUT_LS (PAD_CTL_DRV_MEDIUM)
+#define PIN_HYS (PAD_CTL_HYS_ENABLE)
+#define PIN_HYS_PULL (PAD_CTL_HYS_ENABLE | PAD_CTL_PKE_ENABLE | PAD_CTL_PUE_PULL)
+#define PIN_HYS_KEEP (PAD_CTL_HYS_ENABLE | PAD_CTL_PKE_ENABLE)
+#define PIO_OD  (PIN_HYS_PULL | PAD_CTL_22K_PU | PAD_CTL_ODE_OPENDRAIN_ENABLE | PAD_CTL_DRV_MEDIUM)

Consider to put these defines in include/asm/arch-mx51/iomux.h. They could be usefult for other boards, too.

board/Protonic/prtlvt2/prtlvt2.c line 344

Quoted 4 lines
board/Protonic/prtlvt2/prtlvt2.c new file +4 -0
+	/* Raise the core frequency to 800MHz */
+	/* printf("Core at 400 MHz!\n"); */
+	/* writel(0x1, &mxc_ccm->cacrr); */
+	writel(0x0, &mxc_ccm->cacrr);

Comment is misleading. Remove what is not needed.

board/Protonic/prtlvt2/prtlvt2.c line 452

Quoted 2 lines
board/Protonic/prtlvt2/prtlvt2.c new file +2 -0
+	/* Setup other io's */
+	for(t=0; other_io_conf[t].pin>=0; t++) {

Add spaces when needed.

Quoted 1 line

+#ifdef BOARD_LATE_INIT

Probably it is better (and in mx51evk, too) to remove the #ifdef, because we need to initialize the pmic, else some functionalities cannot work. It is mandatory that power_init is called.

Quoted 17 lines
board/Protonic/prtlvt2/prtlvt2.h +16 -0
+#ifndef __BOARD_FREESCALE_MX51_EVK_H__
+#define __BOARD_FREESCALE_MX51_EVK_H__
+
+#ifndef __ASSEMBLY__
+struct io_board_ctrl {
+	u16 led_ctrl;		/* 0x00 */
+	u16 resv1[0x03];
+	u16 sb_stat;		/* 0x08 */
+	u16 resv2[0x03];
+	u16 int_stat;		/* 0x10 */
+	u16 resv3[0x07];
+	u16 int_rest;		/* 0x20 */
+	u16 resv4[0x0B];
+	u16 int_mask;		/* 0x38 */
+	u16 resv5[0x03];
+	u16 id1;		/* 0x40 */

board/Protonic/prtlvt2/prtlvt2.h line 48

Quoted 10 lines
board/Protonic/prtlvt2/prtlvt2.h +10 -0
+	u16 resv6[0x03];
+	u16 id2;		/* 0x48 */
+	u16 resv7[0x03];
+	u16 version;		/* 0x50 */
+	u16 resv8[0x03];
+	u16 id3;		/* 0x58 */
+	u16 resv9[0x03];
+	u16 sw_reset;		/* 0x60 */
+};
+#endif

Is this structure really used ? I have not seen in code. Or does it come only from mx51evk ? You can remove the whole file, if it is not needed.

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#9 lore raw

PATCH 4/4 Added initial support for PRTLVT2-based boards.

Quoted 8 lines · Stefano Babic

Hi David,

in addition to Wolfgang's comments:

board/Protonic/prtlvt2/prtlvt2.c line 46

Quoted 2 lines
board/Protonic/prtlvt2/prtlvt2.c new file +2 -0
+static u32 system_rev;
+struct io_board_ctrl *mx51_io_board;

Structure is not used, and probably does not match your board. You should drop it.

Ok, I was already wondering what this did here.... I'll just remove it.

Quoted 4 lines
Quoted 4 lines
board/Protonic/prtlvt2/prtlvt2.c new file +4 -0
+#define POUT_HS (PAD_CTL_DRV_HIGH | PAD_CTL_SRE_FAST)
+#define POUT_MS (PAD_CTL_DRV_MAX | PAD_CTL_SRE_FAST)
+#define POUT_LS (PAD_CTL_DRV_MEDIUM)
+#define PIN_HYS (PAD_CTL_HYS_ENABLE)
Quoted 8 lines
Quoted 5 lines

+#define PIN_HYS_PULL (PAD_CTL_HYS_ENABLE | PAD_CTL_PKE_ENABLE | PAD_CTL_PUE_PULL) +#define PIN_HYS_KEEP (PAD_CTL_HYS_ENABLE | PAD_CTL_PKE_ENABLE)
+#define PIO_OD (PIN_HYS_PULL | PAD_CTL_22K_PU | PAD_CTL_ODE_OPENDRAIN_ENABLE | PAD_CTL_DRV_MEDIUM)

Consider to put these defines in include/asm/arch-mx51/iomux.h. They could be usefult for other boards, too.

Hmmm. In that case they should have other names, and one should probably make the set complete with all combinations, and I doubt it'll make sense anymore then....
This subset of possibilities is too specific to our board I fear. I had already been complaining about some seemingly arbitrary combinations of IO-pad settings being used in SoC-driver code in the linux kernel. IMHO, one should never choose a specific combination of settings without excactly knowing it is electrically correct for that specific pin on that specific board... not only because it works, but also from EMC considerations. I also think, that in theory this would make the kernel board-specific in that case... not something one would want I believe. For that reason, I keep thinking that correct and thorough IO-pad setup specific for each board should be done in the boot-loader.... never in the kernel.

OTOH, maybe one could come up with some commonly-used combinations... but which ones?

Quoted 6 lines

board/Protonic/prtlvt2/prtlvt2.c line 344

Quoted 4 lines
board/Protonic/prtlvt2/prtlvt2.c new file +4 -0
+	/* Raise the core frequency to 800MHz */
+	/* printf("Core at 400 MHz!\n"); */
+	/* writel(0x1, &mxc_ccm->cacrr); */
+	writel(0x0, &mxc_ccm->cacrr);

Comment is misleading. Remove what is not needed.

Sorry, I was too lazy to make a configuration option out of this... will fix it.

Quoted 4 lines

board/Protonic/prtlvt2/prtlvt2.c line 452

Quoted 2 lines
board/Protonic/prtlvt2/prtlvt2.c new file +2 -0
+	/* Setup other io's */
+	for(t=0; other_io_conf[t].pin>=0; t++) {

Add spaces when needed.

Ok.

Quoted 5 lines
Quoted 1 line

+#ifdef BOARD_LATE_INIT

Probably it is better (and in mx51evk, too) to remove the #ifdef, because we need to initialize the pmic, else some functionalities cannot work. It is mandatory that power_init is called.

I agree. I had always been "about to remove it" ;-)

Quoted 23 lines
Quoted 23 lines
(quoted diff) +0 -0

b/board/Protonic/prtlvt2/prtlvt2.h +#ifndef
__BOARD_FREESCALE_MX51_EVK_H__

board/Protonic/prtlvt2/prtlvt2.h new file +20 -0
+#define __BOARD_FREESCALE_MX51_EVK_H__
+
+#ifndef __ASSEMBLY__
+struct io_board_ctrl {
+	u16 led_ctrl;		/* 0x00 */
+	u16 resv1[0x03];
+	u16 sb_stat;		/* 0x08 */
+	u16 resv2[0x03];
+	u16 int_stat;		/* 0x10 */
+	u16 resv3[0x07];
+	u16 int_rest;		/* 0x20 */
+	u16 resv4[0x0B];
+	u16 int_mask;		/* 0x38 */
+	u16 resv5[0x03];
+	u16 id1;		/* 0x40 */
+	u16 resv6[0x03];
+	u16 id2;		/* 0x48 */
+	u16 resv7[0x03];
+	u16 version;		/* 0x50 */
+	u16 resv8[0x03];
Quoted 8 lines

board/Protonic/prtlvt2/prtlvt2.h line 48

Quoted 5 lines
board/Protonic/prtlvt2/prtlvt2.h new file +5 -0
+	u16 id3;		/* 0x58 */
+	u16 resv9[0x03];
+	u16 sw_reset;		/* 0x60 */
+};
+#endif

Is this structure really used ? I have not seen in code. Or does it come only from mx51evk ? You can remove the whole file, if it is not needed.

No idea what it does. It's copied from MX51EVK. I will try removing it.

Best regards,

Signature · 2 lines
David Jander
Protonic Holland.
#10 lore raw

PATCH 4/4 Added initial support for PRTLVT2-based boards.

Quoted 5 lines · David Jander

Hmmm. In that case they should have other names, and one should probably make the set complete with all combinations, and I doubt it'll make sense anymore then....
This subset of possibilities is too specific to our board I fear.

Ok, I understand. I have thought it coul be easier to other one to use the same set of combinations.

Quoted 5 lines
Quoted 2 lines

Is this structure really used ? I have not seen in code. Or does it come only from mx51evk ? You can remove the whole file, if it is not needed.

No idea what it does. It's copied from MX51EVK. I will try removing it.

This structure comes from older Freescale's source and makes sense only on another board. I have already merged a patch to drop the usage of the structure inside the board functions, but rather the mx51evk.h is still in mainline. I will cleanup in mx51evk, too.

Best regards,
Stefano

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#11 lore raw

PATCH 2/4 MX51: Added missing pin definition

In reply to David Jander
Quoted 10 lines · David Jander
Signed-off-by David Jander <[email protected]>
---
 arch/arm/include/asm/arch-mx51/mx51_pins.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
arch/arm/include/asm/arch-mx51/mx51_pins.h +0 -0
index ca26f41..c443f13 100644

arch/arm/include/asm/arch-mx51/mx51_pins.h enum iomux_pins { · line 323

Quoted 4 lines
arch/arm/include/asm/arch-mx51/mx51_pins.h +1 -0
@@ -320,6 +320,7 @@ enum iomux_pins {
 	MX51_PIN_DISP1_DAT22 = _MXC_BUILD_NON_GPIO_PIN(0x324, 0x724),
 	MX51_PIN_DISP1_DAT23 = _MXC_BUILD_NON_GPIO_PIN(0x328, 0x728),
 	MX51_PIN_DI1_PIN3 = _MXC_BUILD_NON_GPIO_PIN(0x32C, 0x72C),
+	MX51_PIN_DI1_DISP_CLK = _MXC_BUILD_NON_GPIO_PIN(0, 0x730), /* No MUX register!! */

Line too long.

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#12 lore raw

PATCH 2/4 MX51: Added missing pin definition

Quoted 6 lines · Stefano Babic
Quoted 5 lines · David Jander
Signed-off-by David Jander <[email protected]>
---
 arch/arm/include/asm/arch-mx51/mx51_pins.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
Quoted 13 lines
Quoted 11 lines
(quoted diff) +0 -0

b/arch/arm/include/asm/arch-mx51/mx51_pins.h index ca26f41..c443f13 100644

arch/arm/include/asm/arch-mx51/mx51_pins.h +1 -0
@@ -320,6 +320,7 @@ enum iomux_pins {
 	MX51_PIN_DISP1_DAT22 = _MXC_BUILD_NON_GPIO_PIN(0x324, 0x724),
 	MX51_PIN_DISP1_DAT23 = _MXC_BUILD_NON_GPIO_PIN(0x328, 0x728),
 	MX51_PIN_DI1_PIN3 = _MXC_BUILD_NON_GPIO_PIN(0x32C, 0x72C),
+	MX51_PIN_DI1_DISP_CLK = _MXC_BUILD_NON_GPIO_PIN(0, 0x730), /* No MUX

register!! */

Line too long.

Ok, will fix.

Best regards,

Signature · 2 lines
David Jander
Protonic Holland.
#13 lore raw

PATCH 1/4 MX51: iomux: Added support for mxc_iomux_set_input()

In reply to David Jander
Quoted 2 lines · David Jander
Signed-off-by David Jander <[email protected]>

---

Hi Dave,

Quoted 3 lines
 arch/arm/cpu/armv7/mx51/iomux.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

Probably iut is better you set a more useful comment in your commit.
Instead of "Added support for mxc_iomux_set_input()", you can explain
which is the new feature you provide. Something to explain you add a utility for the "daisy chain" pins, to control the input path to a module when the module can be connected to more as one pin.

The patch is part of a series. However, I can see only the first two patches. Is there something missing ? I do not see any relation between these two patches, too.

arch/arm/cpu/armv7/mx51/iomux.c unsigned int mxc_iomux_get_pad(iomux_pin_name_t pin) · line 170

Quoted 3 lines
arch/arm/cpu/armv7/mx51/iomux.c +3 -0
+void mxc_iomux_set_input(iomux_input_select_t input, u32 config)
+{
+	u32 pad_reg = IOMUXSW_INPUT_CTL+(input*4);

Code styling, you should add spaces:

	u32 pad_reg = IOMUXSW_INPUT_CTL + (input * 4);

arch/arm/cpu/armv7/mx51/iomux.c unsigned int mxc_iomux_get_pad(iomux_pin_name_t pin) · line 172

Quoted 2 lines
arch/arm/cpu/armv7/mx51/iomux.c +2 -0
+	writel(config, pad_reg);
+}

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#14 lore raw

PATCH 1/4 MX51: iomux: Added support for mxc_iomux_set_input ()

Hi Stefano,

Quoted 10 lines · Stefano Babic

Hi Dave,

Quoted 2 lines
 arch/arm/cpu/armv7/mx51/iomux.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

Probably iut is better you set a more useful comment in your commit.
Instead of "Added support for mxc_iomux_set_input()", you can explain
which is the new feature you provide. Something to explain you add a utility for the "daisy chain" pins, to control the input path to a module when the module can be connected to more as one pin.

Hmmm. I thought it was a trivial and obviously missing function to make iomux.c complete. Someone just needed to write it. I didn't think it needed any more explaining than that, but I'll do it in the next version of the patch set (will take a while).

Quoted 3 lines

The patch is part of a series. However, I can see only the first two patches. Is there something missing ? I do not see any relation between these two patches, too.

I sent 4 patches and received them all on the mailing-list. Are you sure you miss two of them?
The first three patches introduce some minimal fixes/additions in order to implement the BSP for PRTLVT2 boards (patch 4/4).

Quoted 7 lines

arch/arm/cpu/armv7/mx51/iomux.c unsigned int mxc_iomux_get_pad(iomux_pin_name_t pin) · line 170

Quoted 3 lines
arch/arm/cpu/armv7/mx51/iomux.c +3 -0
+void mxc_iomux_set_input(iomux_input_select_t input, u32 config)
+{
+	u32 pad_reg = IOMUXSW_INPUT_CTL+(input*4);

Code styling, you should add spaces:

	u32 pad_reg = IOMUXSW_INPUT_CTL + (input * 4);

Ok.

Best regards,

Signature · 2 lines
David Jander
Protonic Holland.
#15 lore raw

PATCH 1/4 MX51: iomux: Added support for mxc_iomux_set_input()

Quoted 2 lines · David Jander

Hi Stefano,

Hi David,

Quoted 5 lines

Hmmm. I thought it was a trivial and obviously missing function to make iomux.c complete.
Someone just needed to write it. I didn't think it needed
any more explaining than that, but I'll do it in the next version of the patch set (will take a while).

Not everybody works with the MX51, and if we do not write some additional info it is difficult to find a relation in the reference manual ;-).

Quoted 2 lines

I sent 4 patches and received them all on the mailing-list. Are you sure you miss two of them?

I have found them on gmane now, thanks.

Best regards,
Stefano

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================