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
