Hi, On Tue, Oct 28, 2014 at 11:29 AM, Simon Glass <s...@chromium.org> wrote: > Hi Chen-Yu, > > On 27 October 2014 20:53, Chen-Yu Tsai <w...@csie.org> wrote: >> Hi, >> >> On Tue, Oct 28, 2014 at 8:05 AM, Simon Glass <s...@chromium.org> wrote: >>> Hi Hans, >>> >>> On 24 October 2014 03:08, Hans de Goede <hdego...@redhat.com> wrote: >>>> Hi, >>>> >>>> On 10/23/2014 06:02 AM, Simon Glass wrote: >>>>> This adds driver model support to the sunxi GPIO driver, using the device >>>>> tree to trigger binding of the driver. The driver will still operate >>>>> without driver model too. >>>>> >>>>> Signed-off-by: Simon Glass <s...@chromium.org> >>>>> --- >>>>> >>>>> Changes in v2: >>>>> - Remove references to exynos and tegra >>>>> - Use the word 'bank' instead of 'port' >>>>> >>>>> drivers/gpio/sunxi_gpio.c | 170 >>>>> ++++++++++++++++++++++++++++++++++++++++++++++ >>>>> include/configs/sun7i.h | 1 + >>>>> 2 files changed, 171 insertions(+) >>>>> >>>>> diff --git a/drivers/gpio/sunxi_gpio.c b/drivers/gpio/sunxi_gpio.c >>>>> index 0c50a8f..44135e5 100644 >>>>> --- a/drivers/gpio/sunxi_gpio.c >>>>> +++ b/drivers/gpio/sunxi_gpio.c >>>>> @@ -11,9 +11,25 @@ >>>>> */ >>>>> >>>>> #include <common.h> >>>>> +#include <dm.h> >>>>> +#include <errno.h> >>>>> +#include <fdtdec.h> >>>>> +#include <malloc.h> >>>>> #include <asm/io.h> >>>>> #include <asm/gpio.h> >>>>> +#include <dm/device-internal.h> >>>>> >>>>> +DECLARE_GLOBAL_DATA_PTR; >>>>> + >>>>> +#define SUNXI_GPIOS_PER_BANK SUNXI_GPIO_A_NR >>>>> + >>>>> +struct sunxi_gpio_platdata { >>>>> + struct sunxi_gpio *regs; >>>>> + const char *bank_name; /* Name of bank, e.g. "B" */ >>>>> + int gpio_count; >>>>> +}; >>>>> + >>>>> +#ifndef CONFIG_DM_GPIO >>>>> static int sunxi_gpio_output(u32 pin, u32 val) >>>>> { >>>>> u32 dat; >>>>> @@ -100,3 +116,157 @@ int sunxi_name_to_gpio(const char *name) >>>>> return -1; >>>>> return group * 32 + pin; >>>>> } >>>>> +#endif >>>>> + >>>>> +#ifdef CONFIG_DM_GPIO >>>>> +static int sunxi_gpio_direction_input(struct udevice *dev, unsigned >>>>> offset) >>>>> +{ >>>>> + struct sunxi_gpio_platdata *plat = dev_get_platdata(dev); >>>>> + >>>>> + sunxi_gpio_set_cfgbank(plat->regs, offset, SUNXI_GPIO_INPUT); >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static int sunxi_gpio_direction_output(struct udevice *dev, unsigned >>>>> offset, >>>>> + int value) >>>>> +{ >>>>> + struct sunxi_gpio_platdata *plat = dev_get_platdata(dev); >>>>> + u32 num = GPIO_NUM(offset); >>>>> + >>>>> + sunxi_gpio_set_cfgbank(plat->regs, offset, SUNXI_GPIO_OUTPUT); >>>>> + clrsetbits_le32(&plat->regs->dat, 1 << num, value ? (1 << num) : 0); >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static int sunxi_gpio_get_value(struct udevice *dev, unsigned offset) >>>>> +{ >>>>> + struct sunxi_gpio_platdata *plat = dev_get_platdata(dev); >>>>> + u32 num = GPIO_NUM(offset); >>>>> + unsigned dat; >>>>> + >>>>> + dat = readl(&plat->regs->dat); >>>>> + dat >>= num; >>>>> + >>>>> + return dat & 0x1; >>>>> +} >>>>> + >>>>> +static int sunxi_gpio_set_value(struct udevice *dev, unsigned offset, >>>>> + int value) >>>>> +{ >>>>> + struct sunxi_gpio_platdata *plat = dev_get_platdata(dev); >>>>> + u32 num = GPIO_NUM(offset); >>>>> + >>>>> + clrsetbits_le32(&plat->regs->dat, 1 << num, value ? (1 << num) : 0); >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static int sunxi_gpio_get_function(struct udevice *dev, unsigned offset) >>>>> +{ >>>>> + struct sunxi_gpio_platdata *plat = dev_get_platdata(dev); >>>>> + int func; >>>>> + >>>>> + func = sunxi_gpio_get_cfgbank(plat->regs, offset); >>>>> + if (func == SUNXI_GPIO_OUTPUT) >>>>> + return GPIOF_OUTPUT; >>>>> + else if (func == SUNXI_GPIO_INPUT) >>>>> + return GPIOF_INPUT; >>>>> + else >>>>> + return GPIOF_FUNC; >>>>> +} >>>>> + >>>>> +static const struct dm_gpio_ops gpio_sunxi_ops = { >>>>> + .direction_input = sunxi_gpio_direction_input, >>>>> + .direction_output = sunxi_gpio_direction_output, >>>>> + .get_value = sunxi_gpio_get_value, >>>>> + .set_value = sunxi_gpio_set_value, >>>>> + .get_function = sunxi_gpio_get_function, >>>>> +}; >>>>> + >>>>> +/** >>>>> + * Returns the name of a GPIO bank >>>>> + * >>>>> + * GPIO banks are named A, B, C, ... >>>>> + * >>>>> + * @bank: Bank number (0, 1..n-1) >>>>> + * @return allocated string containing the name >>>>> + */ >>>>> +static char *gpio_bank_name(int bank) >>>>> +{ >>>>> + char *name; >>>>> + >>>>> + name = malloc(2); >>>>> + if (name) { >>>>> + name[0] = 'A' + bank; >>>>> + name[1] = '\0'; >>>>> + } >>>>> + >>>>> + return name; >>>>> +} >>>>> + >>>>> +static int gpio_sunxi_probe(struct udevice *dev) >>>>> +{ >>>>> + struct sunxi_gpio_platdata *plat = dev_get_platdata(dev); >>>>> + struct gpio_dev_priv *uc_priv = dev->uclass_priv; >>>>> + >>>>> + /* Tell the uclass how many GPIOs we have */ >>>>> + if (plat) { >>>>> + uc_priv->gpio_count = plat->gpio_count; >>>>> + uc_priv->bank_name = plat->bank_name; >>>>> + } >>>>> + >>>>> + return 0; >>>>> +} >>>>> +/** >>>>> + * We have a top-level GPIO device with no actual GPIOs. It has a child >>>>> + * device for each Sunxi bank. >>>>> + */ >>>>> +static int gpio_sunxi_bind(struct udevice *parent) >>>>> +{ >>>>> + struct sunxi_gpio_platdata *plat = parent->platdata; >>>>> + struct sunxi_gpio_reg *ctlr; >>>>> + int bank; >>>>> + int ret; >>>>> + >>>>> + /* If this is a child device, there is nothing to do here */ >>>>> + if (plat) >>>>> + return 0; >>>>> + >>>>> + ctlr = (struct sunxi_gpio_reg *)fdtdec_get_addr(gd->fdt_blob, >>>>> + parent->of_offset, >>>>> "reg"); >>>>> + for (bank = 0; bank < SUNXI_GPIO_BANKS; bank++) { >>>>> + struct sunxi_gpio_platdata *plat; >>>>> + struct udevice *dev; >>>>> + >>>>> + plat = calloc(1, sizeof(*plat)); >>>>> + if (!plat) >>>>> + return -ENOMEM; >>>>> + plat->regs = &ctlr->gpio_bank[bank]; >>>>> + plat->bank_name = gpio_bank_name(bank); >>>>> + plat->gpio_count = SUNXI_GPIOS_PER_BANK; >>>> >>>> This is not correct, a bank have a maximum of 32 gpios but most >>>> have less. I assume that this should be the number of actual pins in >>>> the bank, correct ? >>> >>> Well I worry that we need to maintain compatibility with what is >>> there. At some point we can change it, but for now the GPIO numbering >>> is done assuming 32 GPIOs per bank, right? >> >> The numbering always assumed 32 GPIOs per bank, just some of them may >> be empty. IMHO it simplifies register/bit offset conversions, and >> it's easier to convert the numbers to the designations in the datasheet >> by hand, for example PA0 = 0, PB0 = 32. >> >> We also do this in the kernel driver. But the bindings are 3 cell: >> <bank pin flags>. > > OK, well we can leave the 'holes' for now. > >> >>> When everything is moved to driver model I suppose we can be more clever. >>> >>>> >>>> Our "gpio-pin-numbers" are based on a sparse numbering >>>> scheme assuming 32 pins / bank, and there are assumptions this is >>>> the case in various places, so we cannot fix this until we've >>>> fully gone dm for all gpio usage. But here it would be nice >>>> to have the actual numbers of pins. >>>> >>>> Doing so requires at least one table with bank -> number of gpio-s >>>> mapping. And I think it may also differ on SoC type in some cases >>>> (I would need to take a look at the datasheets) >>> >>> Hoping this can be in the device tree. Do you have a binding for it? >> >> This is in the (kernel) driver, not the device tree bindings. >> So we would need to at least add a table for that. >> >> I don't see any pinmux related stuff in this patch. Does the gpio >> dm handle that? > > No, or at least not yet. Does sunxi have kernel support for pinctrl? > We could perhaps use that binding if it exists. Otherwise I think the > current code is our best bet - we can select the correct serial port > based on static configuration (CONFIG) for now.
It does. But the bindings are based on strings for function descriptions, which implies a whole lookup table in the driver. Not sure this would be great for SPL. Also Linus Walleij (pinctrl maintainer) proposed some new generic bindings, though I don't know if we will ever switch over. CCing Maxime Ripard (sunxi maintainer) on this. ChenYu >>>> >>>> I suggest keeping this as is for now, and put fixing this with >>>> a follow up patch on the todo list, and otherwise this looks good, >>>> so this is: >>>> >>>> Acked-by: Hans de Goede <hdego...@redhat.com> >>> >>> OK >>> >>>> >>>> >>>> >>>>> + >>>>> + 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 sunxi_gpio_ids[] = { >>>>> + { .compatible = "allwinner,sun7i-a20-pinctrl" }, >>>>> + { } >>>>> +}; >>>>> + >>>>> +U_BOOT_DRIVER(gpio_sunxi) = { >>>>> + .name = "gpio_sunxi", >>>>> + .id = UCLASS_GPIO, >>>>> + .ops = &gpio_sunxi_ops, >>>>> + .of_match = sunxi_gpio_ids, >>>>> + .bind = gpio_sunxi_bind, >>>>> + .probe = gpio_sunxi_probe, >>>>> +}; >>>>> +#endif >>>>> diff --git a/include/configs/sun7i.h b/include/configs/sun7i.h >>>>> index 500d0e3..2314e97 100644 >>>>> --- a/include/configs/sun7i.h >>>>> +++ b/include/configs/sun7i.h >>>>> @@ -38,6 +38,7 @@ >>>>> >>>>> #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_DM) >>>>> # define CONFIG_CMD_DM >>>>> +# define CONFIG_DM_GPIO >>>>> #endif >>>>> >>>>> /* _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot