[U-Boot] [PATCH 1/2] gpio: Add DW APB GPIO driver

Marek Vasut
2015-08-02T22:19:50Z

On Sunday, August 02, 2015 at 11:28:13 PM, Simon Glass wrote:
> Hi Marek,

Hi,

> On 27 July 2015 at 14:44, Marek Vasut <marex@denx.de> wrote:
> > Add driver for the DesignWare APB GPIO IP block.
> > This driver is DM capable and probes from DT.
> >
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Simon Glass <sjg@chromium.org>

[...]

> > +#define GPIO_SWPORTA_DR 0x00
> > +#define GPIO_SWPORTA_DDR 0x04
> > +#define GPIO_INTEN 0x30
> > +#define GPIO_INTMASK 0x34
> > +#define GPIO_INTTYPE_LEVEL 0x38
> > +#define GPIO_INT_POLARITY 0x3c
> > +#define GPIO_INTSTATUS 0x40
> > +#define GPIO_PORTA_DEBOUNCE 0x48
> > +#define GPIO_PORTA_EOI 0x4c
> > +#define GPIO_EXT_PORTA 0x50
>
> Should use C structure, right?

My understanding is that we no longer want C structs . Tom ?

[...]

> > +static const struct dm_gpio_ops gpio_dwapb_ops = {
> > + .direction_input = dwapb_gpio_direction_input,
> > + .direction_output = dwapb_gpio_direction_output,
> > + .get_value = dwapb_gpio_get_value,
> > + .set_value = dwapb_gpio_set_value,
>
> Do you want to implement .function?

No, the pinmuxing on SoCFPGA is still in a weird state.

> > +};
> > +
> > +static int gpio_dwapb_probe(struct udevice *dev)
> > +{
> > + struct gpio_dev_priv *priv = dev_get_uclass_priv(dev);
> > + struct gpio_dwapb_platdata *plat = dev->platdata;
> > +
> > + if (!plat)
> > + return 0;
> > +
> > + priv->gpio_count = plat->pins;
> > + priv->bank_name = plat->name;
> > +
> > + return 0;
> > +}
> > +
> > +static int gpio_dwapb_bind(struct udevice *dev)
> > +{
> > + struct gpio_dwapb_platdata *plat = dev_get_platdata(dev);
> > + const void *blob = gd->fdt_blob;
> > + struct udevice *subdev;
> > + fdt_addr_t base;
> > + int node, bank = 0;
> > + const char *name;
> > +
> > + /* If this is a child device, there is nothing to do here */
> > + if (plat)
> > + return 0;
> > +
> > + base = fdtdec_get_addr(blob, dev->of_offset, "reg");
> > + if (base == FDT_ADDR_T_NONE) {
> > + debug("Can't get the GPIO register base address\n");
> > + return -ENXIO;
> > + }
> > +
> > + name = fdt_get_name(blob, dev->of_offset, NULL);
> > +
> > + for (node = fdt_first_subnode(blob, dev->of_offset);
> > + node > 0;
> > + node = fdt_next_subnode(blob, node)) {
> > + int ret;
> > +
> > + if (!fdtdec_get_bool(blob, node, "gpio-controller"))
> > + continue;
> > +
> > + plat = NULL;
> > + plat = calloc(1, sizeof(*plat));
>
> I suppose this should use devm_alloc() now.

Is that even in u-boot/master ? Also, I'm not sure it's a good idea to put
this in if I use this driver in SPL.

> > + if (!plat)
> > + return -ENOMEM;
> > +
> > + plat->base = base;
> > + plat->bank = bank;
> > + plat->pins = fdtdec_get_int(blob, node, "snps,nr-gpios",
> > 0); + snprintf(plat->name, sizeof(plat->name) - 1,
> > "%s-bank%i-", + name, bank);
>
> Why such a long name? That's going to be a pain to type in the 'gpio'
> command.

Do you have a suggestion please ?

Also, I can as well use "gpio <operation> N" , where N is a number.

> > +
> > + ret = device_bind(dev, dev->driver, plat->name,
> > + plat, -1, &subdev);
> > + if (ret)
> > + return ret;
> > +
> > + subdev->of_offset = node;
> > + bank++;
> > + }
> > +
> > +
> > + return 0;
> > +}
> > +
> > +static const struct udevice_id gpio_dwapb_ids[] = {
> > + { .compatible = "snps,dw-apb-gpio" },
> > + { }
> > +};
> > +
> > +U_BOOT_DRIVER(gpio_dwapb) = {
> > + .name = "gpio-dwapb",
> > + .id = UCLASS_GPIO,
> > + .of_match = gpio_dwapb_ids,
> > + .ops = &gpio_dwapb_ops,
> > + .bind = gpio_dwapb_bind,
> > + .probe = gpio_dwapb_probe,
> > +};
> > --
> > 2.1.4
>
> Regards,
> Simon