From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peng Fan Date: Tue, 20 Jan 2015 09:27:06 +0800 Subject: [U-Boot] [PATCH] dm:gpio:mxc get configuration from dtb In-Reply-To: References: <1421647912-18016-1-git-send-email-Peng.Fan@freescale.com> Message-ID: <54BDAEEA.2030401@freescale.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Simon, On 1/20/2015 3:45 AM, Simon Glass wrote: > Hi Peng. > > On 18 January 2015 at 23:11, Peng Fan 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... I've seen Igor's comments.I'll address them. > >> Signed-off-by: Peng Fan >> --- >> 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 >> #include >> #include >> +#ifdef CONFIG_OF_CONTROL >> +#include >> +DECLARE_GLOBAL_DATA_PTR; >> +#endif > Hopefully #ifdef not needed. Ok. I'll try to remove them in v2 patch. > >> 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. Yeah. Agree. Converting the boards to device tree may require lots of change, I am just hacking this in my side for personal interests. Anyway, I'll try to address you all comments to make it better. > > Regards, > Simon Regards, Peng.