Skip to contentSkip to conversation

Adding support for MX35

U-Boot Superseded Stefano Babic 3 participants 46 messages Jan 20, 2011 – Feb 1, 2011

Subsystems (6)
lore
Share & tools
lore
#0 lore raw

Patchset V2 contain patches for SPI sent previously independently to ML and additional patches for I2C.

#1 lore raw

PATCH v2 1/11 Add support for MX35 processor

The patch adds basic support for the Freescale's i.MX35 (arm1136 based) processor.

---
Changes:

Wolfgang Denk:
- Drop __REG macros
- sort lists
- coding styles
- iomux functions must return void
- fix license headers

 arch/arm/cpu/arm1136/mx35/Makefile           |   49 +++
 arch/arm/cpu/arm1136/mx35/generic.c          |  463 ++++++++++++++++++++++++++
 arch/arm/cpu/arm1136/mx35/iomux.c            |  116 +++++++
 arch/arm/cpu/arm1136/mx35/timer.c            |  120 +++++++
 arch/arm/include/asm/arch-mx35/asm-offsets.h |   21 ++
 arch/arm/include/asm/arch-mx35/clock.h       |   45 +++
 arch/arm/include/asm/arch-mx35/crm_regs.h    |  270 +++++++++++++++
 arch/arm/include/asm/arch-mx35/imx-regs.h    |  336 +++++++++++++++++++
 arch/arm/include/asm/arch-mx35/iomux.h       |  303 +++++++++++++++++
 arch/arm/include/asm/arch-mx35/mx35_pins.h   |  355 ++++++++++++++++++++
 arch/arm/include/asm/arch-mx35/sys_proto.h   |   31 ++
 11 files changed, 2109 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/cpu/arm1136/mx35/Makefile
 create mode 100644 arch/arm/cpu/arm1136/mx35/generic.c
 create mode 100644 arch/arm/cpu/arm1136/mx35/iomux.c
 create mode 100644 arch/arm/cpu/arm1136/mx35/timer.c
 create mode 100644 arch/arm/include/asm/arch-mx35/asm-offsets.h
 create mode 100644 arch/arm/include/asm/arch-mx35/clock.h
 create mode 100644 arch/arm/include/asm/arch-mx35/crm_regs.h
 create mode 100644 arch/arm/include/asm/arch-mx35/imx-regs.h
 create mode 100644 arch/arm/include/asm/arch-mx35/iomux.h
 create mode 100644 arch/arm/include/asm/arch-mx35/mx35_pins.h
 create mode 100644 arch/arm/include/asm/arch-mx35/sys_proto.h
arch/arm/cpu/arm1136/mx35/Makefile new file +49 -0
new file mode 100644
index 0000000..9b4cf76
@@ -0,0 +1,49 @@
+#
+# (C) Copyright 2000-2006
+# Wolfgang Denk, DENX Software Engineering, wd at denx.de.
+#
+# (C) Copyright 2008-2009 Freescale Semiconductor, Inc.
+#
+# 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
+#
+
+include $(TOPDIR)/config.mk
+
+LIB	= $(obj)lib$(SOC).o
+
+COBJS	+= generic.o
+COBJS	+= timer.o
+COBJS	+= iomux.o
+
+SRCS	:= $(SOBJS:.o=.S) $(COBJS:.o=.c)
+OBJS	:= $(addprefix $(obj),$(SOBJS) $(COBJS))
+
+all:	$(obj).depend $(LIB)
+
+$(LIB):	$(OBJS)
+	$(call cmd_link_o_target, $(OBJS))
+
+#########################################################################
+
+# defines $(obj).depend target
+include $(SRCTREE)/rules.mk
+
+sinclude $(obj).depend
+
+#########################################################################
arch/arm/cpu/arm1136/mx35/generic.c new file +463 -0

463 lines not on this page · raw message

arch/arm/cpu/arm1136/mx35/iomux.c new file +116 -0

116 lines not on this page · raw message

arch/arm/cpu/arm1136/mx35/timer.c new file +120 -0

120 lines not on this page · raw message

arch/arm/include/asm/arch-mx35/asm-offsets.h new file +21 -0
new file mode 100644
index 0000000..c1dc62a
@@ -0,0 +1,21 @@
+/*
+ * needed for lowlevel_init.S
+ *
+ * These should be auto-generated
+ */
+/* CCM */
+#define CLKCTL_CCMR		0x00
+#define CLKCTL_PDR0		0x04
+#define CLKCTL_PDR1		0x08
+#define CLKCTL_PDR2		0x0C
+#define CLKCTL_PDR3		0x10
+#define CLKCTL_PDR4		0x14
+#define CLKCTL_RCSR		0x18
+#define CLKCTL_MPCTL		0x1C
+#define CLKCTL_PPCTL		0x20
+#define CLKCTL_ACMR		0x24
+#define CLKCTL_COSR		0x28
+#define CLKCTL_CGR0		0x2C
+#define CLKCTL_CGR1		0x30
+#define CLKCTL_CGR2		0x34
+#define CLKCTL_CGR3		0x38
arch/arm/include/asm/arch-mx35/clock.h new file +45 -0

45 lines not on this page · raw message

arch/arm/include/asm/arch-mx35/crm_regs.h new file +270 -0

270 lines not on this page · raw message

arch/arm/include/asm/arch-mx35/imx-regs.h new file +336 -0

336 lines not on this page · raw message

arch/arm/include/asm/arch-mx35/iomux.h new file +303 -0

303 lines not on this page · raw message

arch/arm/include/asm/arch-mx35/mx35_pins.h new file +355 -0

355 lines not on this page · raw message

arch/arm/include/asm/arch-mx35/sys_proto.h new file +31 -0
new file mode 100644
index 0000000..422eb52
@@ -0,0 +1,31 @@
+/*
+ * (C) Copyright 2011
+ * Stefano Babic, DENX Software Engineering, sbabic at denx.de.
+ *
+ * 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
+ */
+
+#ifndef _SYS_PROTO_H_
+#define _SYS_PROTO_H_
+
+u32 get_cpu_rev(void);
+#define is_soc_rev(rev)	((get_cpu_rev() & 0xFF) - rev)
+void sdelay(unsigned long);
+
+#endif
Signature · 1 line
1.7.1
#2 lore raw

PATCH v2 1/11 Add support for MX35 processor

Dear Stefano Babic,

Quoted 4 lines · In message <[email protected]> you wrote:

The patch adds basic support for the Freescale's i.MX35 (arm1136 based) processor.

checkpatch says:

[U-Boot] [PATCH V2 01/11] Add support for MX35 processor
total: 7 errors, 8 warnings, 2109 lines checked

Please fix.

arch/arm/cpu/arm1136/mx35/generic.c line 73

Quoted 11 lines
arch/arm/cpu/arm1136/mx35/generic.c new file +11 -0
+u32 get_cpu_rev(void)
+{
+	int reg;
+	struct iim_regs *iim =
+		(struct iim_regs *)IIM_BASE_ADDR;
+	reg = readl(&iim->iim_srev);
+	if (!reg) {
+		reg = readw(ROMPATCH_REV);
+		reg <<= 4;
+	} else
+		reg += CHIP_REV_1_0;

If there are braces for the "if", then there shall also be braces for the "else" ("Use braces in both branches.")

arch/arm/cpu/arm1136/mx35/generic.c line 194

Quoted 12 lines
arch/arm/cpu/arm1136/mx35/generic.c new file +12 -0
+u32 imx_get_uartclk(void)
+{
+	u32 freq;
+	struct ccm_regs *ccm =
+		(struct ccm_regs *)IMX_CCM_BASE;
+	u32 pdr4 = readl(&ccm->pdr4);
+
+	if (readl(&ccm->pdr3) & MXC_CCM_PDR3_UART_M_U)
+		freq = get_mcu_main_clk();
+	else
+		freq = decode_pll(readl(&ccm->ppctl),
+			CONFIG_MX35_HCLK_FREQ);

Braces needed.

arch/arm/cpu/arm1136/mx35/generic.c line 257

Quoted 8 lines
arch/arm/cpu/arm1136/mx35/generic.c new file +8 -0
+	case USB_CLK:
+		usb_prdf = (reg4 >> 25) & 0x7;
+		usb_podf = (reg4 >> 22) & 0x7;
+		if (reg4 & 0x200)
+			pll = get_mcu_main_clk();
+		else
+			pll = decode_pll(readl(&ccm->ppctl),
+				CONFIG_MX35_HCLK_FREQ);

Ditto. Please fix globally.

Quoted 15 lines

index 0000000..c1dc62a

arch/arm/include/asm/arch-mx35/asm-offsets.h new file +11 -0
@@ -0,0 +1,21 @@
+/*
+ * needed for lowlevel_init.S
+ *
+ * These should be auto-generated
+ */
+/* CCM */
+#define CLKCTL_CCMR		0x00
+#define CLKCTL_PDR0		0x04
+#define CLKCTL_PDR1		0x08
+#define CLKCTL_PDR2		0x0C
+#define CLKCTL_PDR3		0x10

arch/arm/include/asm/arch-mx35/asm-offsets.h line 21

Quoted 10 lines
arch/arm/include/asm/arch-mx35/asm-offsets.h new file +10 -0
+#define CLKCTL_PDR4		0x14
+#define CLKCTL_RCSR		0x18
+#define CLKCTL_MPCTL		0x1C
+#define CLKCTL_PPCTL		0x20
+#define CLKCTL_ACMR		0x24
+#define CLKCTL_COSR		0x28
+#define CLKCTL_CGR0		0x2C
+#define CLKCTL_CGR1		0x30
+#define CLKCTL_CGR2		0x34
+#define CLKCTL_CGR3		0x38

Indeed they should. Why don't you autogenerate these, then?

We have all the tools in place, use them.

Quoted 5 lines
arch/arm/include/asm/arch-mx35/asm-offsets.h new file +5 -0
+#define NFC_BUF_SIZE			0x1000
+#define NFC_BUFSIZE_REG_OFF             (0 + 0x00)
+#define RAM_BUFFER_ADDRESS_REG_OFF      (0 + 0x04)
+#define NAND_FLASH_ADD_REG_OFF          (0 + 0x06)
+#define NAND_FLASH_CMD_REG_OFF          (0 + 0x08)

arch/arm/include/asm/arch-mx35/asm-offsets.h

Quoted 10 lines
arch/arm/include/asm/arch-mx35/asm-offsets.h new file +10 -0
+#define NFC_CONFIGURATION_REG_OFF       (0 + 0x0A)
+#define ECC_STATUS_RESULT_REG_OFF       (0 + 0x0C)
+#define ECC_RSLT_MAIN_AREA_REG_OFF      (0 + 0x0E)
+#define ECC_RSLT_SPARE_AREA_REG_OFF     (0 + 0x10)
+#define NF_WR_PROT_REG_OFF              (0 + 0x12)
+#define NAND_FLASH_WR_PR_ST_REG_OFF     (0 + 0x18)
+#define NAND_FLASH_CONFIG1_REG_OFF      (0 + 0x1A)
+#define NAND_FLASH_CONFIG2_REG_OFF      (0 + 0x1C)
+#define UNLOCK_START_BLK_ADD_REG_OFF    (0 + 0x20)
+#define UNLOCK_END_BLK_ADD_REG_OFF      (0 + 0x22)

Why has this not been converted into a C struct?

...

arch/arm/include/asm/arch-mx35/asm-offsets.h

Quoted 12 lines
arch/arm/include/asm/arch-mx35/asm-offsets.h new file +12 -0
+typedef enum iomux_pin_config {
+	MUX_CONFIG_FUNC = 0,	/*!< used as function */
+	MUX_CONFIG_ALT1,	/*!< used as alternate function 1 */
+	MUX_CONFIG_ALT2,	/*!< used as alternate function 2 */
+	MUX_CONFIG_ALT3,	/*!< used as alternate function 3 */
+	MUX_CONFIG_ALT4,	/*!< used as alternate function 4 */
+	MUX_CONFIG_ALT5,	/*!< used as alternate function 5 */
+	MUX_CONFIG_ALT6,	/*!< used as alternate function 6 */
+	MUX_CONFIG_ALT7,	/*!< used as alternate function 7 */
+	MUX_CONFIG_SION = 0x1 << 4,	/*!< used as LOOPBACK:MUX SION bit */
+	MUX_CONFIG_GPIO = MUX_CONFIG_ALT5,	/*!< used as GPIO */
+} iomux_pin_cfg_t;

/*!< ???

Quoted 1 line

+/*!

/*! ???

...

Quoted 7 lines

+/*!
+ * Request ownership for an IO pin. This function has to be the first one
+ * being called before that pin is used. The caller has to check the
+ * return value to make sure it returns 0.
+ *
+ * @param pin a name defined by \b iomux_pin_name_t
+ * @param cfg an input function as defined in \b #iomux_pin_cfg_t

\b ???

Quoted 2 lines

+ * @param pin a name defined by \b iomux_pin_name_t
+ * @param cfg an input function as defined in \b #iomux_pin_cfg_t

"iomux_pin_name_t", but "#iomux_pin_cfg_t" ???

...

Quoted 1 line

+typedef enum iomux_pins {

...

Quoted 17 lines
arch/arm/include/asm/arch-mx35/asm-offsets.h new file +17 -0
+	MX35_PIN_A0 = _MXC_BUILD_NON_GPIO_PIN(0x28, 0x368),
+	MX35_PIN_A1 = _MXC_BUILD_NON_GPIO_PIN(0x2C, 0x36C),
+	MX35_PIN_A2 = _MXC_BUILD_NON_GPIO_PIN(0x30, 0x370),
+	MX35_PIN_A3 = _MXC_BUILD_NON_GPIO_PIN(0x34, 0x374),
+	MX35_PIN_A4 = _MXC_BUILD_NON_GPIO_PIN(0x38, 0x378),
+	MX35_PIN_A5 = _MXC_BUILD_NON_GPIO_PIN(0x3C, 0x37C),
+	MX35_PIN_A6 = _MXC_BUILD_NON_GPIO_PIN(0x40, 0x380),
+	MX35_PIN_A7 = _MXC_BUILD_NON_GPIO_PIN(0x44, 0x384),
+	MX35_PIN_A8 = _MXC_BUILD_NON_GPIO_PIN(0x48, 0x388),
+	MX35_PIN_A9 = _MXC_BUILD_NON_GPIO_PIN(0x4C, 0x38C),
+	MX35_PIN_A10 = _MXC_BUILD_NON_GPIO_PIN(0x50, 0x390),
+	MX35_PIN_MA10 = _MXC_BUILD_NON_GPIO_PIN(0x54, 0x394),
+	MX35_PIN_A11 = _MXC_BUILD_NON_GPIO_PIN(0x58, 0x398),
+	MX35_PIN_A12 = _MXC_BUILD_NON_GPIO_PIN(0x5C, 0x39C),
+	MX35_PIN_A13 = _MXC_BUILD_NON_GPIO_PIN(0x60, 0x3A0),
+	MX35_PIN_A14 = _MXC_BUILD_NON_GPIO_PIN(0x64, 0x3A4),
+	MX35_PIN_A15 = _MXC_BUILD_NON_GPIO_PIN(0x68, 0x3A8),

arch/arm/include/asm/arch-mx35/asm-offsets.h

Quoted 10 lines
arch/arm/include/asm/arch-mx35/asm-offsets.h new file +10 -0
+	MX35_PIN_A16 = _MXC_BUILD_NON_GPIO_PIN(0x6C, 0x3AC),
+	MX35_PIN_A17 = _MXC_BUILD_NON_GPIO_PIN(0x70, 0x3B0),
+	MX35_PIN_A18 = _MXC_BUILD_NON_GPIO_PIN(0x74, 0x3B4),
+	MX35_PIN_A19 = _MXC_BUILD_NON_GPIO_PIN(0x78, 0x3B8),
+	MX35_PIN_A20 = _MXC_BUILD_NON_GPIO_PIN(0x7C, 0x3BC),
+	MX35_PIN_A21 = _MXC_BUILD_NON_GPIO_PIN(0x80, 0x3C0),
+	MX35_PIN_A22 = _MXC_BUILD_NON_GPIO_PIN(0x84, 0x3C4),
+	MX35_PIN_A23 = _MXC_BUILD_NON_GPIO_PIN(0x88, 0x3C8),
+	MX35_PIN_A24 = _MXC_BUILD_NON_GPIO_PIN(0x8C, 0x3CC),
+	MX35_PIN_A25 = _MXC_BUILD_NON_GPIO_PIN(0x90, 0x3D0),

Note: the following remark is a question, NOT a change request:

Would it not be possible to reduce all these terrible lists? As far as I can tell, the list is built sequentially, with both arguments to _MXC_BUILD_NON_GPIO_PIN() being incremented by 4 for the next register. This begs for automatic code generation, doesn't it?

Best regards,

Wolfgang Denk

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-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The human race has one really effective weapon, and that is laughter.
                                                         - Mark Twain
#3 lore raw

PATCH v2 1/11 Add support for MX35 processor

Quoted 6 lines · Wolfgang Denk

Dear Stefano Babic,

Quoted 3 lines · In message <[email protected]> you wrote:

The patch adds basic support for the Freescale's i.MX35 (arm1136 based) processor.

Quoted 6 lines
Quoted 1 line

checkpatch says:

[U-Boot] [PATCH V2 01/11] Add support for MX35 processor
total: 7 errors, 8 warnings, 2109 lines checked

Mmmhh...checkpatch says:

total: 0 errors, 8 warnings, 2109 lines checked

0001-Add-support-for-MX35-processor.patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see

Warnings are only related to typedef ("do not add typedef"), so I do not know where the errors are coming from. Do you use the --no-tree option as I usually set?

Quoted 15 lines

arch/arm/cpu/arm1136/mx35/generic.c line 194

Quoted 13 lines
arch/arm/cpu/arm1136/mx35/generic.c new file +12 -0
+u32 imx_get_uartclk(void)
+{
+	u32 freq;
+	struct ccm_regs *ccm =
+		(struct ccm_regs *)IMX_CCM_BASE;
+	u32 pdr4 = readl(&ccm->pdr4);
+
+	if (readl(&ccm->pdr3) & MXC_CCM_PDR3_UART_M_U)
+		freq = get_mcu_main_clk();
+	else
+		freq = decode_pll(readl(&ccm->ppctl),
+			CONFIG_MX35_HCLK_FREQ);

Braces needed

checkpatch (and generally accepted in linux, as I can see) requires that single statements must not be surrounded by braces. checkpatch returns a warning, explaining that braces are not needed.

I see in the past some comments requiring to remove braces, but it you prefer to add them. IMHO it is better to follow the same codestyle as linux, using the same tools as checkpatch. I do not know why we have two different results from checkpatch, I try to investigate. I had prefer to use always braces in if statement, to avoid possible errors if some code is added, but since checkpatch complains about it I was convinced to remove them.

Quoted 11 lines

arch/arm/cpu/arm1136/mx35/generic.c line 257

Quoted 9 lines
arch/arm/cpu/arm1136/mx35/generic.c new file +8 -0
+	case USB_CLK:
+		usb_prdf = (reg4 >> 25) & 0x7;
+		usb_podf = (reg4 >> 22) & 0x7;
+		if (reg4 & 0x200)
+			pll = get_mcu_main_clk();
+		else
+			pll = decode_pll(readl(&ccm->ppctl),
+				CONFIG_MX35_HCLK_FREQ);

Ditto. Please fix globally.

See my previous comment. I would prefer to not have a different rule in u-boot, and not go against some provided tool (we use both checkpatch) if not strictly required.

Quoted 4 lines

Indeed they should. Why don't you autogenerate these, then?

We have all the tools in place, use them.

I will see how to use them.

Quoted 11 lines
Quoted 11 lines
arch/arm/include/asm/arch-mx35/imx-regs.h new file +9 -0
+#define NFC_BUF_SIZE			0x1000
+#define NFC_BUFSIZE_REG_OFF             (0 + 0x00)
+#define RAM_BUFFER_ADDRESS_REG_OFF      (0 + 0x04)
+#define NAND_FLASH_ADD_REG_OFF          (0 + 0x06)
+#define NAND_FLASH_CMD_REG_OFF          (0 + 0x08)
+#define NFC_CONFIGURATION_REG_OFF       (0 + 0x0A)
+#define ECC_STATUS_RESULT_REG_OFF       (0 + 0x0C)
+#define ECC_RSLT_MAIN_AREA_REG_OFF      (0 + 0x0E)
+#define ECC_RSLT_SPARE_AREA_REG_OFF     (0 + 0x10)
Quoted 8 lines

arch/arm/include/asm/arch-mx35/imx-regs.h line 186

Quoted 6 lines
arch/arm/include/asm/arch-mx35/imx-regs.h new file +6 -0
+#define NF_WR_PROT_REG_OFF              (0 + 0x12)
+#define NAND_FLASH_WR_PR_ST_REG_OFF     (0 + 0x18)
+#define NAND_FLASH_CONFIG1_REG_OFF      (0 + 0x1A)
+#define NAND_FLASH_CONFIG2_REG_OFF      (0 + 0x1C)
+#define UNLOCK_START_BLK_ADD_REG_OFF    (0 + 0x20)
+#define UNLOCK_END_BLK_ADD_REG_OFF      (0 + 0x22)

Why has this not been converted into a C struct?

I will check, I think I do not require anymore. NAND driver is in mainline and does not require them. Probably I can remove them. In the code I post now I do not use them at all, so probably I can remove them without problems.

Quoted 16 lines

...

arch/arm/include/asm/arch-mx35/iomux.h line 53

Quoted 12 lines
arch/arm/include/asm/arch-mx35/iomux.h new file +12 -0
+typedef enum iomux_pin_config {
+	MUX_CONFIG_FUNC = 0,	/*!< used as function */
+	MUX_CONFIG_ALT1,	/*!< used as alternate function 1 */
+	MUX_CONFIG_ALT2,	/*!< used as alternate function 2 */
+	MUX_CONFIG_ALT3,	/*!< used as alternate function 3 */
+	MUX_CONFIG_ALT4,	/*!< used as alternate function 4 */
+	MUX_CONFIG_ALT5,	/*!< used as alternate function 5 */
+	MUX_CONFIG_ALT6,	/*!< used as alternate function 6 */
+	MUX_CONFIG_ALT7,	/*!< used as alternate function 7 */
+	MUX_CONFIG_SION = 0x1 << 4,	/*!< used as LOOPBACK:MUX SION bit */
+	MUX_CONFIG_GPIO = MUX_CONFIG_ALT5,	/*!< used as GPIO */
+} iomux_pin_cfg_t;

/*!< ???

I forget to remove them, thanks.

Quoted 9 lines

...

Quoted 7 lines

+/*!
+ * Request ownership for an IO pin. This function has to be the first one
+ * being called before that pin is used. The caller has to check the
+ * return value to make sure it returns 0.
+ *
+ * @param pin a name defined by \b iomux_pin_name_t
+ * @param cfg an input function as defined in \b #iomux_pin_cfg_t

Quoted 6 lines

\b ???

Quoted 2 lines

+ * @param pin a name defined by \b iomux_pin_name_t
+ * @param cfg an input function as defined in \b #iomux_pin_cfg_t

"iomux_pin_name_t", but "#iomux_pin_cfg_t" ???

I'll fix them

Quoted 32 lines
Quoted 1 line

+typedef enum iomux_pins {

...

Quoted 17 lines
arch/arm/include/asm/arch-mx35/mx35_pins.h new file +17 -0
+	MX35_PIN_A0 = _MXC_BUILD_NON_GPIO_PIN(0x28, 0x368),
+	MX35_PIN_A1 = _MXC_BUILD_NON_GPIO_PIN(0x2C, 0x36C),
+	MX35_PIN_A2 = _MXC_BUILD_NON_GPIO_PIN(0x30, 0x370),
+	MX35_PIN_A3 = _MXC_BUILD_NON_GPIO_PIN(0x34, 0x374),
+	MX35_PIN_A4 = _MXC_BUILD_NON_GPIO_PIN(0x38, 0x378),
+	MX35_PIN_A5 = _MXC_BUILD_NON_GPIO_PIN(0x3C, 0x37C),
+	MX35_PIN_A6 = _MXC_BUILD_NON_GPIO_PIN(0x40, 0x380),
+	MX35_PIN_A7 = _MXC_BUILD_NON_GPIO_PIN(0x44, 0x384),
+	MX35_PIN_A8 = _MXC_BUILD_NON_GPIO_PIN(0x48, 0x388),
+	MX35_PIN_A9 = _MXC_BUILD_NON_GPIO_PIN(0x4C, 0x38C),
+	MX35_PIN_A10 = _MXC_BUILD_NON_GPIO_PIN(0x50, 0x390),
+	MX35_PIN_MA10 = _MXC_BUILD_NON_GPIO_PIN(0x54, 0x394),
+	MX35_PIN_A11 = _MXC_BUILD_NON_GPIO_PIN(0x58, 0x398),
+	MX35_PIN_A12 = _MXC_BUILD_NON_GPIO_PIN(0x5C, 0x39C),
+	MX35_PIN_A13 = _MXC_BUILD_NON_GPIO_PIN(0x60, 0x3A0),
+	MX35_PIN_A14 = _MXC_BUILD_NON_GPIO_PIN(0x64, 0x3A4),
+	MX35_PIN_A15 = _MXC_BUILD_NON_GPIO_PIN(0x68, 0x3A8),

arch/arm/include/asm/arch-mx35/mx35_pins.h line 153

Quoted 10 lines
arch/arm/include/asm/arch-mx35/mx35_pins.h new file +10 -0
+	MX35_PIN_A16 = _MXC_BUILD_NON_GPIO_PIN(0x6C, 0x3AC),
+	MX35_PIN_A17 = _MXC_BUILD_NON_GPIO_PIN(0x70, 0x3B0),
+	MX35_PIN_A18 = _MXC_BUILD_NON_GPIO_PIN(0x74, 0x3B4),
+	MX35_PIN_A19 = _MXC_BUILD_NON_GPIO_PIN(0x78, 0x3B8),
+	MX35_PIN_A20 = _MXC_BUILD_NON_GPIO_PIN(0x7C, 0x3BC),
+	MX35_PIN_A21 = _MXC_BUILD_NON_GPIO_PIN(0x80, 0x3C0),
+	MX35_PIN_A22 = _MXC_BUILD_NON_GPIO_PIN(0x84, 0x3C4),
+	MX35_PIN_A23 = _MXC_BUILD_NON_GPIO_PIN(0x88, 0x3C8),
+	MX35_PIN_A24 = _MXC_BUILD_NON_GPIO_PIN(0x8C, 0x3CC),
+	MX35_PIN_A25 = _MXC_BUILD_NON_GPIO_PIN(0x90, 0x3D0),

Note: the following remark is a question, NOT a change request:

Quoted 4 lines

Would it not be possible to reduce all these terrible lists? As far as I can tell, the list is built sequentially, with both arguments to _MXC_BUILD_NON_GPIO_PIN() being incremented by 4 for the next register. This begs for automatic code generation, doesn't it?

I do not know if it helps. The list follows exactly the description in user manual, and, if you can see a rule for MX35_PIN_A*, it is not so simply to find one for other pins, specially for the MXC_BUILD_GPIO_PIN. At least, the list is at the moment coherent for all i.MX processors (ok, ugly for all). The name of the pin cannot be generated, and it is the name found in manual. Miore as generated, the list is sorted....

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
=====================================================================
#4 lore raw

PATCH v2 1/11 Add support for MX35 processor

Dear Stefano Babic,

Quoted 14 lines · In message <[email protected]> you wrote:
Quoted 8 lines

arch/arm/cpu/arm1136/mx35/generic.c line 194

Quoted 6 lines
arch/arm/cpu/arm1136/mx35/generic.c new file +5 -0
+	if (readl(&ccm->pdr3) & MXC_CCM_PDR3_UART_M_U)
+		freq = get_mcu_main_clk();
+	else
+		freq = decode_pll(readl(&ccm->ppctl),
+			CONFIG_MX35_HCLK_FREQ);

Braces needed

checkpatch (and generally accepted in linux, as I can see) requires that single statements must not be surrounded by braces. checkpatch returns a warning, explaining that braces are not needed.

Quoted 4 lines

I see in the past some comments requiring to remove braces, but it you prefer to add them. IMHO it is better to follow the same codestyle as linux, using the same tools as checkpatch. I do not know why we have two different results from checkpatch, I try to investigate. I had prefer to

I was running an older version of checkpatch...

Quoted 11 lines
Quoted 10 lines

arch/arm/cpu/arm1136/mx35/generic.c line 257

Quoted 8 lines
arch/arm/cpu/arm1136/mx35/generic.c new file +8 -0
+	case USB_CLK:
+		usb_prdf = (reg4 >> 25) & 0x7;
+		usb_podf = (reg4 >> 22) & 0x7;
+		if (reg4 & 0x200)
+			pll = get_mcu_main_clk();
+		else
+			pll = decode_pll(readl(&ccm->ppctl),
+				CONFIG_MX35_HCLK_FREQ);

Ditto. Please fix globally.

Quoted 3 lines

See my previous comment. I would prefer to not have a different rule in u-boot, and not go against some provided tool (we use both checkpatch) if not strictly required.

Did you try it?

For me, both

+ if (reg4 & 0x200)
+ pll = get_mcu_main_clk();
+ else
+ pll = decode_pll(readl(&ccm->ppctl),
+ CONFIG_MX35_HCLK_FREQ);

and

+ if (reg4 & 0x200) {
+ pll = get_mcu_main_clk();
+ } else {
+ pll = decode_pll(readl(&ccm->ppctl),
+ CONFIG_MX35_HCLK_FREQ);
+ }

generate _NO_ warnings with checkpatch.

I feel that when the "single statement" is split across several lines, eventually even including blank lines (see yesterday's discussion here), then braces are needed.

Quoted 5 lines
Quoted 3 lines

Indeed they should. Why don't you autogenerate these, then?

We have all the tools in place, use them.

I will see how to use them.

See tools/scripts/make-asm-offsets

Quoted 7 lines
Quoted 6 lines

Note: the following remark is a question, NOT a change request:

Would it not be possible to reduce all these terrible lists? As far as I can tell, the list is built sequentially, with both arguments to _MXC_BUILD_NON_GPIO_PIN() being incremented by 4 for the next register. This begs for automatic code generation, doesn't it?

Quoted 6 lines

I do not know if it helps. The list follows exactly the description in user manual, and, if you can see a rule for MX35_PIN_A*, it is not so simply to find one for other pins, specially for the MXC_BUILD_GPIO_PIN. At least, the list is at the moment coherent for all i.MX processors (ok, ugly for all). The name of the pin cannot be generated, and it is the name found in manual. Miore as generated, the list is sorted....

OK, thanks for the explanation.

Best regards,

Wolfgang Denk

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-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"You're just jealous." "What, of an overgrown puppy  with  a  single-
figure IQ?"                      - Terry Pratchett, _Moving Pictures_
#5 lore raw

PATCH v3 1/11 Add support for MX35 processor

In reply to Stefano Babic

The patch adds basic support for the Freescale's i.MX35 (arm1136 based) processor.

The patch adds also a prototype for the initialization of the FEC(ethernet controller) to netdev.h to avoid warnings.

---
Changes:

Wolfgang Denk:
- fix code styling issues reported by checkpatch
- fix missing braces (code styling)
- generates asm-offsets.h.
See arch/arm/cpu/arm1136/mx35/Makefile for rules
- drop unnedded NAND defines

 arch/arm/cpu/arm1136/mx35/Makefile         |   64 ++++
 arch/arm/cpu/arm1136/mx35/asm-offsets.c    |   43 +++
 arch/arm/cpu/arm1136/mx35/generic.c        |  462 ++++++++++++++++++++++++++++
 arch/arm/cpu/arm1136/mx35/iomux.c          |  116 +++++++
 arch/arm/cpu/arm1136/mx35/timer.c          |  120 +++++++
 arch/arm/include/asm/arch-mx35/clock.h     |   45 +++
 arch/arm/include/asm/arch-mx35/crm_regs.h  |  270 ++++++++++++++++
 arch/arm/include/asm/arch-mx35/imx-regs.h  |  291 +++++++++++++++++
 arch/arm/include/asm/arch-mx35/iomux.h     |  295 ++++++++++++++++++
 arch/arm/include/asm/arch-mx35/mx35_pins.h |  355 +++++++++++++++++++++
 arch/arm/include/asm/arch-mx35/sys_proto.h |   31 ++
 include/netdev.h                           |    1 +
 12 files changed, 2093 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/cpu/arm1136/mx35/Makefile
 create mode 100644 arch/arm/cpu/arm1136/mx35/asm-offsets.c
 create mode 100644 arch/arm/cpu/arm1136/mx35/generic.c
 create mode 100644 arch/arm/cpu/arm1136/mx35/iomux.c
 create mode 100644 arch/arm/cpu/arm1136/mx35/timer.c
 create mode 100644 arch/arm/include/asm/arch-mx35/clock.h
 create mode 100644 arch/arm/include/asm/arch-mx35/crm_regs.h
 create mode 100644 arch/arm/include/asm/arch-mx35/imx-regs.h
 create mode 100644 arch/arm/include/asm/arch-mx35/iomux.h
 create mode 100644 arch/arm/include/asm/arch-mx35/mx35_pins.h
 create mode 100644 arch/arm/include/asm/arch-mx35/sys_proto.h
arch/arm/cpu/arm1136/mx35/Makefile new file +64 -0
new file mode 100644
index 0000000..82c7218
@@ -0,0 +1,64 @@
+#
+# (C) Copyright 2000-2006
+# Wolfgang Denk, DENX Software Engineering, wd at denx.de.
+#
+# (C) Copyright 2008-2009 Freescale Semiconductor, Inc.
+#
+# 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
+#
+
+include $(TOPDIR)/config.mk
+
+LIB	= $(obj)lib$(SOC).o
+
+COBJS	+= generic.o
+COBJS	+= timer.o
+COBJS	+= iomux.o
+
+SRCS	:= $(SOBJS:.o=.S) $(COBJS:.o=.c)
+OBJS	:= $(addprefix $(obj),$(SOBJS) $(COBJS))
+
+all:	$(obj).depend $(LIB)
+
+$(LIB):	$(OBJS)
+	$(call cmd_link_o_target, $(OBJS))
+
+$(OBJS) : $(TOPDIR)/include/asm/arch/asm-offsets.h
+
+
+#########################################################################
+
+# defines $(obj).depend target
+include $(SRCTREE)/rules.mk
+
+sinclude $(obj).depend
+
+#########################################################################
+
+$(TOPDIR)/include/asm/arch/asm-offsets.h:	$(TOPDIR)/include/autoconf.mk.dep \
+	./asm-offsets.s
+	@echo Generating $@
+	$(TOPDIR)/tools/scripts/make-asm-offsets ./asm-offsets.s $@
+
+asm-offsets.s:	$(TOPDIR)/include/autoconf.mk.dep \
+	./asm-offsets.c
+	$(CC) -DDO_DEPS_ONLY \
+		$(CFLAGS) $(CFLAGS_$(BCURDIR)/$(@F)) $(CFLAGS_$(BCURDIR)) \
+		-o $@ ./asm-offsets.c -c -S
+
arch/arm/cpu/arm1136/mx35/asm-offsets.c new file +43 -0

43 lines not on this page · raw message

arch/arm/cpu/arm1136/mx35/generic.c new file +462 -0

462 lines not on this page · raw message

arch/arm/cpu/arm1136/mx35/iomux.c new file +116 -0

116 lines not on this page · raw message

arch/arm/cpu/arm1136/mx35/timer.c new file +120 -0

120 lines not on this page · raw message

arch/arm/include/asm/arch-mx35/clock.h new file +45 -0

45 lines not on this page · raw message

arch/arm/include/asm/arch-mx35/crm_regs.h new file +270 -0

270 lines not on this page · raw message

arch/arm/include/asm/arch-mx35/imx-regs.h new file +291 -0

291 lines not on this page · raw message

arch/arm/include/asm/arch-mx35/iomux.h new file +295 -0

295 lines not on this page · raw message

arch/arm/include/asm/arch-mx35/mx35_pins.h new file +355 -0

355 lines not on this page · raw message

arch/arm/include/asm/arch-mx35/sys_proto.h new file +31 -0
new file mode 100644
index 0000000..422eb52
@@ -0,0 +1,31 @@
+/*
+ * (C) Copyright 2011
+ * Stefano Babic, DENX Software Engineering, sbabic at denx.de.
+ *
+ * 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
+ */
+
+#ifndef _SYS_PROTO_H_
+#define _SYS_PROTO_H_
+
+u32 get_cpu_rev(void);
+#define is_soc_rev(rev)	((get_cpu_rev() & 0xFF) - rev)
+void sdelay(unsigned long);
+
+#endif
include/netdev.h +1 -0
index 1a542e8..6f0a971 100644
@@ -93,6 +93,7 @@ int uli526x_initialize(bd_t *bis);
 int xilinx_emaclite_initialize (bd_t *bis, int base_addr);
 int sh_eth_initialize(bd_t *bis);
 int dm9000_initialize(bd_t *bis);
+int fecmxc_initialize(bd_t *bis);
 
 /* Boards with PCI network controllers can call this from their board_eth_init()
  * function to initialize whatever's on board.
Signature · 1 line
1.7.1
#6 lore raw

PATCH v3 1/11 Add support for MX35 processor

Quoted 8 lines · Stefano Babic

The patch adds basic support for the Freescale's i.MX35 (arm1136 based) processor.

The patch adds also a prototype for the initialization of the FEC(ethernet controller) to netdev.h to avoid warnings.

Applied to u-boot-imx, thanks

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
=====================================================================
#7 lore raw

PATCH v2 2/11 serial_mxc: add support for Freescale's i.MX35 processor

In reply to Stefano Babic

The patch adds UART support for the i.MX35 processor.

---
 drivers/serial/serial_mxc.c |   15 ++++++---------
 1 files changed, 6 insertions(+), 9 deletions(-)
drivers/serial/serial_mxc.c +6 -9
index 805f4c5..b9cf9de 100644
@@ -50,17 +50,14 @@
 #define UART_PHYS 0x1001b000
 #elif defined(CONFIG_SYS_MX27_UART6)
 #define UART_PHYS 0x1001c000
-#elif defined(CONFIG_SYS_MX51_UART1)
+#elif defined(CONFIG_SYS_MX35_UART1) || defined(CONFIG_SYS_MX51_UART1) || \
+	defined(CONFIG_SYS_MX53_UART1)
 #define UART_PHYS UART1_BASE_ADDR
-#elif defined(CONFIG_SYS_MX51_UART2)
+#elif defined(CONFIG_SYS_MX35_UART2) || defined(CONFIG_SYS_MX51_UART2) || \
+	defined(CONFIG_SYS_MX53_UART2)
 #define UART_PHYS UART2_BASE_ADDR
-#elif defined(CONFIG_SYS_MX51_UART3)
-#define UART_PHYS UART3_BASE_ADDR
-#elif defined(CONFIG_SYS_MX53_UART1)
-#define UART_PHYS UART1_BASE_ADDR
-#elif defined(CONFIG_SYS_MX53_UART2)
-#define UART_PHYS UART2_BASE_ADDR
-#elif defined(CONFIG_SYS_MX53_UART3)
+#elif defined(CONFIG_SYS_MX35_UART3) || defined(CONFIG_SYS_MX51_UART3) || \
+	defined(CONFIG_SYS_MX53_UART3)
 #define UART_PHYS UART3_BASE_ADDR
 #else
 #error "define CONFIG_SYS_MXxx_UARTx to use the MXC UART driver"
Signature · 1 line
1.7.1
#8 lore raw

PATCH v2 3/11 mxc_i2c: Add support for the i.MX35 processor

In reply to Stefano Babic

---
Changes:

Wolfgang Denk:
- Do not use XXX in defines because are blocked by SPAM filters

 drivers/i2c/mxc_i2c.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)
drivers/i2c/mxc_i2c.c +5 -4
index 1ebec14..7f669ff 100755
@@ -29,9 +29,8 @@
 #if defined(CONFIG_MX31)
 #include <asm/arch/mx31.h>
 #include <asm/arch/mx31-regs.h>
-#endif
-
-#if defined(CONFIG_MX53)
+#else
+#include <asm/arch/imx-regs.h>
 #include <asm/arch/clock.h>
 #endif
 
@@ -66,8 +65,10 @@
 #define I2C_BASE        I2C1_BASE_ADDR
 #elif defined(CONFIG_SYS_I2C_MX53_PORT2)
 #define I2C_BASE        I2C2_BASE_ADDR
+#elif defined(CONFIG_SYS_I2C_MX35_PORT1)
+#define I2C_BASE	I2C_BASE_ADDR
 #else
-#error "define CONFIG_SYS_I2C_MXxx_PORTx to use the I2C driver"
+#error "define CONFIG_SYS_I2C_MX<Processor>_PORTx to use the mx I2C driver"
 #endif
 
 #ifdef DEBUG
Signature · 1 line
1.7.1
#9 lore raw

PATCH v2 3/11 mxc_i2c: Add support for the i.MX35 processor

Hello Stefano,

Quoted 10 lines · Stefano Babic

---
Changes:

Wolfgang Denk:
- Do not use XXX in defines because are blocked by SPAM filters

 drivers/i2c/mxc_i2c.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

bye,
Heiko

Signature · 2 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
#10 lore raw

PATCH v2 4/11 I2C: mxc_i2c: get rid of __REG access

In reply to Stefano Babic

This driver accesses to processor's register via __REG macros, that are removed (or are planned to be removed) and replaced by C structures. This patches replaces all occurrencies of __REG macros.

---
 drivers/i2c/mxc_i2c.c |   68 ++++++++++++++++++++++++++++++------------------
 1 files changed, 42 insertions(+), 26 deletions(-)
drivers/i2c/mxc_i2c.c +42 -26
index 7f669ff..eead502 100755
@@ -23,6 +23,7 @@
  */
 
 #include <common.h>
+#include <asm/io.h>
 
 #if defined(CONFIG_HARD_I2C)
 
@@ -81,15 +82,26 @@ static u16 div[] = { 30, 32, 36, 42, 48, 52, 60, 72, 80, 88, 104, 128, 144,
 	             160, 192, 240, 288, 320, 384, 480, 576, 640, 768, 960,
 	             1152, 1280, 1536, 1920, 2304, 2560, 3072, 3840};
 
+static inline void i2c_reset(void)
+{
+	writew(0, I2C_BASE + I2CR);	/* Reset module */
+	writew(0, I2C_BASE + I2SR);
+	writew(I2CR_IEN, I2C_BASE + I2CR);
+}
+
 void i2c_init(int speed, int unused)
 {
 	int freq;
 	int i;
 
 #if defined(CONFIG_MX31)
+	struct clock_control_regs *sc_regs =
+		(struct clock_control_regs *)CCM_BASE;
+
 	freq = mx31_get_ipg_clk();
 	/* start the required I2C clock */
-	__REG(CCM_CGR0) = __REG(CCM_CGR0) | (3 << I2C_CLK_OFFSET);
+	writel(readl(&sc_regs->cgr0) | (3 << I2C_CLK_OFFSET),
+		&sc_regs->cgr0);
 #else
 	freq = mxc_get_clock(MXC_IPG_PERCLK);
 #endif
@@ -100,28 +112,26 @@ void i2c_init(int speed, int unused)
 
 	DPRINTF("%s: speed: %d\n",__FUNCTION__, speed);
 
-	__REG16(I2C_BASE + I2CR) = 0; /* Reset module */
-	__REG16(I2C_BASE + IFDR) = i;
-	__REG16(I2C_BASE + I2CR) = I2CR_IEN;
-	__REG16(I2C_BASE + I2SR) = 0;
+	writew(i, I2C_BASE + IFDR);
+	i2c_reset();
 }
 
 static int wait_busy(void)
 {
 	int timeout = 10000;
 
-	while (!(__REG16(I2C_BASE + I2SR) & I2SR_IIF) && --timeout)
+	while (!(readw(I2C_BASE + I2SR) & I2SR_IIF) && --timeout)
 		udelay(1);
-	__REG16(I2C_BASE + I2SR) = 0; /* clear interrupt */
+	writew(0, I2C_BASE + I2SR); /* clear interrupt */
 
 	return timeout;
 }
 
 static int tx_byte(u8 byte)
 {
-	__REG16(I2C_BASE + I2DR) = byte;
+	writew(byte, I2C_BASE + I2DR);
 
-	if (!wait_busy() || __REG16(I2C_BASE + I2SR) & I2SR_RX_NO_AK)
+	if (!wait_busy() || readw(I2C_BASE + I2SR) & I2SR_RX_NO_AK)
 		return -1;
 	return 0;
 }
@@ -131,27 +141,27 @@ static int rx_byte(void)
 	if (!wait_busy())
 		return -1;
 
-	return __REG16(I2C_BASE + I2DR);
+	return readw(I2C_BASE + I2DR);
 }
 
 int i2c_probe(uchar chip)
 {
 	int ret;
 
-	__REG16(I2C_BASE + I2CR) = 0; /* Reset module */
-	__REG16(I2C_BASE + I2CR) = I2CR_IEN;
+	writew(0, I2C_BASE + I2CR); /* Reset module */
+	writew(I2CR_IEN, I2C_BASE + I2CR);
 
-	__REG16(I2C_BASE + I2CR) = I2CR_IEN |  I2CR_MSTA | I2CR_MTX;
+	writew(I2CR_IEN |  I2CR_MSTA | I2CR_MTX, I2C_BASE + I2CR);
 	ret = tx_byte(chip << 1);
-	__REG16(I2C_BASE + I2CR) = I2CR_IEN | I2CR_MTX;
+	writew(I2CR_IEN | I2CR_MTX, I2C_BASE + I2CR);
 
 	return ret;
 }
 
 static int i2c_addr(uchar chip, uint addr, int alen)
 {
-	__REG16(I2C_BASE + I2SR) = 0; /* clear interrupt */
-	__REG16(I2C_BASE + I2CR) = I2CR_IEN |  I2CR_MSTA | I2CR_MTX;
+	writew(0, I2C_BASE + I2SR);
+	writew(I2CR_IEN |  I2CR_MSTA | I2CR_MTX, I2C_BASE + I2CR);
 
 	if (tx_byte(chip << 1))
 		return -1;
@@ -167,35 +177,40 @@ int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
 	int timeout = 10000;
 	int ret;
 
-	DPRINTF("%s chip: 0x%02x addr: 0x%04x alen: %d len: %d\n",__FUNCTION__, chip, addr, alen, len);
+	DPRINTF("%s chip: 0x%02x addr: 0x%04x alen: %d len: %d\n",
+		__FUNCTION__, chip, addr, alen, len);
 
 	if (i2c_addr(chip, addr, alen)) {
 		printf("i2c_addr failed\n");
 		return -1;
 	}
 
-	__REG16(I2C_BASE + I2CR) = I2CR_IEN |  I2CR_MSTA | I2CR_MTX | I2CR_RSTA;
+	writew(I2CR_IEN | I2CR_MSTA | I2CR_MTX | I2CR_RSTA, I2C_BASE + I2CR);
 
 	if (tx_byte(chip << 1 | 1))
 		return -1;
 
-	__REG16(I2C_BASE + I2CR) = I2CR_IEN |  I2CR_MSTA | ((len == 1) ? I2CR_TX_NO_AK : 0);
+	writew(I2CR_IEN | I2CR_MSTA |
+		((len == 1) ? I2CR_TX_NO_AK : 0),
+		I2C_BASE + I2CR);
 
-	ret = __REG16(I2C_BASE + I2DR);
+	ret = readw(I2C_BASE + I2DR);
 
 	while (len--) {
 		if ((ret = rx_byte()) < 0)
 			return -1;
 		*buf++ = ret;
 		if (len <= 1)
-			__REG16(I2C_BASE + I2CR) = I2CR_IEN |  I2CR_MSTA | I2CR_TX_NO_AK;
+			writew(I2CR_IEN | I2CR_MSTA |
+				I2CR_TX_NO_AK,
+				I2C_BASE + I2CR);
 	}
 
 	wait_busy();
 
-	__REG16(I2C_BASE + I2CR) = I2CR_IEN;
+	writew(I2CR_IEN, I2C_BASE + I2CR);
 
-	while (__REG16(I2C_BASE + I2SR) & I2SR_IBB && --timeout)
+	while (readw(I2C_BASE + I2SR) & I2SR_IBB && --timeout)
 		udelay(1);
 
 	return 0;
@@ -204,7 +219,8 @@ int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
 int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len)
 {
 	int timeout = 10000;
-	DPRINTF("%s chip: 0x%02x addr: 0x%04x alen: %d len: %d\n",__FUNCTION__, chip, addr, alen, len);
+	DPRINTF("%s chip: 0x%02x addr: 0x%04x alen: %d len: %d\n",
+		__FUNCTION__, chip, addr, alen, len);
 
 	if (i2c_addr(chip, addr, alen))
 		return -1;
@@ -213,9 +229,9 @@ int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len)
 		if (tx_byte(*buf++))
 			return -1;
 
-	__REG16(I2C_BASE + I2CR) = I2CR_IEN;
+	writew(I2CR_IEN, I2C_BASE + I2CR);
 
-	while (__REG16(I2C_BASE + I2SR) & I2SR_IBB && --timeout)
+	while (readw(I2C_BASE + I2SR) & I2SR_IBB && --timeout)
 		udelay(1);
 
 	return 0;
Signature · 1 line
1.7.1
#11 lore raw

PATCH v2 4/11 I2C: mxc_i2c: get rid of __REG access

Dear Stefano Babic,

Quoted 4 lines · In message <[email protected]> you wrote:

This driver accesses to processor's register via __REG macros, that are removed (or are planned to be removed) and replaced by C structures. This patches replaces all occurrencies of __REG macros.

Checkpatch says:

total: 0 errors, 2 warnings, 169 lines checked

Please have a look.

...

drivers/i2c/mxc_i2c.c int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len) · line 223

Quoted 7 lines
drivers/i2c/mxc_i2c.c +2 -1
@@ -204,7 +219,8 @@ int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
 int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len)
 {
 	int timeout = 10000;
-	DPRINTF("%s chip: 0x%02x addr: 0x%04x alen: %d len: %d\n",__FUNCTION__, chip, addr, alen, len);
+	DPRINTF("%s chip: 0x%02x addr: 0x%04x alen: %d len: %d\n",
+		__FUNCTION__, chip, addr, alen, len);

While touching this anyway, can we please convert the DPRINTF() into debug() ?

Best regards,

Wolfgang Denk

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-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Summit meetings tend to be like panda matings. The expectations  are
always high, and the results usually disappointing."   - Robert Orben
#12 lore raw

PATCH v2 4/11 I2C: mxc_i2c: get rid of __REG access

Quoted 11 lines · Wolfgang Denk

Dear Stefano Babic,

Quoted 4 lines · In message <[email protected]> you wrote:

This driver accesses to processor's register via __REG macros, that are removed (or are planned to be removed) and replaced by C structures. This patches replaces all occurrencies of __REG macros.

Checkpatch says:

total: 0 errors, 2 warnings, 169 lines checked

I have seen, but because __FUNCTION__is generally used, I have not replaced the occurrencies with the suggested __func__. At least, not in this patch. I think it is should be done in a separate patch (I will do
in next post).

Quoted 10 lines

drivers/i2c/mxc_i2c.c int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len) · line 223

Quoted 7 lines
drivers/i2c/mxc_i2c.c +2 -1
@@ -204,7 +219,8 @@ int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
 int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len)
 {
 	int timeout = 10000;
-	DPRINTF("%s chip: 0x%02x addr: 0x%04x alen: %d len: %d\n",__FUNCTION__, chip, addr, alen, len);
+	DPRINTF("%s chip: 0x%02x addr: 0x%04x alen: %d len: %d\n",
+		__FUNCTION__, chip, addr, alen, len);

While touching this anyway, can we please convert the DPRINTF() into debug() ?

You missed the following patch that removes the ugly private debug functions..

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
=====================================================================
#13 lore raw

PATCH v3 4/11 I2C: mxc_i2c: get rid of __REG access

In reply to Stefano Babic

This driver accesses to processor's register via __REG macros, that are removed (or are planned to be removed) and replaced by C structures. This patches replaces all occurrencies of __REG macros.

---

Changes:
- fix code styling issues
- replace __FUNCTION__ with __func__

 drivers/i2c/mxc_i2c.c |   72 ++++++++++++++++++++++++++++++-------------------
 1 files changed, 44 insertions(+), 28 deletions(-)
drivers/i2c/mxc_i2c.c +44 -28
index 7f669ff..fd6db18 100755
@@ -23,6 +23,7 @@
  */
 
 #include <common.h>
+#include <asm/io.h>
 
 #if defined(CONFIG_HARD_I2C)
 
@@ -81,15 +82,26 @@ static u16 div[] = { 30, 32, 36, 42, 48, 52, 60, 72, 80, 88, 104, 128, 144,
 	             160, 192, 240, 288, 320, 384, 480, 576, 640, 768, 960,
 	             1152, 1280, 1536, 1920, 2304, 2560, 3072, 3840};
 
+static inline void i2c_reset(void)
+{
+	writew(0, I2C_BASE + I2CR);	/* Reset module */
+	writew(0, I2C_BASE + I2SR);
+	writew(I2CR_IEN, I2C_BASE + I2CR);
+}
+
 void i2c_init(int speed, int unused)
 {
 	int freq;
 	int i;
 
 #if defined(CONFIG_MX31)
+	struct clock_control_regs *sc_regs =
+		(struct clock_control_regs *)CCM_BASE;
+
 	freq = mx31_get_ipg_clk();
 	/* start the required I2C clock */
-	__REG(CCM_CGR0) = __REG(CCM_CGR0) | (3 << I2C_CLK_OFFSET);
+	writel(readl(&sc_regs->cgr0) | (3 << I2C_CLK_OFFSET),
+		&sc_regs->cgr0);
 #else
 	freq = mxc_get_clock(MXC_IPG_PERCLK);
 #endif
@@ -98,30 +110,28 @@ void i2c_init(int speed, int unused)
 		if (freq / div[i] <= speed)
 			break;
 
-	DPRINTF("%s: speed: %d\n",__FUNCTION__, speed);
+	debug("%s: speed: %d\n", __func__, speed);
 
-	__REG16(I2C_BASE + I2CR) = 0; /* Reset module */
-	__REG16(I2C_BASE + IFDR) = i;
-	__REG16(I2C_BASE + I2CR) = I2CR_IEN;
-	__REG16(I2C_BASE + I2SR) = 0;
+	writew(i, I2C_BASE + IFDR);
+	i2c_reset();
 }
 
 static int wait_busy(void)
 {
 	int timeout = 10000;
 
-	while (!(__REG16(I2C_BASE + I2SR) & I2SR_IIF) && --timeout)
+	while (!(readw(I2C_BASE + I2SR) & I2SR_IIF) && --timeout)
 		udelay(1);
-	__REG16(I2C_BASE + I2SR) = 0; /* clear interrupt */
+	writew(0, I2C_BASE + I2SR); /* clear interrupt */
 
 	return timeout;
 }
 
 static int tx_byte(u8 byte)
 {
-	__REG16(I2C_BASE + I2DR) = byte;
+	writew(byte, I2C_BASE + I2DR);
 
-	if (!wait_busy() || __REG16(I2C_BASE + I2SR) & I2SR_RX_NO_AK)
+	if (!wait_busy() || readw(I2C_BASE + I2SR) & I2SR_RX_NO_AK)
 		return -1;
 	return 0;
 }
@@ -131,27 +141,27 @@ static int rx_byte(void)
 	if (!wait_busy())
 		return -1;
 
-	return __REG16(I2C_BASE + I2DR);
+	return readw(I2C_BASE + I2DR);
 }
 
 int i2c_probe(uchar chip)
 {
 	int ret;
 
-	__REG16(I2C_BASE + I2CR) = 0; /* Reset module */
-	__REG16(I2C_BASE + I2CR) = I2CR_IEN;
+	writew(0, I2C_BASE + I2CR); /* Reset module */
+	writew(I2CR_IEN, I2C_BASE + I2CR);
 
-	__REG16(I2C_BASE + I2CR) = I2CR_IEN |  I2CR_MSTA | I2CR_MTX;
+	writew(I2CR_IEN |  I2CR_MSTA | I2CR_MTX, I2C_BASE + I2CR);
 	ret = tx_byte(chip << 1);
-	__REG16(I2C_BASE + I2CR) = I2CR_IEN | I2CR_MTX;
+	writew(I2CR_IEN | I2CR_MTX, I2C_BASE + I2CR);
 
 	return ret;
 }
 
 static int i2c_addr(uchar chip, uint addr, int alen)
 {
-	__REG16(I2C_BASE + I2SR) = 0; /* clear interrupt */
-	__REG16(I2C_BASE + I2CR) = I2CR_IEN |  I2CR_MSTA | I2CR_MTX;
+	writew(0, I2C_BASE + I2SR);
+	writew(I2CR_IEN |  I2CR_MSTA | I2CR_MTX, I2C_BASE + I2CR);
 
 	if (tx_byte(chip << 1))
 		return -1;
@@ -167,35 +177,40 @@ int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
 	int timeout = 10000;
 	int ret;
 
-	DPRINTF("%s chip: 0x%02x addr: 0x%04x alen: %d len: %d\n",__FUNCTION__, chip, addr, alen, len);
+	debug("%s chip: 0x%02x addr: 0x%04x alen: %d len: %d\n",
+		__func__, chip, addr, alen, len);
 
 	if (i2c_addr(chip, addr, alen)) {
 		printf("i2c_addr failed\n");
 		return -1;
 	}
 
-	__REG16(I2C_BASE + I2CR) = I2CR_IEN |  I2CR_MSTA | I2CR_MTX | I2CR_RSTA;
+	writew(I2CR_IEN | I2CR_MSTA | I2CR_MTX | I2CR_RSTA, I2C_BASE + I2CR);
 
 	if (tx_byte(chip << 1 | 1))
 		return -1;
 
-	__REG16(I2C_BASE + I2CR) = I2CR_IEN |  I2CR_MSTA | ((len == 1) ? I2CR_TX_NO_AK : 0);
+	writew(I2CR_IEN | I2CR_MSTA |
+		((len == 1) ? I2CR_TX_NO_AK : 0),
+		I2C_BASE + I2CR);
 
-	ret = __REG16(I2C_BASE + I2DR);
+	ret = readw(I2C_BASE + I2DR);
 
 	while (len--) {
 		if ((ret = rx_byte()) < 0)
 			return -1;
 		*buf++ = ret;
 		if (len <= 1)
-			__REG16(I2C_BASE + I2CR) = I2CR_IEN |  I2CR_MSTA | I2CR_TX_NO_AK;
+			writew(I2CR_IEN | I2CR_MSTA |
+				I2CR_TX_NO_AK,
+				I2C_BASE + I2CR);
 	}
 
 	wait_busy();
 
-	__REG16(I2C_BASE + I2CR) = I2CR_IEN;
+	writew(I2CR_IEN, I2C_BASE + I2CR);
 
-	while (__REG16(I2C_BASE + I2SR) & I2SR_IBB && --timeout)
+	while (readw(I2C_BASE + I2SR) & I2SR_IBB && --timeout)
 		udelay(1);
 
 	return 0;
@@ -203,8 +218,9 @@ int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
 
 int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len)
 {
-	int timeout = 10000;
-	DPRINTF("%s chip: 0x%02x addr: 0x%04x alen: %d len: %d\n",__FUNCTION__, chip, addr, alen, len);
+	int timeout = I2C_MAX_TIMEOUT;
+	debug("%s chip: 0x%02x addr: 0x%04x alen: %d len: %d\n",
+		__func__, chip, addr, alen, len);
 
 	if (i2c_addr(chip, addr, alen))
 		return -1;
@@ -213,9 +229,9 @@ int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len)
 		if (tx_byte(*buf++))
 			return -1;
 
-	__REG16(I2C_BASE + I2CR) = I2CR_IEN;
+	writew(I2CR_IEN, I2C_BASE + I2CR);
 
-	while (__REG16(I2C_BASE + I2SR) & I2SR_IBB && --timeout)
+	while (readw(I2C_BASE + I2SR) & I2SR_IBB && --timeout)
 		udelay(1);
 
 	return 0;
Signature · 1 line
1.7.1
#14 lore raw

PATCH v3 4/11 I2C: mxc_i2c: get rid of __REG access

Hello Stefano,

Quoted 7 lines · Stefano Babic

This driver accesses to processor's register via __REG macros, that are removed (or are planned to be removed) and replaced by C structures. This patches replaces all occurrencies of __REG macros.

Quoted 8 lines

---

Changes:
- fix code styling issues
- replace __FUNCTION__ with __func__

 drivers/i2c/mxc_i2c.c |   72 ++++++++++++++++++++++++++++++-------------------
 1 files changed, 44 insertions(+), 28 deletions(-)

bye,
Heiko

Signature · 2 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
#15 lore raw

PATCH v2 5/11 I2C: mxc_i2c: address failure with mx35 processor

In reply to Stefano Babic

There is sporadic failures when more as one I2C slave is on the bus and the processor tries to communicate with more as one slave.
The problem was seen on a mx35pdk (two I2C slaves, PMIC controller and CAN/RTC chip).

---
 drivers/i2c/mxc_i2c.c |   93 +++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 71 insertions(+), 22 deletions(-)
drivers/i2c/mxc_i2c.c +71 -22
index eead502..5667e68 100755
@@ -72,11 +72,8 @@
 #error "define CONFIG_SYS_I2C_MX<Processor>_PORTx to use the mx I2C driver"
 #endif
 
-#ifdef DEBUG
-#define DPRINTF(args...)  printf(args)
-#else
-#define DPRINTF(args...)
-#endif
+#define I2C_MAX_TIMEOUT		10000
+#define I2C_MAX_RETRIES		3
 
 static u16 div[] = { 30, 32, 36, 42, 48, 52, 60, 72, 80, 88, 104, 128, 144,
 	             160, 192, 240, 288, 320, 384, 480, 576, 640, 768, 960,
@@ -110,37 +107,67 @@ void i2c_init(int speed, int unused)
 		if (freq / div[i] <= speed)
 			break;
 
-	DPRINTF("%s: speed: %d\n",__FUNCTION__, speed);
+	debug("%s: speed: %d\n", __FUNCTION__, speed);
 
 	writew(i, I2C_BASE + IFDR);
 	i2c_reset();
 }
 
+static int wait_idle(void)
+{
+	int timeout = I2C_MAX_TIMEOUT;
+
+	while ((readw(I2C_BASE + I2SR) & I2SR_IBB) && --timeout) {
+		writew(0, I2C_BASE + I2SR);
+		udelay(1);
+	}
+	return timeout ? timeout : (!(readw(I2C_BASE + I2SR) & I2SR_IBB));
+}
+
 static int wait_busy(void)
 {
-	int timeout = 10000;
+	int timeout = I2C_MAX_TIMEOUT;
 
-	while (!(readw(I2C_BASE + I2SR) & I2SR_IIF) && --timeout)
+	while (!(readw(I2C_BASE + I2SR) & I2SR_IBB) && --timeout)
 		udelay(1);
 	writew(0, I2C_BASE + I2SR); /* clear interrupt */
 
 	return timeout;
 }
 
+static int wait_complete(void)
+{
+	int timeout = I2C_MAX_TIMEOUT;
+
+	while ((!(readw(I2C_BASE + I2SR) & I2SR_ICF)) && (--timeout)) {
+		writew(0, I2C_BASE + I2SR);
+		udelay(1);
+	}
+	udelay(200);
+
+	writew(0, I2C_BASE + I2SR);	/* clear interrupt */
+
+	return timeout;
+}
+
+
 static int tx_byte(u8 byte)
 {
 	writew(byte, I2C_BASE + I2DR);
 
-	if (!wait_busy() || readw(I2C_BASE + I2SR) & I2SR_RX_NO_AK)
+	if (!wait_complete() || readw(I2C_BASE + I2SR) & I2SR_RX_NO_AK)
 		return -1;
 	return 0;
 }
 
-static int rx_byte(void)
+static int rx_byte(int last)
 {
-	if (!wait_busy())
+	if (!wait_complete())
 		return -1;
 
+	if (last)
+		writew(I2CR_IEN, I2C_BASE + I2CR);
+
 	return readw(I2C_BASE + I2DR);
 }
 
@@ -160,24 +187,48 @@ int i2c_probe(uchar chip)
 
 static int i2c_addr(uchar chip, uint addr, int alen)
 {
-	writew(0, I2C_BASE + I2SR);
-	writew(I2CR_IEN |  I2CR_MSTA | I2CR_MTX, I2C_BASE + I2CR);
+	int i, retry = 0;
+	for (retry = 0; retry < 3; retry++) {
+		if (wait_idle())
+			break;
+		i2c_reset();
+		for (i = 0; i < I2C_MAX_TIMEOUT; i++)
+			udelay(1);
+	}
+	if (retry >= I2C_MAX_RETRIES) {
+		debug("%s:bus is busy(%x)\n",
+		       __func__, readw(I2C_BASE + I2SR));
+		return -1;
+	}
+	writew(I2CR_IEN | I2CR_MSTA | I2CR_MTX, I2C_BASE + I2CR);
 
-	if (tx_byte(chip << 1))
+	if (!wait_busy()) {
+		debug("%s:trigger start fail(%x)\n",
+		       __func__, readw(I2C_BASE + I2SR));
 		return -1;
+	}
 
+	if (tx_byte(chip << 1) || (readw(I2C_BASE + I2SR) & I2SR_RX_NO_AK)) {
+		debug("%s:chip address cycle fail(%x)\n",
+		       __func__, readw(I2C_BASE + I2SR));
+		return -1;
+	}
 	while (alen--)
-		if (tx_byte((addr >> (alen * 8)) & 0xff))
+		if (tx_byte((addr >> (alen * 8)) & 0xff) ||
+		    (readw(I2C_BASE + I2SR) & I2SR_RX_NO_AK)) {
+			debug("%s:device address cycle fail(%x)\n",
+			       __func__, readw(I2C_BASE + I2SR));
 			return -1;
+		}
 	return 0;
 }
 
 int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
 {
-	int timeout = 10000;
+	int timeout = I2C_MAX_TIMEOUT;
 	int ret;
 
-	DPRINTF("%s chip: 0x%02x addr: 0x%04x alen: %d len: %d\n",
+	debug("%s chip: 0x%02x addr: 0x%04x alen: %d len: %d\n",
 		__FUNCTION__, chip, addr, alen, len);
 
 	if (i2c_addr(chip, addr, alen)) {
@@ -197,7 +248,7 @@ int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
 	ret = readw(I2C_BASE + I2DR);
 
 	while (len--) {
-		if ((ret = rx_byte()) < 0)
+		if ((ret = rx_byte(len == 0)) < 0)
 			return -1;
 		*buf++ = ret;
 		if (len <= 1)
@@ -206,8 +257,6 @@ int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
 				I2C_BASE + I2CR);
 	}
 
-	wait_busy();
-
 	writew(I2CR_IEN, I2C_BASE + I2CR);
 
 	while (readw(I2C_BASE + I2SR) & I2SR_IBB && --timeout)
@@ -218,8 +267,8 @@ int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
 
 int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len)
 {
-	int timeout = 10000;
-	DPRINTF("%s chip: 0x%02x addr: 0x%04x alen: %d len: %d\n",
+	int timeout = I2C_MAX_TIMEOUT;
+	debug("%s chip: 0x%02x addr: 0x%04x alen: %d len: %d\n",
 		__FUNCTION__, chip, addr, alen, len);
 
 	if (i2c_addr(chip, addr, alen))
Signature · 1 line
1.7.1
#16 lore raw

PATCH v2 5/11 I2C: mxc_i2c: address failure with mx35 processor

Dear Stefano Babic,

Quoted 5 lines · In message <[email protected]> you wrote:

There is sporadic failures when more as one I2C slave is on the bus and the processor tries to communicate with more as one slave.
The problem was seen on a mx35pdk (two I2C slaves, PMIC controller and CAN/RTC chip).

Please also mention which approach the patch takes to fix this problem.

Checkpatch says:

total: 1 errors, 1 warnings, 166 lines checked

Please have a look.

drivers/i2c/mxc_i2c.c line 79

Quoted 5 lines
drivers/i2c/mxc_i2c.c +0 -5
-#ifdef DEBUG
-#define DPRINTF(args...)  printf(args)
-#else
-#define DPRINTF(args...)
-#endif

Ah. Please forget the related comment in my previous message.

But please mention this change in the commit message.

Best regards,

Wolfgang Denk

Signature · 4 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
EMACS belongs in <sys/errno.h>: Editor too big!
#17 lore raw

PATCH v2 5/11 I2C: mxc_i2c: address failure with mx35 processor

Quoted 11 lines · Wolfgang Denk

Dear Stefano Babic,

Quoted 5 lines · In message <[email protected]> you wrote:

There is sporadic failures when more as one I2C slave is on the bus and the processor tries to communicate with more as one slave.
The problem was seen on a mx35pdk (two I2C slaves, PMIC controller and CAN/RTC chip).

Please also mention which approach the patch takes to fix this problem.

Ok

Quoted 4 lines

Checkpatch says:

total: 1 errors, 1 warnings, 166 lines checked

I know, but I found the construct ok. checkpatch complains against using assignment in if function, even if it is large used. Again, as we have a tool to check patches, I will change to remove this issue.

Quoted 10 lines

Please have a look.

drivers/i2c/mxc_i2c.c line 79

Quoted 5 lines
drivers/i2c/mxc_i2c.c +0 -5
-#ifdef DEBUG
-#define DPRINTF(args...)  printf(args)
-#else
-#define DPRINTF(args...)
-#endif

Ah. Please forget the related comment in my previous message.

...and you please forget my comment to your comment...

Quoted 2 lines

But please mention this change in the commit message.

I will do it.

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
=====================================================================
#18 lore raw

PATCH v3 5/11 I2C: mxc_i2c: address failure with mx35 processor

In reply to Stefano Babic

There is sporadic failures when more as one I2C slave is on the bus and the processor tries to communicate with more as one slave.
The problem was seen on a mx35pdk (two I2C slaves, PMIC controller and CAN/RTC chip).

The current driver uses the IIF bit in the status register to check if the bus is busy or not. According to the manual, this is not correct, because the IIB bit should be checked. Not only, to check if a transfer is finished must be checked the ICF bit, and this is not tested at all.

This patch comes from analyse with a corresponding driver provided by Freescale as part of the LTIB tool. Comparing the two drivers, it appears that the current u-boot driver checks the wrong bits, and depending on race condition, the transfer can be successful or not.

The patch gets rid also of own debug function (DPRINTF), replaced with the general debug().

Tested on Freescale mx35pdk.

---
Changes:

Wolfgang Denk:
- change commit message explaining the problem and the changes
- describe in commit message the drop of DPRINTF

 drivers/i2c/mxc_i2c.c |   86 ++++++++++++++++++++++++++++++++++++++----------
 1 files changed, 68 insertions(+), 18 deletions(-)
drivers/i2c/mxc_i2c.c +68 -18
index fd6db18..c5ec486 100755
@@ -72,11 +72,8 @@
 #error "define CONFIG_SYS_I2C_MX<Processor>_PORTx to use the mx I2C driver"
 #endif
 
-#ifdef DEBUG
-#define DPRINTF(args...)  printf(args)
-#else
-#define DPRINTF(args...)
-#endif
+#define I2C_MAX_TIMEOUT		10000
+#define I2C_MAX_RETRIES		3
 
 static u16 div[] = { 30, 32, 36, 42, 48, 52, 60, 72, 80, 88, 104, 128, 144,
 	             160, 192, 240, 288, 320, 384, 480, 576, 640, 768, 960,
@@ -116,31 +113,61 @@ void i2c_init(int speed, int unused)
 	i2c_reset();
 }
 
+static int wait_idle(void)
+{
+	int timeout = I2C_MAX_TIMEOUT;
+
+	while ((readw(I2C_BASE + I2SR) & I2SR_IBB) && --timeout) {
+		writew(0, I2C_BASE + I2SR);
+		udelay(1);
+	}
+	return timeout ? timeout : (!(readw(I2C_BASE + I2SR) & I2SR_IBB));
+}
+
 static int wait_busy(void)
 {
-	int timeout = 10000;
+	int timeout = I2C_MAX_TIMEOUT;
 
-	while (!(readw(I2C_BASE + I2SR) & I2SR_IIF) && --timeout)
+	while (!(readw(I2C_BASE + I2SR) & I2SR_IBB) && --timeout)
 		udelay(1);
 	writew(0, I2C_BASE + I2SR); /* clear interrupt */
 
 	return timeout;
 }
 
+static int wait_complete(void)
+{
+	int timeout = I2C_MAX_TIMEOUT;
+
+	while ((!(readw(I2C_BASE + I2SR) & I2SR_ICF)) && (--timeout)) {
+		writew(0, I2C_BASE + I2SR);
+		udelay(1);
+	}
+	udelay(200);
+
+	writew(0, I2C_BASE + I2SR);	/* clear interrupt */
+
+	return timeout;
+}
+
+
 static int tx_byte(u8 byte)
 {
 	writew(byte, I2C_BASE + I2DR);
 
-	if (!wait_busy() || readw(I2C_BASE + I2SR) & I2SR_RX_NO_AK)
+	if (!wait_complete() || readw(I2C_BASE + I2SR) & I2SR_RX_NO_AK)
 		return -1;
 	return 0;
 }
 
-static int rx_byte(void)
+static int rx_byte(int last)
 {
-	if (!wait_busy())
+	if (!wait_complete())
 		return -1;
 
+	if (last)
+		writew(I2CR_IEN, I2C_BASE + I2CR);
+
 	return readw(I2C_BASE + I2DR);
 }
 
@@ -160,21 +187,45 @@ int i2c_probe(uchar chip)
 
 static int i2c_addr(uchar chip, uint addr, int alen)
 {
-	writew(0, I2C_BASE + I2SR);
-	writew(I2CR_IEN |  I2CR_MSTA | I2CR_MTX, I2C_BASE + I2CR);
+	int i, retry = 0;
+	for (retry = 0; retry < 3; retry++) {
+		if (wait_idle())
+			break;
+		i2c_reset();
+		for (i = 0; i < I2C_MAX_TIMEOUT; i++)
+			udelay(1);
+	}
+	if (retry >= I2C_MAX_RETRIES) {
+		debug("%s:bus is busy(%x)\n",
+		       __func__, readw(I2C_BASE + I2SR));
+		return -1;
+	}
+	writew(I2CR_IEN | I2CR_MSTA | I2CR_MTX, I2C_BASE + I2CR);
 
-	if (tx_byte(chip << 1))
+	if (!wait_busy()) {
+		debug("%s:trigger start fail(%x)\n",
+		       __func__, readw(I2C_BASE + I2SR));
 		return -1;
+	}
 
+	if (tx_byte(chip << 1) || (readw(I2C_BASE + I2SR) & I2SR_RX_NO_AK)) {
+		debug("%s:chip address cycle fail(%x)\n",
+		       __func__, readw(I2C_BASE + I2SR));
+		return -1;
+	}
 	while (alen--)
-		if (tx_byte((addr >> (alen * 8)) & 0xff))
+		if (tx_byte((addr >> (alen * 8)) & 0xff) ||
+		    (readw(I2C_BASE + I2SR) & I2SR_RX_NO_AK)) {
+			debug("%s:device address cycle fail(%x)\n",
+			       __func__, readw(I2C_BASE + I2SR));
 			return -1;
+		}
 	return 0;
 }
 
 int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
 {
-	int timeout = 10000;
+	int timeout = I2C_MAX_TIMEOUT;
 	int ret;
 
 	debug("%s chip: 0x%02x addr: 0x%04x alen: %d len: %d\n",
@@ -197,7 +248,8 @@ int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
 	ret = readw(I2C_BASE + I2DR);
 
 	while (len--) {
-		if ((ret = rx_byte()) < 0)
+		ret = rx_byte(len == 0);
+		if (ret  < 0)
 			return -1;
 		*buf++ = ret;
 		if (len <= 1)
@@ -206,8 +258,6 @@ int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
 				I2C_BASE + I2CR);
 	}
 
-	wait_busy();
-
 	writew(I2CR_IEN, I2C_BASE + I2CR);
 
 	while (readw(I2C_BASE + I2SR) & I2SR_IBB && --timeout)
Signature · 1 line
1.7.1
#19 lore raw

PATCH v3 5/11 I2C: mxc_i2c: address failure with mx35 processor

Hello Stefano,

just a question ...

Quoted 40 lines · Stefano Babic

There is sporadic failures when more as one I2C slave is on the bus and the processor tries to communicate with more as one slave.
The problem was seen on a mx35pdk (two I2C slaves, PMIC controller and CAN/RTC chip).

The current driver uses the IIF bit in the status register to check if the bus is busy or not. According to the manual, this is not correct, because the IIB bit should be checked. Not only, to check if a transfer is finished must be checked the ICF bit, and this is not tested at all.

This patch comes from analyse with a corresponding driver provided by Freescale as part of the LTIB tool. Comparing the two drivers, it appears that the current u-boot driver checks the wrong bits, and depending on race condition, the transfer can be successful or not.

The patch gets rid also of own debug function (DPRINTF), replaced with the general debug().

Tested on Freescale mx35pdk.

---
Changes:

Wolfgang Denk:
- change commit message explaining the problem and the changes
- describe in commit message the drop of DPRINTF

 drivers/i2c/mxc_i2c.c |   86 ++++++++++++++++++++++++++++++++++++++----------
 1 files changed, 68 insertions(+), 18 deletions(-)
drivers/i2c/mxc_i2c.c +0 -0
index fd6db18..c5ec486 100755

[...]

Quoted 27 lines
drivers/i2c/mxc_i2c.c +13 -2
@@ -116,31 +113,61 @@ void i2c_init(int speed, int unused)
 	i2c_reset();
 }
 
+static int wait_idle(void)
+{
+	int timeout = I2C_MAX_TIMEOUT;
+
+	while ((readw(I2C_BASE + I2SR) & I2SR_IBB) && --timeout) {
+		writew(0, I2C_BASE + I2SR);
+		udelay(1);
+	}
+	return timeout ? timeout : (!(readw(I2C_BASE + I2SR) & I2SR_IBB));
+}
+
 static int wait_busy(void)
 {
-	int timeout = 10000;
+	int timeout = I2C_MAX_TIMEOUT;
 
-	while (!(readw(I2C_BASE + I2SR) & I2SR_IIF) && --timeout)
+	while (!(readw(I2C_BASE + I2SR) & I2SR_IBB) && --timeout)
 		udelay(1);
 	writew(0, I2C_BASE + I2SR); /* clear interrupt */
 
 	return timeout;
 }

drivers/i2c/mxc_i2c.c void i2c_init(int speed, int unused) · line 146

Quoted 10 lines
drivers/i2c/mxc_i2c.c +9 -0
 
+static int wait_complete(void)
+{
+	int timeout = I2C_MAX_TIMEOUT;
+
+	while ((!(readw(I2C_BASE + I2SR) & I2SR_ICF)) && (--timeout)) {
+		writew(0, I2C_BASE + I2SR);
+		udelay(1);
+	}
+	udelay(200);

Why is this delay necessary? Why exactly 200? Is this documented somewhere in the doc?

bye,
Heiko

Signature · 2 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
#20 lore raw

PATCH v3 5/11 I2C: mxc_i2c: address failure with mx35 processor

Quoted 4 lines · Heiko Schocher
Quoted 4 lines
drivers/i2c/mxc_i2c.c +4 -0
+static int wait_complete(void)
+{
+	int timeout = I2C_MAX_TIMEOUT;
+
Quoted 8 lines

drivers/i2c/mxc_i2c.c void i2c_init(int speed, int unused) · line 146

Quoted 5 lines
drivers/i2c/mxc_i2c.c +5 -0
+	while ((!(readw(I2C_BASE + I2SR) & I2SR_ICF)) && (--timeout)) {
+		writew(0, I2C_BASE + I2SR);
+		udelay(1);
+	}
+	udelay(200);

Why is this delay necessary? Why exactly 200? Is this documented somewhere in the doc?

Rather I do not have a clear explanation. In the manual there is a hint regarding a delay that SW must introduce after setting the RSTA bit and before writing data into the I2DR register. Really this delay should be very short, but I checked removing the udelay() or decreasing the value to some uSec and it does not work.

This is the output with debugging after removing the udelay call:

i2c_read chip: 0x08 addr: 0x0007 alen: 1 len: 3
i2c_addr:chip address cycle fail(a1)
i2c_addr failed
i2c_read chip: 0x08 addr: 0x001e alen: 1 len: 3
i2c_addr:chip address cycle fail(a1)
i2c_addr failed

This happens in the i2_read() call, and it is the function where the RSTA bit is set. I can only presume the two things are related. I checked then with the driver provided by Freescale in the LTIB, and also in this driver a delay is set after checking transfer is completed.

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
=====================================================================
#21 lore raw

PATCH v3 5/11 I2C: mxc_i2c: address failure with mx35 processor

Hello Stefano,

Quoted 28 lines · Stefano Babic
Quoted 11 lines · Heiko Schocher

drivers/i2c/mxc_i2c.c void i2c_init(int speed, int unused) · line 146

Quoted 9 lines
drivers/i2c/mxc_i2c.c +9 -0
+static int wait_complete(void)
+{
+	int timeout = I2C_MAX_TIMEOUT;
+
+	while ((!(readw(I2C_BASE + I2SR) & I2SR_ICF)) && (--timeout)) {
+		writew(0, I2C_BASE + I2SR);
+		udelay(1);
+	}
+	udelay(200);

Why is this delay necessary? Why exactly 200? Is this documented somewhere in the doc?

Rather I do not have a clear explanation. In the manual there is a hint regarding a delay that SW must introduce after setting the RSTA bit and before writing data into the I2DR register. Really this delay should be very short, but I checked removing the udelay() or decreasing the value to some uSec and it does not work.

This is the output with debugging after removing the udelay call:

i2c_read chip: 0x08 addr: 0x0007 alen: 1 len: 3
i2c_addr:chip address cycle fail(a1)
i2c_addr failed
i2c_read chip: 0x08 addr: 0x001e alen: 1 len: 3
i2c_addr:chip address cycle fail(a1)
i2c_addr failed

Quoted 4 lines

This happens in the i2_read() call, and it is the function where the RSTA bit is set. I can only presume the two things are related. I checked then with the driver provided by Freescale in the LTIB, and also in this driver a delay is set after checking transfer is completed.

Thanks for the explanation, so:

bye,
Heiko

Signature · 2 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
#22 lore raw

PATCH v2 6/11 Add basic support for Freescale's mc9sdz60

In reply to Stefano Babic

The patch adds helper funtions for basic access to the registers of the MC9sdz60 chip (multifunctional device with RTC and CAN) via I2C interface.

---
 drivers/misc/Makefile   |    5 ++-
 drivers/misc/mc9sdz60.c |   51 ++++++++++++++++++++++++++++
 include/mc9sdz60.h      |   84 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 138 insertions(+), 2 deletions(-)
 create mode 100644 drivers/misc/mc9sdz60.c
 create mode 100644 include/mc9sdz60.h
drivers/misc/Makefile +3 -2
index a76bd4e..b152486 100644
@@ -28,12 +28,13 @@ LIB	:= $(obj)libmisc.o
 COBJS-$(CONFIG_ALI152X) += ali512x.o
 COBJS-$(CONFIG_DS4510)  += ds4510.o
 COBJS-$(CONFIG_FSL_LAW) += fsl_law.o
+COBJS-$(CONFIG_FSL_PMIC) += fsl_pmic.o
 COBJS-$(CONFIG_GPIO_LED) += gpio_led.o
+COBJS-$(CONFIG_FSL_MC9SDZ60) += mc9sdz60.o
 COBJS-$(CONFIG_NS87308) += ns87308.o
+COBJS-$(CONFIG_PDSP188x) += pdsp188x.o
 COBJS-$(CONFIG_STATUS_LED) += status_led.o
 COBJS-$(CONFIG_TWL4030_LED) += twl4030_led.o
-COBJS-$(CONFIG_FSL_PMIC) += fsl_pmic.o
-COBJS-$(CONFIG_PDSP188x) += pdsp188x.o
 
 COBJS	:= $(COBJS-y)
 SRCS	:= $(COBJS:.o=.c)
drivers/misc/mc9sdz60.c new file +51 -0

51 lines not on this page · raw message

include/mc9sdz60.h new file +84 -0

84 lines not on this page · raw message

Signature · 1 line
1.7.1
#23 lore raw

PATCH v2 7/11 SPI: mxc_spi: add support for i.MX35 processor

In reply to Stefano Babic
---
 drivers/spi/mxc_spi.c |   96 ++++++++++++++++++++++++++++++++++++------------
 1 files changed, 72 insertions(+), 24 deletions(-)
drivers/spi/mxc_spi.c +72 -24
index d558137..5670714 100644
@@ -70,6 +70,8 @@ static unsigned long spi_bases[] = {
 	0x53f84000,
 };
 
+#define mxc_get_clock(x)	mx31_get_ipg_clk()
+
 #elif defined(CONFIG_MX51)
 #include <asm/arch/imx-regs.h>
 #include <asm/arch/clock.h>
@@ -111,6 +113,45 @@ static unsigned long spi_bases[] = {
 	CSPI2_BASE_ADDR,
 	CSPI3_BASE_ADDR,
 };
+
+#elif defined(CONFIG_MX35)
+
+#include <asm/arch/imx-regs.h>
+#include <asm/arch/clock.h>
+
+#define MXC_CSPIRXDATA		0x00
+#define MXC_CSPITXDATA		0x04
+#define MXC_CSPICTRL		0x08
+#define MXC_CSPIINT		0x0C
+#define MXC_CSPIDMA		0x10
+#define MXC_CSPISTAT		0x14
+#define MXC_CSPIPERIOD		0x18
+#define MXC_CSPITEST		0x1C
+#define MXC_CSPIRESET		0x00
+
+#define MXC_CSPICTRL_EN		(1 << 0)
+#define MXC_CSPICTRL_MODE	(1 << 1)
+#define MXC_CSPICTRL_XCH	(1 << 2)
+#define MXC_CSPICTRL_SMC	(1 << 3)
+#define MXC_CSPICTRL_POL	(1 << 4)
+#define MXC_CSPICTRL_PHA	(1 << 5)
+#define MXC_CSPICTRL_SSCTL	(1 << 6)
+#define MXC_CSPICTRL_SSPOL	(1 << 7)
+#define MXC_CSPICTRL_CHIPSELECT(x)	(((x) & 0x3) << 12)
+#define MXC_CSPICTRL_BITCOUNT(x)	(((x) & 0xfff) << 20)
+#define MXC_CSPICTRL_DATARATE(x)	(((x) & 0x7) << 16)
+#define MXC_CSPICTRL_TC		(1 << 7)
+#define MXC_CSPICTRL_RXOVF	(1 << 6)
+#define MXC_CSPICTRL_MAXBITS	0xfff
+
+#define MXC_CSPIPERIOD_32KHZ	(1 << 15)
+#define MAX_SPI_BYTES	4
+
+static unsigned long spi_bases[] = {
+	0x43fa4000,
+	0x50010000,
+};
+
 #else
 #error "Unsupported architecture"
 #endif
@@ -158,8 +199,35 @@ void spi_cs_deactivate(struct spi_slave *slave)
 			      !(mxcs->ss_pol));
 }
 
-#ifdef CONFIG_MX51
-static s32 spi_cfg(struct mxc_spi_slave *mxcs, unsigned int cs,
+#if defined(CONFIG_MX31) || defined(CONFIG_MX35)
+static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs,
+		unsigned int max_hz, unsigned int mode)
+{
+	unsigned int ctrl_reg;
+
+	ctrl_reg = MXC_CSPICTRL_CHIPSELECT(cs) |
+		MXC_CSPICTRL_BITCOUNT(MXC_CSPICTRL_MAXBITS) |
+		MXC_CSPICTRL_DATARATE(7) | /* FIXME: calculate data rate */
+		MXC_CSPICTRL_EN |
+#ifdef CONFIG_MX35
+		MXC_CSPICTRL_SSCTL |
+#endif
+		MXC_CSPICTRL_MODE;
+
+	if (mode & SPI_CPHA)
+		ctrl_reg |= MXC_CSPICTRL_PHA;
+	if (mode & SPI_CPOL)
+		ctrl_reg |= MXC_CSPICTRL_POL;
+	if (mode & SPI_CS_HIGH)
+		ctrl_reg |= MXC_CSPICTRL_SSPOL;
+	mxcs->ctrl_reg = ctrl_reg;
+
+	return 0;
+}
+#endif
+
+#if defined(CONFIG_MX51)
+static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs,
 		unsigned int max_hz, unsigned int mode)
 {
 	u32 clk_src = mxc_get_clock(MXC_CSPI_CLK);
@@ -227,7 +295,7 @@ static s32 spi_cfg(struct mxc_spi_slave *mxcs, unsigned int cs,
 
 	/*
 	 * Configuration register setup
-	 * The MX51 has support different setup for each SS
+	 * The MX51 supports different setup for each SS
 	 */
 	reg_config = (reg_config & ~(1 << (cs + MXC_CSPICON_SSPOL))) |
 		(ss_pol << (cs + MXC_CSPICON_SSPOL));
@@ -363,7 +431,6 @@ int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen,
 
 }
 
-
 int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
 		void *din, unsigned long flags)
 {
@@ -441,7 +508,6 @@ static int decode_cs(struct mxc_spi_slave *mxcs, unsigned int cs)
 struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
 			unsigned int max_hz, unsigned int mode)
 {
-	unsigned int ctrl_reg;
 	struct mxc_spi_slave *mxcs;
 	int ret;
 
@@ -467,30 +533,12 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
 	mxcs->base = spi_bases[bus];
 	mxcs->ss_pol = (mode & SPI_CS_HIGH) ? 1 : 0;
 
-#ifdef CONFIG_MX51
-	/* Can be used for i.MX31 too ? */
-	ctrl_reg = 0;
-	ret = spi_cfg(mxcs, cs, max_hz, mode);
+	ret = spi_cfg_mxc(mxcs, cs, max_hz, mode);
 	if (ret) {
 		printf("mxc_spi: cannot setup SPI controller\n");
 		free(mxcs);
 		return NULL;
 	}
-#else
-	ctrl_reg = MXC_CSPICTRL_CHIPSELECT(cs) |
-		MXC_CSPICTRL_BITCOUNT(31) |
-		MXC_CSPICTRL_DATARATE(7) | /* FIXME: calculate data rate */
-		MXC_CSPICTRL_EN |
-		MXC_CSPICTRL_MODE;
-
-	if (mode & SPI_CPHA)
-		ctrl_reg |= MXC_CSPICTRL_PHA;
-	if (mode & SPI_CPOL)
-		ctrl_reg |= MXC_CSPICTRL_POL;
-	if (mode & SPI_CS_HIGH)
-		ctrl_reg |= MXC_CSPICTRL_SSPOL;
-	mxcs->ctrl_reg = ctrl_reg;
-#endif
 	return &mxcs->slave;
 }
 
Signature · 1 line
1.7.1
#24 lore raw

PATCH v2 8/11 SPI: mxc_spi: fix swapping bug and add missing swapping in unaligned rx case

In reply to Stefano Babic

From: Anatolij Gustschin <[email protected]>

We need to shift only one time in each cycle in the swapping loop for unaligned tx case. Currently two byte shift operations are performed in each loop cycle causing zero gaps in the transmited data, so not all data scheduled for transmition is actually transmited.

The proper swapping in unaligned rx case is missing, so add it as we need to put the received data into the rx buffer in the correct byte order.

---
 drivers/spi/mxc_spi.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
drivers/spi/mxc_spi.c +4 -4
index 5670714..3e99afd 100644
@@ -372,7 +372,7 @@ int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen,
 			/* Buffer is not 32-bit aligned */
 			if ((unsigned long)dout & 0x03) {
 				data = 0;
-				for (i = 0; i < 4; i++, data <<= 8) {
+				for (i = 0; i < 4; i++) {
 					data = (data << 8) | (*dout++ & 0xFF);
 				}
 			} else {
@@ -405,11 +405,11 @@ int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen,
 	if (bitlen % 32) {
 		data = reg_read(mxcs->base + MXC_CSPIRXDATA);
 		cnt = (bitlen % 32) / 8;
+		data = cpu_to_be32(data) >> ((sizeof(data) - cnt) * 8);
 		debug("SPI Rx unaligned: 0x%x\n", data);
 		if (din) {
-			for (i = 0; i < cnt; i++, data >>= 8) {
-				*din++ = data & 0xFF;
-			}
+			memcpy(din, &data, cnt);
+			din += cnt;
 		}
 		nbytes -= cnt;
 	}
Signature · 1 line
1.7.1
#25 lore raw

PATCH v2 8/11 SPI: mxc_spi: fix swapping bug and add missing swapping in unaligned rx case

Dear Stefano Babic,

Quoted 8 lines · In message <[email protected]> you wrote:

From: Anatolij Gustschin <[email protected]>

We need to shift only one time in each cycle in the swapping loop for unaligned tx case. Currently two byte shift operations are performed in each loop cycle causing zero gaps in the transmited data, so not all data scheduled for transmition is actually transmited.

Quoted 6 lines

The proper swapping in unaligned rx case is missing, so add it as we need to put the received data into the rx buffer in the correct byte order.

Checkpatch says:

[U-Boot] [PATCH V2 08/11] SPI: mxc_spi: fix swapping bug and add
total: 0 errors, 1 warnings, 22 lines checked

Please have a look.

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
The complexity of software is an essential property, not an  acciden-
tal  one. Hence, descriptions of a software entity that abstract away
its complexity often abstract away its essence.    - Fred Brooks, Jr.
#26 lore raw

PATCH v2 8/11 SPI: mxc_spi: fix swapping bug and add missing swapping in unaligned rx case

Quoted 15 lines · Wolfgang Denk

Dear Stefano Babic,

Quoted 12 lines · In message <[email protected]> you wrote:

From: Anatolij Gustschin <[email protected]>

We need to shift only one time in each cycle in the swapping loop for unaligned tx case. Currently two byte shift operations are performed in each loop cycle causing zero gaps in the transmited data, so not all data scheduled for transmition is actually transmited.

The proper swapping in unaligned rx case is missing, so add it as we need to put the received data into the rx buffer in the correct byte order.

Quoted 7 lines
Quoted 2 lines

Checkpatch says:

[U-Boot] [PATCH V2 08/11] SPI: mxc_spi: fix swapping bug and add
total: 0 errors, 1 warnings, 22 lines checked

WARNING: braces {} are not necessary for single statement blocks #31: FILE: drivers/spi/mxc_spi.c:375:
+ for (i = 0; i < 4; i++) {
data = (data << 8) | (*dout++ & 0xFF);
}

total: 0 errors, 1 warnings, 22 lines checked

This is exactly what you prefer, but checkpatch complains. IMHO I prefer to fix in this patch removing braces, and do not change in previous patches where you suggest to add braces (and then checkpatch will complain about it).

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: [email protected]
=====================================================================
#27 lore raw

PATCH v2 8/11 SPI: mxc_spi: fix swapping bug and add missing swapping in unaligned rx case

Dear Stefano Babic,

Quoted 10 lines · In message <[email protected]> you wrote:
Quoted 3 lines

[U-Boot] [PATCH V2 08/11] SPI: mxc_spi: fix swapping bug and add
total: 0 errors, 1 warnings, 22 lines checked

WARNING: braces {} are not necessary for single statement blocks #31: FILE: drivers/spi/mxc_spi.c:375:

drivers/spi/mxc_spi.c +1 -0
+				for (i = 0; i < 4; i++) {
 					data = (data << 8) | (*dout++ & 0xFF);
 				}
Quoted 6 lines

total: 0 errors, 1 warnings, 22 lines checked

This is exactly what you prefer, but checkpatch complains. IMHO I prefer to fix in this patch removing braces, and do not change in previous patches where you suggest to add braces (and then checkpatch will complain about it).

These are two _different_ situations.

Here we have a single statement on a single line. This needs no braces.

In the other case, we had a single stament split over multiple lines, and with multiple lines I want to see braces. And checkpatch appears to be happy with braces then, too (and without as well).

Best regards,

Wolfgang Denk

Signature · 4 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
"Ada is PL/I trying to be Smalltalk.                 - Codoso diBlini
#28 lore raw

PATCH v3 8/11 SPI: mxc_spi: fix swapping bug and add missing swapping in unaligned rx case

In reply to Stefano Babic

From: Anatolij Gustschin <[email protected]>

We need to shift only one time in each cycle in the swapping loop for unaligned tx case. Currently two byte shift operations are performed in each loop cycle causing zero gaps in the transmited data, so not all data scheduled for transmition is actually transmited.

The proper swapping in unaligned rx case is missing, so add it as we need to put the received data into the rx buffer in the correct byte order.

---

Changes:
- code styling

 drivers/spi/mxc_spi.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)
drivers/spi/mxc_spi.c +4 -5
index 5670714..dadf228 100644
@@ -372,9 +372,8 @@ int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen,
 			/* Buffer is not 32-bit aligned */
 			if ((unsigned long)dout & 0x03) {
 				data = 0;
-				for (i = 0; i < 4; i++, data <<= 8) {
+				for (i = 0; i < 4; i++)
 					data = (data << 8) | (*dout++ & 0xFF);
-				}
 			} else {
 				data = *(u32 *)dout;
 				data = cpu_to_be32(data);
@@ -405,11 +404,11 @@ int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen,
 	if (bitlen % 32) {
 		data = reg_read(mxcs->base + MXC_CSPIRXDATA);
 		cnt = (bitlen % 32) / 8;
+		data = cpu_to_be32(data) >> ((sizeof(data) - cnt) * 8);
 		debug("SPI Rx unaligned: 0x%x\n", data);
 		if (din) {
-			for (i = 0; i < cnt; i++, data >>= 8) {
-				*din++ = data & 0xFF;
-			}
+			memcpy(din, &data, cnt);
+			din += cnt;
 		}
 		nbytes -= cnt;
 	}
Signature · 1 line
1.7.1
#29 lore raw

PATCH v3 8/11 SPI: mxc_spi: fix swapping bug and add missing swapping in unaligned rx case

Quoted 8 lines · Stefano Babic

From: Anatolij Gustschin <[email protected]>

We need to shift only one time in each cycle in the swapping loop for unaligned tx case. Currently two byte shift operations are performed in each loop cycle causing zero gaps in the transmited data, so not all data scheduled for transmition is actually transmited.

Quoted 6 lines

The proper swapping in unaligned rx case is missing, so add it as we need to put the received data into the rx buffer in the correct byte order.

Applied to u-boot-imx, thanks

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
=====================================================================
#30 lore raw

PATCH v2 9/11 SPI: mxc_spi: add SPI clock calculation and setup to the driver

In reply to Stefano Babic

From: Anatolij Gustschin <[email protected]>

The MXC SPI driver didn't calculate the SPI clock up to now and just used highest possible divider 512 for DATA RATE in the control register. This results in very low transfer rates.

The patch adds code to calculate and setup the SPI clock frequency for transfers.

---
Changes:

Sergei Shtylyov:
- fix commit message

 drivers/spi/mxc_spi.c |   23 ++++++++++++++++++++++-
 1 files changed, 22 insertions(+), 1 deletions(-)
drivers/spi/mxc_spi.c +22 -1
index 3e99afd..ee7675b 100644
@@ -199,15 +199,36 @@ void spi_cs_deactivate(struct spi_slave *slave)
 			      !(mxcs->ss_pol));
 }
 
+u32 get_cspi_div(u32 div)
+{
+	int i;
+
+	for (i = 0; i < 8; i++) {
+		if (div <= (4 << i))
+			return i;
+	}
+	return i;
+}
+
 #if defined(CONFIG_MX31) || defined(CONFIG_MX35)
 static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs,
 		unsigned int max_hz, unsigned int mode)
 {
 	unsigned int ctrl_reg;
+	u32 clk_src;
+	u32 div;
+
+	clk_src = mxc_get_clock(MXC_CSPI_CLK);
+
+	div = clk_src / max_hz;
+	div = get_cspi_div(div);
+
+	debug("clk %d Hz, div %d, real clk %d Hz\n",
+		max_hz, div, clk_src / (4 << div));
 
 	ctrl_reg = MXC_CSPICTRL_CHIPSELECT(cs) |
 		MXC_CSPICTRL_BITCOUNT(MXC_CSPICTRL_MAXBITS) |
-		MXC_CSPICTRL_DATARATE(7) | /* FIXME: calculate data rate */
+		MXC_CSPICTRL_DATARATE(div) |
 		MXC_CSPICTRL_EN |
 #ifdef CONFIG_MX35
 		MXC_CSPICTRL_SSCTL |
Signature · 1 line
1.7.1
#31 lore raw

PATCH v2 9/11 SPI: mxc_spi: add SPI clock calculation and setup to the driver

Quoted 7 lines · Stefano Babic

From: Anatolij Gustschin <[email protected]>

The MXC SPI driver didn't calculate the SPI clock up to now and just used highest possible divider 512 for DATA RATE in the control register. This results in very low transfer rates.

Quoted 5 lines

The patch adds code to calculate and setup the SPI clock frequency for transfers.

Applied to u-boot-imx, thanks

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
=====================================================================
#32 lore raw

PATCH v2 10/11 SPI: mxc_spi: replace fixed offsets with structures

In reply to Stefano Babic

This patch cleans driver code replacing all accesses to registers with fixed offsets with a corresponding structure.

---
 arch/arm/include/asm/arch-mx31/mx31-regs.h |   11 +++
 arch/arm/include/asm/arch-mx35/imx-regs.h  |   12 ++++
 arch/arm/include/asm/arch-mx5/imx-regs.h   |   12 ++++
 drivers/spi/mxc_spi.c                      |   94 ++++++++-------------------
 4 files changed, 63 insertions(+), 66 deletions(-)
arch/arm/include/asm/arch-mx31/mx31-regs.h +11 -0
index 46ed47c..105f7d8 100644
@@ -64,6 +64,17 @@ struct gpio_regs {
 	u32	gpio_psr;
 };
 
+struct cspi_regs {
+	u32 rxdata;
+	u32 txdata;
+	u32 ctrl;
+	u32 intr;
+	u32 dma;
+	u32 stat;
+	u32 period;
+	u32 test;
+};
+
 #define IOMUX_PADNUM_MASK	0x1ff
 #define IOMUX_PIN(gpionum, padnum) ((padnum) & IOMUX_PADNUM_MASK)
 
arch/arm/include/asm/arch-mx35/imx-regs.h +12 -0
index e96a37d..6299ba9 100644
@@ -309,6 +309,18 @@ struct gpt_regs {
 	u32 counter;	/* counter */
 };
 
+/* CSPI registers */
+struct cspi_regs {
+	u32 rxdata;
+	u32 txdata;
+	u32 ctrl;
+	u32 intr;
+	u32 dma;
+	u32 stat;
+	u32 period;
+	u32 test;
+};
+
 /* Watchdog Timer (WDOG) registers */
 struct wdog_regs {
 	u16 wcr;	/* Control */
arch/arm/include/asm/arch-mx5/imx-regs.h +12 -0
index 09b69f6..a1849f8 100755
@@ -256,6 +256,18 @@ struct src {
 	u32	simr;
 };
 
+/* CSPI registers */
+struct cspi_regs {
+	u32 rxdata;
+	u32 txdata;
+	u32 ctrl;
+	u32 cfg;
+	u32 intr;
+	u32 dma;
+	u32 stat;
+	u32 period;
+};
+
 struct iim_regs {
 	u32	stat;
 	u32	statm;
drivers/spi/mxc_spi.c +28 -66

221 lines not on this page · raw message

Signature · 1 line
1.7.1
#33 lore raw

PATCH v2 10/11 SPI: mxc_spi: replace fixed offsets with structures

Dear Stefano Babic,

Quoted 3 lines · In message <[email protected]> you wrote:

This patch cleans driver code replacing all accesses to registers with fixed offsets with a corresponding structure.

Checkpatch says:

[U-Boot] [PATCH V2 10/11] SPI: mxc_spi: replace fixed offsets with
total: 3 errors, 0 warnings, 274 lines checked

Please fix.

Best regards,

Wolfgang Denk

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-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"I think trash is the most important manifestation of culture we have
in my lifetime."                                      - Johnny Legend
#34 lore raw

PATCH v2 10/11 SPI: mxc_spi: replace fixed offsets with structures

Quoted 7 lines · Wolfgang Denk

Dear Stefano Babic,

Quoted 3 lines · In message <[email protected]> you wrote:

This patch cleans driver code replacing all accesses to registers with fixed offsets with a corresponding structure.

Quoted 6 lines

Checkpatch says:

[U-Boot] [PATCH V2 10/11] SPI: mxc_spi: replace fixed offsets with
total: 3 errors, 0 warnings, 274 lines checked

Please fix.

ok, we are clearly a problem using checkpatch:

checkpatch.pl --no-tree 0010-SPI-mxc_spi-replace-fixed-offsets-with-structures.patch
total: 0 errors, 0 warnings, 274 lines checked

0010-SPI-mxc_spi-replace-fixed-offsets-with-structures.patch has no obvious style problems and is ready for submission.

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
=====================================================================
#35 lore raw

PATCH v2 11/11 Add support for Freescale's mx35pdk board.

In reply to Stefano Babic

The patch adds suupport for the Freescale's mx35pdk board (known as well as mx35_3stack).

The board boots from the NOR flash. Following devices are supported:
- two ethernet devices (FEC and SMC911x on debug board)
- I2C
- PMIC (MC13892) via I2C interface
- UART
- NOR flash (64MB)
- NAND flash (2GB)
- basic access to mc9sdz60 registers via I2C interface

---
 MAINTAINERS                             |    3 +-
 board/freescale/mx35pdk/Makefile        |   49 ++++
 board/freescale/mx35pdk/lowlevel_init.S |  363 +++++++++++++++++++++++++++++++
 board/freescale/mx35pdk/mx35pdk.c       |  309 ++++++++++++++++++++++++++
 board/freescale/mx35pdk/mx35pdk.h       |  101 +++++++++
 boards.cfg                              |    1 +
 doc/README.mx35pdk                      |  188 ++++++++++++++++
 include/configs/mx35pdk.h               |  303 ++++++++++++++++++++++++++
 8 files changed, 1316 insertions(+), 1 deletions(-)
 create mode 100644 board/freescale/mx35pdk/Makefile
 create mode 100644 board/freescale/mx35pdk/lowlevel_init.S
 create mode 100644 board/freescale/mx35pdk/mx35pdk.c
 create mode 100644 board/freescale/mx35pdk/mx35pdk.h
 create mode 100644 doc/README.mx35pdk
 create mode 100644 include/configs/mx35pdk.h
MAINTAINERS +2 -1
index 79aa596..0c3ddc1 100644
@@ -552,9 +552,10 @@ Rowel Atienza <[email protected]>
 Stefano Babic <[email protected]>
 
 	ea20		davinci
+	mx35pdk		i.MX35
+	mx51evk		i.MX51
 	polaris		xscale
 	trizepsiv	xscale
-	mx51evk		i.MX51
  	vision2		i.MX51
 
 Jason Liu <[email protected]>
board/freescale/mx35pdk/Makefile new file +49 -0

49 lines not on this page · raw message

board/freescale/mx35pdk/lowlevel_init.S new file +363 -0

363 lines not on this page · raw message

board/freescale/mx35pdk/mx35pdk.c new file +309 -0

309 lines not on this page · raw message

board/freescale/mx35pdk/mx35pdk.h new file +101 -0

101 lines not on this page · raw message

boards.cfg +1 -0
index d312ee8..6fb3882 100644
@@ -38,6 +38,7 @@
 qong                         arm         arm1136     -                   davedenx       mx31
 mx31ads                      arm         arm1136     -                   freescale      mx31
 imx31_litekit                arm         arm1136     -                   logicpd        mx31
+mx35pdk                      arm         arm1136     -                   freescale      mx35
 omap2420h4                   arm         arm1136     -                   ti             omap24xx
 tnetv107x_evm                arm         arm1176     tnetv107xevm        ti             tnetv107x
 armadillo                    arm         arm720t
doc/README.mx35pdk new file +188 -0

188 lines not on this page · raw message

include/configs/mx35pdk.h new file +303 -0

303 lines not on this page · raw message

Signature · 1 line
1.7.1
#36 lore raw

PATCH v2 11/11 Add support for Freescale's mx35pdk board.

Dear Stefano Babic,

Quoted 2 lines · In message <[email protected]> you wrote:

The patch adds suupport for the Freescale's mx35pdk board (known as well as mx35_3stack).

Checkpatch says:

[U-Boot] [PATCH V2 11/11] Add support for Freescale's mx35pdk board.
total: 0 errors, 1 warnings, 1331 lines checked

(Prototype for smc911x_initialize() is in netdev.h).

...

Quoted 13 lines

+.macro init_clock

board/freescale/mx35pdk/lowlevel_init.S new file +12 -0
+	ldr r0, =CCM_BASE_ADDR
+
+        /* default CLKO to 1/32 of the ARM core*/
+        ldr r1, [r0, #CLKCTL_COSR]
+        bic r1, r1, #0x00000FF00
+        bic r1, r1, #0x0000000FF
+        mov r2, #0x00006C00
+        add r2, r2, #0x67
+        orr r1, r1, r2
+        str r1, [r0, #CLKCTL_COSR]
+
+        ldr r2, =CCM_CCMR_CONFIG

board/freescale/mx35pdk/lowlevel_init.S line 162

Quoted 10 lines
board/freescale/mx35pdk/lowlevel_init.S new file +10 -0
+        str r2, [r0, #CLKCTL_CCMR]
+
+	check_soc_version r1, r2
+	cmp r1, #CHIP_REV_2_0
+	ldrhs r3, =CCM_MPLL_532_HZ
+	bhs 1f
+	ldr r2, [r0, #CLKCTL_PDR0]
+        tst r2, #CLKMODE_CONSUMER
+        ldrne r3, =CCM_MPLL_532_HZ  /* consumer path*/
+        ldreq r3, =CCM_MPLL_399_HZ  /* auto path*/

etc. etc.

Please use TAB for indentation.

board/freescale/mx35pdk/mx35pdk.c line 259

Quoted 12 lines
board/freescale/mx35pdk/mx35pdk.c new file +12 -0
+int checkboard(void)
+{
+	struct ccm_regs *ccm =
+		(struct ccm_regs *)IMX_CCM_BASE;
+
+	puts("Board: MX35 PDK ");
+
+	/*
+	 * Be sure that I2C is initialized to check
+	 * the board revision
+	 */
+	i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);

Error checking?

board/freescale/mx35pdk/mx35pdk.c line 265

Quoted 5 lines
board/freescale/mx35pdk/mx35pdk.c new file +5 -0
+	/* Print board revision */
+	puts(board_detect() ? "2.0" : "1.0");
+ 
+	/* Print CPU revision */
+	puts(" i.MX35 ");

I mentioned this before: If you make board_detect() return 1 or 2, you can combine the calls to puts() for example like that:

	printf("Board: MX35 PDK %d.0 i.MX35 ", board_detect());

board/freescale/mx35pdk/mx35pdk.c line 269

Quoted 4 lines
board/freescale/mx35pdk/mx35pdk.c new file +4 -0
+	if (get_cpu_rev() & CHIP_REV_2_0)
+		puts("2.0 [");
+	else
+		puts("1.0 [");

Eventually similar improvment could be done here. [Just a hint - feel free to post-pone into a later patch if this affects other boards as well.]

Best regards,

Wolfgang Denk

Signature · 7 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
Romulan women are not like Vulcan females. We are  not  dedicated  to
pure logic and the sterility of non-emotion.
	-- Romulan Commander, "The Enterprise Incident",
	   stardate 5027.3
#37 lore raw

PATCH v2 11/11 Add support for Freescale's mx35pdk board.

Quoted 6 lines · Wolfgang Denk

Dear Stefano Babic,

Quoted 2 lines · In message <[email protected]> you wrote:

The patch adds suupport for the Freescale's mx35pdk board (known as well as mx35_3stack).

Quoted 6 lines

Checkpatch says:

[U-Boot] [PATCH V2 11/11] Add support for Freescale's mx35pdk board.
total: 0 errors, 1 warnings, 1331 lines checked

(Prototype for smc911x_initialize() is in netdev.h).

Thanks, I have not search with attention. I fix it.

Quoted 1 line

Please use TAB for indentation.

Thanks - in this case checkpatch reports nothing, and I suppose everything is ok.

Quoted 16 lines
Quoted 4 lines
board/freescale/mx35pdk/mx35pdk.c new file +2 -0
+int checkboard(void)
+{

board/freescale/mx35pdk/mx35pdk.c line 259

Quoted 10 lines
board/freescale/mx35pdk/mx35pdk.c new file +10 -0
+	struct ccm_regs *ccm =
+		(struct ccm_regs *)IMX_CCM_BASE;
+
+	puts("Board: MX35 PDK ");
+
+	/*
+	 * Be sure that I2C is initialized to check
+	 * the board revision
+	 */
+	i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);

Error checking?

I wil ladd it

Quoted 9 lines

board/freescale/mx35pdk/mx35pdk.c line 265

Quoted 6 lines
board/freescale/mx35pdk/mx35pdk.c new file +5 -0
+	/* Print board revision */
+	puts(board_detect() ? "2.0" : "1.0");
+ 
+	/* Print CPU revision */
+	puts(" i.MX35 ");

I mentioned this before: If you make board_detect() return 1 or 2, you can combine the calls to puts() for example like that:

I was unsure, it seemed to me easier to understand as it is implemented now, becaues board_detect() is used to detect if the PMIC is installed or not. It returns 0 or 1, and tell if the test for the PMIC was successful.

Probably the former version of the board has no pmic at all or was not connected to I2C. So in another part of code board_detect is used in boolean form:

if (board_detect()) {

I thought that to combine the result makes some confusion. Probably it is clearer to use get_board_rev() instead of board_detect() and to extract the revision number from the returned u32:

printf("Board: MX35 PDK %d.0 i.MX35 ", (get_board_rev() >> 8) & 0xFF);

Quoted 3 lines

Eventually similar improvment could be done here. [Just a hint - feel free to post-pone into a later patch if this affects other boards as well.]

Agree, to change this I would prefer a patch for all i.MX boards at the same time. They use at the moment the same approach I use now.

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
=====================================================================
#38 lore raw

PATCH v2 11/11 Add support for Freescale's mx35pdk board.

Dear Stefano Babic,

Quoted 15 lines · In message <[email protected]> you wrote:
Quoted 9 lines

board/freescale/mx35pdk/mx35pdk.c line 265

Quoted 6 lines
board/freescale/mx35pdk/mx35pdk.c new file +5 -0
+	/* Print board revision */
+	puts(board_detect() ? "2.0" : "1.0");
+ 
+	/* Print CPU revision */
+	puts(" i.MX35 ");

I mentioned this before: If you make board_detect() return 1 or 2, you can combine the calls to puts() for example like that:

I was unsure, it seemed to me easier to understand as it is implemented now, becaues board_detect() is used to detect if the PMIC is installed or not. It returns 0 or 1, and tell if the test for the PMIC was successful.

Quoted 5 lines

Probably the former version of the board has no pmic at all or was not connected to I2C. So in another part of code board_detect is used in boolean form:

if (board_detect()) {

This function should be called pmic_detect() or so.

Quoted 5 lines

I thought that to combine the result makes some confusion. Probably it is clearer to use get_board_rev() instead of board_detect() and to extract the revision number from the returned u32:

printf("Board: MX35 PDK %d.0 i.MX35 ", (get_board_rev() >> 8) & 0xFF);

Agreed.

Best regards,

Wolfgang Denk

Signature · 7 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
It's certainly  convenient  the  way  the  crime  (or  condition)  of
stupidity   carries   with   it  its  own  punishment,  automatically
admisistered without remorse, pity, or prejudice. :-)
         -- Tom Christiansen in <[email protected]>
#39 lore raw

PATCH v3 11/11 Add support for Freescale's mx35pdk board.

In reply to Stefano Babic

The patch adds suupport for the Freescale's mx35pdk board (known as well as mx35_3stack).

The board boots from the NOR flash. Following devices are supported:
- two ethernet devices (FEC and SMC911x on debug board)
- I2C
- PMIC (MC13892) via I2C interface
- UART
- NOR flash (64MB)
- NAND flash (2GB)
- basic access to mc9sdz60 registers via I2C interface

---
Changes:

Wolfgang Denk:
- replace board_detect with a more correct name
- use the output of get_board_rev to print the board revision
- remove leading spaces from lowlevel_init.S

 MAINTAINERS                             |    3 +-
 board/freescale/mx35pdk/Makefile        |   49 ++++
 board/freescale/mx35pdk/lowlevel_init.S |  363 +++++++++++++++++++++++++++++++
 board/freescale/mx35pdk/mx35pdk.c       |  300 +++++++++++++++++++++++++
 board/freescale/mx35pdk/mx35pdk.h       |  101 +++++++++
 boards.cfg                              |    1 +
 doc/README.mx35pdk                      |  188 ++++++++++++++++
 include/configs/mx35pdk.h               |  303 ++++++++++++++++++++++++++
 8 files changed, 1307 insertions(+), 1 deletions(-)
 create mode 100644 board/freescale/mx35pdk/Makefile
 create mode 100644 board/freescale/mx35pdk/lowlevel_init.S
 create mode 100644 board/freescale/mx35pdk/mx35pdk.c
 create mode 100644 board/freescale/mx35pdk/mx35pdk.h
 create mode 100644 doc/README.mx35pdk
 create mode 100644 include/configs/mx35pdk.h
MAINTAINERS +2 -1
index 79aa596..0c3ddc1 100644
@@ -552,9 +552,10 @@ Rowel Atienza <[email protected]>
 Stefano Babic <[email protected]>
 
 	ea20		davinci
+	mx35pdk		i.MX35
+	mx51evk		i.MX51
 	polaris		xscale
 	trizepsiv	xscale
-	mx51evk		i.MX51
  	vision2		i.MX51
 
 Jason Liu <[email protected]>
board/freescale/mx35pdk/Makefile new file +49 -0

49 lines not on this page · raw message

board/freescale/mx35pdk/lowlevel_init.S new file +363 -0

363 lines not on this page · raw message

board/freescale/mx35pdk/mx35pdk.c new file +300 -0

300 lines not on this page · raw message

board/freescale/mx35pdk/mx35pdk.h new file +101 -0

101 lines not on this page · raw message

boards.cfg +1 -0
index d312ee8..6fb3882 100644
@@ -38,6 +38,7 @@
 qong                         arm         arm1136     -                   davedenx       mx31
 mx31ads                      arm         arm1136     -                   freescale      mx31
 imx31_litekit                arm         arm1136     -                   logicpd        mx31
+mx35pdk                      arm         arm1136     -                   freescale      mx35
 omap2420h4                   arm         arm1136     -                   ti             omap24xx
 tnetv107x_evm                arm         arm1176     tnetv107xevm        ti             tnetv107x
 armadillo                    arm         arm720t
doc/README.mx35pdk new file +188 -0

188 lines not on this page · raw message

include/configs/mx35pdk.h new file +303 -0

303 lines not on this page · raw message

Signature · 1 line
1.7.1
#40 lore raw

PATCH v4 11/11 Add support for Freescale's mx35pdk board.

The patch adds suupport for the Freescale's mx35pdk board (known as well as mx35_3stack).

The board boots from the NOR flash. Following devices are supported:
- two ethernet devices (FEC and SMC911x on debug board)
- I2C
- PMIC (MC13892) via I2C interface
- UART
- NOR flash (64MB)
- NAND flash (2GB)
- basic access to mc9sdz60 registers via I2C interface

---
Please ignore V3 of this patch : the board version was not correctly printed. Sorry for noise.

Changes:

Wolfgang Denk:
- replace board_detect with a more correct name
- use the output of get_board_rev to print the board revision
- remove leading spaces from lowlevel_init.S

 MAINTAINERS                             |    3 +-
 board/freescale/mx35pdk/Makefile        |   49 ++++
 board/freescale/mx35pdk/lowlevel_init.S |  363 +++++++++++++++++++++++++++++++
 board/freescale/mx35pdk/mx35pdk.c       |  298 +++++++++++++++++++++++++
 board/freescale/mx35pdk/mx35pdk.h       |  101 +++++++++
 boards.cfg                              |    1 +
 doc/README.mx35pdk                      |  188 ++++++++++++++++
 include/configs/mx35pdk.h               |  303 ++++++++++++++++++++++++++
 8 files changed, 1305 insertions(+), 1 deletions(-)
 create mode 100644 board/freescale/mx35pdk/Makefile
 create mode 100644 board/freescale/mx35pdk/lowlevel_init.S
 create mode 100644 board/freescale/mx35pdk/mx35pdk.c
 create mode 100644 board/freescale/mx35pdk/mx35pdk.h
 create mode 100644 doc/README.mx35pdk
 create mode 100644 include/configs/mx35pdk.h
MAINTAINERS +2 -1
index 79aa596..0c3ddc1 100644
@@ -552,9 +552,10 @@ Rowel Atienza <[email protected]>
 Stefano Babic <[email protected]>
 
 	ea20		davinci
+	mx35pdk		i.MX35
+	mx51evk		i.MX51
 	polaris		xscale
 	trizepsiv	xscale
-	mx51evk		i.MX51
  	vision2		i.MX51
 
 Jason Liu <[email protected]>
board/freescale/mx35pdk/Makefile new file +49 -0

49 lines not on this page · raw message

board/freescale/mx35pdk/lowlevel_init.S new file +363 -0

363 lines not on this page · raw message

board/freescale/mx35pdk/mx35pdk.c new file +298 -0

298 lines not on this page · raw message

board/freescale/mx35pdk/mx35pdk.h new file +101 -0

101 lines not on this page · raw message

boards.cfg +1 -0
index d312ee8..6fb3882 100644
@@ -38,6 +38,7 @@
 qong                         arm         arm1136     -                   davedenx       mx31
 mx31ads                      arm         arm1136     -                   freescale      mx31
 imx31_litekit                arm         arm1136     -                   logicpd        mx31
+mx35pdk                      arm         arm1136     -                   freescale      mx35
 omap2420h4                   arm         arm1136     -                   ti             omap24xx
 tnetv107x_evm                arm         arm1176     tnetv107xevm        ti             tnetv107x
 armadillo                    arm         arm720t
doc/README.mx35pdk new file +188 -0

188 lines not on this page · raw message

include/configs/mx35pdk.h new file +303 -0

303 lines not on this page · raw message

Signature · 1 line
1.7.1
#41 lore raw

PATCH v4 11/11 Add support for Freescale's mx35pdk board.

Quoted 13 lines · Stefano Babic

The patch adds suupport for the Freescale's mx35pdk board (known as well as mx35_3stack).

The board boots from the NOR flash. Following devices are supported:
- two ethernet devices (FEC and SMC911x on debug board)
- I2C
- PMIC (MC13892) via I2C interface
- UART
- NOR flash (64MB)
- NAND flash (2GB)
- basic access to mc9sdz60 registers via I2C interface

Quoted 1 line

Applied to u-boot-imx, thanks

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
=====================================================================
#42 lore raw

PATCH v3 11/11 Add support for Freescale's mx35pdk board.

In reply to Stefano Babic
Quoted 13 lines · Stefano Babic

The patch adds suupport for the Freescale's mx35pdk board (known as well as mx35_3stack).

The board boots from the NOR flash. Following devices are supported:
- two ethernet devices (FEC and SMC911x on debug board)
- I2C
- PMIC (MC13892) via I2C interface
- UART
- NOR flash (64MB)
- NAND flash (2GB)
- basic access to mc9sdz60 registers via I2C interface

Quoted 1 line

Applied to u-boot-imx, thanks

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
=====================================================================
#43 lore raw
In reply to Stefano Babic

Dear Stefano Babic,

Quoted 2 lines · In message <[email protected]> you wrote:

Patchset V2 contain patches for SPI sent previously independently to ML and additional patches for I2C.

I just learned that I was running an old version of checkpatch, which turned out some false positives (and my short visual inspection did not recognize this).

Sorry for the flase alarms.

The remaining warnings are:

[U-Boot] [PATCH V2 01/11] Add support for MX35 processor
total: 0 errors, 8 warnings, 2109 lines checked

[U-Boot] [PATCH V2 04/11] I2C: mxc_i2c: get rid of __REG access
total: 0 errors, 2 warnings, 169 lines checked

[U-Boot] [PATCH V2 05/11] I2C: mxc_i2c: address failure with mx35
total: 1 errors, 1 warnings, 166 lines checked

[U-Boot] [PATCH V2 08/11] SPI: mxc_spi: fix swapping bug and add
total: 0 errors, 1 warnings, 22 lines checked

[U-Boot] [PATCH V2 11/11] Add support for Freescale's mx35pdk board.
total: 0 errors, 1 warnings, 1331 lines checked

Best regards,

Wolfgang Denk

Signature · 4 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
What was sliced bread the greatest thing since?
#44 lore raw
Quoted 11 lines · Wolfgang Denk

Dear Stefano Babic,

Quoted 2 lines · In message <[email protected]> you wrote:

Patchset V2 contain patches for SPI sent previously independently to ML and additional patches for I2C.

I just learned that I was running an old version of checkpatch, which turned out some false positives (and my short visual inspection did not recognize this)

Sorry for the flase alarms.

No problem, this explains the the different results. I ran checkpatch on the patchset.

Quoted 5 lines

The remaining warnings are:

[U-Boot] [PATCH V2 01/11] Add support for MX35 processor
total: 0 errors, 8 warnings, 2109 lines checked

They are related to typedef (mmmhh...there is one regarding usage of volatile, I will fix it). I do not think that using typedef is forbidden.

Quoted 3 lines

[U-Boot] [PATCH V2 04/11] I2C: mxc_i2c: get rid of __REG access
total: 0 errors, 2 warnings, 169 lines checked

__FUNCTION__, I will fix it

Quoted 3 lines

[U-Boot] [PATCH V2 05/11] I2C: mxc_i2c: address failure with mx35
total: 1 errors, 1 warnings, 166 lines checked

See my previous comment. In any case, I will change to avoid checkpatch complains

Quoted 3 lines

[U-Boot] [PATCH V2 08/11] SPI: mxc_spi: fix swapping bug and add
total: 0 errors, 1 warnings, 22 lines checked

Braces, braces...do you agree to follow checkpatch's rule (no braces for single statement) ?

Quoted 3 lines

[U-Boot] [PATCH V2 11/11] Add support for Freescale's mx35pdk board.
total: 0 errors, 1 warnings, 1331 lines checked

You already give me the solution

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
=====================================================================
#45 lore raw

Dear Stefano Babic,

Quoted 6 lines · In message <[email protected]> you wrote:
Quoted 3 lines

[U-Boot] [PATCH V2 08/11] SPI: mxc_spi: fix swapping bug and add
total: 0 errors, 1 warnings, 22 lines checked

Braces, braces...do you agree to follow checkpatch's rule (no braces for single statement) ?

Try it out :-)

Best regards,

Wolfgang Denk

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-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Faith may be defined briefly as an illogical belief in the  occurence
of the improbable.                                    - H. L. Mencken