Hi Marek, On 2 August 2015 at 16:19, Marek Vasut <ma...@denx.de> wrote: > 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 <ma...@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 <ma...@denx.de> >> > Cc: Simon Glass <s...@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.
Yes it's very new. Only in dm/master. It's only compiled in when enabled, so you can still use it in SPL. Up to you. At some point I think we should convert all driver allocs to use devm so that we know what allocation is going on. > >> > + 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 ? A, B, C is good if you have <=26 banks. Tegra does that. Exynos does PA0, PA1, PB0, PC0, PC1, etc. > > 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 Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot