From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sudeep.KarkadaNagesha@arm.com (Sudeep KarkadaNagesha) Date: Thu, 01 Aug 2013 19:20:36 +0100 Subject: [PATCH v3 1/3] ARM: bcm281xx: Add GPIO driver In-Reply-To: <1374870345-8276-2-git-send-email-markus.mayer@linaro.org> References: <1374870345-8276-1-git-send-email-markus.mayer@linaro.org> <1374870345-8276-2-git-send-email-markus.mayer@linaro.org> Message-ID: <51FAA6F4.2010002@arm.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 26/07/13 21:25, Markus Mayer wrote: > From: Markus Mayer > > This patch adds the GPIO driver for the bcm281xx family of chips. > May be I am missing my mails, but I could not find even in archives. The patch says its v3, I could not trace v1/v2 and not change log. > Signed-off-by: Markus Mayer > Reviewed-by: Ray Jui > Reviewed-by: Christian Daudt > Reviewed-by: Tim Kryger > Reviewed-by: Matt Porter > Reviewed-by: Alex Elder > --- > drivers/gpio/gpio-bcm-kona.c | 697 ++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 697 insertions(+) > create mode 100644 drivers/gpio/gpio-bcm-kona.c > > diff --git a/drivers/gpio/gpio-bcm-kona.c b/drivers/gpio/gpio-bcm-kona.c > new file mode 100644 > index 0000000..ca6cb7e > --- /dev/null > +++ b/drivers/gpio/gpio-bcm-kona.c > @@ -0,0 +1,697 @@ > +/* > + * Copyright (C) 2012-2013 Broadcom Corporation > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License as > + * published by the Free Software Foundation version 2. > + * > + * This program is distributed "as is" WITHOUT ANY WARRANTY of any > + * kind, whether express or implied; without even the implied warranty > + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define BCM_GPIO_PASSWD 0x00a5a501 > +#define GPIO_PER_BANK 32 > + > +#define GPIO_BANK(gpio) ((gpio) >> 5) > +#define GPIO_BIT(gpio) ((gpio) & (GPIO_PER_BANK - 1)) > + > +#define GPIO_GPOR0_OFFSET 0x00000000 > +#define GPIO_GPIR0_OFFSET 0x00000020 > +#define GPIO_GPORS0_OFFSET 0x00000040 > +#define GPIO_GPORC0_OFFSET 0x00000060 > +#define GPIO_ISR0_OFFSET 0x00000080 > +#define GPIO_IMR0_OFFSET 0x000000A0 > +#define GPIO_IMRC0_OFFSET 0x000000C0 > +#define GPIO_GPCTR0_OFFSET 0x00000100 > +#define GPIO_GPPLSR0_OFFSET 0x00000500 > +#define GPIO_GPPWR_OFFSET 0x00000520 > + Why these macros can't be something like this ? #define GPIO_GPOR_OFFSET(bank) (0x00000000 + (bank) << 2) > +#define GPIO_GPCTR0_DBR_SHIFT 5 > +#define GPIO_GPCTR0_DBR_MASK 0x000001E0 > + > +#define GPIO_GPCTR0_ITR_SHIFT 3 > +#define GPIO_GPCTR0_ITR_MASK 0x00000018 > +#define GPIO_GPCTR0_ITR_CMD_RISING_EDGE 0x00000001 > +#define GPIO_GPCTR0_ITR_CMD_FALLING_EDGE 0x00000002 > +#define GPIO_GPCTR0_ITR_CMD_BOTH_EDGE 0x00000003 > + > +#define GPIO_GPCTR0_IOTR_MASK 0x00000001 > +#define GPIO_GPCTR0_IOTR_CMD_0UTPUT 0x00000000 > +#define GPIO_GPCTR0_IOTR_CMD_INPUT 0x00000001 > + > +#define GPIO_GPCTR0_DB_ENABLE_MASK 0x00000100 > + > +#define LOCK_CODE 0xFFFFFFFF > +#define UNLOCK_CODE 0x0 > + > +struct bcm_kona_gpio_bank { > + int id; > + int irq; > +}; > + > +struct bcm_kona_gpio { > + struct irq_domain *irq_domain; > + void __iomem *reg_base; > + int num_bank; > + int irq_base; > + spinlock_t lock; > + struct bcm_kona_gpio_bank *banks; > +}; > + > +static struct gpio_chip bcm_gpio_chip; > + Can this be embed in bcm_kona_gpio? You can then get rid of all the accesses to global variable bcm_kona_gpio. > +static void bcm_kona_gpio_irq_mask(struct irq_data *d); > +static void bcm_kona_gpio_irq_unmask(struct irq_data *d); > + > +static struct bcm_kona_gpio bcm_kona_gpio; > + Can this be allocated dynamically ? And the changed global pointer can be accessed only in probe. You can remove all direct access once you embed as mentioned above. > +static inline u32 bcm_kona_gpio_out_status(int bank) > +{ > + return GPIO_GPOR0_OFFSET + (bank << 2); > +} > + > +static inline u32 bcm_kona_gpio_in_status(int bank) > +{ > + return GPIO_GPIR0_OFFSET + (bank << 2); > +} > + > +static inline u32 bcm_kona_gpio_out_set(int bank) > +{ > + return GPIO_GPORS0_OFFSET + (bank << 2); > +} > + > +static inline u32 bcm_kona_gpio_out_clear(int bank) > +{ > + return GPIO_GPORC0_OFFSET + (bank << 2); > +} > + > +static inline u32 bcm_kona_gpio_int_status(int bank) > +{ > + return GPIO_ISR0_OFFSET + (bank << 2); > +} > + > +static inline u32 bcm_kona_gpio_int_mask(int bank) > +{ > + return GPIO_IMR0_OFFSET + (bank << 2); > +} > + > +static inline u32 bcm_kona_gpio_int_mskclr(int bank) > +{ > + return GPIO_IMRC0_OFFSET + (bank << 2); > +} > + > +static inline u32 bcm_kona_gpio_pwd_status(int bank) > +{ > + return GPIO_GPPLSR0_OFFSET + (bank << 2); > +} > + > +static inline u32 bcm_kona_gpio_control(int gpio) > +{ > + return GPIO_GPCTR0_OFFSET + (gpio << 2); > +} All these functions can go away if you can have the macros as shown above ? > + > +static int bcm_kona_count_irq_resources(struct platform_device *pdev) > +{ > + struct resource *res; > + int count = 0; > + > + for (;;) { > + res = platform_get_resource(pdev, IORESOURCE_IRQ, count); platform_get_irq instead > + if (!res) > + break; > + count++; > + } > + return count; > +} > + > +static void bcm_kona_gpio_set_lockcode_bank(int bank_id, int lockcode) > +{ > + void __iomem *reg_base = bcm_kona_gpio.reg_base; pass gpio_chip to avoid using global > + /* This register is password protected */ > + writel(BCM_GPIO_PASSWD, reg_base + GPIO_GPPWR_OFFSET); > + /* Lock the bank */ > + writel(lockcode, reg_base + bcm_kona_gpio_pwd_status(bank_id)); > +} > + > +static inline void bcm_kona_gpio_lock_bank(int bank_id) ditto > +{ > + bcm_kona_gpio_set_lockcode_bank(bank_id, LOCK_CODE); > +} > + > +static inline void bcm_kona_gpio_unlock_bank(int bank_id) ditto > +{ > + bcm_kona_gpio_set_lockcode_bank(bank_id, UNLOCK_CODE); > +} > + > +static void bcm_kona_gpio_set(struct gpio_chip *chip, unsigned gpio, > + int value) > +{ > + int bank_id = GPIO_BANK(gpio); > + int bit = GPIO_BIT(gpio); > + void __iomem *reg_base = bcm_kona_gpio.reg_base; fetch bcm_kona_gpio from gpio_chip > + u32 val, reg_offset; > + unsigned long flags; > + > + spin_lock_irqsave(&bcm_kona_gpio.lock, flags); > + bcm_kona_gpio_unlock_bank(bank_id); > + > + /* determine the GPIO pin direction */ > + val = readl(reg_base + bcm_kona_gpio_control(gpio)); > + val &= GPIO_GPCTR0_IOTR_MASK; > + > + /* this function only applies to output pin */ > + if (GPIO_GPCTR0_IOTR_CMD_INPUT == val) > + return; > + > + reg_offset = value ? bcm_kona_gpio_out_set(bank_id) : > + bcm_kona_gpio_out_clear(bank_id); > + > + val = readl(reg_base + reg_offset); > + val |= 1 << bit; > + writel(val, reg_base + reg_offset); > + > + bcm_kona_gpio_lock_bank(bank_id); > + spin_unlock_irqrestore(&bcm_kona_gpio.lock, flags); > +} > + > +static int bcm_kona_gpio_get(struct gpio_chip *chip, unsigned gpio) > +{ > + int bank_id = GPIO_BANK(gpio); > + int bit = GPIO_BIT(gpio); > + void __iomem *reg_base = bcm_kona_gpio.reg_base; > + u32 val, reg_offset; > + unsigned long flags; > + > + spin_lock_irqsave(&bcm_kona_gpio.lock, flags); > + bcm_kona_gpio_unlock_bank(bank_id); > + > + /* determine the GPIO pin direction */ > + val = readl(reg_base + bcm_kona_gpio_control(gpio)); > + val &= GPIO_GPCTR0_IOTR_MASK; > + > + /* read the GPIO bank status */ > + reg_offset = (GPIO_GPCTR0_IOTR_CMD_INPUT == val) ? > + bcm_kona_gpio_in_status(bank_id) : > + bcm_kona_gpio_out_status(bank_id); > + val = readl(reg_base + reg_offset); > + > + bcm_kona_gpio_lock_bank(bank_id); > + spin_unlock_irqrestore(&bcm_kona_gpio.lock, flags); > + > + /* return the specified bit status */ > + return (val >> bit) & 1; > +} > + > +static int bcm_kona_gpio_direction_input(struct gpio_chip *chip, > + unsigned gpio) > +{ > + void __iomem *reg_base = bcm_kona_gpio.reg_base; > + u32 val; > + unsigned long flags; > + int bank_id = GPIO_BANK(gpio); > + > + spin_lock_irqsave(&bcm_kona_gpio.lock, flags); > + bcm_kona_gpio_unlock_bank(bank_id); > + > + val = readl(reg_base + bcm_kona_gpio_control(gpio)); > + val &= ~GPIO_GPCTR0_IOTR_MASK; > + val |= GPIO_GPCTR0_IOTR_CMD_INPUT; > + writel(val, reg_base + bcm_kona_gpio_control(gpio)); > + > + bcm_kona_gpio_lock_bank(bank_id); > + spin_unlock_irqrestore(&bcm_kona_gpio.lock, flags); > + > + return 0; > +} > + > +static int bcm_kona_gpio_direction_output(struct gpio_chip *chip, > + unsigned gpio, int value) > +{ > + void __iomem *reg_base = bcm_kona_gpio.reg_base; > + int bank_id = GPIO_BANK(gpio); > + int bit = GPIO_BIT(gpio); > + u32 val, reg_offset; > + unsigned long flags; > + > + spin_lock_irqsave(&bcm_kona_gpio.lock, flags); > + bcm_kona_gpio_unlock_bank(bank_id); > + > + val = readl(reg_base + bcm_kona_gpio_control(gpio)); > + val &= ~GPIO_GPCTR0_IOTR_MASK; > + val |= GPIO_GPCTR0_IOTR_CMD_0UTPUT; > + writel(val, reg_base + bcm_kona_gpio_control(gpio)); > + reg_offset = value ? bcm_kona_gpio_out_set(bank_id) : > + bcm_kona_gpio_out_clear(bank_id); > + > + val = readl(reg_base + reg_offset); > + val |= 1 << bit; > + writel(val, reg_base + reg_offset); > + > + bcm_kona_gpio_lock_bank(bank_id); > + spin_unlock_irqrestore(&bcm_kona_gpio.lock, flags); > + > + return 0; > +} > + > +static int bcm_kona_gpio_to_irq(struct gpio_chip *chip, unsigned gpio) > +{ > + if (gpio >= bcm_gpio_chip.ngpio) is bcm_gpio_chip != chip ? Globals are even if you have that value passed as argument ? > + return -ENXIO; > + return irq_create_mapping(bcm_kona_gpio.irq_domain, gpio); > +} > + > +static int bcm_kona_gpio_set_debounce(struct gpio_chip *chip, unsigned gpio, > + unsigned debounce) > +{ > + void __iomem *reg_base = bcm_kona_gpio.reg_base; > + u32 val, res; > + unsigned long flags; > + int bank_id = GPIO_BANK(gpio); > + > + /* debounce must be 1-128ms (or 0) */ > + if ((debounce > 0 && debounce < 1000) || debounce > 128000) { > + dev_err(chip->dev, "Debounce value %u not in range\n", > + debounce); > + return -EINVAL; > + } > + > + /* calculate debounce bit value */ > + if (debounce != 0) { > + /* Convert to ms */ > + debounce /= 1000; > + > + /* find the MSB */ > + res = fls(debounce) - 1; > + > + /* Check if MSB-1 is set (round up or down) */ > + if (res > 0 && (debounce & 1 << (res - 1))) > + res++; > + } > + > + /* spin lock for read-modify-write of the GPIO register */ > + spin_lock_irqsave(&bcm_kona_gpio.lock, flags); > + bcm_kona_gpio_unlock_bank(bank_id); > + > + val = readl(reg_base + bcm_kona_gpio_control(gpio)); > + val &= ~GPIO_GPCTR0_DBR_MASK; > + > + if (debounce == 0) { > + /* disable debounce */ > + val &= ~GPIO_GPCTR0_DB_ENABLE_MASK; > + } else { > + val |= GPIO_GPCTR0_DB_ENABLE_MASK | > + (res << GPIO_GPCTR0_DBR_SHIFT); > + } > + > + writel(val, reg_base + bcm_kona_gpio_control(gpio)); > + > + bcm_kona_gpio_lock_bank(bank_id); > + spin_unlock_irqrestore(&bcm_kona_gpio.lock, flags); > + > + return 0; > +} > + > +static int bcm_kona_gpio_request(struct gpio_chip *chip, unsigned gpio) > +{ > + struct irq_data d; > + > + d.hwirq = gpio; > + bcm_kona_gpio_irq_unmask(&d); > + > + return 0; > +} > + > +static void bcm_kona_gpio_free(struct gpio_chip *chip, unsigned gpio) > +{ > + struct irq_data d; > + > + d.hwirq = gpio; > + bcm_kona_gpio_irq_mask(&d); > +} > + > +static struct gpio_chip bcm_gpio_chip = { > + .label = "bcm-kona-gpio", > + .request = bcm_kona_gpio_request, > + .free = bcm_kona_gpio_free, > + .direction_input = bcm_kona_gpio_direction_input, > + .get = bcm_kona_gpio_get, > + .direction_output = bcm_kona_gpio_direction_output, > + .set = bcm_kona_gpio_set, > + .set_debounce = bcm_kona_gpio_set_debounce, > + .to_irq = bcm_kona_gpio_to_irq, > + .base = 0, > +}; > + > +static void bcm_kona_gpio_irq_ack(struct irq_data *d) > +{ > + int gpio = d->hwirq; > + int bank_id = GPIO_BANK(gpio); > + int bit = GPIO_BIT(gpio); > + void __iomem *reg_base = bcm_kona_gpio.reg_base; > + u32 val; > + unsigned long flags; > + > + spin_lock_irqsave(&bcm_kona_gpio.lock, flags); > + bcm_kona_gpio_unlock_bank(bank_id); > + > + val = readl(reg_base + bcm_kona_gpio_int_status(bank_id)); > + val |= 1 << bit; > + writel(val, reg_base + bcm_kona_gpio_int_status(bank_id)); > + > + bcm_kona_gpio_lock_bank(bank_id); > + spin_unlock_irqrestore(&bcm_kona_gpio.lock, flags); > +} > + > +static void bcm_kona_gpio_irq_mask(struct irq_data *d) > +{ > + int gpio = d->hwirq; > + int bank_id = GPIO_BANK(gpio); > + int bit = GPIO_BIT(gpio); > + void __iomem *reg_base = bcm_kona_gpio.reg_base; > + u32 val; > + unsigned long flags; > + > + spin_lock_irqsave(&bcm_kona_gpio.lock, flags); > + bcm_kona_gpio_unlock_bank(bank_id); > + > + val = readl(reg_base + bcm_kona_gpio_int_mask(bank_id)); > + val |= 1 << bit; > + writel(val, reg_base + bcm_kona_gpio_int_mask(bank_id)); > + > + bcm_kona_gpio_lock_bank(bank_id); > + spin_unlock_irqrestore(&bcm_kona_gpio.lock, flags); > +} > + > +static void bcm_kona_gpio_irq_unmask(struct irq_data *d) > +{ > + int gpio = d->hwirq; > + int bank_id = GPIO_BANK(gpio); > + int bit = GPIO_BIT(gpio); > + void __iomem *reg_base = bcm_kona_gpio.reg_base; > + u32 val; > + unsigned long flags; > + > + spin_lock_irqsave(&bcm_kona_gpio.lock, flags); > + bcm_kona_gpio_unlock_bank(bank_id); > + > + val = readl(reg_base + bcm_kona_gpio_int_mskclr(bank_id)); > + val |= 1 << bit; > + writel(val, reg_base + bcm_kona_gpio_int_mskclr(bank_id)); > + > + bcm_kona_gpio_lock_bank(bank_id); > + spin_unlock_irqrestore(&bcm_kona_gpio.lock, flags); > +} > + > +static int bcm_kona_gpio_irq_set_type(struct irq_data *d, unsigned int type) > +{ > + int gpio = d->hwirq; > + void __iomem *reg_base = bcm_kona_gpio.reg_base; irq_*_get_irq_chip_* series of helpers will be useful in these case You have chip, chip_data, handler_data in irq_data to avoid all these global data. > + u32 lvl_type; > + u32 val; > + unsigned long flags; > + int bank_id = GPIO_BANK(gpio); > + > + switch (type & IRQ_TYPE_SENSE_MASK) { > + case IRQ_TYPE_EDGE_RISING: > + lvl_type = GPIO_GPCTR0_ITR_CMD_RISING_EDGE; > + break; > + > + case IRQ_TYPE_EDGE_FALLING: > + lvl_type = GPIO_GPCTR0_ITR_CMD_FALLING_EDGE; > + break; > + > + case IRQ_TYPE_EDGE_BOTH: > + lvl_type = GPIO_GPCTR0_ITR_CMD_BOTH_EDGE; > + break; > + > + case IRQ_TYPE_LEVEL_HIGH: > + case IRQ_TYPE_LEVEL_LOW: > + /* BCM GPIO doesn't support level triggering */ > + default: > + dev_err(bcm_gpio_chip.dev, "Invalid BCM GPIO irq type 0x%x\n", > + type); > + return -EINVAL; > + } > + > + spin_lock_irqsave(&bcm_kona_gpio.lock, flags); > + bcm_kona_gpio_unlock_bank(bank_id); > + > + val = readl(reg_base + bcm_kona_gpio_control(gpio)); > + val &= ~GPIO_GPCTR0_ITR_MASK; > + val |= lvl_type << GPIO_GPCTR0_ITR_SHIFT; > + writel(val, reg_base + bcm_kona_gpio_control(gpio)); > + > + bcm_kona_gpio_lock_bank(bank_id); > + spin_unlock_irqrestore(&bcm_kona_gpio.lock, flags); > + > + return 0; > +} > + > +static void bcm_kona_gpio_irq_handler(unsigned int irq, struct irq_desc *desc) > +{ > + int bit, bank_id; > + void __iomem *reg_base = bcm_kona_gpio.reg_base; > + unsigned long sta; > + struct bcm_kona_gpio_bank *bank = irq_get_handler_data(irq); > + struct irq_chip *chip = irq_desc_get_chip(desc); > + > + chained_irq_enter(chip, desc); > + > + bank_id = bank->id; > + bcm_kona_gpio_unlock_bank(bank_id); > + > + sta = readl(reg_base + bcm_kona_gpio_int_status(bank_id)) & > + (~(readl(reg_base + bcm_kona_gpio_int_mask(bank_id)))); > + > + for_each_set_bit(bit, &sta, 32) { > + /* > + * Clear interrupt before handler is called so we don't > + * miss any interrupt occurred during executing them. > + */ > + writel(readl(reg_base + bcm_kona_gpio_int_status(bank_id)) | > + (1 << bit), > + reg_base + bcm_kona_gpio_int_status(bank_id)); > + > + /* Invoke interrupt handler */ > + generic_handle_irq(gpio_to_irq(GPIO_PER_BANK * bank_id + bit)); > + } > + > + bcm_kona_gpio_lock_bank(bank_id); > + > + chained_irq_exit(chip, desc); > +} > + > +static struct irq_chip bcm_gpio_irq_chip = { > + .name = "bcm-kona-gpio", > + .irq_ack = bcm_kona_gpio_irq_ack, > + .irq_mask = bcm_kona_gpio_irq_mask, > + .irq_unmask = bcm_kona_gpio_irq_unmask, > + .irq_set_type = bcm_kona_gpio_irq_set_type, > +}; > + > +static struct __initconst of_device_id bcm_kona_gpio_of_match[] = { > + { .compatible = "brcm,kona-gpio" }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, kona_gpio_match); > + > +/* > + * This lock class tells lockdep that GPIO irqs are in a different > + * category than their parents, so it won't report false recursion. > + */ > +static struct lock_class_key gpio_lock_class; > + > +static int bcm_kona_gpio_irq_map(struct irq_domain *d, unsigned int virq, > + irq_hw_number_t hwirq) > +{ > + irq_set_chip_data(virq, &d->host_data); > + irq_set_lockdep_class(virq, &gpio_lock_class); > + irq_set_chip_and_handler(virq, &bcm_gpio_irq_chip, handle_simple_irq); > + irq_set_nested_thread(virq, 1); > + set_irq_flags(virq, IRQF_VALID); > + > + return 0; > +} > + > +static void bcm_kona_gpio_irq_unmap(struct irq_domain *d, unsigned int virq) > +{ > + irq_set_chip_and_handler(virq, NULL, NULL); > + irq_set_chip_data(virq, NULL); > +} > + > +static struct irq_domain_ops bcm_kona_irq_ops = { > + .map = bcm_kona_gpio_irq_map, > + .unmap = bcm_kona_gpio_irq_unmap, > + .xlate = irq_domain_xlate_twocell, > +}; > + > +static void bcm_kona_gpio_reset(int num_bank) > +{ > + int i; > + void __iomem *reg_base = bcm_kona_gpio.reg_base; > + > + /* disable interrupts and clear status */ > + for (i = 0; i < num_bank; i++) { > + bcm_kona_gpio_unlock_bank(i); > + writel(0xFFFFFFFF, reg_base + bcm_kona_gpio_int_mask(i)); > + writel(0xFFFFFFFF, reg_base + bcm_kona_gpio_int_status(i)); > + bcm_kona_gpio_lock_bank(i); > + } > +} > + > +static int bcm_kona_gpio_probe(struct platform_device *pdev) > +{ > + const struct of_device_id *match; > + struct resource *res; > + struct bcm_kona_gpio_bank *bank; > + int ret; > + int i; > + > + match = of_match_device(bcm_kona_gpio_of_match, &pdev->dev); > + if (!match) { > + dev_err(&pdev->dev, "Failed to find gpio controller\n"); > + return -ENODEV; > + } > + > + bcm_kona_gpio.num_bank = bcm_kona_count_irq_resources(pdev); > + if (bcm_kona_gpio.num_bank == 0) { > + dev_err(&pdev->dev, "Couldn't determine # GPIO banks\n"); > + return -ENOENT; > + } > + bcm_gpio_chip.of_node = pdev->dev.of_node; > + bcm_gpio_chip.dev = &pdev->dev; > + bcm_gpio_chip.ngpio = bcm_kona_gpio.num_bank * GPIO_PER_BANK; > + bcm_kona_gpio.banks = devm_kzalloc(&pdev->dev, > + bcm_kona_gpio.num_bank * > + sizeof(*bcm_kona_gpio.banks), > + GFP_KERNEL); > + if (!bcm_kona_gpio.banks) { > + dev_err(&pdev->dev, "Couldn't allocate bank structure\n"); > + return -ENOMEM; > + } > + > + bcm_kona_gpio.irq_base = irq_alloc_descs(-1, 0, bcm_gpio_chip.ngpio, > + 0); > + if (bcm_kona_gpio.irq_base < 0) { > + dev_err(&pdev->dev, "Couldn't allocate IRQ numbers\n"); > + return -ENXIO; > + } > + bcm_kona_gpio.irq_domain = irq_domain_add_linear(pdev->dev.of_node, > + bcm_gpio_chip.ngpio, > + &bcm_kona_irq_ops, > + &bcm_kona_gpio); > + if (!bcm_kona_gpio.irq_domain) { > + dev_err(&pdev->dev, "Couldn't allocate IRQ domain\n"); > + ret = -ENXIO; > + goto err_irq_descs; > + } > + for (i = 0; i < bcm_kona_gpio.num_bank; i++) { > + bank = &bcm_kona_gpio.banks[i]; > + bank->id = i; > + bank->irq = platform_get_irq(pdev, i); > + if (bank->irq < 0) { > + dev_err(&pdev->dev, "Couldn't get IRQ for bank %d", i); > + ret = -ENOENT; > + goto err_irq_domain; > + } > + } > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) { > + dev_err(&pdev->dev, "Missing MEM resource\n"); > + ret = -ENXIO; > + goto err_irq_domain; > + } > + > + bcm_kona_gpio.reg_base = devm_request_and_ioremap(&pdev->dev, res); > + if (!bcm_kona_gpio.reg_base) { > + dev_err(&pdev->dev, "Couldn't ioremap regs\n"); > + ret = -ENXIO; > + goto err_irq_domain; > + } > + dev_info(&pdev->dev, "Setting up Kona GPIO at 0x%p (phys %#x)\n", > + bcm_kona_gpio.reg_base, res->start); > + > + bcm_kona_gpio_reset(bcm_kona_gpio.num_bank); > + > + ret = gpiochip_add(&bcm_gpio_chip); > + if (ret < 0) { > + dev_err(&pdev->dev, "Couldn't add GPIO chip -- %d\n", ret); > + goto err_irq_domain; > + } > + for (i = 0; i < bcm_gpio_chip.ngpio; i++) { > + int irq = bcm_kona_gpio_to_irq(&bcm_gpio_chip, i); > + bank = &bcm_kona_gpio.banks[GPIO_BANK(i)]; > + irq_set_lockdep_class(irq, &gpio_lock_class); > + irq_set_chip_data(irq, bank); > + irq_set_chip_and_handler(irq, &bcm_gpio_irq_chip, > + handle_simple_irq); > + set_irq_flags(irq, IRQF_VALID); > + } > + for (i = 0; i < bcm_kona_gpio.num_bank; i++) { > + bank = &bcm_kona_gpio.banks[i]; > + > + irq_set_chained_handler(bank->irq, bcm_kona_gpio_irq_handler); > + irq_set_handler_data(bank->irq, bank); > + } > + > + spin_lock_init(&bcm_kona_gpio.lock); > + platform_set_drvdata to set it as bcm_kona_gpio so that you need not access the global pointer anywhere else. In fact it can removed I think. > + return 0; > + > +err_irq_domain: > + irq_domain_remove(bcm_kona_gpio.irq_domain); > + > +err_irq_descs: > + irq_free_descs(bcm_kona_gpio.irq_base, bcm_gpio_chip.ngpio); > + > + return ret; > +} > + > +static int bcm_kona_gpio_remove(struct platform_device *pdev) > +{ > + int i, ret; > + struct bcm_kona_gpio_bank *bank; > + > + for (i = 0; i < bcm_gpio_chip.ngpio; i++) may be bcm_kona_gpio as driver data will help here ? > + gpio_free(i); > + ret = gpiochip_remove(&bcm_gpio_chip); > + if (ret < 0) { > + dev_err(&pdev->dev, "Couldn't remove GPIO chip -- %d\n", ret); > + return ret; > + } > + for (i = 0; i < bcm_kona_gpio.num_bank; i++) { > + bank = &bcm_kona_gpio.banks[i]; > + irq_set_chained_handler(bank->irq, NULL); > + irq_set_handler_data(bank->irq, NULL); > + } > + irq_domain_remove(bcm_kona_gpio.irq_domain); > + irq_free_descs(bcm_kona_gpio.irq_base, bcm_gpio_chip.ngpio); > + > + return 0; > +} > + > +static struct platform_driver bcm_kona_gpio_driver = { > + .driver = { > + .name = "bcm-kona-gpio", > + .owner = THIS_MODULE, > + .of_match_table = bcm_kona_gpio_of_match, > + }, > + .probe = bcm_kona_gpio_probe, > + .remove = bcm_kona_gpio_remove, > +}; > + > +module_platform_driver(bcm_kona_gpio_driver); > + > +MODULE_AUTHOR("Broadcom"); > +MODULE_DESCRIPTION("Broadcom Kona GPIO Driver"); > +MODULE_LICENSE("GPL v2"); IMO, you don't require any global data or pointers if probe populates all the pdev, irq/gpio_chip data member appropriately Regards, Sudeep