[U-Boot] [PATCH v3 4/4] dm:gpio:mxc add DT support

[Igor Grinberg <grinberg@compulab.co.il>]

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 <Peng.Fan@freescale.com>

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 <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)
>> +{
>> +    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.