[U-Boot] [PATCH 1/7] i2c: Use __weak instead of __attribute__((weak, alias))

[U-Boot] [PATCH 1/7] i2c: Use __weak instead of __attribute__((weak, alias))

[Marek Vasut]

Use __weak from linux/compiler.h instead of __attribute__((weak, alias))
to define overridable function. This patch is intended as a cleanup patch
to bring some consistency into the code.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Heiko Schocher <hs@denx.de>
---
 common/cmd_i2c.c |   16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/common/cmd_i2c.c b/common/cmd_i2c.c
index 4438db5..c8990ef 100644
--- a/common/cmd_i2c.c
+++ b/common/cmd_i2c.c
@@ -82,6 +82,7 @@
 #include <i2c.h>
 #include <malloc.h>
 #include <asm/byteorder.h>
+#include <linux/compiler.h>
 
 /* Display values from last command.
  * Memory modify remembered values are different from display memory.
@@ -133,30 +134,27 @@ DECLARE_GLOBAL_DATA_PTR;
 #define DISP_LINE_LEN	16
 
 /* implement possible board specific board init */
-static void __def_i2c_init_board(void)
+__weak
+void i2c_init_board(void)
 {
 	return;
 }
-void i2c_init_board(void)
-	__attribute__((weak, alias("__def_i2c_init_board")));
 
 /* TODO: Implement architecture-specific get/set functions */
-static unsigned int __def_i2c_get_bus_speed(void)
+__weak
+unsigned int i2c_get_bus_speed(void)
 {
 	return CONFIG_SYS_I2C_SPEED;
 }
-unsigned int i2c_get_bus_speed(void)
-	__attribute__((weak, alias("__def_i2c_get_bus_speed")));
 
-static int __def_i2c_set_bus_speed(unsigned int speed)
+__weak
+int i2c_set_bus_speed(unsigned int speed)
 {
 	if (speed != CONFIG_SYS_I2C_SPEED)
 		return -1;
 
 	return 0;
 }
-int i2c_set_bus_speed(unsigned int)
-	__attribute__((weak, alias("__def_i2c_set_bus_speed")));
 
 /*
  * get_alen: small parser helper function to get address length
-- 
1.7.10.4

[U-Boot] [PATCH 2/7] i2c: Staticize local functions in mxc i2c driver

[Marek Vasut]

Some functions in the MXC i2c driver were not static, fix this by
making them so.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Heiko Schocher <hs@denx.de>
Cc: Stefano Babic <sbabic@denx.de>
---
 drivers/i2c/mxc_i2c.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
index 18270b9..a73b10b 100644
--- a/drivers/i2c/mxc_i2c.c
+++ b/drivers/i2c/mxc_i2c.c
@@ -115,7 +115,7 @@ static uint8_t i2c_imx_get_clk(unsigned int rate)
 /*
  * Set I2C Bus speed
  */
-int bus_i2c_set_bus_speed(void *base, int speed)
+static int bus_i2c_set_bus_speed(void *base, int speed)
 {
 	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)base;
 	u8 clk_idx = i2c_imx_get_clk(speed);
@@ -133,7 +133,7 @@ int bus_i2c_set_bus_speed(void *base, int speed)
 /*
  * Get I2C Speed
  */
-unsigned int bus_i2c_get_bus_speed(void *base)
+static unsigned int bus_i2c_get_bus_speed(void *base)
 {
 	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)base;
 	u8 clk_idx = readb(&i2c_regs->ifdr);
-- 
1.7.10.4

[U-Boot] [PATCH 3/7] i2c: kerneldoc: Add kerneldoc annotations to cmd_i2c.c

[Marek Vasut]

Add kerneldoc style documentation into cmd_i2c.c to properly describe
all overridable functions and most of the command interface.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Heiko Schocher <hs@denx.de>
---
 common/cmd_i2c.c |  194 +++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 179 insertions(+), 15 deletions(-)

diff --git a/common/cmd_i2c.c b/common/cmd_i2c.c
index c8990ef..e7df2e4 100644
--- a/common/cmd_i2c.c
+++ b/common/cmd_i2c.c
@@ -133,7 +133,13 @@ DECLARE_GLOBAL_DATA_PTR;
 
 #define DISP_LINE_LEN	16
 
-/* implement possible board specific board init */
+/**
+ * i2c_init_board() - Board-specific I2C bus init
+ *
+ * This function is the default no-op implementation of I2C bus
+ * initialization. This function can be overriden by board-specific
+ * implementation if needed.
+ */
 __weak
 void i2c_init_board(void)
 {
@@ -141,12 +147,38 @@ void i2c_init_board(void)
 }
 
 /* TODO: Implement architecture-specific get/set functions */
+
+/**
+ * i2c_get_bus_speed() - Return I2C bus speed
+ *
+ * This function is the default implementation of function for retrieveing
+ * the current I2C bus speed in Hz.
+ *
+ * A driver implementing runtime switching of I2C bus speed must override
+ * this function to report the speed correctly. Simple or legacy drivers
+ * can use this fallback.
+ *
+ * Returns I2C bus speed in Hz.
+ */
 __weak
 unsigned int i2c_get_bus_speed(void)
 {
 	return CONFIG_SYS_I2C_SPEED;
 }
 
+/**
+ * i2c_set_bus_speed() - Configure I2C bus speed
+ * @speed:	Newly set speed of the I2C bus in Hz
+ *
+ * This function is the default implementation of function for setting
+ * the I2C bus speed in Hz.
+ *
+ * A driver implementing runtime switching of I2C bus speed must override
+ * this function to report the speed correctly. Simple or legacy drivers
+ * can use this fallback.
+ *
+ * Returns zero on success, negative value on error.
+ */
 __weak
 int i2c_set_bus_speed(unsigned int speed)
 {
@@ -156,9 +188,10 @@ int i2c_set_bus_speed(unsigned int speed)
 	return 0;
 }
 
-/*
- * get_alen: small parser helper function to get address length
- * returns the address length
+/**
+ * get_alen() - Small parser helper function to get address length
+ *
+ * Returns the address length.
  */
 static uint get_alen(char *arg)
 {
@@ -176,11 +209,19 @@ static uint get_alen(char *arg)
 	return alen;
 }
 
-/*
+/**
+ * do_i2c_read() - Handle the "i2c read" command-line command
+ * @cmdtp:	Command data struct pointer
+ * @flag:	Command flag
+ * @argc:	Command-line argument count
+ * @argv:	Array of command-line arguments
+ *
+ * Returns zero on success, CMD_RET_USAGE in case of misuse and negative
+ * on error.
+ *
  * Syntax:
  *	i2c read {i2c_chip} {devaddr}{.0, .1, .2} {len} {memaddr}
  */
-
 static int do_i2c_read ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	u_char	chip;
@@ -269,7 +310,16 @@ static int do_i2c_write(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[
 	return 0;
 }
 
-/*
+/**
+ * do_i2c_md() - Handle the "i2c md" command-line command
+ * @cmdtp:	Command data struct pointer
+ * @flag:	Command flag
+ * @argc:	Command-line argument count
+ * @argv:	Array of command-line arguments
+ *
+ * Returns zero on success, CMD_RET_USAGE in case of misuse and negative
+ * on error.
+ *
  * Syntax:
  *	i2c md {i2c_chip} {addr}{.0, .1, .2} {len}
  */
@@ -361,8 +411,15 @@ static int do_i2c_md ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]
 	return 0;
 }
 
-
-/* Write (fill) memory
+/**
+ * do_i2c_mw() - Handle the "i2c mw" command-line command
+ * @cmdtp:	Command data struct pointer
+ * @flag:	Command flag
+ * @argc:	Command-line argument count
+ * @argv:	Array of command-line arguments
+ *
+ * Returns zero on success, CMD_RET_USAGE in case of misuse and negative
+ * on error.
  *
  * Syntax:
  *	i2c mw {i2c_chip} {addr}{.0, .1, .2} {data} [{count}]
@@ -419,10 +476,20 @@ static int do_i2c_mw ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]
 #endif
 	}
 
-	return (0);
+	return 0;
 }
 
-/* Calculate a CRC on memory
+/**
+ * do_i2c_crc() - Handle the "i2c crc32" command-line command
+ * @cmdtp:	Command data struct pointer
+ * @flag:	Command flag
+ * @argc:	Command-line argument count
+ * @argv:	Array of command-line arguments
+ *
+ * Calculate a CRC on memory
+ *
+ * Returns zero on success, CMD_RET_USAGE in case of misuse and negative
+ * on error.
  *
  * Syntax:
  *	i2c crc32 {i2c_chip} {addr}{.0, .1, .2} {count}
@@ -479,13 +546,22 @@ static int do_i2c_crc (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]
 	return 0;
 }
 
-/* Modify memory.
+/**
+ * mod_i2c_mem() - Handle the "i2c mm" and "i2c nm" command-line command
+ * @cmdtp:	Command data struct pointer
+ * @flag:	Command flag
+ * @argc:	Command-line argument count
+ * @argv:	Array of command-line arguments
+ *
+ * Modify memory.
+ *
+ * Returns zero on success, CMD_RET_USAGE in case of misuse and negative
+ * on error.
  *
  * Syntax:
  *	i2c mm{.b, .w, .l} {i2c_chip} {addr}{.0, .1, .2}
  *	i2c nm{.b, .w, .l} {i2c_chip} {addr}{.0, .1, .2}
  */
-
 static int
 mod_i2c_mem(cmd_tbl_t *cmdtp, int incrflag, int flag, int argc, char * const argv[])
 {
@@ -601,7 +677,16 @@ mod_i2c_mem(cmd_tbl_t *cmdtp, int incrflag, int flag, int argc, char * const arg
 	return 0;
 }
 
-/*
+/**
+ * do_i2c_probe() - Handle the "i2c probe" command-line command
+ * @cmdtp:	Command data struct pointer
+ * @flag:	Command flag
+ * @argc:	Command-line argument count
+ * @argv:	Array of command-line arguments
+ *
+ * Returns zero on success, CMD_RET_USAGE in case of misuse and negative
+ * on error.
+ *
  * Syntax:
  *	i2c probe {addr}
  *
@@ -655,7 +740,16 @@ static int do_i2c_probe (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv
 	return (0 == found);
 }
 
-/*
+/**
+ * do_i2c_loop() - Handle the "i2c loop" command-line command
+ * @cmdtp:	Command data struct pointer
+ * @flag:	Command flag
+ * @argc:	Command-line argument count
+ * @argv:	Array of command-line arguments
+ *
+ * Returns zero on success, CMD_RET_USAGE in case of misuse and negative
+ * on error.
+ *
  * Syntax:
  *	i2c loop {i2c_chip} {addr}{.0, .1, .2} [{length}] [{delay}]
  *	{length} - Number of bytes to read
@@ -716,6 +810,8 @@ static int do_i2c_loop(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]
 /*
  * The SDRAM command is separately configured because many
  * (most?) embedded boards don't use SDRAM DIMMs.
+ *
+ * FIXME: Document and probably move elsewhere!
  */
 #if defined(CONFIG_CMD_SDRAM)
 static void print_ddr2_tcyc (u_char const b)
@@ -1245,6 +1341,15 @@ static int do_sdram (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 #endif
 
 #if defined(CONFIG_I2C_MUX)
+/**
+ * do_i2c_add_bus() - Handle the "i2c bus" command-line command
+ * @cmdtp:	Command data struct pointer
+ * @flag:	Command flag
+ * @argc:	Command-line argument count
+ * @argv:	Array of command-line arguments
+ *
+ * Returns zero always.
+ */
 static int do_i2c_add_bus(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 {
 	int ret=0;
@@ -1274,6 +1379,16 @@ static int do_i2c_add_bus(cmd_tbl_t * cmdtp, int flag, int argc, char * const ar
 #endif  /* CONFIG_I2C_MUX */
 
 #if defined(CONFIG_I2C_MULTI_BUS)
+/**
+ * do_i2c_bus_num() - Handle the "i2c dev" command-line command
+ * @cmdtp:	Command data struct pointer
+ * @flag:	Command flag
+ * @argc:	Command-line argument count
+ * @argv:	Array of command-line arguments
+ *
+ * Returns zero on success, CMD_RET_USAGE in case of misuse and negative
+ * on error.
+ */
 static int do_i2c_bus_num(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 {
 	int bus_idx, ret=0;
@@ -1292,6 +1407,16 @@ static int do_i2c_bus_num(cmd_tbl_t * cmdtp, int flag, int argc, char * const ar
 }
 #endif  /* CONFIG_I2C_MULTI_BUS */
 
+/**
+ * do_i2c_bus_speed() - Handle the "i2c speed" command-line command
+ * @cmdtp:	Command data struct pointer
+ * @flag:	Command flag
+ * @argc:	Command-line argument count
+ * @argv:	Array of command-line arguments
+ *
+ * Returns zero on success, CMD_RET_USAGE in case of misuse and negative
+ * on error.
+ */
 static int do_i2c_bus_speed(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 {
 	int speed, ret=0;
@@ -1309,16 +1434,45 @@ static int do_i2c_bus_speed(cmd_tbl_t * cmdtp, int flag, int argc, char * const
 	return ret;
 }
 
+/**
+ * do_i2c_mm() - Handle the "i2c mm" command-line command
+ * @cmdtp:	Command data struct pointer
+ * @flag:	Command flag
+ * @argc:	Command-line argument count
+ * @argv:	Array of command-line arguments
+ *
+ * Returns zero on success, CMD_RET_USAGE in case of misuse and negative
+ * on error.
+ */
 static int do_i2c_mm(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 {
 	return mod_i2c_mem (cmdtp, 1, flag, argc, argv);
 }
 
+/**
+ * do_i2c_nm() - Handle the "i2c nm" command-line command
+ * @cmdtp:	Command data struct pointer
+ * @flag:	Command flag
+ * @argc:	Command-line argument count
+ * @argv:	Array of command-line arguments
+ *
+ * Returns zero on success, CMD_RET_USAGE in case of misuse and negative
+ * on error.
+ */
 static int do_i2c_nm(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 {
 	return mod_i2c_mem (cmdtp, 0, flag, argc, argv);
 }
 
+/**
+ * do_i2c_reset() - Handle the "i2c reset" command-line command
+ * @cmdtp:	Command data struct pointer
+ * @flag:	Command flag
+ * @argc:	Command-line argument count
+ * @argv:	Array of command-line arguments
+ *
+ * Returns zero always.
+ */
 static int do_i2c_reset(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 {
 	i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
@@ -1354,6 +1508,16 @@ void i2c_reloc(void) {
 }
 #endif
 
+/**
+ * do_i2c() - Handle the "i2c" command-line command
+ * @cmdtp:	Command data struct pointer
+ * @flag:	Command flag
+ * @argc:	Command-line argument count
+ * @argv:	Array of command-line arguments
+ *
+ * Returns zero on success, CMD_RET_USAGE in case of misuse and negative
+ * on error.
+ */
 static int do_i2c(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 {
 	cmd_tbl_t *c;
-- 
1.7.10.4

[U-Boot] [PATCH 4/7] i2c: mxs: Abstract out the MXS I2C speed setup

[Marek Vasut]

This patch pulls out the I2C speed setup from the i2c_init() call
and implements the bus configuration lookup table with register
values that needs to be programmed into the I2C IP to run at
particular speed.

This patch is a first step towards implementing run-time I2C bus
speed configuration for the MXS I2C IP.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@freescale.com>
---
 drivers/i2c/mxs_i2c.c |   57 ++++++++++++++++++++++++++++++++-----------------
 1 file changed, 37 insertions(+), 20 deletions(-)

diff --git a/drivers/i2c/mxs_i2c.c b/drivers/i2c/mxs_i2c.c
index 2a193c2..98f6e8c 100644
--- a/drivers/i2c/mxs_i2c.c
+++ b/drivers/i2c/mxs_i2c.c
@@ -210,34 +210,51 @@ int i2c_probe(uchar chip)
 	return ret;
 }
 
+static struct mxs_i2c_speed_table {
+	uint32_t	speed;
+	uint32_t	timing0;
+	uint32_t	timing1;
+} mxs_i2c_tbl[] = {
+	{
+		100000,
+		(0x0078 << I2C_TIMING0_HIGH_COUNT_OFFSET) |
+		(0x0030 << I2C_TIMING0_RCV_COUNT_OFFSET),
+		(0x0080 << I2C_TIMING1_LOW_COUNT_OFFSET) |
+		(0x0030 << I2C_TIMING1_XMIT_COUNT_OFFSET)
+	},
+	{
+		400000,
+		(0x000f << I2C_TIMING0_HIGH_COUNT_OFFSET) |
+		(0x0007 << I2C_TIMING0_RCV_COUNT_OFFSET),
+		(0x001f << I2C_TIMING1_LOW_COUNT_OFFSET) |
+		(0x000f << I2C_TIMING1_XMIT_COUNT_OFFSET),
+	}
+};
+
+static struct mxs_i2c_speed_table *mxs_i2c_speed_to_cfg(uint32_t speed)
+{
+	int i;
+	for (i = 0; i < ARRAY_SIZE(mxs_i2c_tbl); i++)
+		if (mxs_i2c_tbl[i].speed == speed)
+			return &mxs_i2c_tbl[i];
+	return NULL;
+}
+
 void i2c_init(int speed, int slaveadd)
 {
 	struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE;
+	struct mxs_i2c_speed_table *spd = mxs_i2c_speed_to_cfg(speed);
 
-	mxs_i2c_reset();
-
-	switch (speed) {
-	case 100000:
-		writel((0x0078 << I2C_TIMING0_HIGH_COUNT_OFFSET) |
-			(0x0030 << I2C_TIMING0_RCV_COUNT_OFFSET),
-			&i2c_regs->hw_i2c_timing0);
-		writel((0x0080 << I2C_TIMING1_LOW_COUNT_OFFSET) |
-			(0x0030 << I2C_TIMING1_XMIT_COUNT_OFFSET),
-			&i2c_regs->hw_i2c_timing1);
-		break;
-	case 400000:
-		writel((0x000f << I2C_TIMING0_HIGH_COUNT_OFFSET) |
-			(0x0007 << I2C_TIMING0_RCV_COUNT_OFFSET),
-			&i2c_regs->hw_i2c_timing0);
-		writel((0x001f << I2C_TIMING1_LOW_COUNT_OFFSET) |
-			(0x000f << I2C_TIMING1_XMIT_COUNT_OFFSET),
-			&i2c_regs->hw_i2c_timing1);
-		break;
-	default:
+	if (!spd) {
 		printf("MXS I2C: Invalid speed selected (%d Hz)\n", speed);
 		return;
 	}
 
+	mxs_i2c_reset();
+
+	writel(spd->timing0, &i2c_regs->hw_i2c_timing0);
+	writel(spd->timing1, &i2c_regs->hw_i2c_timing1);
+
 	writel((0x0015 << I2C_TIMING2_BUS_FREE_OFFSET) |
 		(0x000d << I2C_TIMING2_LEADIN_COUNT_OFFSET),
 		&i2c_regs->hw_i2c_timing2);
-- 
1.7.10.4

[U-Boot] [PATCH 4/7] i2c: mxs: Abstract out the MXS I2C speed setup

[Wolfgang Denk]

Dear Marek,

In message <1352766871-892-4-git-send-email-marex@denx.de> you wrote:
> This patch pulls out the I2C speed setup from the i2c_init() call
> and implements the bus configuration lookup table with register
> values that needs to be programmed into the I2C IP to run at
> particular speed.
> 
> This patch is a first step towards implementing run-time I2C bus
> speed configuration for the MXS I2C IP.

Thanks.

> +static struct mxs_i2c_speed_table {
> +	uint32_t	speed;
> +	uint32_t	timing0;
> +	uint32_t	timing1;
> +} mxs_i2c_tbl[] = {
> +	{
> +		100000,
> +		(0x0078 << I2C_TIMING0_HIGH_COUNT_OFFSET) |
> +		(0x0030 << I2C_TIMING0_RCV_COUNT_OFFSET),
> +		(0x0080 << I2C_TIMING1_LOW_COUNT_OFFSET) |
> +		(0x0030 << I2C_TIMING1_XMIT_COUNT_OFFSET)
> +	},
> +	{
> +		400000,
> +		(0x000f << I2C_TIMING0_HIGH_COUNT_OFFSET) |
> +		(0x0007 << I2C_TIMING0_RCV_COUNT_OFFSET),
> +		(0x001f << I2C_TIMING1_LOW_COUNT_OFFSET) |
> +		(0x000f << I2C_TIMING1_XMIT_COUNT_OFFSET),
> +	}
> +};

Do we really need such a compile-time initialized table which will
have to include all possible I2C speeds anybody is ever going to use
on any board?

And if board XXX wants to use a funny I2C clock, we have to add yet
another entry to this common file?   Such a solution does not scale.

Can we not rather calculate these register values for any arbitrary
I2C clock given?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Microsoft Compatibility:
     your old Windows 3.11 application crash exactly as the new ones.

[U-Boot] [PATCH 4/7] i2c: mxs: Abstract out the MXS I2C speed setup

[Marek Vasut]

Dear Wolfgang Denk,

> Dear Marek,
> 
> In message <1352766871-892-4-git-send-email-marex@denx.de> you wrote:
> > This patch pulls out the I2C speed setup from the i2c_init() call
> > and implements the bus configuration lookup table with register
> > values that needs to be programmed into the I2C IP to run at
> > particular speed.
> > 
> > This patch is a first step towards implementing run-time I2C bus
> > speed configuration for the MXS I2C IP.
> 
> Thanks.
> 
> > +static struct mxs_i2c_speed_table {
> > +	uint32_t	speed;
> > +	uint32_t	timing0;
> > +	uint32_t	timing1;
> > +} mxs_i2c_tbl[] = {
> > +	{
> > +		100000,
> > +		(0x0078 << I2C_TIMING0_HIGH_COUNT_OFFSET) |
> > +		(0x0030 << I2C_TIMING0_RCV_COUNT_OFFSET),
> > +		(0x0080 << I2C_TIMING1_LOW_COUNT_OFFSET) |
> > +		(0x0030 << I2C_TIMING1_XMIT_COUNT_OFFSET)
> > +	},
> > +	{
> > +		400000,
> > +		(0x000f << I2C_TIMING0_HIGH_COUNT_OFFSET) |
> > +		(0x0007 << I2C_TIMING0_RCV_COUNT_OFFSET),
> > +		(0x001f << I2C_TIMING1_LOW_COUNT_OFFSET) |
> > +		(0x000f << I2C_TIMING1_XMIT_COUNT_OFFSET),
> > +	}
> > +};
> 
> Do we really need such a compile-time initialized table which will
> have to include all possible I2C speeds anybody is ever going to use
> on any board?

Yes

> And if board XXX wants to use a funny I2C clock, we have to add yet
> another entry to this common file?   Such a solution does not scale.

The problem is, the algorithm to compute these values is not described in the 
MX28 manual. There're only values for 100 and 400kHz speed in the manual.

> Can we not rather calculate these register values for any arbitrary
> I2C clock given?

That's what I'd love to do ... no luck so far. That's why there is the crappy 
table.

> Best regards,
> 
> Wolfgang Denk

Best regards,
Marek Vasut

[U-Boot] [PATCH 5/7] i2c: mxs: Implement i2c_get/set_bus_speed()

[Marek Vasut]

This patch implements the setup and retrieval functions for the I2C
bus speed on the MXS I2C IP.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@freescale.com>
---
 drivers/i2c/mxs_i2c.c |   45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/drivers/i2c/mxs_i2c.c b/drivers/i2c/mxs_i2c.c
index 98f6e8c..4152242 100644
--- a/drivers/i2c/mxs_i2c.c
+++ b/drivers/i2c/mxs_i2c.c
@@ -240,6 +240,51 @@ static struct mxs_i2c_speed_table *mxs_i2c_speed_to_cfg(uint32_t speed)
 	return NULL;
 }
 
+static uint32_t mxs_i2c_cfg_to_speed(uint32_t timing0, uint32_t timing1)
+{
+	int i;
+	for (i = 0; i < ARRAY_SIZE(mxs_i2c_tbl); i++) {
+		if (mxs_i2c_tbl[i].timing0 != timing0)
+			continue;
+		if (mxs_i2c_tbl[i].timing1 != timing1)
+			continue;
+		return mxs_i2c_tbl[i].speed;
+	}
+
+	return 0;
+}
+
+int i2c_set_bus_speed(unsigned int speed)
+{
+	struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE;
+	struct mxs_i2c_speed_table *spd = mxs_i2c_speed_to_cfg(speed);
+
+	if (!spd) {
+		printf("MXS I2C: Invalid speed selected (%d Hz)\n", speed);
+		return -EINVAL;
+	}
+
+	writel(spd->timing0, &i2c_regs->hw_i2c_timing0);
+	writel(spd->timing1, &i2c_regs->hw_i2c_timing1);
+
+	writel((0x0015 << I2C_TIMING2_BUS_FREE_OFFSET) |
+		(0x000d << I2C_TIMING2_LEADIN_COUNT_OFFSET),
+		&i2c_regs->hw_i2c_timing2);
+
+	return 0;
+}
+
+unsigned int i2c_get_bus_speed(void)
+{
+	struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE;
+	uint32_t timing0, timing1;
+
+	timing0 = readl(&i2c_regs->hw_i2c_timing0);
+	timing1 = readl(&i2c_regs->hw_i2c_timing1);
+
+	return mxs_i2c_cfg_to_speed(timing0, timing1);
+}
+
 void i2c_init(int speed, int slaveadd)
 {
 	struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE;
-- 
1.7.10.4

[U-Boot] [PATCH 6/7] i2c: mxs: Use i2c_set_bus_speed() in i2c_init()

[Marek Vasut]

Use i2c_set_bus_speed() in i2c_init() within the mxs i2c driver
to avoid duplication of code.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@freescale.com>
---
 drivers/i2c/mxs_i2c.c |   16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/drivers/i2c/mxs_i2c.c b/drivers/i2c/mxs_i2c.c
index 4152242..3771452 100644
--- a/drivers/i2c/mxs_i2c.c
+++ b/drivers/i2c/mxs_i2c.c
@@ -287,22 +287,8 @@ unsigned int i2c_get_bus_speed(void)
 
 void i2c_init(int speed, int slaveadd)
 {
-	struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE;
-	struct mxs_i2c_speed_table *spd = mxs_i2c_speed_to_cfg(speed);
-
-	if (!spd) {
-		printf("MXS I2C: Invalid speed selected (%d Hz)\n", speed);
-		return;
-	}
-
 	mxs_i2c_reset();
-
-	writel(spd->timing0, &i2c_regs->hw_i2c_timing0);
-	writel(spd->timing1, &i2c_regs->hw_i2c_timing1);
-
-	writel((0x0015 << I2C_TIMING2_BUS_FREE_OFFSET) |
-		(0x000d << I2C_TIMING2_LEADIN_COUNT_OFFSET),
-		&i2c_regs->hw_i2c_timing2);
+	i2c_set_bus_speed(speed);
 
 	return;
 }
-- 
1.7.10.4

[U-Boot] [PATCH 7/7] i2c: mxs: Fix TIMING2 register value

[Marek Vasut]

According to FSL, the value in the TIMING2 register shall be 0x00300030
instead of what's written in the datasheet. This new value correlates
with older STMP36xx datasheet. Issues were detected in Linux when this
register was misconfigured, so write this correct value.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@freescale.com>
---
 drivers/i2c/mxs_i2c.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/mxs_i2c.c b/drivers/i2c/mxs_i2c.c
index 3771452..006fb91 100644
--- a/drivers/i2c/mxs_i2c.c
+++ b/drivers/i2c/mxs_i2c.c
@@ -267,8 +267,8 @@ int i2c_set_bus_speed(unsigned int speed)
 	writel(spd->timing0, &i2c_regs->hw_i2c_timing0);
 	writel(spd->timing1, &i2c_regs->hw_i2c_timing1);
 
-	writel((0x0015 << I2C_TIMING2_BUS_FREE_OFFSET) |
-		(0x000d << I2C_TIMING2_LEADIN_COUNT_OFFSET),
+	writel((0x0030 << I2C_TIMING2_BUS_FREE_OFFSET) |
+		(0x0030 << I2C_TIMING2_LEADIN_COUNT_OFFSET),
 		&i2c_regs->hw_i2c_timing2);
 
 	return 0;
-- 
1.7.10.4

[U-Boot] [PATCH 1/7] i2c: Use __weak instead of __attribute__((weak, alias))

[Wolfgang Denk]

Dear Marek Vasut,

In message <1352766871-892-1-git-send-email-marex@denx.de> you wrote:
> Use __weak from linux/compiler.h instead of __attribute__((weak, alias))
> to define overridable function. This patch is intended as a cleanup patch
> to bring some consistency into the code.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Heiko Schocher <hs@denx.de>
> ---
>  common/cmd_i2c.c |   16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)

The actual definition of __weak is in include/linux/compiler-gcc.h and
looks like this:

	#define __weak	__attribute__((weak))

which means you omit the ", alias" part of the existing code.

Are you 100% sure that this has no impacts on the behaviour?

In my understanding, "weak" and "weak, alias" are not exactly the
same...

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
While most peoples' opinions change, the conviction of their correct-
ness never does.

[U-Boot] [PATCH 1/7] i2c: Use __weak instead of __attribute__((weak, alias))

[Marek Vasut]

Dear Wolfgang Denk,

> Dear Marek Vasut,
> 
> In message <1352766871-892-1-git-send-email-marex@denx.de> you wrote:
> > Use __weak from linux/compiler.h instead of __attribute__((weak, alias))
> > to define overridable function. This patch is intended as a cleanup patch
> > to bring some consistency into the code.
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Heiko Schocher <hs@denx.de>
> > ---
> > 
> >  common/cmd_i2c.c |   16 +++++++---------
> >  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> The actual definition of __weak is in include/linux/compiler-gcc.h and
> looks like this:
> 
> 	#define __weak	__attribute__((weak))
> 
> which means you omit the ", alias" part of the existing code.
> 
> Are you 100% sure that this has no impacts on the behaviour?

Yes

> In my understanding, "weak" and "weak, alias" are not exactly the
> same...

Can you please elaborate? The point of this alias here is to call __def_i2c_*() 
in case the overriding function isn't defined. Otherwise call the overriding 
function. The __def_i2c_*() is never called directly.

> Best regards,
> 
> Wolfgang Denk

Best regards,
Marek Vasut