Hi Peng, On 21 January 2015 at 04:09, Peng Fan <peng....@freescale.com> 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 <peng....@freescale.com> > --- > 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 <fdtdec.h> > +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)
s/device/dev/ as that is the convention. > +{ > + struct mxc_gpio_plat *plat = device->platdata; > + struct gpio_regs *regs; > + > + if (plat) > + return 0; Please add a comment as to why. > + > + regs = dev_get_addr(device); > + if (!regs) > + return -ENXIO; -ENODEV I think? > + > + plat = calloc(1, sizeof(*plat)); > + if (!plat) > + return -ENOMEM; Can you use the auto-alloc feature instead? Trying to keep memory allocations out of drivers unless it is for buffers, etc. > + > + plat->regs = regs; > + plat->bank_index = device->req_seq; Why store this? You can access it anytime using the device pointer. > + 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, Masahiro added a function for this.: .of_match = of_match_ptr(mxc_gpio_ids), But I'm not completely sure if this is wanted, since you include this information even when not using device tree. > + .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 Can we remove the casts by changing the type to ulong? > }; > > U_BOOT_DEVICES(mxc_gpios) = { > @@ -321,3 +355,4 @@ U_BOOT_DEVICES(mxc_gpios) = { > #endif > }; > #endif > +#endif > -- > 1.8.4 > > Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot