From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peng Fan Date: Fri, 23 Jan 2015 02:56:17 +0800 Subject: [U-Boot] [PATCH v3 4/4] dm:gpio:mxc add DT support In-Reply-To: <54C0B70E.507@compulab.co.il> References: <1421838596-4176-1-git-send-email-Peng.Fan@freescale.com> <1421838596-4176-5-git-send-email-Peng.Fan@freescale.com> <54C04D23.7000804@freescale.com> <54C0B70E.507@compulab.co.il> Message-ID: <54C147D1.1030301@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, Igor Reply below. On 1/22/2015 4:38 PM, Igor Grinberg wrote: > Hi Peng, > > On 01/22/15 03:06, Peng Fan wrote: >> Hi Igor, >> >> Just kindly remind, did you miss this one? Since you ack the other patches in this patch set. > Nope, I did not miss it. > I just haven't looked at it yet... > >> On 1/21/2015 7:09 PM, Peng Fan wrote: >>> This patch add DT support for mxc gpio driver. >>> >>> There are one place using CONFIG_OF_CONTROL macro. >>> 1. The U_BOOT_DEVICES and mxc_plat array are complied out. To DT, >>> platdata is alloced using calloc, so there is no need to use mxc_plat. >>> >>> The following situations are tested, and all work fine: >>> 1. with DM, without DT >>> 2. with DM and DT >>> 3. without DM >>> 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` >>> These will be done in future patches by step. >>> >>> Signed-off-by: Peng Fan > Besides the question below, looks good. > >>> --- >>> drivers/gpio/mxc_gpio.c | 69 +++++++++++++++++++++++++++++++++++++------------ >>> 1 file changed, 52 insertions(+), 17 deletions(-) >>> >>> diff --git a/drivers/gpio/mxc_gpio.c b/drivers/gpio/mxc_gpio.c >>> index c52dd19..0766b78 100644 >>> --- a/drivers/gpio/mxc_gpio.c >>> +++ b/drivers/gpio/mxc_gpio.c >>> @@ -151,6 +151,9 @@ int gpio_direction_output(unsigned gpio, int value) >>> #endif >>> #ifdef CONFIG_DM_GPIO >>> +#include >>> +DECLARE_GLOBAL_DATA_PTR; >>> + >>> static int mxc_gpio_is_output(struct gpio_regs *regs, int offset) >>> { >>> u32 val; >>> @@ -259,23 +262,6 @@ static const struct dm_gpio_ops gpio_mxc_ops = { >>> .get_function = mxc_gpio_get_function, >>> }; >>> -static const struct mxc_gpio_plat mxc_plat[] = { >>> - { 0, (struct gpio_regs *)GPIO1_BASE_ADDR }, >>> - { 1, (struct gpio_regs *)GPIO2_BASE_ADDR }, >>> - { 2, (struct gpio_regs *)GPIO3_BASE_ADDR }, >>> -#if defined(CONFIG_MX25) || defined(CONFIG_MX27) || defined(CONFIG_MX51) || \ >>> - defined(CONFIG_MX53) || defined(CONFIG_MX6) >>> - { 3, (struct gpio_regs *)GPIO4_BASE_ADDR }, >>> -#endif >>> -#if defined(CONFIG_MX27) || defined(CONFIG_MX53) || defined(CONFIG_MX6) >>> - { 4, (struct gpio_regs *)GPIO5_BASE_ADDR }, >>> - { 5, (struct gpio_regs *)GPIO6_BASE_ADDR }, >>> -#endif >>> -#if defined(CONFIG_MX53) || defined(CONFIG_MX6) >>> - { 6, (struct gpio_regs *)GPIO7_BASE_ADDR }, >>> -#endif >>> -}; >>> - >>> static int mxc_gpio_probe(struct udevice *dev) >>> { >>> struct mxc_bank_info *bank = dev_get_priv(dev); >>> @@ -296,12 +282,60 @@ static int mxc_gpio_probe(struct udevice *dev) >>> return 0; >>> } >>> +static int mxc_gpio_bind(struct udevice *device) >>> +{ >>> + struct mxc_gpio_plat *plat = device->platdata; >>> + struct gpio_regs *regs; >>> + >>> + if (plat) >>> + return 0; >>> + >>> + regs = dev_get_addr(device); >>> + if (!regs) >>> + return -ENXIO; >>> + >>> + plat = calloc(1, sizeof(*plat)); >>> + if (!plat) >>> + return -ENOMEM; >>> + >>> + plat->regs = regs; >>> + plat->bank_index = device->req_seq; > Can we assume that in mxc_gpio case, device->req_seq will never equal -1? To NO DT situation, "if (plat) return0;" will directly return, because plat is staticlly intialized in source file. To DT, in aliases, "gpio0=&gpio1" and etc should be added, to make req_seq work. If "reg" property or "alises" are not provied, req_seq will be -1. Here I want aliases, because want bank_index from 0 to max bank, but this can not be guaranteed. If reg property is not provided, dev_get_addr will return error. And plat->bank_index = device->req_seq will not be executed. If reg property is provided, but alises "gpiox=&gpioy" are not provied, there is not a good way to check req_seq exceeds max bank, since I do not want to include `#define xxMAX_BANK yy`. So I did not test this "req_seq < 0 " situation. > >>> + device->platdata = plat; >>> + >>> + return 0; >>> +} >>> + >>> +static const struct udevice_id mxc_gpio_ids[] = { >>> + { .compatible = "fsl,imx35-gpio" }, >>> + { } >>> +}; >>> + >>> 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), >>> + .of_match = mxc_gpio_ids, >>> + .bind = mxc_gpio_bind, >>> +}; >>> + >>> +#ifndef CONFIG_OF_CONTROL >>> +static const struct mxc_gpio_plat mxc_plat[] = { >>> + { 0, (struct gpio_regs *)GPIO1_BASE_ADDR }, >>> + { 1, (struct gpio_regs *)GPIO2_BASE_ADDR }, >>> + { 2, (struct gpio_regs *)GPIO3_BASE_ADDR }, >>> +#if defined(CONFIG_MX25) || defined(CONFIG_MX27) || defined(CONFIG_MX51) || \ >>> + defined(CONFIG_MX53) || defined(CONFIG_MX6) >>> + { 3, (struct gpio_regs *)GPIO4_BASE_ADDR }, >>> +#endif >>> +#if defined(CONFIG_MX27) || defined(CONFIG_MX53) || defined(CONFIG_MX6) >>> + { 4, (struct gpio_regs *)GPIO5_BASE_ADDR }, >>> + { 5, (struct gpio_regs *)GPIO6_BASE_ADDR }, >>> +#endif >>> +#if defined(CONFIG_MX53) || defined(CONFIG_MX6) >>> + { 6, (struct gpio_regs *)GPIO7_BASE_ADDR }, >>> +#endif >>> }; >>> U_BOOT_DEVICES(mxc_gpios) = { >>> @@ -321,3 +355,4 @@ U_BOOT_DEVICES(mxc_gpios) = { >>> #endif >>> }; >>> #endif >>> +#endif >> Thanks, >> Peng. >> Thanks, Peng.