[U-Boot] [PATCH v2 1/2] wandboard: Don't use I2C speed Kconfig settings with DM_I2C
When using DM_I2C the speed value supplied to setup_i2c() is not used, so this code required CONFIG_SYS_MXC_I2C[12]_SPEED to be defined to compile, but did not actually use them.
Change this so we no longer need to define an unused macro to compile in DM_I2C mode. Also make it more clear that they do not control the bus speed. Otherwise it is quite easy to mistakenly believe they are used to set the bus speed.
Cc: Heiko Schocher hs@denx.de Cc: Anatolij Gustschin agust@denx.de Signed-off-by: Trent Piepho tpiepho@impinj.com --- board/wandboard/wandboard.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/board/wandboard/wandboard.c b/board/wandboard/wandboard.c index 69fbc8b690..9d7a94ff9d 100644 --- a/board/wandboard/wandboard.c +++ b/board/wandboard/wandboard.c @@ -46,6 +46,15 @@ DECLARE_GLOBAL_DATA_PTR; #define ETH_PHY_AR8035_POWER IMX_GPIO_NR(7, 13) #define REV_DETECTION IMX_GPIO_NR(2, 28)
+/* Speed defined in Kconfig is only applicable when not using DM_I2C. */ +#ifdef CONFIG_DM_I2C +#define I2C1_SPEED_NON_DM 0 +#define I2C2_SPEED_NON_DM 0 +#else +#define I2C1_SPEED_NON_DM CONFIG_SYS_MXC_I2C1_SPEED +#define I2C2_SPEED_NON_DM CONFIG_SYS_MXC_I2C2_SPEED +#endif + static bool with_pmic;
int dram_init(void) @@ -463,13 +472,13 @@ int board_init(void) gd->bd->bi_boot_params = PHYS_SDRAM + 0x100;
#if defined(CONFIG_VIDEO_IPUV3) - setup_i2c(1, CONFIG_SYS_MXC_I2C1_SPEED, 0x7f, &mx6dl_i2c2_pad_info); + setup_i2c(1, I2C1_SPEED_NON_DM, 0x7f, &mx6dl_i2c2_pad_info); if (is_mx6dq() || is_mx6dqp()) { - setup_i2c(1, CONFIG_SYS_MXC_I2C1_SPEED, 0x7f, &mx6q_i2c2_pad_info); - setup_i2c(2, CONFIG_SYS_MXC_I2C2_SPEED, 0x7f, &mx6q_i2c3_pad_info); + setup_i2c(1, I2C1_SPEED_NON_DM, 0x7f, &mx6q_i2c2_pad_info); + setup_i2c(2, I2C2_SPEED_NON_DM, 0x7f, &mx6q_i2c3_pad_info); } else { - setup_i2c(1, CONFIG_SYS_MXC_I2C1_SPEED, 0x7f, &mx6dl_i2c2_pad_info); - setup_i2c(2, CONFIG_SYS_MXC_I2C2_SPEED, 0x7f, &mx6dl_i2c3_pad_info); + setup_i2c(1, I2C1_SPEED_NON_DM, 0x7f, &mx6dl_i2c2_pad_info); + setup_i2c(2, I2C2_SPEED_NON_DM, 0x7f, &mx6dl_i2c3_pad_info); }
setup_display();
These options only apply when not using DM_I2C. When using device trees, the dt will enable and control the speeds of the I2C controller(s) and these configuration options have no effect.
So disable them in DM_I2C mode. Otherwise they show up as decoys, and make it look like one is enabling I2C controllers and setting the speed when really it's doing nothing.
However, a system using a SPL build will not use DM_I2C in the SPL, even if DM_I2C is enabled for the main u-boot. And so the SPL might use the kconfig based I2C speed controls while the main u-boot does not.
Cc: Sriram Dash sriram.dash@nxp.com Cc: Priyanka Jain priyanka.jain@nxp.com Cc: Heiko Schocher hs@denx.de Signed-off-by: Trent Piepho tpiepho@impinj.com --- Changes from v1: Added patch in series to fix wandboard build issue Enable settings if SPL is enabled, as SPL will not use DM_I2C
drivers/i2c/Kconfig | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig index 215624020f..095a9bc6a4 100644 --- a/drivers/i2c/Kconfig +++ b/drivers/i2c/Kconfig @@ -161,7 +161,10 @@ config SYS_I2C_MXC channels and operating on standard mode up to 100 kbits/s and fast mode up to 400 kbits/s.
-if SYS_I2C_MXC +# These settings are not used with DM_I2C, however SPL doesn't use +# DM_I2C even if DM_I2C is enabled, and so might use these settings even +# when main u-boot does not! +if SYS_I2C_MXC && (!DM_I2C || SPL) config SYS_I2C_MXC_I2C1 bool "NXP MXC I2C1" help
On Wed, 8 May 2019 23:30:01 +0000 Trent Piepho tpiepho@impinj.com wrote: ...
diff --git a/board/wandboard/wandboard.c b/board/wandboard/wandboard.c index 69fbc8b690..9d7a94ff9d 100644 --- a/board/wandboard/wandboard.c +++ b/board/wandboard/wandboard.c @@ -46,6 +46,15 @@ DECLARE_GLOBAL_DATA_PTR; #define ETH_PHY_AR8035_POWER IMX_GPIO_NR(7, 13) #define REV_DETECTION IMX_GPIO_NR(2, 28)
+/* Speed defined in Kconfig is only applicable when not using DM_I2C. */ +#ifdef CONFIG_DM_I2C +#define I2C1_SPEED_NON_DM 0 +#define I2C2_SPEED_NON_DM 0 +#else +#define I2C1_SPEED_NON_DM CONFIG_SYS_MXC_I2C1_SPEED +#define I2C2_SPEED_NON_DM CONFIG_SYS_MXC_I2C2_SPEED
Shouldn't we change this to
#ifdef CONFIG_DM_I2C #define I2C2_SPEED_NON_DM 0 #define I2C3_SPEED_NON_DM 0 #else #define I2C2_SPEED_NON_DM CONFIG_SYS_MXC_I2C2_SPEED #define I2C3_SPEED_NON_DM CONFIG_SYS_MXC_I2C3_SPEED #endif ... setup_i2c(1, I2C2_SPEED_NON_DM, 0x7f, &mx6q_i2c2_pad_info); setup_i2c(2, I2C3_SPEED_NON_DM, 0x7f, &mx6q_i2c3_pad_info); ?
Because the first argument to setup_i2c() is the bus number which starts counting from 0, but the CONFIG_SYS_MXC_I2C* start counting from 1. This doesn't affect the actual configuration since the speed value is currently the same for all buses. But it is more accurate.
-- Anatolij
On Thu, 2019-05-09 at 08:20 +0200, Anatolij Gustschin wrote:
On Wed, 8 May 2019 23:30:01 +0000 Trent Piepho tpiepho@impinj.com wrote: ...
diff --git a/board/wandboard/wandboard.c b/board/wandboard/wandboard.c index 69fbc8b690..9d7a94ff9d 100644 --- a/board/wandboard/wandboard.c +++ b/board/wandboard/wandboard.c @@ -46,6 +46,15 @@ DECLARE_GLOBAL_DATA_PTR; #define ETH_PHY_AR8035_POWER IMX_GPIO_NR(7, 13) #define REV_DETECTION IMX_GPIO_NR(2, 28)
+/* Speed defined in Kconfig is only applicable when not using DM_I2C. */ +#ifdef CONFIG_DM_I2C +#define I2C1_SPEED_NON_DM 0 +#define I2C2_SPEED_NON_DM 0 +#else +#define I2C1_SPEED_NON_DM CONFIG_SYS_MXC_I2C1_SPEED +#define I2C2_SPEED_NON_DM CONFIG_SYS_MXC_I2C2_SPEED
Shouldn't we change this to
#ifdef CONFIG_DM_I2C #define I2C2_SPEED_NON_DM 0 #define I2C3_SPEED_NON_DM 0 #else #define I2C2_SPEED_NON_DM CONFIG_SYS_MXC_I2C2_SPEED #define I2C3_SPEED_NON_DM CONFIG_SYS_MXC_I2C3_SPEED #endif ... setup_i2c(1, I2C2_SPEED_NON_DM, 0x7f, &mx6q_i2c2_pad_info); setup_i2c(2, I2C3_SPEED_NON_DM, 0x7f, &mx6q_i2c3_pad_info); ?
Because the first argument to setup_i2c() is the bus number which starts counting from 0, but the CONFIG_SYS_MXC_I2C* start counting from 1. This doesn't affect the actual configuration since the speed value is currently the same for all buses. But it is more accurate.
Seems like it should have been, but the existing code uses I2C1_SPEED and I2C2_SPEED. Probably a bug. But if I change it here, then it will affect existing config files, which were done when the bug existed.
Also I see it calls setup_i2c(1, ...) twice, which seems like another bug.
participants (2)
-
Anatolij Gustschin -
Trent Piepho