[U-Boot] [PATCH v9 12/12] dm: exynos: gpio: Convert to driver model

[Przemyslaw Marczak]

Hello,

On 10/20/2014 05:31 PM, Simon Glass wrote:
> Hi,
>
> On 20 October 2014 08:55, Przemyslaw Marczak <p.marczak at samsung.com> wrote:
>> Hello Simon,
>>
>>
>> On 10/15/2014 10:38 AM, Simon Glass wrote:
>>>
>>> Convert the exynos GPIO driver to driver model. This implements the
>>> generic
>>> GPIO interface but not the extra Exynos-specific functions.
>>>
>>> Signed-off-by: Simon Glass <sjg at chromium.org>
>>> ---
>>>
>>> Changes in v9:
>>> - Add missing compatible strings from exynos_gpio_ids[]
>>>
>>> Changes in v8:
>>> - Enable driver model for smdkc100 and s5p_goni separately
>>>
>>> Changes in v7:
>>> - Bring in patches from the SPI series to move post-reloc DM init earlier
>>>
>>> Changes in v6: None
>>> Changes in v5:
>>> - Remove RFC label now that build errors are fixed
>>> - Tidy up and update cover letter message
>>> - Avoid reordering functions
>>>
>>> Changes in v4:
>>> - Add patches for exynos GPIO support
>>>
>>>    drivers/gpio/s5p_gpio.c         | 424
>>> +++++++++++++++++++++++++++-------------
>>>    include/configs/exynos-common.h |   4 +
>>>    include/configs/s5p_goni.h      |   3 +
>>>    include/configs/smdkc100.h      |   3 +
>>>    4 files changed, 298 insertions(+), 136 deletions(-)
>>>
>>> diff --git a/drivers/gpio/s5p_gpio.c b/drivers/gpio/s5p_gpio.c
>>> index 99f2dd8..13d74eb 100644
>>> --- a/drivers/gpio/s5p_gpio.c
>>> +++ b/drivers/gpio/s5p_gpio.c
>>> @@ -6,8 +6,15 @@
>>>     */
>>>
>>
>> ..snip..
>>
>>> +/**
>>> + * We have a top-level GPIO device with no actual GPIOs. It has a child
>>> + * device for each Exynos GPIO bank.
>>> + */
>>> +static int gpio_exynos_bind(struct udevice *parent)
>>> +{
>>> +       struct exynos_gpio_platdata *plat = parent->platdata;
>>> +       struct s5p_gpio_bank *bank, *base;
>>> +       const void *blob = gd->fdt_blob;
>>> +       int node;
>>> +
>>> +       /* If this is a child device, there is nothing to do here */
>>> +       if (plat)
>>> +               return 0;
>>> +
>>> +       base = (struct s5p_gpio_bank *)fdtdec_get_addr(gd->fdt_blob,
>>> +                                                  parent->of_offset,
>>> "reg");
>>> +       for (node = fdt_first_subnode(blob, parent->of_offset), bank =
>>> base;
>>> +            node > 0;
>>> +            node = fdt_next_subnode(blob, node), bank++) {
>>
>>
>> Here (bank++) you assume, that order of subnodes of each pinctrl is the same
>> like in dts file - as I wrote in comment to patch 04 - it isn't.
>>
>>> +               struct exynos_gpio_platdata *plat;
>>> +               struct udevice *dev;
>>> +               fdt_addr_t reg;
>>> +               int ret;
>>> +
>>> +               if (!fdtdec_get_bool(blob, node, "gpio-controller"))
>>> +                       continue;
>>> +               plat = calloc(1, sizeof(*plat));
>>> +               if (!plat)
>>> +                       return -ENOMEM;
>>
>>
>> Some parts of GPIO banks, for which "reg" can be found are prober.
>>
>>
>>> +               reg = fdtdec_get_addr(blob, node, "reg");
>>> +               if (reg != FDT_ADDR_T_NONE)
>>> +                       bank = (struct s5p_gpio_bank *)((ulong)base +
>>> reg);
>>> +               plat->bank = bank;
>>> +               plat->bank_name = fdt_get_name(blob, node, NULL);
>>> +               debug("dev at %p: %s\n", bank, plat->bank_name);
>>> +
>>> +               ret = device_bind(parent, parent->driver,
>>> +                                       plat->bank_name, plat, -1, &dev);
>>> +               if (ret)
>>> +                       return ret;
>>> +               dev->of_offset = parent->of_offset;
>>> +       }
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static const struct udevice_id exynos_gpio_ids[] = {
>>> +       { .compatible = "samsung,s5pc100-pinctrl" },
>>> +       { .compatible = "samsung,s5pc110-pinctrl" },
>>> +       { .compatible = "samsung,exynos4210-pinctrl" },
>>> +       { .compatible = "samsung,exynos4x12-pinctrl" },
>>> +       { .compatible = "samsung,exynos5250-pinctrl" },
>>> +       { .compatible = "samsung,exynos5420-pinctrl" },
>>> +       { }
>>
>>
>> ... snip ...
>>
>> And this is not the end of issues:)
>>
>> Unfortunately Exynos4xxx has a gaps in GPIO bank address space, and the big
>> gpio enum lists includes that here:
>> arch/arm/include/asm/arch-exynos/gpio.h
>>
>> This works for Exynos5, because it's enum list is linear.
>>
>> And the driver gpio-uclass.c - assumes that the gpio numbers are linear.
>> When I removed the gaps in enum for 4x12 -then gpio is mapped good.
>>
>> And after remove the enum gaps, it also requires fitting the gpios in dts
>> files, e.g. cd-gpio, pwr-gpios for sdhci host.
>>
>> Sorry for such late reply in v9 but, before it seems working fine - I didn't
>> try to check the sd card before.
>>
>> I hope that after that fixes, everything will work fine:)
>
> Thanks very much for testing this. As mentioned on the other patch,
> please try u-boot-dm/next and see how things look.
>
> Regards,
> Simon
>

No problem, it's also important for me. I will check the "next" in a 
few minutes.

Regards,
-- 
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com