From mboxrd@z Thu Jan 1 00:00:00 1970 From: Przemyslaw Marczak Date: Mon, 20 Oct 2014 18:06:20 +0200 Subject: [U-Boot] [PATCH v9 12/12] dm: exynos: gpio: Convert to driver model In-Reply-To: <54452FF9.1010207@samsung.com> References: <1413362282-25451-1-git-send-email-sjg@chromium.org> <1413362282-25451-13-git-send-email-sjg@chromium.org> <5445227A.2090102@samsung.com> <54452FF9.1010207@samsung.com> Message-ID: <544532FC.4040202@samsung.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hello again, On 10/20/2014 05:53 PM, Przemyslaw Marczak wrote: > Hello, > > On 10/20/2014 05:31 PM, Simon Glass wrote: >> Hi, >> >> On 20 October 2014 08:55, Przemyslaw Marczak >> 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 >>>> --- >>>> >>>> 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, After a quick look into the code I suppose that the issues are not fixed, but it is tested for sure:) So the main required changes: 1. Remove/fix all "**pinctrl-uboot.dtsi" files and move the cells size into "pinctrl.dtsi" 2. Remove the gaps in Exynos4xxx gpio enum: arch/arm/include/asm/arch-exynos/gpio. and now I checked that the same is for s5pc110, here: arch/arm/include/asm/arch-s5pc1xx/gpio.h (only S5PC110_GPIO_H00) 3. Fix exynos4xxx boards *.dts after remove the gaps in gpio enum. So I think, this is all. Best Regards, -- Przemyslaw Marczak Samsung R&D Institute Poland Samsung Electronics p.marczak at samsung.com