| Message ID | 1414036962-28463-6-git-send-email-sjg@chromium.org |
|---|---|
| State | Superseded |
| Delegated to: | Hans de Goede |
| Headers | show |
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@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 ? 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) 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@redhat.com> > + > + 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 > > /* > Regards, Hans
Hi Hans, On 24 October 2014 03:08, Hans de Goede <hdegoede@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@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? 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? > > 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@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 >> >> /* >> > > Regards, > > Hans Regards, Simon
Hi, On Tue, Oct 28, 2014 at 8:05 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Hans, > > On 24 October 2014 03:08, Hans de Goede <hdegoede@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@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@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 >>> >>> /*
Hi Chen-Yu, On 27 October 2014 20:53, Chen-Yu Tsai <wens@csie.org> wrote: > Hi, > > On Tue, Oct 28, 2014 at 8:05 AM, Simon Glass <sjg@chromium.org> wrote: >> Hi Hans, >> >> On 24 October 2014 03:08, Hans de Goede <hdegoede@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@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 <hdegoede@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 >>>> >>>> /*
Hi, On Tue, Oct 28, 2014 at 11:29 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Chen-Yu, > > On 27 October 2014 20:53, Chen-Yu Tsai <wens@csie.org> wrote: >> Hi, >> >> On Tue, Oct 28, 2014 at 8:05 AM, Simon Glass <sjg@chromium.org> wrote: >>> Hi Hans, >>> >>> On 24 October 2014 03:08, Hans de Goede <hdegoede@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@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 <hdegoede@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 >>>>> >>>>> /*
Hi, On Tue, Oct 28, 2014 at 11:39:07AM +0800, Chen-Yu Tsai wrote: > Hi, > >>> 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. I'm not planning to move to the new bindings any time soon, and no one is actively working on that to my knowledge either. Maxime
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; + + 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 /*
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@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(+)