| Message ID | 1376938761-13657-2-git-send-email-markus.mayer@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hello Markus, Le lundi 19 août 2013 11:59:19 Markus Mayer a écrit : > From: Markus Mayer <mmayer@broadcom.com> > > This patch adds the GPIO driver for the bcm281xx family of chips. > > Signed-off-by: Markus Mayer <markus.mayer@linaro.org> > Reviewed-by: Christian Daudt <csd@broadcom.com> > Reviewed-by: Tim Kryger <tim.kryger@linaro.org> > Reviewed-by: Matt Porter <matt.porter@linaro.org> > --- > .../devicetree/bindings/gpio/gpio-bcm-kona.txt | 43 ++ Since the vendor prefix has been changed from brcm to bcm, should that also be reflected in the Device Tree binding documentation file?
On 19 August 2013 12:02, Florian Fainelli <f.fainelli@gmail.com> wrote: > Hello Markus, > > Le lundi 19 août 2013 11:59:19 Markus Mayer a écrit : >> From: Markus Mayer <mmayer@broadcom.com> >> >> This patch adds the GPIO driver for the bcm281xx family of chips. >> >> Signed-off-by: Markus Mayer <markus.mayer@linaro.org> >> Reviewed-by: Christian Daudt <csd@broadcom.com> >> Reviewed-by: Tim Kryger <tim.kryger@linaro.org> >> Reviewed-by: Matt Porter <matt.porter@linaro.org> >> --- >> .../devicetree/bindings/gpio/gpio-bcm-kona.txt | 43 ++ > > Since the vendor prefix has been changed from brcm to bcm, should that also be > reflected in the Device Tree binding documentation file? Actually, it is the other way around. "brcm," was there first, but we overlooked that and started upstreaming code with the "bcm," vendor prefix. There are various patches floating around out there to change our use of "bcm," to "brcm," to make it conform to the established vendor prefixes. Regards, -Markus
Le lundi 19 août 2013 12:05:55 Markus Mayer a écrit : > On 19 August 2013 12:02, Florian Fainelli <f.fainelli@gmail.com> wrote: > > Hello Markus, > > > > Le lundi 19 août 2013 11:59:19 Markus Mayer a écrit : > >> From: Markus Mayer <mmayer@broadcom.com> > >> > >> This patch adds the GPIO driver for the bcm281xx family of chips. > >> > >> Signed-off-by: Markus Mayer <markus.mayer@linaro.org> > >> Reviewed-by: Christian Daudt <csd@broadcom.com> > >> Reviewed-by: Tim Kryger <tim.kryger@linaro.org> > >> Reviewed-by: Matt Porter <matt.porter@linaro.org> > >> --- > >> > >> .../devicetree/bindings/gpio/gpio-bcm-kona.txt | 43 ++ > > > > Since the vendor prefix has been changed from brcm to bcm, should that > > also be reflected in the Device Tree binding documentation file? > > Actually, it is the other way around. "brcm," was there first, but we > overlooked that and started upstreaming code with the "bcm," vendor > prefix. There are various patches floating around out there to change > our use of "bcm," to "brcm," to make it conform to the established > vendor prefixes. Ermm yes, I realized that after hitting send. Netherless, should this be renamed to "gpio-brcm-kona.txt" directly such that the patches you mention do not have to get updated to patch this one too?
On 19 August 2013 12:11, Florian Fainelli <f.fainelli@gmail.com> wrote: > Le lundi 19 août 2013 12:05:55 Markus Mayer a écrit : >> On 19 August 2013 12:02, Florian Fainelli <f.fainelli@gmail.com> wrote: >> > Hello Markus, >> > >> > Le lundi 19 août 2013 11:59:19 Markus Mayer a écrit : >> >> From: Markus Mayer <mmayer@broadcom.com> >> >> >> >> This patch adds the GPIO driver for the bcm281xx family of chips. >> >> >> >> Signed-off-by: Markus Mayer <markus.mayer@linaro.org> >> >> Reviewed-by: Christian Daudt <csd@broadcom.com> >> >> Reviewed-by: Tim Kryger <tim.kryger@linaro.org> >> >> Reviewed-by: Matt Porter <matt.porter@linaro.org> >> >> --- >> >> >> >> .../devicetree/bindings/gpio/gpio-bcm-kona.txt | 43 ++ >> > >> > Since the vendor prefix has been changed from brcm to bcm, should that >> > also be reflected in the Device Tree binding documentation file? >> >> Actually, it is the other way around. "brcm," was there first, but we >> overlooked that and started upstreaming code with the "bcm," vendor >> prefix. There are various patches floating around out there to change >> our use of "bcm," to "brcm," to make it conform to the established >> vendor prefixes. > > Ermm yes, I realized that after hitting send. Netherless, should this be > renamed to "gpio-brcm-kona.txt" directly such that the patches you mention do > not have to get updated to patch this one too? After long discussions on how to handle the "bcm" vs. "brcm" situation, we said we would be renaming the "bcm," vendor prefixes to "brcm,", but would otherwise refrain from renaming files, variables, function names, etc. except if the file name contains the vendor prefix itself. The thing is that Broadcom is exclusively using the BCM abbreviation internally. Except for our stock ticker, nothing is named "BRCM". :-) So, the less "brcm" is out there, the better it is. Unfortunately, it's too late for the vendor prefix. I can see how a file called gpio-bcm-kona.txt talking about the "brcm," could be a bit confusing. It's still straight forward enough to find out that the definition of "brcm,kona-gpio" is in the gpio-bcm-kona.txt file. And the boundary of what's called "brcm" is still firm: just the vendor prefix. But if we renamed it to gpio-brcm-kona.txt, then it would be confusing why there are not other files named *brcm* and the boundary of what is named how would become quite blurry. Regards, -Markus
Le lundi 19 août 2013 12:23:47 Markus Mayer a écrit : > On 19 August 2013 12:11, Florian Fainelli <f.fainelli@gmail.com> wrote: > > Le lundi 19 août 2013 12:05:55 Markus Mayer a écrit : > >> On 19 August 2013 12:02, Florian Fainelli <f.fainelli@gmail.com> wrote: > >> > Hello Markus, > >> > > >> > Le lundi 19 août 2013 11:59:19 Markus Mayer a écrit : > >> >> From: Markus Mayer <mmayer@broadcom.com> > >> >> > >> >> This patch adds the GPIO driver for the bcm281xx family of chips. > >> >> > >> >> Signed-off-by: Markus Mayer <markus.mayer@linaro.org> > >> >> Reviewed-by: Christian Daudt <csd@broadcom.com> > >> >> Reviewed-by: Tim Kryger <tim.kryger@linaro.org> > >> >> Reviewed-by: Matt Porter <matt.porter@linaro.org> > >> >> --- > >> >> > >> >> .../devicetree/bindings/gpio/gpio-bcm-kona.txt | 43 ++ > >> > > >> > Since the vendor prefix has been changed from brcm to bcm, should that > >> > also be reflected in the Device Tree binding documentation file? > >> > >> Actually, it is the other way around. "brcm," was there first, but we > >> overlooked that and started upstreaming code with the "bcm," vendor > >> prefix. There are various patches floating around out there to change > >> our use of "bcm," to "brcm," to make it conform to the established > >> vendor prefixes. > > > > Ermm yes, I realized that after hitting send. Netherless, should this be > > renamed to "gpio-brcm-kona.txt" directly such that the patches you mention > > do not have to get updated to patch this one too? > > After long discussions on how to handle the "bcm" vs. "brcm" > situation, we said we would be renaming the "bcm," vendor prefixes to > "brcm,", but would otherwise refrain from renaming files, variables, > function names, etc. except if the file name contains the vendor > prefix itself. > > The thing is that Broadcom is exclusively using the BCM abbreviation > internally. Except for our stock ticker, nothing is named "BRCM". :-) > So, the less "brcm" is out there, the better it is. Unfortunately, > it's too late for the vendor prefix. > > I can see how a file called gpio-bcm-kona.txt talking about the > "brcm," could be a bit confusing. It's still straight forward enough > to find out that the definition of "brcm,kona-gpio" is in the > gpio-bcm-kona.txt file. And the boundary of what's called "brcm" is > still firm: just the vendor prefix. > > But if we renamed it to gpio-brcm-kona.txt, then it would be confusing > why there are not other files named *brcm* and the boundary of what is > named how would become quite blurry. I am happy with that explanation, thanks! That works for me too.
Hi Markus, Please see my comments inline. On Monday 19 of August 2013 11:59:19 Markus Mayer wrote: > From: Markus Mayer <mmayer@broadcom.com> > > This patch adds the GPIO driver for the bcm281xx family of chips. > > Signed-off-by: Markus Mayer <markus.mayer@linaro.org> > Reviewed-by: Christian Daudt <csd@broadcom.com> > Reviewed-by: Tim Kryger <tim.kryger@linaro.org> > Reviewed-by: Matt Porter <matt.porter@linaro.org> > --- > .../devicetree/bindings/gpio/gpio-bcm-kona.txt | 43 ++ > drivers/gpio/gpio-bcm-kona.c | 658 > ++++++++++++++++++++ 2 files changed, 701 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/gpio/gpio-bcm-kona.txt create mode > 100644 drivers/gpio/gpio-bcm-kona.c > > diff --git a/Documentation/devicetree/bindings/gpio/gpio-bcm-kona.txt > b/Documentation/devicetree/bindings/gpio/gpio-bcm-kona.txt new file > mode 100644 > index 0000000..70b2faf > --- /dev/null > +++ b/Documentation/devicetree/bindings/gpio/gpio-bcm-kona.txt > @@ -0,0 +1,43 @@ > +Broadcom Kona Family GPIO > +------------------------- > + > +This GPIO driver is used in the following Broadcom SoCs: > + BCM11130, BCM11140, BCM11351, BCM28145, BCM28155 > + > +The GPIO controller only supports edge, not level, triggering of > interrupts. + It should be noted that following properties are required, like it is done in most of binding descriptions, for example by adding Required properties: before the following list of properties. > +- compatible: "brcm,kona-gpio" > +- reg: Physical base address and length of the controller's registers. > +- interrupts: The interrupt outputs from the controller. There is one > GPIO + interrupt per GPIO bank. The number of GPIO banks is HW > configurable. What do you mean by HW configurable? Does it mean that it depends from SoC or you can have different bank configurations on the same SoC? How does the driver know the layout of GPIO banks? The code suggests that the number of interrupts is used as bank count. While this saves you from adding any new property or any other way of getting this information, I'm not sure if it's completely correct. I'd like to hear others' opinion on this, though. > The + number of interrupts listed depends on the number > of GPIO banks on the + SoC. The interrupts must be ordered by bank, > starting with bank 0. +- #gpio-cells: Should be <2>. The first cell is > the pin number, the second + cell is used to specify optional > parameters: > + - bit 0 specifies polarity (0 for normal, 1 for inverted) Maybe some reference to generic GPIO flags could be nice here? > +- #interrupt-cells: Should be <2>. The first cell is the GPIO number. > + The second cell is used to specify flags: > + - trigger type (bits[1:0]): > + 1 = low-to-high edge triggered. > + 2 = high-to-low edge triggered. > + 3 = low-to-high or high-to-low edge triggered This seems to be a subset of generic interrupt flags specified in bindings/interrupt-controller/interrupts.txt. Maybe this should be mentioned here? > + Valid values are 1, 2, 3 > +- gpio-controller: Marks the device node as a GPIO controller. > +- interrupt-controller: Marks the device node as an interrupt > controller. + > +Example: > + gpio: gpio@35003000 { > + compatible = "brcm,bcm11351-gpio", "brcm,kona-gpio"; > + reg = <0x35003000 0x800>; > + interrupts = > + <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH > + GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH > + GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH > + GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH > + GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH > + GIC_SPI 111 IRQ_TYPE_LEVEL_HIGH>; > + #gpio-cells = <2>; > + #interrupt-cells = <2>; > + gpio-controller; > + interrupt-controller; > + }; > diff --git a/drivers/gpio/gpio-bcm-kona.c b/drivers/gpio/gpio-bcm-kona.c > new file mode 100644 > index 0000000..9715b06 > --- /dev/null > +++ b/drivers/gpio/gpio-bcm-kona.c > @@ -0,0 +1,658 @@ > +/* > + * 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 <linux/io.h> > +#include <linux/gpio.h> > +#include <linux/of_device.h> > +#include <linux/module.h> > +#include <linux/irqdomain.h> > +#include <linux/irqchip/chained_irq.h> > + > +#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_OUT_STATUS(bank) (0x00000000 + ((bank) << 2)) > +#define GPIO_IN_STATUS(bank) (0x00000020 + ((bank) << 2)) > +#define GPIO_OUT_SET(bank) (0x00000040 + ((bank) << 2)) > +#define GPIO_OUT_CLEAR(bank) (0x00000060 + ((bank) << 2)) > +#define GPIO_INT_STATUS(bank) (0x00000080 + ((bank) << 2)) > +#define GPIO_INT_MASK(bank) (0x000000A0 + ((bank) << 2)) nit: Hex numbers should be lowercase. > +#define GPIO_INT_MSKCLR(bank) (0x000000C0 + ((bank) << 2)) Ditto. > +#define GPIO_CONTROL(bank) (0x00000100 + ((bank) << 2)) > +#define GPIO_PWD_STATUS(bank) (0x00000500 + ((bank) << 2)) > + > +#define GPIO_GPPWR_OFFSET 0x00000520 > + > +#define GPIO_GPCTR0_DBR_SHIFT 5 > +#define GPIO_GPCTR0_DBR_MASK 0x000001E0 Ditto. > + > +#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 Ditto. > +#define UNLOCK_CODE 0x00000000 > + > +struct bcm_kona_gpio { > + void __iomem *reg_base; > + int num_bank; > + int irq_base; > + spinlock_t lock; > + struct gpio_chip gpio_chip; > + struct irq_domain *irq_domain; > + struct bcm_kona_gpio_bank *banks; > + struct platform_device *pdev; > +}; > + > +struct bcm_kona_gpio_bank { > + int id; > + int irq; > + /* Used in the interrupt handler */ > + void __iomem *reg_base; > +}; > + > +static inline struct bcm_kona_gpio *to_kona_gpio(struct gpio_chip > *chip) +{ > + return container_of(chip, struct bcm_kona_gpio, gpio_chip); > +} > + > +/* This function counts IRQ entries in the device tree */ > +static int bcm_kona_count_irq_resources(struct platform_device *pdev) > +{ > + int i, ret; > + > + for (i = 0;; i++) { > + ret = platform_get_irq(pdev, i); > + if (ret < 0) > + break; > + } > + return i; > +} Well, if this driver supports only device tree, you can use of_count_irq() instead of this function. > + > +static void bcm_kona_gpio_set_lockcode_bank(void __iomem *reg_base, int > bank_id, + int lockcode) > +{ > + writel(BCM_GPIO_PASSWD, reg_base + GPIO_GPPWR_OFFSET); > + writel(lockcode, reg_base + GPIO_PWD_STATUS(bank_id)); > +} > + > +static inline void bcm_kona_gpio_lock_bank(void __iomem *reg_base, int > bank_id) +{ > + bcm_kona_gpio_set_lockcode_bank(reg_base, bank_id, LOCK_CODE); > +} > + > +static inline void bcm_kona_gpio_unlock_bank(void __iomem *reg_base, > + int bank_id) > +{ > + bcm_kona_gpio_set_lockcode_bank(reg_base, bank_id, UNLOCK_CODE); > +} > + > +static void bcm_kona_gpio_set(struct gpio_chip *chip, unsigned gpio, > + int value) > +{ > + struct bcm_kona_gpio *kona_gpio; > + void __iomem *reg_base; > + int bank_id = GPIO_BANK(gpio); > + int bit = GPIO_BIT(gpio); > + u32 val, reg_offset; > + unsigned long flags; > + > + kona_gpio = to_kona_gpio(chip); > + reg_base = kona_gpio->reg_base; > + spin_lock_irqsave(&kona_gpio->lock, flags); > + bcm_kona_gpio_unlock_bank(reg_base, bank_id); > + > + /* determine the GPIO pin direction */ > + val = readl(reg_base + 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 ? GPIO_OUT_SET(bank_id) : 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(reg_base, bank_id); > + spin_unlock_irqrestore(&kona_gpio->lock, flags); > +} > + > +static int bcm_kona_gpio_get(struct gpio_chip *chip, unsigned gpio) > +{ > + struct bcm_kona_gpio *kona_gpio; > + void __iomem *reg_base; > + int bank_id = GPIO_BANK(gpio); > + int bit = GPIO_BIT(gpio); > + u32 val, reg_offset; > + unsigned long flags; > + > + kona_gpio = to_kona_gpio(chip); > + reg_base = kona_gpio->reg_base; > + spin_lock_irqsave(&kona_gpio->lock, flags); > + bcm_kona_gpio_unlock_bank(reg_base, bank_id); > + > + /* determine the GPIO pin direction */ > + val = readl(reg_base + GPIO_CONTROL(gpio)); > + val &= GPIO_GPCTR0_IOTR_MASK; > + > + /* read the GPIO bank status */ > + reg_offset = (GPIO_GPCTR0_IOTR_CMD_INPUT == val) ? > + GPIO_IN_STATUS(bank_id) : > + GPIO_OUT_STATUS(bank_id); > + val = readl(reg_base + reg_offset); > + > + bcm_kona_gpio_lock_bank(reg_base, bank_id); > + spin_unlock_irqrestore(&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) > +{ > + struct bcm_kona_gpio *kona_gpio; > + void __iomem *reg_base; > + u32 val; > + unsigned long flags; > + int bank_id = GPIO_BANK(gpio); > + > + kona_gpio = to_kona_gpio(chip); > + reg_base = kona_gpio->reg_base; > + spin_lock_irqsave(&kona_gpio->lock, flags); > + bcm_kona_gpio_unlock_bank(reg_base, bank_id); > + > + val = readl(reg_base + GPIO_CONTROL(gpio)); > + val &= ~GPIO_GPCTR0_IOTR_MASK; > + val |= GPIO_GPCTR0_IOTR_CMD_INPUT; > + writel(val, reg_base + GPIO_CONTROL(gpio)); > + > + bcm_kona_gpio_lock_bank(reg_base, bank_id); > + spin_unlock_irqrestore(&kona_gpio->lock, flags); > + > + return 0; > +} > + > +static int bcm_kona_gpio_direction_output(struct gpio_chip *chip, > + unsigned gpio, int value) > +{ > + struct bcm_kona_gpio *kona_gpio; > + void __iomem *reg_base; > + int bank_id = GPIO_BANK(gpio); > + int bit = GPIO_BIT(gpio); > + u32 val, reg_offset; > + unsigned long flags; > + > + kona_gpio = to_kona_gpio(chip); > + reg_base = kona_gpio->reg_base; > + spin_lock_irqsave(&kona_gpio->lock, flags); > + bcm_kona_gpio_unlock_bank(reg_base, bank_id); > + > + val = readl(reg_base + GPIO_CONTROL(gpio)); > + val &= ~GPIO_GPCTR0_IOTR_MASK; > + val |= GPIO_GPCTR0_IOTR_CMD_0UTPUT; > + writel(val, reg_base + GPIO_CONTROL(gpio)); > + reg_offset = value ? GPIO_OUT_SET(bank_id) : 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(reg_base, bank_id); > + spin_unlock_irqrestore(&kona_gpio->lock, flags); > + > + return 0; > +} > + > +static int bcm_kona_gpio_to_irq(struct gpio_chip *chip, unsigned gpio) > +{ > + struct bcm_kona_gpio *kona_gpio; > + > + kona_gpio = to_kona_gpio(chip); > + if (gpio >= kona_gpio->gpio_chip.ngpio) > + return -ENXIO; > + return irq_create_mapping(kona_gpio->irq_domain, gpio); > +} > + > +static int bcm_kona_gpio_set_debounce(struct gpio_chip *chip, unsigned > gpio, + unsigned debounce) > +{ > + struct bcm_kona_gpio *kona_gpio; > + void __iomem *reg_base; > + u32 val, res; > + unsigned long flags; > + int bank_id = GPIO_BANK(gpio); > + > + kona_gpio = to_kona_gpio(chip); > + reg_base = kona_gpio->reg_base; > + /* 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(&kona_gpio->lock, flags); > + bcm_kona_gpio_unlock_bank(reg_base, bank_id); > + > + val = readl(reg_base + 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 + GPIO_CONTROL(gpio)); > + > + bcm_kona_gpio_lock_bank(reg_base, bank_id); > + spin_unlock_irqrestore(&kona_gpio->lock, flags); > + > + return 0; > +} > + > +static struct gpio_chip template_chip = { > + .label = "bcm-kona-gpio", > + .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) > +{ > + struct bcm_kona_gpio *kona_gpio; > + void __iomem *reg_base; > + int gpio = d->hwirq; > + int bank_id = GPIO_BANK(gpio); > + int bit = GPIO_BIT(gpio); > + u32 val; > + unsigned long flags; > + > + kona_gpio = irq_data_get_irq_chip_data(d); > + reg_base = kona_gpio->reg_base; > + spin_lock_irqsave(&kona_gpio->lock, flags); > + bcm_kona_gpio_unlock_bank(reg_base, bank_id); > + > + val = readl(reg_base + GPIO_INT_STATUS(bank_id)); > + val |= 1 << bit; > + writel(val, reg_base + GPIO_INT_STATUS(bank_id)); > + > + bcm_kona_gpio_lock_bank(reg_base, bank_id); > + spin_unlock_irqrestore(&kona_gpio->lock, flags); > +} > + > +static void bcm_kona_gpio_irq_mask(struct irq_data *d) > +{ > + struct bcm_kona_gpio *kona_gpio; > + void __iomem *reg_base; > + int gpio = d->hwirq; > + int bank_id = GPIO_BANK(gpio); > + int bit = GPIO_BIT(gpio); > + u32 val; > + unsigned long flags; > + > + kona_gpio = irq_data_get_irq_chip_data(d); > + reg_base = kona_gpio->reg_base; > + spin_lock_irqsave(&kona_gpio->lock, flags); > + bcm_kona_gpio_unlock_bank(reg_base, bank_id); > + > + val = readl(reg_base + GPIO_INT_MASK(bank_id)); > + val |= 1 << bit; > + writel(val, reg_base + GPIO_INT_MASK(bank_id)); > + > + bcm_kona_gpio_lock_bank(reg_base, bank_id); > + spin_unlock_irqrestore(&kona_gpio->lock, flags); > +} > + > +static void bcm_kona_gpio_irq_unmask(struct irq_data *d) > +{ > + struct bcm_kona_gpio *kona_gpio; > + void __iomem *reg_base; > + int gpio = d->hwirq; > + int bank_id = GPIO_BANK(gpio); > + int bit = GPIO_BIT(gpio); > + u32 val; > + unsigned long flags; > + > + kona_gpio = irq_data_get_irq_chip_data(d); > + reg_base = kona_gpio->reg_base; > + spin_lock_irqsave(&kona_gpio->lock, flags); > + bcm_kona_gpio_unlock_bank(reg_base, bank_id); > + > + val = readl(reg_base + GPIO_INT_MSKCLR(bank_id)); > + val |= 1 << bit; > + writel(val, reg_base + GPIO_INT_MSKCLR(bank_id)); > + > + bcm_kona_gpio_lock_bank(reg_base, bank_id); > + spin_unlock_irqrestore(&kona_gpio->lock, flags); > +} > + > +static int bcm_kona_gpio_irq_set_type(struct irq_data *d, unsigned int > type) +{ > + struct bcm_kona_gpio *kona_gpio; > + void __iomem *reg_base; > + int gpio = d->hwirq; > + u32 lvl_type; > + u32 val; > + unsigned long flags; > + int bank_id = GPIO_BANK(gpio); > + > + kona_gpio = irq_data_get_irq_chip_data(d); > + reg_base = kona_gpio->reg_base; > + 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(kona_gpio->gpio_chip.dev, > + "Invalid BCM GPIO irq type 0x%x\n", type); > + return -EINVAL; > + } > + > + spin_lock_irqsave(&kona_gpio->lock, flags); > + bcm_kona_gpio_unlock_bank(reg_base, bank_id); > + > + val = readl(reg_base + GPIO_CONTROL(gpio)); > + val &= ~GPIO_GPCTR0_ITR_MASK; > + val |= lvl_type << GPIO_GPCTR0_ITR_SHIFT; > + writel(val, reg_base + GPIO_CONTROL(gpio)); > + > + bcm_kona_gpio_lock_bank(reg_base, bank_id); > + spin_unlock_irqrestore(&kona_gpio->lock, flags); > + > + return 0; > +} > + > +static void bcm_kona_gpio_irq_handler(unsigned int irq, struct irq_desc > *desc) +{ > + void __iomem *reg_base; > + int bit, bank_id; > + 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); > + > + /* > + * For bank interrupts, we can't use chip_data to store the kona_gpio > + * pointer, since GIC needs it for its own purposes. Therefore, we get > + * our pointer from the bank structure. > + */ > + reg_base = bank->reg_base; > + bank_id = bank->id; > + bcm_kona_gpio_unlock_bank(reg_base, bank_id); > + > + sta = readl(reg_base + GPIO_INT_STATUS(bank_id)) & > + (~(readl(reg_base + 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 + GPIO_INT_STATUS(bank_id)) | > + (1 << bit), > + reg_base + GPIO_INT_STATUS(bank_id)); > + /* Invoke interrupt handler */ > + generic_handle_irq(gpio_to_irq(GPIO_PER_BANK * bank_id + bit)); Calling the full gpio_to_irq() here in interrupt handler looks like an overkill for me. If you have 1:1 GPIO to interrupt mapping (and irq_chip ops above suggest so) then you could simply call irq_find_mapping() with your IRQ domain and GPIO number as the hwirq argument. You would have to somehow retrieve the IRQ domain pointer, though. (Possibly adding it to bcm_kona_gpio_bank struct wouldn't be too bad idea.) > + } > + > + bcm_kona_gpio_lock_bank(reg_base, 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, bcm_kona_gpio_of_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) > +{ > + int ret; > + > + ret = irq_set_chip_data(virq, d->host_data); > + if (ret < 0) > + return ret; > + 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(struct bcm_kona_gpio *kona_gpio) > +{ > + void __iomem *reg_base; > + int i; > + > + reg_base = kona_gpio->reg_base; > + /* disable interrupts and clear status */ > + for (i = 0; i < kona_gpio->num_bank; i++) { > + bcm_kona_gpio_unlock_bank(reg_base, i); > + writel(0xFFFFFFFF, reg_base + GPIO_INT_MASK(i)); > + writel(0xFFFFFFFF, reg_base + GPIO_INT_STATUS(i)); > + bcm_kona_gpio_lock_bank(reg_base, i); > + } > +} > + > +static int bcm_kona_gpio_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + const struct of_device_id *match; > + struct resource *res; > + struct bcm_kona_gpio_bank *bank; > + struct bcm_kona_gpio *kona_gpio; > + struct gpio_chip *chip; > + int ret; > + int i; > + > + match = of_match_device(bcm_kona_gpio_of_match, dev); > + if (!match) { > + dev_err(dev, "Failed to find gpio controller\n"); > + return -ENODEV; > + } > + > + kona_gpio = devm_kzalloc(dev, sizeof(*kona_gpio), GFP_KERNEL); > + if (!kona_gpio) { > + dev_err(dev, "Failed to allocate memory for GPIO device"); nit: devm_kzalloc() already prints an error message if allocation fails. > + return -ENOMEM; > + } > + kona_gpio->gpio_chip = template_chip; > + chip = &kona_gpio->gpio_chip; > + kona_gpio->num_bank = bcm_kona_count_irq_resources(pdev); of_irq_count() could be used here, if the driver is supposed to be DT-only, but as I said before, I'm not sure if this is the right way to get bank count from DT. > + if (kona_gpio->num_bank == 0) { > + dev_err(dev, "Couldn't determine # GPIO banks\n"); > + return -ENOENT; > + } > + kona_gpio->banks = devm_kzalloc(dev, > + kona_gpio->num_bank * > + sizeof(*kona_gpio->banks), > + GFP_KERNEL); > + if (!kona_gpio->banks) { > + dev_err(dev, "Couldn't allocate bank structure\n"); nit: devm_kzalloc() already prints an error on failure. > + return -ENOMEM; > + } > + > + kona_gpio->pdev = pdev; > + platform_set_drvdata(pdev, kona_gpio); > + chip->of_node = dev->of_node; > + chip->ngpio = kona_gpio->num_bank * GPIO_PER_BANK; > + > + kona_gpio->irq_base = irq_alloc_descs(-1, 0, chip->ngpio, 0); Why do you need a static base here? Anyway, it doesn't seem to be used anywhere in this driver, so I guess it might some kind of leftover. > + if (kona_gpio->irq_base < 0) { > + dev_err(dev, "Couldn't allocate IRQ numbers\n"); > + return -ENXIO; > + } > + > + kona_gpio->irq_domain = irq_domain_add_linear(dev->of_node, > + chip->ngpio, > + &bcm_kona_irq_ops, > + kona_gpio); > + if (!kona_gpio->irq_domain) { > + dev_err(dev, "Couldn't allocate IRQ domain\n"); > + ret = -ENXIO; > + goto err_irq_descs; > + } > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) { > + dev_err(dev, "Missing MEM resource\n"); > + ret = -ENXIO; > + goto err_irq_domain; > + } You don't need to check the return value of platform_get_resource(), because devm_ioremap_resource() already does it. > + > + kona_gpio->reg_base = devm_request_and_ioremap(dev, res); devm_request_and_ioremap() is deprecated. devm_ioremap_resource() should be used in new code (remembering that it returns ERR_PTR() on errors, not NULL, like the old one). > + if (!kona_gpio->reg_base) { > + dev_err(dev, "Couldn't ioremap regs\n"); nit: devm_ioremap_resource() already prints an error on failure. Best regards, Tomasz
On 08/19/2013 12:59 PM, Markus Mayer wrote: > This patch adds the GPIO driver for the bcm281xx family of chips. > diff --git a/Documentation/devicetree/bindings/gpio/gpio-bcm-kona.txt b/Documentation/devicetree/bindings/gpio/gpio-bcm-kona.txt The simple way to avoid the discussion re: the name of the file might have been to name it exactly matching the compatible value it documents, i.e. brcm,kona-gpio.txt. That's what I personally prefer, but it's not mandatory in any way at all. > +Broadcom Kona Family GPIO > +------------------------- > + > +This GPIO driver is used in the following Broadcom SoCs: > + BCM11130, BCM11140, BCM11351, BCM28145, BCM28155 > + > +The GPIO controller only supports edge, not level, triggering of interrupts. > + > +- compatible: "brcm,kona-gpio" Per recent discussions, all the supported compatible values should be spelled out here, which I assume are: brcm,kona-gpio brcm,bcm11130 brcm,bcm11140 brcm,bcm11351 brcm,bcm28145 brcm,bcm28155 Documenting all of them allows drivers to implement quirks/workarounds based on individual SoCs if ever needed. Aside from that, the binding seems reasonable, so the binding, Acked-by: Stephen Warren <swarren@nvidia.com>
On 08/19/2013 04:47 PM, Tomasz Figa wrote: > Hi Markus, > > Please see my comments inline. > > On Monday 19 of August 2013 11:59:19 Markus Mayer wrote: >> This patch adds the GPIO driver for the bcm281xx family of chips. >> diff --git a/Documentation/devicetree/bindings/gpio/gpio-bcm-kona.txt >> +- interrupts: The interrupt outputs from the controller. There is one >> GPIO + interrupt per GPIO bank. The number of GPIO banks is HW >> configurable. > > What do you mean by HW configurable? Does it mean that it depends from > SoC or you can have different bank configurations on the same SoC? the wording is a little odd; I was going to comment on it, but thought it was probably clear enough. > How does the driver know the layout of GPIO banks? The code suggests that > the number of interrupts is used as bank count. While this saves you from > adding any new property or any other way of getting this information, I'm > not sure if it's completely correct. I'd like to hear others' opinion on > this, though. I expect a lookup table with key being the exact compatible value, and output being the # banks would be a reasonable driver implementation. Keying off the number of entries in interrupts seems fine too, since it should work out OK (Tegra does this IIRC).
On Mon, Aug 19, 2013 at 8:59 PM, Markus Mayer <markus.mayer@linaro.org> wrote: > From: Markus Mayer <mmayer@broadcom.com> > > This patch adds the GPIO driver for the bcm281xx family of chips. > > Signed-off-by: Markus Mayer <markus.mayer@linaro.org> > Reviewed-by: Christian Daudt <csd@broadcom.com> > Reviewed-by: Tim Kryger <tim.kryger@linaro.org> > Reviewed-by: Matt Porter <matt.porter@linaro.org> This is getting into nice shape quickly, but at this point the driver will still miss the v3.12 merge window as it seems, so let's aim for v3.13. > +/* This function counts IRQ entries in the device tree */ > +static int bcm_kona_count_irq_resources(struct platform_device *pdev) > +{ > + int i, ret; > + > + for (i = 0;; i++) { > + ret = platform_get_irq(pdev, i); > + if (ret < 0) > + break; > + } > + return i; > +} This looks weird, and it is not operating on a device tree but on the platform resources created by the device tree core, so atleast remove that comment. What it does is just count the number of IRQs for this device, no matter where it came from, right? > +static void bcm_kona_gpio_set(struct gpio_chip *chip, unsigned gpio, > + int value) > +{ (...) > + val = readl(reg_base + reg_offset); > + val |= 1 << bit; I would really like you to do like this: #include <linux/bitops.h> val |= BIT(bit); (...) > +static int bcm_kona_gpio_get(struct gpio_chip *chip, unsigned gpio) > +{ (...) > + /* return the specified bit status */ > + return (val >> bit) & 1; Please do like this: return !!(val & bit); > +static int bcm_kona_gpio_direction_output(struct gpio_chip *chip, > + unsigned gpio, int value) > +{ (...) > + val = readl(reg_base + reg_offset); > + val |= 1 << bit; val |= BIT(bit); > +static void bcm_kona_gpio_irq_handler(unsigned int irq, struct irq_desc *desc) > +{ > + void __iomem *reg_base; > + int bit, bank_id; > + 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); > + > + /* > + * For bank interrupts, we can't use chip_data to store the kona_gpio > + * pointer, since GIC needs it for its own purposes. Therefore, we get > + * our pointer from the bank structure. > + */ > + reg_base = bank->reg_base; > + bank_id = bank->id; > + bcm_kona_gpio_unlock_bank(reg_base, bank_id); > + > + sta = readl(reg_base + GPIO_INT_STATUS(bank_id)) & > + (~(readl(reg_base + 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 + GPIO_INT_STATUS(bank_id)) | > + (1 << bit), Use BIT(bit) > + reg_base + GPIO_INT_STATUS(bank_id)); > + /* Invoke interrupt handler */ > + generic_handle_irq(gpio_to_irq(GPIO_PER_BANK * bank_id + bit)); > + } Usually you may want to re-read thet status for each iteration of this loop, if IRQs appear as you are processing. > +static int bcm_kona_gpio_probe(struct platform_device *pdev) > +{ (...) > + kona_gpio->irq_base = irq_alloc_descs(-1, 0, chip->ngpio, 0); > + if (kona_gpio->irq_base < 0) { > + dev_err(dev, "Couldn't allocate IRQ numbers\n"); > + return -ENXIO; > + } As mentioned by Tomasz this looks strange. Apart from this last thing, the rest is really nitpicking comments and no blockers. Yours, Linus Walleij
On 23 August 2013 10:34, Linus Walleij <linus.walleij@linaro.org> wrote: > On Mon, Aug 19, 2013 at 8:59 PM, Markus Mayer <markus.mayer@linaro.org> wrote: > >> From: Markus Mayer <mmayer@broadcom.com> >> >> This patch adds the GPIO driver for the bcm281xx family of chips. >> >> Signed-off-by: Markus Mayer <markus.mayer@linaro.org> >> Reviewed-by: Christian Daudt <csd@broadcom.com> >> Reviewed-by: Tim Kryger <tim.kryger@linaro.org> >> Reviewed-by: Matt Porter <matt.porter@linaro.org> > > This is getting into nice shape quickly, but at this point the driver > will still miss the v3.12 merge window as it seems, so let's > aim for v3.13. Yes, it is starting to look that way. >> +/* This function counts IRQ entries in the device tree */ >> +static int bcm_kona_count_irq_resources(struct platform_device *pdev) >> +{ >> + int i, ret; >> + >> + for (i = 0;; i++) { >> + ret = platform_get_irq(pdev, i); >> + if (ret < 0) >> + break; >> + } >> + return i; >> +} > > This looks weird, and it is not operating on a device tree but > on the platform resources created by the device tree core, > so atleast remove that comment. What it does is just count > the number of IRQs for this device, no matter where it came > from, right? That function is actually gone now in my current working version. I have taken Thomesz's advice and am using of_count_irq(). You are right that we only care about the number of IRQs, no matter where they came from. In our case, they'd always come from device tree, though. >> +static void bcm_kona_gpio_set(struct gpio_chip *chip, unsigned gpio, >> + int value) >> +{ > (...) >> + val = readl(reg_base + reg_offset); >> + val |= 1 << bit; > > I would really like you to do like this: > > #include <linux/bitops.h> > > val |= BIT(bit); I'll incorporate the bit operations here and below. [...] >> +static void bcm_kona_gpio_irq_handler(unsigned int irq, struct irq_desc *desc) >> +{ >> + void __iomem *reg_base; >> + int bit, bank_id; >> + 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); >> + >> + /* >> + * For bank interrupts, we can't use chip_data to store the kona_gpio >> + * pointer, since GIC needs it for its own purposes. Therefore, we get >> + * our pointer from the bank structure. >> + */ >> + reg_base = bank->reg_base; >> + bank_id = bank->id; >> + bcm_kona_gpio_unlock_bank(reg_base, bank_id); >> + >> + sta = readl(reg_base + GPIO_INT_STATUS(bank_id)) & >> + (~(readl(reg_base + 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 + GPIO_INT_STATUS(bank_id)) | >> + (1 << bit), > > Use BIT(bit) > >> + reg_base + GPIO_INT_STATUS(bank_id)); >> + /* Invoke interrupt handler */ >> + generic_handle_irq(gpio_to_irq(GPIO_PER_BANK * bank_id + bit)); >> + } > > Usually you may want to re-read thet status for each iteration > of this loop, if IRQs appear as you are processing. Will do. >> +static int bcm_kona_gpio_probe(struct platform_device *pdev) >> +{ > (...) >> + kona_gpio->irq_base = irq_alloc_descs(-1, 0, chip->ngpio, 0); >> + if (kona_gpio->irq_base < 0) { >> + dev_err(dev, "Couldn't allocate IRQ numbers\n"); >> + return -ENXIO; >> + } > > As mentioned by Tomasz this looks strange. Yep, it does, and it's gone now. It was a forgotten and overlooked piece of code from an old version of the driver that used legacy mapping. I took it out. Thanks, -Markus
>>> +static void bcm_kona_gpio_irq_handler(unsigned int irq, struct irq_desc *desc) >>> +{ >>> + void __iomem *reg_base; >>> + int bit, bank_id; >>> + 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); >>> + >>> + /* >>> + * For bank interrupts, we can't use chip_data to store the kona_gpio >>> + * pointer, since GIC needs it for its own purposes. Therefore, we get >>> + * our pointer from the bank structure. >>> + */ >>> + reg_base = bank->reg_base; >>> + bank_id = bank->id; >>> + bcm_kona_gpio_unlock_bank(reg_base, bank_id); >>> + >>> + sta = readl(reg_base + GPIO_INT_STATUS(bank_id)) & >>> + (~(readl(reg_base + 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 + GPIO_INT_STATUS(bank_id)) | >>> + (1 << bit), >> >> Use BIT(bit) >> >>> + reg_base + GPIO_INT_STATUS(bank_id)); >>> + /* Invoke interrupt handler */ >>> + generic_handle_irq(gpio_to_irq(GPIO_PER_BANK * bank_id + bit)); >>> + } >> >> Usually you may want to re-read thet status for each iteration >> of this loop, if IRQs appear as you are processing. > > Will do. I have a follow-up question regarding status reads for each iteration. Is this what you are looking for? for (;;) { sta = readl(reg_base + GPIO_INT_STATUS(bank_id)) & (~(readl(reg_base + GPIO_INT_MASK(bank_id)))); if (sta == 0) break; for_each_set_bit(bit, &sta, 32) { int gpio = GPIO_PER_BANK * bank_id + bit; int virq = irq_find_mapping(bank->kona_gpio->irq_domain, gpio); /* * Clear interrupt before handler is called so we don't * miss any interrupt occurred during executing them. */ writel(readl(reg_base + GPIO_INT_STATUS(bank_id)) | BIT(bit), reg_base + GPIO_INT_STATUS(bank_id)); /* Invoke interrupt handler */ generic_handle_irq(virq); } } It's an extra loop, since I am not sure it would work to modify "sta" within the for_each_set_bit loop. Say an interrupt occurred whose bit we already checked, we'd still miss that. Thanks, -Markus
On Fri, Aug 23, 2013 at 9:30 PM, Markus Mayer <markus.mayer@linaro.org> wrote: > I have a follow-up question regarding status reads for each iteration. > Is this what you are looking for? > > for (;;) { > sta = readl(reg_base + GPIO_INT_STATUS(bank_id)) & > (~(readl(reg_base + GPIO_INT_MASK(bank_id)))); > if (sta == 0) > break; Yes, but follow the design pattern of other drivers such as the drivers/irqchip/irq-vic.c: static int handle_one_vic(struct vic_device *vic, struct pt_regs *regs) { u32 stat, irq; int handled = 0; while ((stat = readl_relaxed(vic->base + VIC_IRQ_STATUS))) { irq = ffs(stat) - 1; handle_IRQ(irq_find_mapping(vic->domain, irq), regs); handled = 1; } } I.e use that nice while-construction. Yours, Linus Walleij
diff --git a/Documentation/devicetree/bindings/gpio/gpio-bcm-kona.txt b/Documentation/devicetree/bindings/gpio/gpio-bcm-kona.txt new file mode 100644 index 0000000..70b2faf --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/gpio-bcm-kona.txt @@ -0,0 +1,43 @@ +Broadcom Kona Family GPIO +------------------------- + +This GPIO driver is used in the following Broadcom SoCs: + BCM11130, BCM11140, BCM11351, BCM28145, BCM28155 + +The GPIO controller only supports edge, not level, triggering of interrupts. + +- compatible: "brcm,kona-gpio" +- reg: Physical base address and length of the controller's registers. +- interrupts: The interrupt outputs from the controller. There is one GPIO + interrupt per GPIO bank. The number of GPIO banks is HW configurable. The + number of interrupts listed depends on the number of GPIO banks on the + SoC. The interrupts must be ordered by bank, starting with bank 0. +- #gpio-cells: Should be <2>. The first cell is the pin number, the second + cell is used to specify optional parameters: + - bit 0 specifies polarity (0 for normal, 1 for inverted) +- #interrupt-cells: Should be <2>. The first cell is the GPIO number. + The second cell is used to specify flags: + - trigger type (bits[1:0]): + 1 = low-to-high edge triggered. + 2 = high-to-low edge triggered. + 3 = low-to-high or high-to-low edge triggered + Valid values are 1, 2, 3 +- gpio-controller: Marks the device node as a GPIO controller. +- interrupt-controller: Marks the device node as an interrupt controller. + +Example: + gpio: gpio@35003000 { + compatible = "brcm,bcm11351-gpio", "brcm,kona-gpio"; + reg = <0x35003000 0x800>; + interrupts = + <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH + GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH + GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH + GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH + GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH + GIC_SPI 111 IRQ_TYPE_LEVEL_HIGH>; + #gpio-cells = <2>; + #interrupt-cells = <2>; + gpio-controller; + interrupt-controller; + }; diff --git a/drivers/gpio/gpio-bcm-kona.c b/drivers/gpio/gpio-bcm-kona.c new file mode 100644 index 0000000..9715b06 --- /dev/null +++ b/drivers/gpio/gpio-bcm-kona.c @@ -0,0 +1,658 @@ +/* + * 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 <linux/io.h> +#include <linux/gpio.h> +#include <linux/of_device.h> +#include <linux/module.h> +#include <linux/irqdomain.h> +#include <linux/irqchip/chained_irq.h> + +#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_OUT_STATUS(bank) (0x00000000 + ((bank) << 2)) +#define GPIO_IN_STATUS(bank) (0x00000020 + ((bank) << 2)) +#define GPIO_OUT_SET(bank) (0x00000040 + ((bank) << 2)) +#define GPIO_OUT_CLEAR(bank) (0x00000060 + ((bank) << 2)) +#define GPIO_INT_STATUS(bank) (0x00000080 + ((bank) << 2)) +#define GPIO_INT_MASK(bank) (0x000000A0 + ((bank) << 2)) +#define GPIO_INT_MSKCLR(bank) (0x000000C0 + ((bank) << 2)) +#define GPIO_CONTROL(bank) (0x00000100 + ((bank) << 2)) +#define GPIO_PWD_STATUS(bank) (0x00000500 + ((bank) << 2)) + +#define GPIO_GPPWR_OFFSET 0x00000520 + +#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 0x00000000 + +struct bcm_kona_gpio { + void __iomem *reg_base; + int num_bank; + int irq_base; + spinlock_t lock; + struct gpio_chip gpio_chip; + struct irq_domain *irq_domain; + struct bcm_kona_gpio_bank *banks; + struct platform_device *pdev; +}; + +struct bcm_kona_gpio_bank { + int id; + int irq; + /* Used in the interrupt handler */ + void __iomem *reg_base; +}; + +static inline struct bcm_kona_gpio *to_kona_gpio(struct gpio_chip *chip) +{ + return container_of(chip, struct bcm_kona_gpio, gpio_chip); +} + +/* This function counts IRQ entries in the device tree */ +static int bcm_kona_count_irq_resources(struct platform_device *pdev) +{ + int i, ret; + + for (i = 0;; i++) { + ret = platform_get_irq(pdev, i); + if (ret < 0) + break; + } + return i; +} + +static void bcm_kona_gpio_set_lockcode_bank(void __iomem *reg_base, int bank_id, + int lockcode) +{ + writel(BCM_GPIO_PASSWD, reg_base + GPIO_GPPWR_OFFSET); + writel(lockcode, reg_base + GPIO_PWD_STATUS(bank_id)); +} + +static inline void bcm_kona_gpio_lock_bank(void __iomem *reg_base, int bank_id) +{ + bcm_kona_gpio_set_lockcode_bank(reg_base, bank_id, LOCK_CODE); +} + +static inline void bcm_kona_gpio_unlock_bank(void __iomem *reg_base, + int bank_id) +{ + bcm_kona_gpio_set_lockcode_bank(reg_base, bank_id, UNLOCK_CODE); +} + +static void bcm_kona_gpio_set(struct gpio_chip *chip, unsigned gpio, + int value) +{ + struct bcm_kona_gpio *kona_gpio; + void __iomem *reg_base; + int bank_id = GPIO_BANK(gpio); + int bit = GPIO_BIT(gpio); + u32 val, reg_offset; + unsigned long flags; + + kona_gpio = to_kona_gpio(chip); + reg_base = kona_gpio->reg_base; + spin_lock_irqsave(&kona_gpio->lock, flags); + bcm_kona_gpio_unlock_bank(reg_base, bank_id); + + /* determine the GPIO pin direction */ + val = readl(reg_base + 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 ? GPIO_OUT_SET(bank_id) : 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(reg_base, bank_id); + spin_unlock_irqrestore(&kona_gpio->lock, flags); +} + +static int bcm_kona_gpio_get(struct gpio_chip *chip, unsigned gpio) +{ + struct bcm_kona_gpio *kona_gpio; + void __iomem *reg_base; + int bank_id = GPIO_BANK(gpio); + int bit = GPIO_BIT(gpio); + u32 val, reg_offset; + unsigned long flags; + + kona_gpio = to_kona_gpio(chip); + reg_base = kona_gpio->reg_base; + spin_lock_irqsave(&kona_gpio->lock, flags); + bcm_kona_gpio_unlock_bank(reg_base, bank_id); + + /* determine the GPIO pin direction */ + val = readl(reg_base + GPIO_CONTROL(gpio)); + val &= GPIO_GPCTR0_IOTR_MASK; + + /* read the GPIO bank status */ + reg_offset = (GPIO_GPCTR0_IOTR_CMD_INPUT == val) ? + GPIO_IN_STATUS(bank_id) : + GPIO_OUT_STATUS(bank_id); + val = readl(reg_base + reg_offset); + + bcm_kona_gpio_lock_bank(reg_base, bank_id); + spin_unlock_irqrestore(&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) +{ + struct bcm_kona_gpio *kona_gpio; + void __iomem *reg_base; + u32 val; + unsigned long flags; + int bank_id = GPIO_BANK(gpio); + + kona_gpio = to_kona_gpio(chip); + reg_base = kona_gpio->reg_base; + spin_lock_irqsave(&kona_gpio->lock, flags); + bcm_kona_gpio_unlock_bank(reg_base, bank_id); + + val = readl(reg_base + GPIO_CONTROL(gpio)); + val &= ~GPIO_GPCTR0_IOTR_MASK; + val |= GPIO_GPCTR0_IOTR_CMD_INPUT; + writel(val, reg_base + GPIO_CONTROL(gpio)); + + bcm_kona_gpio_lock_bank(reg_base, bank_id); + spin_unlock_irqrestore(&kona_gpio->lock, flags); + + return 0; +} + +static int bcm_kona_gpio_direction_output(struct gpio_chip *chip, + unsigned gpio, int value) +{ + struct bcm_kona_gpio *kona_gpio; + void __iomem *reg_base; + int bank_id = GPIO_BANK(gpio); + int bit = GPIO_BIT(gpio); + u32 val, reg_offset; + unsigned long flags; + + kona_gpio = to_kona_gpio(chip); + reg_base = kona_gpio->reg_base; + spin_lock_irqsave(&kona_gpio->lock, flags); + bcm_kona_gpio_unlock_bank(reg_base, bank_id); + + val = readl(reg_base + GPIO_CONTROL(gpio)); + val &= ~GPIO_GPCTR0_IOTR_MASK; + val |= GPIO_GPCTR0_IOTR_CMD_0UTPUT; + writel(val, reg_base + GPIO_CONTROL(gpio)); + reg_offset = value ? GPIO_OUT_SET(bank_id) : 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(reg_base, bank_id); + spin_unlock_irqrestore(&kona_gpio->lock, flags); + + return 0; +} + +static int bcm_kona_gpio_to_irq(struct gpio_chip *chip, unsigned gpio) +{ + struct bcm_kona_gpio *kona_gpio; + + kona_gpio = to_kona_gpio(chip); + if (gpio >= kona_gpio->gpio_chip.ngpio) + return -ENXIO; + return irq_create_mapping(kona_gpio->irq_domain, gpio); +} + +static int bcm_kona_gpio_set_debounce(struct gpio_chip *chip, unsigned gpio, + unsigned debounce) +{ + struct bcm_kona_gpio *kona_gpio; + void __iomem *reg_base; + u32 val, res; + unsigned long flags; + int bank_id = GPIO_BANK(gpio); + + kona_gpio = to_kona_gpio(chip); + reg_base = kona_gpio->reg_base; + /* 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(&kona_gpio->lock, flags); + bcm_kona_gpio_unlock_bank(reg_base, bank_id); + + val = readl(reg_base + 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 + GPIO_CONTROL(gpio)); + + bcm_kona_gpio_lock_bank(reg_base, bank_id); + spin_unlock_irqrestore(&kona_gpio->lock, flags); + + return 0; +} + +static struct gpio_chip template_chip = { + .label = "bcm-kona-gpio", + .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) +{ + struct bcm_kona_gpio *kona_gpio; + void __iomem *reg_base; + int gpio = d->hwirq; + int bank_id = GPIO_BANK(gpio); + int bit = GPIO_BIT(gpio); + u32 val; + unsigned long flags; + + kona_gpio = irq_data_get_irq_chip_data(d); + reg_base = kona_gpio->reg_base; + spin_lock_irqsave(&kona_gpio->lock, flags); + bcm_kona_gpio_unlock_bank(reg_base, bank_id); + + val = readl(reg_base + GPIO_INT_STATUS(bank_id)); + val |= 1 << bit; + writel(val, reg_base + GPIO_INT_STATUS(bank_id)); + + bcm_kona_gpio_lock_bank(reg_base, bank_id); + spin_unlock_irqrestore(&kona_gpio->lock, flags); +} + +static void bcm_kona_gpio_irq_mask(struct irq_data *d) +{ + struct bcm_kona_gpio *kona_gpio; + void __iomem *reg_base; + int gpio = d->hwirq; + int bank_id = GPIO_BANK(gpio); + int bit = GPIO_BIT(gpio); + u32 val; + unsigned long flags; + + kona_gpio = irq_data_get_irq_chip_data(d); + reg_base = kona_gpio->reg_base; + spin_lock_irqsave(&kona_gpio->lock, flags); + bcm_kona_gpio_unlock_bank(reg_base, bank_id); + + val = readl(reg_base + GPIO_INT_MASK(bank_id)); + val |= 1 << bit; + writel(val, reg_base + GPIO_INT_MASK(bank_id)); + + bcm_kona_gpio_lock_bank(reg_base, bank_id); + spin_unlock_irqrestore(&kona_gpio->lock, flags); +} + +static void bcm_kona_gpio_irq_unmask(struct irq_data *d) +{ + struct bcm_kona_gpio *kona_gpio; + void __iomem *reg_base; + int gpio = d->hwirq; + int bank_id = GPIO_BANK(gpio); + int bit = GPIO_BIT(gpio); + u32 val; + unsigned long flags; + + kona_gpio = irq_data_get_irq_chip_data(d); + reg_base = kona_gpio->reg_base; + spin_lock_irqsave(&kona_gpio->lock, flags); + bcm_kona_gpio_unlock_bank(reg_base, bank_id); + + val = readl(reg_base + GPIO_INT_MSKCLR(bank_id)); + val |= 1 << bit; + writel(val, reg_base + GPIO_INT_MSKCLR(bank_id)); + + bcm_kona_gpio_lock_bank(reg_base, bank_id); + spin_unlock_irqrestore(&kona_gpio->lock, flags); +} + +static int bcm_kona_gpio_irq_set_type(struct irq_data *d, unsigned int type) +{ + struct bcm_kona_gpio *kona_gpio; + void __iomem *reg_base; + int gpio = d->hwirq; + u32 lvl_type; + u32 val; + unsigned long flags; + int bank_id = GPIO_BANK(gpio); + + kona_gpio = irq_data_get_irq_chip_data(d); + reg_base = kona_gpio->reg_base; + 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(kona_gpio->gpio_chip.dev, + "Invalid BCM GPIO irq type 0x%x\n", type); + return -EINVAL; + } + + spin_lock_irqsave(&kona_gpio->lock, flags); + bcm_kona_gpio_unlock_bank(reg_base, bank_id); + + val = readl(reg_base + GPIO_CONTROL(gpio)); + val &= ~GPIO_GPCTR0_ITR_MASK; + val |= lvl_type << GPIO_GPCTR0_ITR_SHIFT; + writel(val, reg_base + GPIO_CONTROL(gpio)); + + bcm_kona_gpio_lock_bank(reg_base, bank_id); + spin_unlock_irqrestore(&kona_gpio->lock, flags); + + return 0; +} + +static void bcm_kona_gpio_irq_handler(unsigned int irq, struct irq_desc *desc) +{ + void __iomem *reg_base; + int bit, bank_id; + 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); + + /* + * For bank interrupts, we can't use chip_data to store the kona_gpio + * pointer, since GIC needs it for its own purposes. Therefore, we get + * our pointer from the bank structure. + */ + reg_base = bank->reg_base; + bank_id = bank->id; + bcm_kona_gpio_unlock_bank(reg_base, bank_id); + + sta = readl(reg_base + GPIO_INT_STATUS(bank_id)) & + (~(readl(reg_base + 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 + GPIO_INT_STATUS(bank_id)) | + (1 << bit), + reg_base + 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(reg_base, 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, bcm_kona_gpio_of_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) +{ + int ret; + + ret = irq_set_chip_data(virq, d->host_data); + if (ret < 0) + return ret; + 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(struct bcm_kona_gpio *kona_gpio) +{ + void __iomem *reg_base; + int i; + + reg_base = kona_gpio->reg_base; + /* disable interrupts and clear status */ + for (i = 0; i < kona_gpio->num_bank; i++) { + bcm_kona_gpio_unlock_bank(reg_base, i); + writel(0xFFFFFFFF, reg_base + GPIO_INT_MASK(i)); + writel(0xFFFFFFFF, reg_base + GPIO_INT_STATUS(i)); + bcm_kona_gpio_lock_bank(reg_base, i); + } +} + +static int bcm_kona_gpio_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + const struct of_device_id *match; + struct resource *res; + struct bcm_kona_gpio_bank *bank; + struct bcm_kona_gpio *kona_gpio; + struct gpio_chip *chip; + int ret; + int i; + + match = of_match_device(bcm_kona_gpio_of_match, dev); + if (!match) { + dev_err(dev, "Failed to find gpio controller\n"); + return -ENODEV; + } + + kona_gpio = devm_kzalloc(dev, sizeof(*kona_gpio), GFP_KERNEL); + if (!kona_gpio) { + dev_err(dev, "Failed to allocate memory for GPIO device"); + return -ENOMEM; + } + kona_gpio->gpio_chip = template_chip; + chip = &kona_gpio->gpio_chip; + kona_gpio->num_bank = bcm_kona_count_irq_resources(pdev); + if (kona_gpio->num_bank == 0) { + dev_err(dev, "Couldn't determine # GPIO banks\n"); + return -ENOENT; + } + kona_gpio->banks = devm_kzalloc(dev, + kona_gpio->num_bank * + sizeof(*kona_gpio->banks), + GFP_KERNEL); + if (!kona_gpio->banks) { + dev_err(dev, "Couldn't allocate bank structure\n"); + return -ENOMEM; + } + + kona_gpio->pdev = pdev; + platform_set_drvdata(pdev, kona_gpio); + chip->of_node = dev->of_node; + chip->ngpio = kona_gpio->num_bank * GPIO_PER_BANK; + + kona_gpio->irq_base = irq_alloc_descs(-1, 0, chip->ngpio, 0); + if (kona_gpio->irq_base < 0) { + dev_err(dev, "Couldn't allocate IRQ numbers\n"); + return -ENXIO; + } + + kona_gpio->irq_domain = irq_domain_add_linear(dev->of_node, + chip->ngpio, + &bcm_kona_irq_ops, + kona_gpio); + if (!kona_gpio->irq_domain) { + dev_err(dev, "Couldn't allocate IRQ domain\n"); + ret = -ENXIO; + goto err_irq_descs; + } + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) { + dev_err(dev, "Missing MEM resource\n"); + ret = -ENXIO; + goto err_irq_domain; + } + + kona_gpio->reg_base = devm_request_and_ioremap(dev, res); + if (!kona_gpio->reg_base) { + dev_err(dev, "Couldn't ioremap regs\n"); + ret = -ENXIO; + goto err_irq_domain; + } + + for (i = 0; i < kona_gpio->num_bank; i++) { + bank = &kona_gpio->banks[i]; + bank->id = i; + bank->irq = platform_get_irq(pdev, i); + bank->reg_base = kona_gpio->reg_base; + if (bank->irq < 0) { + dev_err(dev, "Couldn't get IRQ for bank %d", i); + ret = -ENOENT; + goto err_irq_domain; + } + } + + dev_info(&pdev->dev, "Setting up Kona GPIO at 0x%p (phys %#x)\n", + kona_gpio->reg_base, res->start); + + bcm_kona_gpio_reset(kona_gpio); + + ret = gpiochip_add(chip); + if (ret < 0) { + dev_err(dev, "Couldn't add GPIO chip -- %d\n", ret); + goto err_irq_domain; + } + for (i = 0; i < chip->ngpio; i++) { + int irq = bcm_kona_gpio_to_irq(chip, i); + bank = &kona_gpio->banks[GPIO_BANK(i)]; + irq_set_lockdep_class(irq, &gpio_lock_class); + irq_set_chip_and_handler(irq, &bcm_gpio_irq_chip, + handle_simple_irq); + set_irq_flags(irq, IRQF_VALID); + } + for (i = 0; i < kona_gpio->num_bank; i++) { + bank = &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(&kona_gpio->lock); + + return 0; + +err_irq_domain: + irq_domain_remove(kona_gpio->irq_domain); + +err_irq_descs: + irq_free_descs(kona_gpio->irq_base, chip->ngpio); + + return ret; +} + +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, +}; + +module_platform_driver(bcm_kona_gpio_driver); + +MODULE_AUTHOR("Broadcom"); +MODULE_DESCRIPTION("Broadcom Kona GPIO Driver"); +MODULE_LICENSE("GPL v2");