[U-Boot] [U-Boot, v2, 5/6] dm: sunxi: Modify the GPIO driver to support driver model

Chen-Yu Tsai wens at csie.org
Tue Oct 28 03:53:00 CET 2014


Hi,

On Tue, Oct 28, 2014 at 8:05 AM, Simon Glass <sjg at chromium.org> wrote:
> Hi Hans,
>
> On 24 October 2014 03:08, Hans de Goede <hdegoede at 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 <sjg at 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>.

> 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?


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 <hdegoede at 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
>>>
>>>  /*


More information about the U-Boot mailing list