On Mon, Dec 15, 2014 at 3:46 PM, Álvaro Fernández Rojas <[email protected]> wrote:
Please add a description for your bindings in Documentation/devicetree/bindings/gpio > Signed-off-by: Álvaro Fernández Rojas <[email protected]> > --- > v2: use vendor for DT properties. > > diff --git a/target/linux/brcm63xx/config-3.14 > b/target/linux/brcm63xx/config-3.14 > index dd27f47..f94c567 100644 > --- a/target/linux/brcm63xx/config-3.14 > +++ b/target/linux/brcm63xx/config-3.14 > @@ -76,6 +76,7 @@ CONFIG_GENERIC_IRQ_SHOW=y > CONFIG_GENERIC_PCI_IOMAP=y > CONFIG_GENERIC_SMP_IDLE_THREAD=y > CONFIG_GPIOLIB=y > +CONFIG_GPIO_BCM6345=y > CONFIG_GPIO_DEVRES=y > CONFIG_GPIO_SYSFS=y > # CONFIG_HAMRADIO is not set > diff --git a/target/linux/brcm63xx/config-3.18 > b/target/linux/brcm63xx/config-3.18 > index e3cf020..7957d02 100644 > --- a/target/linux/brcm63xx/config-3.18 > +++ b/target/linux/brcm63xx/config-3.18 > @@ -80,6 +80,7 @@ CONFIG_GENERIC_IRQ_SHOW=y > CONFIG_GENERIC_PCI_IOMAP=y > CONFIG_GENERIC_SMP_IDLE_THREAD=y > CONFIG_GPIOLIB=y > +CONFIG_GPIO_BCM6345=y > CONFIG_GPIO_DEVRES=y > CONFIG_GPIO_SYSFS=y > # CONFIG_HAMRADIO is not set > diff --git > a/target/linux/brcm63xx/patches-3.14/374-GPIO-add-bcm6345-driver.patch > b/target/linux/brcm63xx/patches-3.14/374-GPIO-add-bcm6345-driver.patch > new file mode 100644 > index 0000000..847933b > --- /dev/null > +++ b/target/linux/brcm63xx/patches-3.14/374-GPIO-add-bcm6345-driver.patch > @@ -0,0 +1,244 @@ > +--- /dev/null > ++++ b/drivers/gpio/gpio-bcm6345.c > +@@ -0,0 +1,216 @@ > ++/* > ++ * This file is subject to the terms and conditions of the GNU General > Public > ++ * License. See the file "COPYING" in the main directory of this archive > ++ * for more details. > ++ * > ++ * Copyright (C) 2008 Maxime Bizon <[email protected]> > ++ * Copyright (C) 2008-2011 Florian Fainelli <[email protected]> > ++ * Copyright (C) 2014 Álvaro Fernández Rojas <[email protected]> > ++ */ > ++ > ++#include <linux/kernel.h> > ++#include <linux/module.h> > ++#include <linux/spinlock.h> > ++#include <linux/platform_device.h> > ++#include <linux/gpio.h> > ++ > ++enum bcm6345_gpio_reg { > ++ GPIO_REG_CTL_HI = 0, > ++ GPIO_REG_CTL_LO, > ++ GPIO_REG_DATA_HI, > ++ GPIO_REG_DATA_LO, > ++ GPIO_REG_MAX > ++}; > ++ > ++struct bcm6345_gpio_chip { > ++ struct gpio_chip chip; > ++ u8 regs[GPIO_REG_MAX]; I don't think we need to be that flexible, as there are essentially only two (three) layouts: bcm6345 and everything else, so we can cover these through appropriate compatible strings (the arm chips and the newest mips one have five registers instead of two for a total of 160 gpios). Also the reg property should only cover what you need, not the whole gpio range. So only e.g. <0xfffe0404 0x8> in case of bcm6345, and <0xfffe0400 0x10> / <0xfffe0080 0x10> / <0x10000080 0x10> for anything else. Or add both as separate resources and use reg-names to distinguish them. You could also use the length of the registers to infer the offsets of the hi/lo registers, and don't need to tell bcm6345 and later apart. > ++ > ++ spinlock_t lock; > ++ void __iomem *membase; > ++}; > ++ > ++static inline struct bcm6345_gpio_chip *to_bcm6345_gpio(struct gpio_chip > *chip) > ++{ > ++ struct bcm6345_gpio_chip *bg; > ++ > ++ bg = container_of(chip, struct bcm6345_gpio_chip, chip); > ++ > ++ return bg; > ++} > ++ > ++static inline u32 bc_gpio_r32(struct bcm6345_gpio_chip *bg, u8 reg) Please call this bcm6345_gpio_read32. > ++{ > ++ return ioread32be(bg->membase + bg->regs[reg]); > ++} > ++ > ++static inline void bc_gpio_w32(struct bcm6345_gpio_chip *bg, u8 reg, u32 > val) And this one bcm6345_gpio_write32 > ++{ > ++ iowrite32be(val, bg->membase + bg->regs[reg]); > ++} > ++ > ++static void bcm6345_gpio_set(struct gpio_chip *chip, unsigned gpio, int > value) > ++{ > ++ struct bcm6345_gpio_chip *bg = to_bcm6345_gpio(chip); > ++ unsigned long flags; > ++ u32 mask, val; > ++ u8 reg; > ++ > ++ if (gpio < 32) { > ++ reg = GPIO_REG_DATA_LO; > ++ mask = BIT(gpio); > ++ } else { > ++ reg = GPIO_REG_DATA_HI; > ++ mask = BIT(gpio - 32); > ++ } I would suggest doing something similar like I did for the "reverse" order of the interrupt registers. Let the bcm6456_gpio_chip have u8 data_reg[2]; u8 dir_reg[2]; which you then populate on probe with { 0x0 }, { 0x4 } for bcm6345, and { 0x4, 0x0 } and { 0xc, 0x8 } Then you can do just reg = bg->data_reg[gpio / 32]; mask = BIT(gpio % 32); ... val = bcm6345_gpio_read32(bg, reg); ... This will also make it easy to expand it for support of more than 64 gpios. > ++ > ++ spin_lock_irqsave(&bg->lock, flags); > ++ val = bc_gpio_r32(bg, reg); > ++ if (value) > ++ val |= mask; > ++ else > ++ val &= ~mask; > ++ bc_gpio_w32(bg, reg, val); > ++ spin_unlock_irqrestore(&bg->lock, flags); > ++} > ++ > ++static int bcm6345_gpio_get(struct gpio_chip *chip, unsigned gpio) > ++{ > ++ struct bcm6345_gpio_chip *bg = to_bcm6345_gpio(chip); > ++ u32 mask; > ++ u8 reg; > ++ > ++ if (gpio < 32) { > ++ reg = GPIO_REG_DATA_LO; > ++ mask = BIT(gpio); > ++ } else { > ++ reg = GPIO_REG_DATA_HI; > ++ mask = BIT(gpio - 32); > ++ } > ++ > ++ return !!(bc_gpio_r32(bg, reg) & mask); > ++} > ++ > ++static int bcm6345_gpio_direction_input(struct gpio_chip *chip, unsigned > gpio) > ++{ > ++ struct bcm6345_gpio_chip *bg = to_bcm6345_gpio(chip); > ++ unsigned long flags; > ++ u32 mask, val; > ++ u8 reg; > ++ > ++ if (gpio < 32) { > ++ reg = GPIO_REG_CTL_LO; > ++ mask = BIT(gpio); > ++ } else { > ++ reg = GPIO_REG_CTL_HI; > ++ mask = BIT(gpio - 32); > ++ } > ++ > ++ spin_lock_irqsave(&bg->lock, flags); > ++ val = bc_gpio_r32(bg, reg) & ~mask; > ++ bc_gpio_w32(bg, reg, val); > ++ spin_unlock_irqrestore(&bg->lock, flags); > ++ > ++ return 0; > ++} > ++ > ++static int bcm6345_gpio_direction_output(struct gpio_chip *chip, unsigned > gpio) > ++{ > ++ struct bcm6345_gpio_chip *bg = to_bcm6345_gpio(chip); > ++ unsigned long flags; > ++ u32 mask, val; > ++ u8 reg; > ++ > ++ if (gpio < 32) { > ++ reg = GPIO_REG_CTL_LO; > ++ mask = BIT(gpio); > ++ } else { > ++ reg = GPIO_REG_CTL_HI; > ++ mask = BIT(gpio - 32); > ++ } > ++ > ++ spin_lock_irqsave(&bg->lock, flags); > ++ val = bc_gpio_r32(bg, reg) | mask; > ++ bc_gpio_w32(bg, reg, val); > ++ spin_unlock_irqrestore(&bg->lock, flags); > ++ > ++ return 0; > ++} > ++ > ++int __init bcm6345_gpio_probe(struct platform_device *pdev) > ++{ > ++ struct device_node *np = pdev->dev.of_node; > ++ struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > ++ struct bcm6345_gpio_chip *bg; > ++ const __be32 *ngpio, *gpiobase; > ++ > ++ if (!res) { > ++ dev_err(&pdev->dev, "failed to find resource\n"); > ++ return -ENOMEM; > ++ } > ++ > ++ bg = devm_kzalloc(&pdev->dev, > ++ sizeof(struct gpio_chip), GFP_KERNEL); > ++ if (!bg) > ++ return -ENOMEM; > ++ > ++ bg->membase = devm_ioremap_resource(&pdev->dev, res); > ++ if (IS_ERR(bg->membase)) { > ++ dev_err(&pdev->dev, "cannot remap I/O memory region\n"); No need to add error messages here, ioremap will already complain for you. > ++ return -ENOMEM; > ++ } > ++ > ++ if (of_property_read_u8_array(np, "brcm,register-map", > ++ bg->regs, GPIO_REG_MAX)) { > ++ dev_err(&pdev->dev, "failed to read register definition\n"); As I explain above, no need for this one. > ++ return -EINVAL; > ++ } > ++ > ++ ngpio = of_get_property(np, "brcm,num-gpios", NULL); > ++ if (!ngpio) { > ++ dev_err(&pdev->dev, "failed to read number of pins\n"); > ++ return -EINVAL; > ++ } > ++ > ++ gpiobase = of_get_property(np, "brcm,gpio-base", NULL); NAK on this part. This should be at best set in the driver itself, but not be in the devicetree itself. > ++ if (gpiobase) > ++ bg->chip.base = be32_to_cpu(*gpiobase); > ++ else > ++ bg->chip.base = -1; > ++ > ++ spin_lock_init(&bg->lock); > ++ > ++ bg->chip.dev = &pdev->dev; > ++ bg->chip.label = dev_name(&pdev->dev); > ++ bg->chip.of_node = np; > ++ bg->chip.ngpio = be32_to_cpu(*ngpio); No need to manually convert it, just use of_property_read_u32(). > ++ bg->chip.direction_input = bcm6345_gpio_direction_input; > ++ bg->chip.direction_output = bcm6345_gpio_direction_output; > ++ bg->chip.get = bcm6345_gpio_get; > ++ bg->chip.set = bcm6345_gpio_set; > ++ > ++ dev_info(&pdev->dev, "registering %d gpios\n", bg->chip.ngpio); > ++ > ++ return gpiochip_add(&bg->chip); > ++} > ++ > ++static struct of_device_id bcm6345_gpio_match[] = { > ++ { .compatible = "brcm,bcm6345-gpio" }, > ++ { }, > ++}; > ++MODULE_DEVICE_TABLE(of, bcm6345_gpio_match); > ++ > ++static struct platform_driver bcm6345_gpio_driver = { > ++ .driver = { > ++ .name = "bcm6345-gpio", > ++ .owner = THIS_MODULE, > ++ .of_match_table = bcm6345_gpio_match, > ++ }, > ++ .probe = bcm6345_gpio_probe, > ++}; > ++ > ++int __init bcm6345_gpio_init(void) > ++{ > ++ return platform_driver_register(&bcm6345_gpio_driver); > ++} > ++subsys_initcall(bcm6345_gpio_init); > +--- a/drivers/gpio/Kconfig > ++++ b/drivers/gpio/Kconfig > +@@ -108,6 +108,12 @@ config GPIO_MAX730X > + > + comment "Memory mapped GPIO drivers:" > + > ++config GPIO_BCM6345 > ++ bool "Broadcom 6345 GPIO Support" > ++ depends on BCM63XX > ++ help > ++ Say yes here to support the Broadcom 6345 SoC GPIO device > ++ > + config GPIO_CLPS711X > + tristate "CLPS711X GPIO support" > + depends on ARCH_CLPS711X || COMPILE_TEST > +--- a/drivers/gpio/Makefile > ++++ b/drivers/gpio/Makefile > +@@ -17,6 +17,7 @@ obj-$(CONFIG_GPIO_ADP5588) += gpio-adp55 > + obj-$(CONFIG_GPIO_AMD8111) += gpio-amd8111.o > + obj-$(CONFIG_GPIO_ARIZONA) += gpio-arizona.o > + obj-$(CONFIG_GPIO_BCM_KONA) += gpio-bcm-kona.o > ++obj-$(CONFIG_GPIO_BCM6345) += gpio-bcm6345.o > + obj-$(CONFIG_GPIO_BT8XX) += gpio-bt8xx.o > + obj-$(CONFIG_GPIO_CLPS711X) += gpio-clps711x.o > + obj-$(CONFIG_GPIO_CS5535) += gpio-cs5535.o Jonas _______________________________________________ openwrt-devel mailing list [email protected] https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
