From 6dbc6e6f58556369bf999cd7d9793586f1b0e4b4 Mon Sep 17 00:00:00 2001 From: Phil Reid Date: Thu, 13 Jun 2019 12:10:23 +0800 Subject: [PATCH 1/5] pinctrl: mcp23s08: Fix add_data and irqchip_add_nested call order Currently probing of the mcp23s08 results in an error message "detected irqchip that is shared with multiple gpiochips: please fix the driver" This is due to the following: Call to mcp23s08_irqchip_setup() with call hierarchy: mcp23s08_irqchip_setup() gpiochip_irqchip_add_nested() gpiochip_irqchip_add_key() gpiochip_set_irq_hooks() Call to devm_gpiochip_add_data() with call hierarchy: devm_gpiochip_add_data() gpiochip_add_data_with_key() gpiochip_add_irqchip() gpiochip_set_irq_hooks() The gpiochip_add_irqchip() returns immediately if there isn't a irqchip but we added a irqchip due to the previous mcp23s08_irqchip_setup() call. So it calls gpiochip_set_irq_hooks() a second time. Fix this by moving the call to devm_gpiochip_add_data before the call to mcp23s08_irqchip_setup Fixes: 02e389e63e35 ("pinctrl: mcp23s08: fix irq setup order") Suggested-by: Marco Felsch Signed-off-by: Phil Reid Signed-off-by: Linus Walleij --- drivers/pinctrl/pinctrl-mcp23s08.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c index 568ca96cdb6d..3a235487e38d 100644 --- a/drivers/pinctrl/pinctrl-mcp23s08.c +++ b/drivers/pinctrl/pinctrl-mcp23s08.c @@ -771,6 +771,10 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev, if (ret < 0) goto fail; + ret = devm_gpiochip_add_data(dev, &mcp->chip, mcp); + if (ret < 0) + goto fail; + mcp->irq_controller = device_property_read_bool(dev, "interrupt-controller"); if (mcp->irq && mcp->irq_controller) { @@ -812,10 +816,6 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev, goto fail; } - ret = devm_gpiochip_add_data(dev, &mcp->chip, mcp); - if (ret < 0) - goto fail; - if (one_regmap_config) { mcp->pinctrl_desc.name = devm_kasprintf(dev, GFP_KERNEL, "mcp23xxx-pinctrl.%d", raw_chip_address); From f2818ba3a0125670cb9999bb5a65ebb631a8da2f Mon Sep 17 00:00:00 2001 From: Alexandre Belloni Date: Thu, 20 Jun 2019 20:30:36 +0200 Subject: [PATCH 2/5] pinctrl: ocelot: fix gpio direction for pins after 31 The third argument passed to REG is not the correct one and ocelot_gpio_set_direction is not working for pins after 31. Fix that by passing the pin number instead of the modulo 32 value. Fixes: da801ab56ad8 pinctrl: ocelot: add MSCC Jaguar2 support Signed-off-by: Alexandre Belloni Signed-off-by: Linus Walleij --- drivers/pinctrl/pinctrl-ocelot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/pinctrl/pinctrl-ocelot.c b/drivers/pinctrl/pinctrl-ocelot.c index 3b4ca52d2456..d2478db975bd 100644 --- a/drivers/pinctrl/pinctrl-ocelot.c +++ b/drivers/pinctrl/pinctrl-ocelot.c @@ -432,7 +432,7 @@ static int ocelot_gpio_set_direction(struct pinctrl_dev *pctldev, struct ocelot_pinctrl *info = pinctrl_dev_get_drvdata(pctldev); unsigned int p = pin % 32; - regmap_update_bits(info->map, REG(OCELOT_GPIO_OE, info, p), BIT(p), + regmap_update_bits(info->map, REG(OCELOT_GPIO_OE, info, pin), BIT(p), input ? 0 : BIT(p)); return 0; From 4b36082e2e09c2769710756390d54cfca563ed96 Mon Sep 17 00:00:00 2001 From: Alexandre Belloni Date: Thu, 20 Jun 2019 20:30:37 +0200 Subject: [PATCH 3/5] pinctrl: ocelot: fix pinmuxing for pins after 31 The actual layout for OCELOT_GPIO_ALT[01] when there are more than 32 pins is interleaved, i.e. OCELOT_GPIO_ALT0[0], OCELOT_GPIO_ALT1[0], OCELOT_GPIO_ALT0[1], OCELOT_GPIO_ALT1[1]. Introduce a new REG_ALT macro to facilitate the register offset calculation and use it where necessary. Fixes: da801ab56ad8 pinctrl: ocelot: add MSCC Jaguar2 support Signed-off-by: Alexandre Belloni Signed-off-by: Linus Walleij --- drivers/pinctrl/pinctrl-ocelot.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/drivers/pinctrl/pinctrl-ocelot.c b/drivers/pinctrl/pinctrl-ocelot.c index d2478db975bd..fb76fb2e9ea5 100644 --- a/drivers/pinctrl/pinctrl-ocelot.c +++ b/drivers/pinctrl/pinctrl-ocelot.c @@ -396,7 +396,7 @@ static int ocelot_pin_function_idx(struct ocelot_pinctrl *info, return -1; } -#define REG(r, info, p) ((r) * (info)->stride + (4 * ((p) / 32))) +#define REG_ALT(msb, info, p) (OCELOT_GPIO_ALT0 * (info)->stride + 4 * ((msb) + ((info)->stride * ((p) / 32)))) static int ocelot_pinmux_set_mux(struct pinctrl_dev *pctldev, unsigned int selector, unsigned int group) @@ -412,19 +412,21 @@ static int ocelot_pinmux_set_mux(struct pinctrl_dev *pctldev, /* * f is encoded on two bits. - * bit 0 of f goes in BIT(pin) of ALT0, bit 1 of f goes in BIT(pin) of - * ALT1 + * bit 0 of f goes in BIT(pin) of ALT[0], bit 1 of f goes in BIT(pin) of + * ALT[1] * This is racy because both registers can't be updated at the same time * but it doesn't matter much for now. */ - regmap_update_bits(info->map, REG(OCELOT_GPIO_ALT0, info, pin->pin), + regmap_update_bits(info->map, REG_ALT(0, info, pin->pin), BIT(p), f << p); - regmap_update_bits(info->map, REG(OCELOT_GPIO_ALT1, info, pin->pin), + regmap_update_bits(info->map, REG_ALT(1, info, pin->pin), BIT(p), f << (p - 1)); return 0; } +#define REG(r, info, p) ((r) * (info)->stride + (4 * ((p) / 32))) + static int ocelot_gpio_set_direction(struct pinctrl_dev *pctldev, struct pinctrl_gpio_range *range, unsigned int pin, bool input) @@ -445,9 +447,9 @@ static int ocelot_gpio_request_enable(struct pinctrl_dev *pctldev, struct ocelot_pinctrl *info = pinctrl_dev_get_drvdata(pctldev); unsigned int p = offset % 32; - regmap_update_bits(info->map, REG(OCELOT_GPIO_ALT0, info, offset), + regmap_update_bits(info->map, REG_ALT(0, info, offset), BIT(p), 0); - regmap_update_bits(info->map, REG(OCELOT_GPIO_ALT1, info, offset), + regmap_update_bits(info->map, REG_ALT(1, info, offset), BIT(p), 0); return 0; From 35594bc7cecf3a78504b590e350570e8f4d7779e Mon Sep 17 00:00:00 2001 From: Nicolas Boichat Date: Mon, 29 Apr 2019 11:55:14 +0800 Subject: [PATCH 4/5] pinctrl: mediatek: Ignore interrupts that are wake only during resume Before suspending, mtk-eint would set the interrupt mask to the one in wake_mask. However, some of these interrupts may not have a corresponding interrupt handler, or the interrupt may be disabled. On resume, the eint irq handler would trigger nevertheless, and irq/pm.c:irq_pm_check_wakeup would be called, which would try to call irq_disable. However, if the interrupt is not enabled (irqd_irq_disabled(&desc->irq_data) is true), the call does nothing, and the interrupt is left enabled in the eint driver. Especially for level-sensitive interrupts, this will lead to an interrupt storm on resume. If we detect that an interrupt is only in wake_mask, but not in cur_mask, we can just mask it out immediately (as mtk_eint_resume would do anyway at a later stage in the resume sequence, when restoring cur_mask). Fixes: bf22ff45bed6 ("genirq: Avoid unnecessary low level irq function calls") Signed-off-by: Nicolas Boichat Acked-by: Sean Wang Signed-off-by: Linus Walleij --- drivers/pinctrl/mediatek/mtk-eint.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/drivers/pinctrl/mediatek/mtk-eint.c b/drivers/pinctrl/mediatek/mtk-eint.c index f464f8cd274b..737385e86beb 100644 --- a/drivers/pinctrl/mediatek/mtk-eint.c +++ b/drivers/pinctrl/mediatek/mtk-eint.c @@ -318,7 +318,7 @@ static void mtk_eint_irq_handler(struct irq_desc *desc) struct irq_chip *chip = irq_desc_get_chip(desc); struct mtk_eint *eint = irq_desc_get_handler_data(desc); unsigned int status, eint_num; - int offset, index, virq; + int offset, mask_offset, index, virq; void __iomem *reg = mtk_eint_get_offset(eint, 0, eint->regs->stat); int dual_edge, start_level, curr_level; @@ -328,10 +328,24 @@ static void mtk_eint_irq_handler(struct irq_desc *desc) status = readl(reg); while (status) { offset = __ffs(status); + mask_offset = eint_num >> 5; index = eint_num + offset; virq = irq_find_mapping(eint->domain, index); status &= ~BIT(offset); + /* + * If we get an interrupt on pin that was only required + * for wake (but no real interrupt requested), mask the + * interrupt (as would mtk_eint_resume do anyway later + * in the resume sequence). + */ + if (eint->wake_mask[mask_offset] & BIT(offset) && + !(eint->cur_mask[mask_offset] & BIT(offset))) { + writel_relaxed(BIT(offset), reg - + eint->regs->stat + + eint->regs->mask_set); + } + dual_edge = eint->dual_edge[index]; if (dual_edge) { /* From 9d957a959bc8c3dfe37572ac8e99affb5a885965 Mon Sep 17 00:00:00 2001 From: Nicolas Boichat Date: Wed, 26 Jun 2019 11:54:45 +0800 Subject: [PATCH 5/5] pinctrl: mediatek: Update cur_mask in mask/mask ops During suspend/resume, mtk_eint_mask may be called while wake_mask is active. For example, this happens if a wake-source with an active interrupt handler wakes the system: irq/pm.c:irq_pm_check_wakeup would disable the interrupt, so that it can be handled later on in the resume flow. However, this may happen before mtk_eint_do_resume is called: in this case, wake_mask is loaded, and cur_mask is restored from an older copy, re-enabling the interrupt, and causing an interrupt storm (especially for level interrupts). Step by step, for a line that has both wake and interrupt enabled: 1. cur_mask[irq] = 1; wake_mask[irq] = 1; EINT_EN[irq] = 1 (interrupt enabled at hardware level) 2. System suspends, resumes due to that line (at this stage EINT_EN == wake_mask) 3. irq_pm_check_wakeup is called, and disables the interrupt => EINT_EN[irq] = 0, but we still have cur_mask[irq] = 1 4. mtk_eint_do_resume is called, and restores EINT_EN = cur_mask, so it reenables EINT_EN[irq] = 1 => interrupt storm as the driver is not yet ready to handle the interrupt. This patch fixes the issue in step 3, by recording all mask/unmask changes in cur_mask. This also avoids the need to read the current mask in eint_do_suspend, and we can remove mtk_eint_chip_read_mask function. The interrupt will be re-enabled properly later on, sometimes after mtk_eint_do_resume, when the driver is ready to handle it. Fixes: 58a5e1b64bb0 ("pinctrl: mediatek: Implement wake handler and suspend resume") Signed-off-by: Nicolas Boichat Acked-by: Sean Wang Signed-off-by: Linus Walleij --- drivers/pinctrl/mediatek/mtk-eint.c | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/drivers/pinctrl/mediatek/mtk-eint.c b/drivers/pinctrl/mediatek/mtk-eint.c index 737385e86beb..7e526bcf5e0b 100644 --- a/drivers/pinctrl/mediatek/mtk-eint.c +++ b/drivers/pinctrl/mediatek/mtk-eint.c @@ -113,6 +113,8 @@ static void mtk_eint_mask(struct irq_data *d) void __iomem *reg = mtk_eint_get_offset(eint, d->hwirq, eint->regs->mask_set); + eint->cur_mask[d->hwirq >> 5] &= ~mask; + writel(mask, reg); } @@ -123,6 +125,8 @@ static void mtk_eint_unmask(struct irq_data *d) void __iomem *reg = mtk_eint_get_offset(eint, d->hwirq, eint->regs->mask_clr); + eint->cur_mask[d->hwirq >> 5] |= mask; + writel(mask, reg); if (eint->dual_edge[d->hwirq]) @@ -217,19 +221,6 @@ static void mtk_eint_chip_write_mask(const struct mtk_eint *eint, } } -static void mtk_eint_chip_read_mask(const struct mtk_eint *eint, - void __iomem *base, u32 *buf) -{ - int port; - void __iomem *reg; - - for (port = 0; port < eint->hw->ports; port++) { - reg = base + eint->regs->mask + (port << 2); - buf[port] = ~readl_relaxed(reg); - /* Mask is 0 when irq is enabled, and 1 when disabled. */ - } -} - static int mtk_eint_irq_request_resources(struct irq_data *d) { struct mtk_eint *eint = irq_data_get_irq_chip_data(d); @@ -384,7 +375,6 @@ static void mtk_eint_irq_handler(struct irq_desc *desc) int mtk_eint_do_suspend(struct mtk_eint *eint) { - mtk_eint_chip_read_mask(eint, eint->base, eint->cur_mask); mtk_eint_chip_write_mask(eint, eint->base, eint->wake_mask); return 0;