On Wed, Apr 23, 2014 at 10:31 AM, Stefano Babic sbabic@denx.de wrote:
On 03/04/2014 08:01, Tim Harvey wrote:
use the new iomux function and a macro to create a multi-dimensional array of iomux values without duplicating the defintions.
Signed-off-by: Tim Harvey tharvey@gateworks.com
board/gateworks/gw_ventana/gw_ventana.c | 497 ++++++++++++++++++++------------ 1 file changed, 316 insertions(+), 181 deletions(-)
diff --git a/board/gateworks/gw_ventana/gw_ventana.c b/board/gateworks/gw_ventana/gw_ventana.c index 2113740..ebf7e7d 100644 --- a/board/gateworks/gw_ventana/gw_ventana.c +++ b/board/gateworks/gw_ventana/gw_ventana.c @@ -40,6 +40,17 @@
DECLARE_GLOBAL_DATA_PTR;
+#define IOMUX(x) (MX6Q_##x), (MX6DL_##x) +#define SETUP_PAD(def) \ +if (is_cpu_type(MXC_CPU_MX6Q)) { \
imx_iomux_v3_setup_pad(MX6Q_##def); \+} else { \
imx_iomux_v3_setup_pad(MX6DL_##def); \+}
This macro should be available for other boards, too.
Yes - I'm moving these macros to iomux-v3.h
+#define SETUP_PADS(x) \
imx_iomux_v3_setup_multiple_pads_array(x, \ARRAY_SIZE(x)/2, is_cpu_type(MXC_CPU_MX6Q) ? 0 : 1, 2)/* GPIO's common to all baseboards */ #define GP_PHY_RST IMX_GPIO_NR(1, 30) #define GP_USB_OTG_PWR IMX_GPIO_NR(3, 22) @@ -94,109 +105,145 @@ int board_type;
/* UART1: Function varies per baseboard */ iomux_v3_cfg_t const uart1_pads[] = {
MX6_PAD_SD3_DAT6__UART1_RX_DATA | MUX_PAD_CTRL(UART_PAD_CTRL),MX6_PAD_SD3_DAT7__UART1_TX_DATA | MUX_PAD_CTRL(UART_PAD_CTRL),
IOMUX(PAD_SD3_DAT6__UART1_RX_DATA | MUX_PAD_CTRL(UART_PAD_CTRL)),IOMUX(PAD_SD3_DAT7__UART1_TX_DATA | MUX_PAD_CTRL(UART_PAD_CTRL)),};
/* UART2: Serial Console */ iomux_v3_cfg_t const uart2_pads[] = {
MX6_PAD_SD4_DAT7__UART2_TX_DATA | MUX_PAD_CTRL(UART_PAD_CTRL),MX6_PAD_SD4_DAT4__UART2_RX_DATA | MUX_PAD_CTRL(UART_PAD_CTRL),
IOMUX(PAD_SD4_DAT7__UART2_TX_DATA | MUX_PAD_CTRL(UART_PAD_CTRL)),IOMUX(PAD_SD4_DAT4__UART2_RX_DATA | MUX_PAD_CTRL(UART_PAD_CTRL)),};
#define PC MUX_PAD_CTRL(I2C_PAD_CTRL)
/* I2C1: GSC */ -struct i2c_pads_info i2c_pad_info0 = { +struct i2c_pads_info mx6q_i2c_pad_info0 = { .scl = {
.i2c_mode = MX6_PAD_EIM_D21__I2C1_SCL | PC,.gpio_mode = MX6_PAD_EIM_D21__GPIO3_IO21 | PC,
.i2c_mode = MX6Q_PAD_EIM_D21__I2C1_SCL | PC,.gpio_mode = MX6Q_PAD_EIM_D21__GPIO3_IO21 | PC,What have you changed here ?
I don't have a solution for a pretty macro that avoids duplicating struct i2c_pads_info so I'm creating two versions of the struct; one with MX6Q_* and the other (below) for imx6dl/solo with MX6DL_*
.gp = IMX_GPIO_NR(3, 21) }, .sda = {
.i2c_mode = MX6_PAD_EIM_D28__I2C1_SDA | PC,.gpio_mode = MX6_PAD_EIM_D28__GPIO3_IO28 | PC,
.i2c_mode = MX6Q_PAD_EIM_D28__I2C1_SDA | PC,.gpio_mode = MX6Q_PAD_EIM_D28__GPIO3_IO28 | PC,.gp = IMX_GPIO_NR(3, 28)}+}; +struct i2c_pads_info mx6dl_i2c_pad_info0 = {
.scl = {.i2c_mode = MX6DL_PAD_EIM_D21__I2C1_SCL | PC,.gpio_mode = MX6DL_PAD_EIM_D21__GPIO3_IO21 | PC,.gp = IMX_GPIO_NR(3, 21)},.sda = {.i2c_mode = MX6DL_PAD_EIM_D28__I2C1_SDA | PC,.gpio_mode = MX6DL_PAD_EIM_D28__GPIO3_IO28 | PC, .gp = IMX_GPIO_NR(3, 28) }};
/* I2C2: PMIC/PCIe Switch/PCIe Clock/Mezz */ -struct i2c_pads_info i2c_pad_info1 = { +struct i2c_pads_info mx6q_i2c_pad_info1 = {
.scl = {.i2c_mode = MX6Q_PAD_KEY_COL3__I2C2_SCL | PC,.gpio_mode = MX6Q_PAD_KEY_COL3__GPIO4_IO12 | PC,.gp = IMX_GPIO_NR(4, 12)},.sda = {.i2c_mode = MX6Q_PAD_KEY_ROW3__I2C2_SDA | PC,.gpio_mode = MX6Q_PAD_KEY_ROW3__GPIO4_IO13 | PC,.gp = IMX_GPIO_NR(4, 13)}+}; +struct i2c_pads_info mx6dl_i2c_pad_info1 = { .scl = {
.i2c_mode = MX6_PAD_KEY_COL3__I2C2_SCL | PC,.gpio_mode = MX6_PAD_KEY_COL3__GPIO4_IO12 | PC,
.i2c_mode = MX6DL_PAD_KEY_COL3__I2C2_SCL | PC,.gpio_mode = MX6DL_PAD_KEY_COL3__GPIO4_IO12 | PC, .gp = IMX_GPIO_NR(4, 12) }, .sda = {
.i2c_mode = MX6_PAD_KEY_ROW3__I2C2_SDA | PC,.gpio_mode = MX6_PAD_KEY_ROW3__GPIO4_IO13 | PC,
.i2c_mode = MX6DL_PAD_KEY_ROW3__I2C2_SDA | PC,.gpio_mode = MX6DL_PAD_KEY_ROW3__GPIO4_IO13 | PC, .gp = IMX_GPIO_NR(4, 13) }};
/* I2C3: Misc/Expansion */ -struct i2c_pads_info i2c_pad_info2 = { +struct i2c_pads_info mx6q_i2c_pad_info2 = { .scl = {
.i2c_mode = MX6_PAD_GPIO_3__I2C3_SCL | PC,.gpio_mode = MX6_PAD_GPIO_3__GPIO1_IO03 | PC,
.i2c_mode = MX6Q_PAD_GPIO_3__I2C3_SCL | PC,.gpio_mode = MX6Q_PAD_GPIO_3__GPIO1_IO03 | PC, .gp = IMX_GPIO_NR(1, 3) }, .sda = {
.i2c_mode = MX6_PAD_GPIO_6__I2C3_SDA | PC,.gpio_mode = MX6_PAD_GPIO_6__GPIO1_IO06 | PC,
.i2c_mode = MX6Q_PAD_GPIO_6__I2C3_SDA | PC,.gpio_mode = MX6Q_PAD_GPIO_6__GPIO1_IO06 | PC,.gp = IMX_GPIO_NR(1, 6)}+};
It seems you have already tried but you have not found a solution for this. Anyway, repeating the same structure for all variants looks bad. The solution with SETUP_PADS() and IOMUX is pretty better.
No, I haven't found a pretty solution for dealing with 2 values of struct i2c_pads_info to pass to imx's setup_i2c. I can try to create a new setup_i2c_array and macro similar to what I did for imx_iomux_v3_setup_multiple_pads, or we can leave that to a future patch in case anyone has any better ideas?
+struct i2c_pads_info mx6dl_i2c_pad_info2 = {
.scl = {.i2c_mode = MX6DL_PAD_GPIO_3__I2C3_SCL | PC,.gpio_mode = MX6DL_PAD_GPIO_3__GPIO1_IO03 | PC,.gp = IMX_GPIO_NR(1, 3)},.sda = {.i2c_mode = MX6DL_PAD_GPIO_6__I2C3_SDA | PC,.gpio_mode = MX6DL_PAD_GPIO_6__GPIO1_IO06 | PC, .gp = IMX_GPIO_NR(1, 6) }};
<snip>
#ifdef CONFIG_CMD_NAND @@ -205,7 +252,7 @@ static void setup_gpmi_nand(void) struct mxc_ccm_reg *mxc_ccm = (struct mxc_ccm_reg *)CCM_BASE_ADDR;
/* config gpmi nand iomux */
imx_iomux_v3_setup_multiple_pads(nfc_pads, ARRAY_SIZE(nfc_pads));
SETUP_PADS(nfc_pads);I will only suggest to use another name for the macro. SETUP_PADS seems too generic and could conflict in future with other SOCs. IMX6_SETUP_PADS ?
My thought is to move this to iomux-v3.h where IOMUX_PAD and NEW_PAD_CTRL macro's are defined. Those are short and not SoC specific because only boards using imx iomux-v3 would include these. My preference is to try and keep the macro name very short otherwise we have to use a lot of line breaks for existing code that fits a pad name and mux control within 80 lines.
How about the following in iomux-v3.h:
/* define a set of pads for IMX6Q/IMX6DUAL and IMX6DL/IMX6SOLO */ IOMUX_PADS(x) /* setup cpu specific pad based on struct declared using IOMUX_PADS(...) */ SETUP_IOMUX_PAD(def) /* setup multiple cpu specific pads based on struct declared using IOMUX_PADS(...) */ SETUP_IOMUX_PADS(def)
Regards,
Tim