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

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

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);

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;

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);

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

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

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;

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; }

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;

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

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
participants (2)
-
Marek Vasut
-
Wolfgang Denk