Re: [U-Boot] [PATCH v3 4/4] dm:gpio:mxc add DT support
Hi Peng, On 24 January 2015 at 07:34, Peng Fan wrote: > Hi Simon, > > > On 1/23/2015 5:26 AM, Simon Glass wrote: >> >> Hi Peng, >> >> On 21 January 2015 at 04:09, 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 >>> --- >>> 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) >> >> s/device/dev/ as that is the convention. > > Will fix this. >> >> >>> +{ >>> + struct mxc_gpio_plat *plat = device->platdata; >>> + struct gpio_regs *regs; >>> + >>> + if (plat) >>> + return 0; >> >> Please add a comment as to why. > > Ok. >> >> >>> + >>> + regs = dev_get_addr(device); >>> + if (!regs) >>> + return -ENXIO; >> >> -ENODEV I think? > > Yeah. Right. >>> >>> + >>> + 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. > > I want the DM code can support DT and no DT. To no DT, platdata is defined > in U_BOOT_DEVICES. > If using auto-alloc feature, but DT is not supported, is it conflict with > platdata in U_BOOT_DEVICES? Yes it will. But please add a TODO in the code to remove this when every board is converted to driver model and you don't need this. >> >> >>> + >>> + plat->regs = regs; >>> + plat->bank_index = device->req_seq; >> >> Why store this? You can access it anytime using the device pointer. > > To no DT, bank_index is statically intialized in mxc_plat array. I do not > want to introudce `#ifdef CONFIG_OF_CONTROL` in probe function and introudce > `if (dev->of_offset == -1)`, so > store it to bank_index. > To no DT, `if(plat) return 0;` will return. So plat->bank_index = > device->req_seq will only effects for DT. > Just want to support DT and no DT. OK I think I understand. >> >> >>> + 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 = mx
Re: [U-Boot] [PATCH v3 4/4] dm:gpio:mxc add DT support
Hi Simon, On 1/23/2015 5:26 AM, Simon Glass wrote: Hi Peng, On 21 January 2015 at 04:09, 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 --- 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) s/device/dev/ as that is the convention. Will fix this. +{ + struct mxc_gpio_plat *plat = device->platdata; + struct gpio_regs *regs; + + if (plat) + return 0; Please add a comment as to why. Ok. + + regs = dev_get_addr(device); + if (!regs) + return -ENXIO; -ENODEV I think? Yeah. Right. + + 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. I want the DM code can support DT and no DT. To no DT, platdata is defined in U_BOOT_DEVICES. If using auto-alloc feature, but DT is not supported, is it conflict with platdata in U_BOOT_DEVICES? + + plat->regs = regs; + plat->bank_index = device->req_seq; Why store this? You can access it anytime using the device pointer. To no DT, bank_index is statically intialized in mxc_plat array. I do not want to introudce `#ifdef CONFIG_OF_CONTROL` in probe function and introudce `if (dev->of_offset == -1)`, so store it to bank_index. To no DT, `if(plat) return 0;` will return. So plat->bank_index = device->req_seq will only effects for DT. Just want to support DT and no DT. + 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. Thanks,I'll try this. I am not sure whether using of_match_ptr will make compiler complain mxc_gpio_ids `defined but not used` for no DT, since `#ifdef xx` is not recommended to compiled out `mxc_gpio_ids` for no DT. + .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(CONF
Re: [U-Boot] [PATCH v3 4/4] dm:gpio:mxc add DT support
Hi Peng, On 21 January 2015 at 04:09, 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 > --- > 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) 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 >
Re: [U-Boot] [PATCH v3 4/4] dm:gpio:mxc add DT support
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 }, +
Re: [U-Boot] [PATCH v3 4/4] dm:gpio:mxc add DT support
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? >> +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. > -- Regards, Igor. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 4/4] dm:gpio:mxc add DT support
Hi Igor, Just kindly remind, did you miss this one? Since you ack the other patches in this patch set. 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 --- 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; + 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. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v3 4/4] dm:gpio:mxc add DT support
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 --- 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; + 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 -- 1.8.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot