From b83bd893f3a04d35f5bfcf399c1034660e5b2403 Mon Sep 17 00:00:00 2001 From: Stefan Wahren Date: Mon, 31 Oct 2016 13:21:33 +0000 Subject: pinctrl: bcm2835: reduce GPPUD set-up time Since the BCM2835 datasheet doesn't exactly specify the set-up time for the GPIO Pull-up/down Clock Registers there was an assumption of 150 cycles at a clock rate of 1 MHz. During a discussion [1] in the Raspberry Pi forum it turns out that clock rate refers to the VPU which has a rate of 250 MHz. So we can reduce the delay to a sensible value and update the comment above. I tested this optimization with a Raspberry Pi B and a multimeter. [1] - https://www.raspberrypi.org/forums/viewtopic.php?f=72&t=163352 Signed-off-by: Stefan Wahren Reviewed-by: Eric Anholt Signed-off-by: Linus Walleij --- drivers/pinctrl/bcm/pinctrl-bcm2835.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'drivers/pinctrl/bcm') diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c index fa77165..b2dd278 100644 --- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c +++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c @@ -917,12 +917,14 @@ static int bcm2835_pinconf_set(struct pinctrl_dev *pctldev, bcm2835_gpio_wr(pc, GPPUD, arg & 3); /* - * Docs say to wait 150 cycles, but not of what. We assume a - * 1 MHz clock here, which is pretty slow... + * BCM2835 datasheet say to wait 150 cycles, but not of what. + * But the VideoCore firmware delay for this operation + * based nearly on the same amount of VPU cycles and this clock + * runs at 250 MHz. */ - udelay(150); + udelay(1); bcm2835_gpio_wr(pc, GPPUDCLK0 + (off * 4), BIT(bit)); - udelay(150); + udelay(1); bcm2835_gpio_wr(pc, GPPUDCLK0 + (off * 4), 0); } /* for each config */ -- cgit v1.1 From 00445b5d5866c7b31bcfc59e907c9062100b4b0a Mon Sep 17 00:00:00 2001 From: Phil Elwell Date: Tue, 24 Feb 2015 13:40:50 +0000 Subject: pinctrl: bcm2835: Fix ints for GPIOs 28-31 & 46-53 Contrary to the documentation, the BCM2835 GPIO controller actually has four interrupt lines - one each for the three IRQ groups and one common. Confusingly, the GPIO interrupt groups don't correspond directly with the GPIO control banks. Instead, GPIOs 0-27 generate IRQ GPIO0, 28-45 IRQ GPIO1 and 46-53 IRQ GPIO2. Awkwardly, the GPIOs for IRQ GPIO1 straddle two 32-entry GPIO banks, so split out a function to process the interrupts for a single GPIO bank. Cc: Stefan Wahren Cc: Stephen Warren Signed-off-by: Phil Elwell Acked-by: Eric Anholt Signed-off-by: Linus Walleij --- drivers/pinctrl/bcm/pinctrl-bcm2835.c | 50 ++++++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 12 deletions(-) (limited to 'drivers/pinctrl/bcm') diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c index b2dd278..9587d67 100644 --- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c +++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c @@ -47,6 +47,7 @@ #define MODULE_NAME "pinctrl-bcm2835" #define BCM2835_NUM_GPIOS 54 #define BCM2835_NUM_BANKS 2 +#define BCM2835_NUM_IRQS 3 #define BCM2835_PIN_BITMAP_SZ \ DIV_ROUND_UP(BCM2835_NUM_GPIOS, sizeof(unsigned long) * 8) @@ -88,13 +89,13 @@ enum bcm2835_pinconf_pull { struct bcm2835_gpio_irqdata { struct bcm2835_pinctrl *pc; - int bank; + int irqgroup; }; struct bcm2835_pinctrl { struct device *dev; void __iomem *base; - int irq[BCM2835_NUM_BANKS]; + int irq[BCM2835_NUM_IRQS]; /* note: locking assumes each bank will have its own unsigned long */ unsigned long enabled_irq_map[BCM2835_NUM_BANKS]; @@ -105,7 +106,7 @@ struct bcm2835_pinctrl { struct gpio_chip gpio_chip; struct pinctrl_gpio_range gpio_range; - struct bcm2835_gpio_irqdata irq_data[BCM2835_NUM_BANKS]; + struct bcm2835_gpio_irqdata irq_data[BCM2835_NUM_IRQS]; spinlock_t irq_lock[BCM2835_NUM_BANKS]; }; @@ -391,17 +392,16 @@ static struct gpio_chip bcm2835_gpio_chip = { .can_sleep = false, }; -static irqreturn_t bcm2835_gpio_irq_handler(int irq, void *dev_id) +static int bcm2835_gpio_irq_handle_bank(struct bcm2835_pinctrl *pc, + unsigned int bank, u32 mask) { - struct bcm2835_gpio_irqdata *irqdata = dev_id; - struct bcm2835_pinctrl *pc = irqdata->pc; - int bank = irqdata->bank; unsigned long events; unsigned offset; unsigned gpio; unsigned int type; events = bcm2835_gpio_rd(pc, GPEDS0 + bank * 4); + events &= mask; events &= pc->enabled_irq_map[bank]; for_each_set_bit(offset, &events, 32) { gpio = (32 * bank) + offset; @@ -409,7 +409,30 @@ static irqreturn_t bcm2835_gpio_irq_handler(int irq, void *dev_id) generic_handle_irq(irq_linear_revmap(pc->irq_domain, gpio)); } - return events ? IRQ_HANDLED : IRQ_NONE; + + return (events != 0); +} + +static irqreturn_t bcm2835_gpio_irq_handler(int irq, void *dev_id) +{ + struct bcm2835_gpio_irqdata *irqdata = dev_id; + struct bcm2835_pinctrl *pc = irqdata->pc; + int handled = 0; + + switch (irqdata->irqgroup) { + case 0: /* IRQ0 covers GPIOs 0-27 */ + handled = bcm2835_gpio_irq_handle_bank(pc, 0, 0x0fffffff); + break; + case 1: /* IRQ1 covers GPIOs 28-45 */ + handled = bcm2835_gpio_irq_handle_bank(pc, 0, 0xf0000000) | + bcm2835_gpio_irq_handle_bank(pc, 1, 0x00003fff); + break; + case 2: /* IRQ2 covers GPIOs 46-53 */ + handled = bcm2835_gpio_irq_handle_bank(pc, 1, 0x003fc000); + break; + } + + return handled ? IRQ_HANDLED : IRQ_NONE; } static inline void __bcm2835_gpio_irq_config(struct bcm2835_pinctrl *pc, @@ -1000,8 +1023,6 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev) for (i = 0; i < BCM2835_NUM_BANKS; i++) { unsigned long events; unsigned offset; - int len; - char *name; /* clear event detection flags */ bcm2835_gpio_wr(pc, GPREN0 + i * 4, 0); @@ -1016,10 +1037,15 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev) for_each_set_bit(offset, &events, 32) bcm2835_gpio_wr(pc, GPEDS0 + i * 4, BIT(offset)); + spin_lock_init(&pc->irq_lock[i]); + } + + for (i = 0; i < BCM2835_NUM_IRQS; i++) { + int len; + char *name; pc->irq[i] = irq_of_parse_and_map(np, i); pc->irq_data[i].pc = pc; - pc->irq_data[i].bank = i; - spin_lock_init(&pc->irq_lock[i]); + pc->irq_data[i].irqgroup = i; len = strlen(dev_name(pc->dev)) + 16; name = devm_kzalloc(pc->dev, len, GFP_KERNEL); -- cgit v1.1 From ccca1ad5db0e97710b69f9a3895e03290c47898b Mon Sep 17 00:00:00 2001 From: Phil Elwell Date: Fri, 6 May 2016 12:32:47 +0100 Subject: pinctrl: bcm2835: Return pins to inputs when freed When dynamically unloading overlays, it is important that freed pins are restored to being inputs to prevent functions from being enabled in multiple places at once. Cc: Stefan Wahren Cc: Stephen Warren Signed-off-by: Phil Elwell Acked-by: Eric Anholt Signed-off-by: Linus Walleij --- drivers/pinctrl/bcm/pinctrl-bcm2835.c | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'drivers/pinctrl/bcm') diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c index 9587d67..6128359 100644 --- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c +++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c @@ -844,6 +844,16 @@ static const struct pinctrl_ops bcm2835_pctl_ops = { .dt_free_map = bcm2835_pctl_dt_free_map, }; +static int bcm2835_pmx_free(struct pinctrl_dev *pctldev, + unsigned offset) +{ + struct bcm2835_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev); + + /* disable by setting to GPIO_IN */ + bcm2835_pinctrl_fsel_set(pc, offset, BCM2835_FSEL_GPIO_IN); + return 0; +} + static int bcm2835_pmx_get_functions_count(struct pinctrl_dev *pctldev) { return BCM2835_FSEL_COUNT; @@ -903,6 +913,7 @@ static int bcm2835_pmx_gpio_set_direction(struct pinctrl_dev *pctldev, } static const struct pinmux_ops bcm2835_pmx_ops = { + .free = bcm2835_pmx_free, .get_functions_count = bcm2835_pmx_get_functions_count, .get_function_name = bcm2835_pmx_get_function_name, .get_function_groups = bcm2835_pmx_get_function_groups, -- cgit v1.1 From 85ae9e512f437cd09bf61564bdba29ab88bab3e3 Mon Sep 17 00:00:00 2001 From: Linus Walleij Date: Mon, 14 Nov 2016 18:48:19 +0100 Subject: pinctrl: bcm2835: switch to GPIOLIB_IRQCHIP It should be possible to use the GPIOLIB_IRQCHIP helper library with the BCM2835 driver since it is a pretty straight forward cascaded irqchip. The only difference from other drivers is that the BCM2835 has several banks for a single gpiochip, and each bank has a separate IRQ line. Instead of creating one gpiochip per bank, a single gpiochip covers all banks GPIO lines. This makes it necessary to resolve the bank ID in the IRQ handler. The GPIOLIB_IRQCHIP allows several IRQs to be cascaded off the same gpiochip by calling gpiochip_set_chained_irqchip() repeatedly, but we have been a bit short on examples for how this should be handled in practice, so this is intended as an example of how this can be achieved. The old code did not model the chip as a chained interrupt handler, but this patch also rectifies that situation. Cc: Stephen Warren Tested-by: Stefan Wahren Tested-by: Eric Anholt Acked-by: Eric Anholt Signed-off-by: Linus Walleij --- drivers/pinctrl/bcm/Kconfig | 1 + drivers/pinctrl/bcm/pinctrl-bcm2835.c | 140 ++++++++++++++++------------------ 2 files changed, 65 insertions(+), 76 deletions(-) (limited to 'drivers/pinctrl/bcm') diff --git a/drivers/pinctrl/bcm/Kconfig b/drivers/pinctrl/bcm/Kconfig index 6324677..8968dd7 100644 --- a/drivers/pinctrl/bcm/Kconfig +++ b/drivers/pinctrl/bcm/Kconfig @@ -20,6 +20,7 @@ config PINCTRL_BCM2835 bool select PINMUX select PINCONF + select GPIOLIB_IRQCHIP config PINCTRL_IPROC_GPIO bool "Broadcom iProc GPIO (with PINCONF) driver" diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c index 6128359..1bb38d0 100644 --- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c +++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c @@ -24,11 +24,9 @@ #include #include #include -#include #include #include #include -#include #include #include #include @@ -87,11 +85,6 @@ enum bcm2835_pinconf_pull { #define BCM2835_PINCONF_UNPACK_PARAM(_conf_) ((_conf_) >> 16) #define BCM2835_PINCONF_UNPACK_ARG(_conf_) ((_conf_) & 0xffff) -struct bcm2835_gpio_irqdata { - struct bcm2835_pinctrl *pc; - int irqgroup; -}; - struct bcm2835_pinctrl { struct device *dev; void __iomem *base; @@ -102,16 +95,13 @@ struct bcm2835_pinctrl { unsigned int irq_type[BCM2835_NUM_GPIOS]; struct pinctrl_dev *pctl_dev; - struct irq_domain *irq_domain; struct gpio_chip gpio_chip; struct pinctrl_gpio_range gpio_range; - struct bcm2835_gpio_irqdata irq_data[BCM2835_NUM_IRQS]; + int irq_group[BCM2835_NUM_IRQS]; spinlock_t irq_lock[BCM2835_NUM_BANKS]; }; -static struct lock_class_key gpio_lock_class; - /* pins are just named GPIO0..GPIO53 */ #define BCM2835_GPIO_PIN(a) PINCTRL_PIN(a, "gpio" #a) static struct pinctrl_pin_desc bcm2835_gpio_pins[] = { @@ -369,13 +359,6 @@ static int bcm2835_gpio_direction_output(struct gpio_chip *chip, return pinctrl_gpio_direction_output(chip->base + offset); } -static int bcm2835_gpio_to_irq(struct gpio_chip *chip, unsigned offset) -{ - struct bcm2835_pinctrl *pc = gpiochip_get_data(chip); - - return irq_linear_revmap(pc->irq_domain, offset); -} - static struct gpio_chip bcm2835_gpio_chip = { .label = MODULE_NAME, .owner = THIS_MODULE, @@ -386,14 +369,13 @@ static struct gpio_chip bcm2835_gpio_chip = { .get_direction = bcm2835_gpio_get_direction, .get = bcm2835_gpio_get, .set = bcm2835_gpio_set, - .to_irq = bcm2835_gpio_to_irq, .base = -1, .ngpio = BCM2835_NUM_GPIOS, .can_sleep = false, }; -static int bcm2835_gpio_irq_handle_bank(struct bcm2835_pinctrl *pc, - unsigned int bank, u32 mask) +static void bcm2835_gpio_irq_handle_bank(struct bcm2835_pinctrl *pc, + unsigned int bank, u32 mask) { unsigned long events; unsigned offset; @@ -405,34 +387,49 @@ static int bcm2835_gpio_irq_handle_bank(struct bcm2835_pinctrl *pc, events &= pc->enabled_irq_map[bank]; for_each_set_bit(offset, &events, 32) { gpio = (32 * bank) + offset; + /* FIXME: no clue why the code looks up the type here */ type = pc->irq_type[gpio]; - generic_handle_irq(irq_linear_revmap(pc->irq_domain, gpio)); + generic_handle_irq(irq_linear_revmap(pc->gpio_chip.irqdomain, + gpio)); } - - return (events != 0); } -static irqreturn_t bcm2835_gpio_irq_handler(int irq, void *dev_id) +static void bcm2835_gpio_irq_handler(struct irq_desc *desc) { - struct bcm2835_gpio_irqdata *irqdata = dev_id; - struct bcm2835_pinctrl *pc = irqdata->pc; - int handled = 0; + struct gpio_chip *chip = irq_desc_get_handler_data(desc); + struct bcm2835_pinctrl *pc = gpiochip_get_data(chip); + struct irq_chip *host_chip = irq_desc_get_chip(desc); + int irq = irq_desc_get_irq(desc); + int group; + int i; + + for (i = 0; i < ARRAY_SIZE(pc->irq); i++) { + if (pc->irq[i] == irq) { + group = pc->irq_group[i]; + break; + } + } + /* This should not happen, every IRQ has a bank */ + if (i == ARRAY_SIZE(pc->irq)) + BUG(); - switch (irqdata->irqgroup) { + chained_irq_enter(host_chip, desc); + + switch (group) { case 0: /* IRQ0 covers GPIOs 0-27 */ - handled = bcm2835_gpio_irq_handle_bank(pc, 0, 0x0fffffff); + bcm2835_gpio_irq_handle_bank(pc, 0, 0x0fffffff); break; case 1: /* IRQ1 covers GPIOs 28-45 */ - handled = bcm2835_gpio_irq_handle_bank(pc, 0, 0xf0000000) | - bcm2835_gpio_irq_handle_bank(pc, 1, 0x00003fff); + bcm2835_gpio_irq_handle_bank(pc, 0, 0xf0000000); + bcm2835_gpio_irq_handle_bank(pc, 1, 0x00003fff); break; case 2: /* IRQ2 covers GPIOs 46-53 */ - handled = bcm2835_gpio_irq_handle_bank(pc, 1, 0x003fc000); + bcm2835_gpio_irq_handle_bank(pc, 1, 0x003fc000); break; } - return handled ? IRQ_HANDLED : IRQ_NONE; + chained_irq_exit(host_chip, desc); } static inline void __bcm2835_gpio_irq_config(struct bcm2835_pinctrl *pc, @@ -478,7 +475,8 @@ static void bcm2835_gpio_irq_config(struct bcm2835_pinctrl *pc, static void bcm2835_gpio_irq_enable(struct irq_data *data) { - struct bcm2835_pinctrl *pc = irq_data_get_irq_chip_data(data); + struct gpio_chip *chip = irq_data_get_irq_chip_data(data); + struct bcm2835_pinctrl *pc = gpiochip_get_data(chip); unsigned gpio = irqd_to_hwirq(data); unsigned offset = GPIO_REG_SHIFT(gpio); unsigned bank = GPIO_REG_OFFSET(gpio); @@ -492,7 +490,8 @@ static void bcm2835_gpio_irq_enable(struct irq_data *data) static void bcm2835_gpio_irq_disable(struct irq_data *data) { - struct bcm2835_pinctrl *pc = irq_data_get_irq_chip_data(data); + struct gpio_chip *chip = irq_data_get_irq_chip_data(data); + struct bcm2835_pinctrl *pc = gpiochip_get_data(chip); unsigned gpio = irqd_to_hwirq(data); unsigned offset = GPIO_REG_SHIFT(gpio); unsigned bank = GPIO_REG_OFFSET(gpio); @@ -598,7 +597,8 @@ static int __bcm2835_gpio_irq_set_type_enabled(struct bcm2835_pinctrl *pc, static int bcm2835_gpio_irq_set_type(struct irq_data *data, unsigned int type) { - struct bcm2835_pinctrl *pc = irq_data_get_irq_chip_data(data); + struct gpio_chip *chip = irq_data_get_irq_chip_data(data); + struct bcm2835_pinctrl *pc = gpiochip_get_data(chip); unsigned gpio = irqd_to_hwirq(data); unsigned offset = GPIO_REG_SHIFT(gpio); unsigned bank = GPIO_REG_OFFSET(gpio); @@ -624,7 +624,8 @@ static int bcm2835_gpio_irq_set_type(struct irq_data *data, unsigned int type) static void bcm2835_gpio_irq_ack(struct irq_data *data) { - struct bcm2835_pinctrl *pc = irq_data_get_irq_chip_data(data); + struct gpio_chip *chip = irq_data_get_irq_chip_data(data); + struct bcm2835_pinctrl *pc = gpiochip_get_data(chip); unsigned gpio = irqd_to_hwirq(data); bcm2835_gpio_set_bit(pc, GPEDS0, gpio); @@ -667,10 +668,11 @@ static void bcm2835_pctl_pin_dbg_show(struct pinctrl_dev *pctldev, unsigned offset) { struct bcm2835_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev); + struct gpio_chip *chip = &pc->gpio_chip; enum bcm2835_fsel fsel = bcm2835_pinctrl_fsel_get(pc, offset); const char *fname = bcm2835_functions[fsel]; int value = bcm2835_gpio_get_bit(pc, GPLEV0, offset); - int irq = irq_find_mapping(pc->irq_domain, offset); + int irq = irq_find_mapping(chip->irqdomain, offset); seq_printf(s, "function %s in %s; irq %d (%s)", fname, value ? "hi" : "lo", @@ -1016,21 +1018,6 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev) pc->gpio_chip.parent = dev; pc->gpio_chip.of_node = np; - pc->irq_domain = irq_domain_add_linear(np, BCM2835_NUM_GPIOS, - &irq_domain_simple_ops, NULL); - if (!pc->irq_domain) { - dev_err(dev, "could not create IRQ domain\n"); - return -ENOMEM; - } - - for (i = 0; i < BCM2835_NUM_GPIOS; i++) { - int irq = irq_create_mapping(pc->irq_domain, i); - irq_set_lockdep_class(irq, &gpio_lock_class); - irq_set_chip_and_handler(irq, &bcm2835_gpio_irq_chip, - handle_level_irq); - irq_set_chip_data(irq, pc); - } - for (i = 0; i < BCM2835_NUM_BANKS; i++) { unsigned long events; unsigned offset; @@ -1051,34 +1038,35 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev) spin_lock_init(&pc->irq_lock[i]); } - for (i = 0; i < BCM2835_NUM_IRQS; i++) { - int len; - char *name; - pc->irq[i] = irq_of_parse_and_map(np, i); - pc->irq_data[i].pc = pc; - pc->irq_data[i].irqgroup = i; - - len = strlen(dev_name(pc->dev)) + 16; - name = devm_kzalloc(pc->dev, len, GFP_KERNEL); - if (!name) - return -ENOMEM; - snprintf(name, len, "%s:bank%d", dev_name(pc->dev), i); - - err = devm_request_irq(dev, pc->irq[i], - bcm2835_gpio_irq_handler, IRQF_SHARED, - name, &pc->irq_data[i]); - if (err) { - dev_err(dev, "unable to request IRQ %d\n", pc->irq[i]); - return err; - } - } - err = gpiochip_add_data(&pc->gpio_chip, pc); if (err) { dev_err(dev, "could not add GPIO chip\n"); return err; } + err = gpiochip_irqchip_add(&pc->gpio_chip, &bcm2835_gpio_irq_chip, + 0, handle_level_irq, IRQ_TYPE_NONE); + if (err) { + dev_info(dev, "could not add irqchip\n"); + return err; + } + + for (i = 0; i < BCM2835_NUM_IRQS; i++) { + pc->irq[i] = irq_of_parse_and_map(np, i); + pc->irq_group[i] = i; + /* + * Use the same handler for all groups: this is necessary + * since we use one gpiochip to cover all lines - the + * irq handler then needs to figure out which group and + * bank that was firing the IRQ and look up the per-group + * and bank data. + */ + gpiochip_set_chained_irqchip(&pc->gpio_chip, + &bcm2835_gpio_irq_chip, + pc->irq[i], + bcm2835_gpio_irq_handler); + } + pc->pctl_dev = devm_pinctrl_register(dev, &bcm2835_pinctrl_desc, pc); if (IS_ERR(pc->pctl_dev)) { gpiochip_remove(&pc->gpio_chip); -- cgit v1.1