On Wed, Apr 23, 2014 at 10:31 AM, Stefano Babic <sba...@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 <thar...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
