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.
Regards,
Simon
>
>
> 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
