[U-Boot] [PATCH] dm:gpio:mxc get configuration from dtb

[Simon Glass]

Hi Peng.

On 18 January 2015 at 23:11, Peng Fan <Peng.Fan at freescale.com> wrote:
> This patch supports getting gpios' configuration from dtb.
> CONFIG_OF_CONTROL is used to indicated which part is for device tree,
> and which is not.
>
> This patch is already tested on mx6sxsabresd board. Since device tree
> has not been upstreamed, if want to test this patch. The followings
> need to be done.
>  + pieces of code does not gpio_request when using gpio_direction_xxx and
>    etc, need to request gpio.
>  + move the gpio settings from board_early_init_f to board_init
>  + define CONFIG_DM ,CONFIG_DM_GPIO and CONFIG_OF_CONTROL
>  + Add device tree file and do related configuration in
>    `make ARCH=arm menuconfig`
>

Sorry I am going to repeat some of Igor's comments...

> Signed-off-by: Peng Fan <Peng.Fan at freescale.com>
> ---
>  drivers/gpio/mxc_gpio.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 76 insertions(+)
>
> diff --git a/drivers/gpio/mxc_gpio.c b/drivers/gpio/mxc_gpio.c
> index 8bb9e39..8603068 100644
> --- a/drivers/gpio/mxc_gpio.c
> +++ b/drivers/gpio/mxc_gpio.c
> @@ -14,6 +14,10 @@
>  #include <asm/arch/imx-regs.h>
>  #include <asm/gpio.h>
>  #include <asm/io.h>
> +#ifdef CONFIG_OF_CONTROL
> +#include <fdtdec.h>
> +DECLARE_GLOBAL_DATA_PTR;
> +#endif

Hopefully #ifdef not needed.

>
>  enum mxc_gpio_direction {
>         MXC_GPIO_DIRECTION_IN,
> @@ -258,6 +262,7 @@ static const struct dm_gpio_ops gpio_mxc_ops = {
>         .get_function           = mxc_gpio_get_function,
>  };
>
> +#ifndef CONFIG_OF_CONTROL
>  static const struct mxc_gpio_plat mxc_plat[] = {
>         { (struct gpio_regs *)GPIO1_BASE_ADDR },
>         { (struct gpio_regs *)GPIO2_BASE_ADDR },
> @@ -274,6 +279,7 @@ static const struct mxc_gpio_plat mxc_plat[] = {
>         { (struct gpio_regs *)GPIO7_BASE_ADDR },
>  #endif

Same here.

>  };
> +#endif
>
>  static int mxc_gpio_probe(struct udevice *dev)
>  {
> @@ -283,7 +289,19 @@ static int mxc_gpio_probe(struct udevice *dev)
>         int banknum;
>         char name[18], *str;
>
> +#ifdef CONFIG_OF_CONTROL
> +       /*
> +        * In dts file add:
> +        * aliases {
> +        *      gpio0 = &gpio1;
> +        *      gpio1 = &gpio2;
> +        *      .....
> +        * };
> +        * Then set banknum accoring dev's seq number. */
> +       banknum = dev->seq;
> +#else
>         banknum = plat - mxc_plat;
> +#endif
>         sprintf(name, "GPIO%d_", banknum + 1);
>         str = strdup(name);
>         if (!str)
> @@ -295,14 +313,71 @@ static int mxc_gpio_probe(struct udevice *dev)
>         return 0;
>  }
>
> +#ifdef CONFIG_OF_CONTROL
> +static int mxc_gpio_bind(struct udevice *device)
> +{
> +       struct mxc_gpio_plat *plat = device->platdata;
> +       struct gpio_regs *ctrl;
> +
> +       if (plat)
> +               return 0;
> +       /*
> +        * In the dts file, gpiox bank are as following:
> +        *      gpio1: gpio at 0209c000 {
> +        *              compatible = "fsl,imx6q-gpio", "fsl,imx35-gpio";
> +        *              reg = <0x0209c000 0x4000>;
> +        *              interrupts = <0 66 0x04 0 67 0x04>;
> +        *              gpio-controller;
> +        *              #gpio-cells = <2>;
> +        *              interrupt-controller;
> +        *              #interrupt-cells = <2>;
> +        *      };
> +        *
> +        *      gpio2: gpio at 020a0000 {
> +        *              compatible = "fsl,imx6q-gpio", "fsl,imx35-gpio";
> +        *              reg = <0x020a0000 0x4000>;
> +        *              interrupts = <0 68 0x04 0 69 0x04>;
> +        *              gpio-controller;
> +        *              #gpio-cells = <2>;
> +        *              interrupt-controller;
> +        *              #interrupt-cells = <2>;
> +        *      };
> +        *
> +        * gpio1 is the 1st bank, gpio2 is the 2nd bank and gpio3 ....
> +        */
> +
> +       ctrl = (struct gpio_regs *)fdtdec_get_addr(gd->fdt_blob,
> +                                                  device->of_offset, "reg");
> +       plat = calloc(1, sizeof(*plat));
> +       if (!plat)
> +               return -ENOMEM;
> +
> +       plat->regs = ctrl;
> +
> +       device->platdata = plat;
> +
> +       return 0;
> +}
> +
> +static const struct udevice_id mxc_gpio_ids[] = {
> +       { .compatible = "fsl,imx35-gpio" },
> +       { }
> +};
> +#endif
> +
>  U_BOOT_DRIVER(gpio_mxc) = {
>         .name   = "gpio_mxc",
>         .id     = UCLASS_GPIO,
>         .ops    = &gpio_mxc_ops,
>         .probe  = mxc_gpio_probe,
>         .priv_auto_alloc_size = sizeof(struct mxc_bank_info),
> +#ifdef CONFIG_OF_CONTROL
> +       .of_match = mxc_gpio_ids,
> +       .bind   = mxc_gpio_bind,
> +#endif
>  };
>
> +#ifndef CONFIG_OF_CONTROL
>  U_BOOT_DEVICES(mxc_gpios) = {
>         { "gpio_mxc", &mxc_plat[0] },
>         { "gpio_mxc", &mxc_plat[1] },
> @@ -320,3 +395,4 @@ U_BOOT_DEVICES(mxc_gpios) = {
>  #endif
>  };
>  #endif
> +#endif

Overall I wonder why you don't just convert the boards to device tree?
It might be more work, but it would be a lot cleaner.

Regards,
Simon