Use the bank name as the irqchip name. This name is later visible in /proc/interrupts, what makes it possible to easily identify each GPIO interrupt. /proc/interrupts before this patch: 143: 0 exynos4210_wkup_irq_chip 7 Edge hdmi 144: 0 exynos4210_wkup_irq_chip 6 Level wm8994 145: 1 exynos4210_wkup_irq_chip 7 Edge max77686-pmic, max77686-rtc 146: 1 exynos_gpio_irq_chip 3 Edge 3-0048 /proc/interrupts after this patch: 143: 0 gpx3 7 Edge hdmi 144: 0 gpx3 6 Level wm8994 145: 1 gpx0 7 Edge max77686-pmic, max77686-rtc 146: 1 gpm2 3 Edge 3-0048 Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- drivers/pinctrl/samsung/pinctrl-exynos.c | 27 +++++++++++++++--------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c index 84501c785473..1c87cf41602a 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos.c +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c @@ -207,7 +207,7 @@ static void exynos_irq_release_resources(struct irq_data *irqd) /* * irq_chip for gpio interrupts. */ -static struct exynos_irq_chip exynos_gpio_irq_chip = { +static const struct exynos_irq_chip exynos_gpio_irq_chip __initconst = { .chip = { .name = "exynos_gpio_irq_chip", .irq_unmask = exynos_irq_unmask, @@ -313,7 +313,13 @@ int exynos_eint_gpio_init(struct samsung_pinctrl_drv_data *d) goto err_domains; } - bank->irq_chip = &exynos_gpio_irq_chip; + bank->irq_chip = kmemdup(&exynos_gpio_irq_chip, + sizeof(*bank->irq_chip), GFP_KERNEL); + if (!bank->irq_chip) { + ret = -ENOMEM; + goto err_domains; + } + bank->irq_chip->chip.name = bank->name; } return 0; @@ -521,7 +527,7 @@ int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d) struct samsung_pin_bank *bank; struct exynos_weint_data *weint_data; struct exynos_muxed_weint_data *muxed_data; - struct exynos_irq_chip *irq_chip; + const struct exynos_irq_chip *irq_chip; unsigned int muxed_banks = 0; unsigned int i; int idx, irq; @@ -531,12 +537,7 @@ int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d) match = of_match_node(exynos_wkup_irq_ids, np); if (match) { - irq_chip = kmemdup(match->data, - sizeof(*irq_chip), GFP_KERNEL); - if (!irq_chip) { - of_node_put(np); - return -ENOMEM; - } + irq_chip = match->data; wkup_np = np; break; } @@ -557,7 +558,13 @@ int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d) return -ENXIO; } - bank->irq_chip = irq_chip; + bank->irq_chip = kmemdup(irq_chip, sizeof(*irq_chip), + GFP_KERNEL); + if (!bank->irq_chip) { + of_node_put(wkup_np); + return -ENOMEM; + } + bank->irq_chip->chip.name = bank->name; if (!of_find_property(bank->of_node, "interrupts", NULL)) { bank->eint_type = EINT_TYPE_WKUP_MUX; -- 2.17.1
[-- Attachment #1: Type: text/plain, Size: 1711 bytes --] Hi Marek, I love your patch! Perhaps something to improve: [auto build test WARNING on pinctrl-samsung/for-next] [also build test WARNING on pinctrl/devel v5.7] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Marek-Szyprowski/pinctrl-samsung-Use-bank-name-as-irqchip-name/20200609-162531 base: https://git.kernel.org/pub/scm/linux/kernel/git/pinctrl/samsung.git for-next config: arm64-randconfig-r004-20200607 (attached as .config) compiler: aarch64-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>, old ones prefixed by <<): >> WARNING: modpost: vmlinux.o(.text+0x997f04): Section mismatch in reference from the function exynos_eint_gpio_init() to the variable .init.rodata:exynos_gpio_irq_chip The function exynos_eint_gpio_init() references the variable __initconst exynos_gpio_irq_chip. This is often because exynos_eint_gpio_init lacks a __initconst annotation or the annotation of exynos_gpio_irq_chip is wrong. --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 39831 bytes --]
On Tue, Jun 09, 2020 at 10:23:29AM +0200, Marek Szyprowski wrote: > Use the bank name as the irqchip name. This name is later visible in > /proc/interrupts, what makes it possible to easily identify each > GPIO interrupt. > > /proc/interrupts before this patch: > 143: 0 exynos4210_wkup_irq_chip 7 Edge hdmi > 144: 0 exynos4210_wkup_irq_chip 6 Level wm8994 > 145: 1 exynos4210_wkup_irq_chip 7 Edge max77686-pmic, max77686-rtc > 146: 1 exynos_gpio_irq_chip 3 Edge 3-0048 > > /proc/interrupts after this patch: > 143: 0 gpx3 7 Edge hdmi > 144: 0 gpx3 6 Level wm8994 > 145: 1 gpx0 7 Edge max77686-pmic, max77686-rtc > 146: 1 gpm2 3 Edge 3-0048 > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> Hi Marek, Nice idea! > --- > drivers/pinctrl/samsung/pinctrl-exynos.c | 27 +++++++++++++++--------- > 1 file changed, 17 insertions(+), 10 deletions(-) > > diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c > index 84501c785473..1c87cf41602a 100644 > --- a/drivers/pinctrl/samsung/pinctrl-exynos.c > +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c > @@ -207,7 +207,7 @@ static void exynos_irq_release_resources(struct irq_data *irqd) > /* > * irq_chip for gpio interrupts. > */ > -static struct exynos_irq_chip exynos_gpio_irq_chip = { > +static const struct exynos_irq_chip exynos_gpio_irq_chip __initconst = { > .chip = { > .name = "exynos_gpio_irq_chip", > .irq_unmask = exynos_irq_unmask, > @@ -313,7 +313,13 @@ int exynos_eint_gpio_init(struct samsung_pinctrl_drv_data *d) > goto err_domains; > } > > - bank->irq_chip = &exynos_gpio_irq_chip; > + bank->irq_chip = kmemdup(&exynos_gpio_irq_chip, > + sizeof(*bank->irq_chip), GFP_KERNEL); You cannot reference initconst memory from non-init function. Build with SECTION_MISMATCH to see the warnings. > + if (!bank->irq_chip) { irq_domain_remove() > + ret = -ENOMEM; > + goto err_domains; > + } > + bank->irq_chip->chip.name = bank->name; > } > > return 0; > @@ -521,7 +527,7 @@ int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d) > struct samsung_pin_bank *bank; > struct exynos_weint_data *weint_data; > struct exynos_muxed_weint_data *muxed_data; > - struct exynos_irq_chip *irq_chip; > + const struct exynos_irq_chip *irq_chip; > unsigned int muxed_banks = 0; > unsigned int i; > int idx, irq; > @@ -531,12 +537,7 @@ int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d) > > match = of_match_node(exynos_wkup_irq_ids, np); > if (match) { > - irq_chip = kmemdup(match->data, > - sizeof(*irq_chip), GFP_KERNEL); > - if (!irq_chip) { > - of_node_put(np); > - return -ENOMEM; > - } > + irq_chip = match->data; > wkup_np = np; > break; > } > @@ -557,7 +558,13 @@ int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d) > return -ENXIO; > } > > - bank->irq_chip = irq_chip; > + bank->irq_chip = kmemdup(irq_chip, sizeof(*irq_chip), > + GFP_KERNEL); > + if (!bank->irq_chip) { > + of_node_put(wkup_np); irq_domain_remove() Best regards, Krzysztof > + return -ENOMEM; > + } > + bank->irq_chip->chip.name = bank->name; > > if (!of_find_property(bank->of_node, "interrupts", NULL)) { > bank->eint_type = EINT_TYPE_WKUP_MUX; > -- > 2.17.1 >
From: Marek Szyprowski <m.szyprowski@samsung.com> Use the bank name as the irqchip name. This name is later visible in /proc/interrupts, what makes it possible to easily identify each GPIO interrupt. /proc/interrupts before this patch: 143: 0 exynos4210_wkup_irq_chip 7 Edge hdmi 144: 0 exynos4210_wkup_irq_chip 6 Level wm8994 145: 1 exynos4210_wkup_irq_chip 7 Edge max77686-pmic, max77686-rtc 146: 1 exynos_gpio_irq_chip 3 Edge 3-0048 /proc/interrupts after this patch: 143: 0 gpx3 7 Edge hdmi 144: 0 gpx3 6 Level wm8994 145: 1 gpx0 7 Edge max77686-pmic, max77686-rtc 146: 1 gpm2 3 Edge 3-0048 Handling of the eint_wake_mask_value has been reworked, because each bank has now its own exynos_irq_chip structure allocated. Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> --- Hi Linus, I have only one patch queued for Samsung pinctrl. Can you apply it directly? Best regards, Krzysztof --- drivers/pinctrl/samsung/pinctrl-exynos.c | 58 +++++++++++++----------- 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c index 84501c785473..b9ea09fabf84 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos.c +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c @@ -38,7 +38,7 @@ struct exynos_irq_chip { u32 eint_con; u32 eint_mask; u32 eint_pend; - u32 eint_wake_mask_value; + u32 *eint_wake_mask_value; u32 eint_wake_mask_reg; void (*set_eint_wakeup_mask)(struct samsung_pinctrl_drv_data *drvdata, struct exynos_irq_chip *irq_chip); @@ -207,7 +207,7 @@ static void exynos_irq_release_resources(struct irq_data *irqd) /* * irq_chip for gpio interrupts. */ -static struct exynos_irq_chip exynos_gpio_irq_chip = { +static const struct exynos_irq_chip exynos_gpio_irq_chip __initconst = { .chip = { .name = "exynos_gpio_irq_chip", .irq_unmask = exynos_irq_unmask, @@ -274,7 +274,7 @@ struct exynos_eint_gpio_save { * exynos_eint_gpio_init() - setup handling of external gpio interrupts. * @d: driver data of samsung pinctrl driver. */ -int exynos_eint_gpio_init(struct samsung_pinctrl_drv_data *d) +__init int exynos_eint_gpio_init(struct samsung_pinctrl_drv_data *d) { struct samsung_pin_bank *bank; struct device *dev = d->dev; @@ -297,6 +297,15 @@ int exynos_eint_gpio_init(struct samsung_pinctrl_drv_data *d) for (i = 0; i < d->nr_banks; ++i, ++bank) { if (bank->eint_type != EINT_TYPE_GPIO) continue; + + bank->irq_chip = devm_kmemdup(dev, &exynos_gpio_irq_chip, + sizeof(*bank->irq_chip), GFP_KERNEL); + if (!bank->irq_chip) { + ret = -ENOMEM; + goto err_domains; + } + bank->irq_chip->chip.name = bank->name; + bank->irq_domain = irq_domain_add_linear(bank->of_node, bank->nr_pins, &exynos_eint_irqd_ops, bank); if (!bank->irq_domain) { @@ -313,7 +322,6 @@ int exynos_eint_gpio_init(struct samsung_pinctrl_drv_data *d) goto err_domains; } - bank->irq_chip = &exynos_gpio_irq_chip; } return 0; @@ -338,9 +346,9 @@ static int exynos_wkup_irq_set_wake(struct irq_data *irqd, unsigned int on) pr_info("wake %s for irq %d\n", on ? "enabled" : "disabled", irqd->irq); if (!on) - our_chip->eint_wake_mask_value |= bit; + *our_chip->eint_wake_mask_value |= bit; else - our_chip->eint_wake_mask_value &= ~bit; + *our_chip->eint_wake_mask_value &= ~bit; return 0; } @@ -360,10 +368,10 @@ exynos_pinctrl_set_eint_wakeup_mask(struct samsung_pinctrl_drv_data *drvdata, pmu_regs = drvdata->retention_ctrl->priv; dev_info(drvdata->dev, "Setting external wakeup interrupt mask: 0x%x\n", - irq_chip->eint_wake_mask_value); + *irq_chip->eint_wake_mask_value); regmap_write(pmu_regs, irq_chip->eint_wake_mask_reg, - irq_chip->eint_wake_mask_value); + *irq_chip->eint_wake_mask_value); } static void @@ -382,10 +390,11 @@ s5pv210_pinctrl_set_eint_wakeup_mask(struct samsung_pinctrl_drv_data *drvdata, clk_base = (void __iomem *) drvdata->retention_ctrl->priv; - __raw_writel(irq_chip->eint_wake_mask_value, + __raw_writel(*irq_chip->eint_wake_mask_value, clk_base + irq_chip->eint_wake_mask_reg); } +static u32 eint_wake_mask_value = EXYNOS_EINT_WAKEUP_MASK_DISABLED; /* * irq_chip for wakeup interrupts */ @@ -403,7 +412,7 @@ static const struct exynos_irq_chip s5pv210_wkup_irq_chip __initconst = { .eint_con = EXYNOS_WKUP_ECON_OFFSET, .eint_mask = EXYNOS_WKUP_EMASK_OFFSET, .eint_pend = EXYNOS_WKUP_EPEND_OFFSET, - .eint_wake_mask_value = EXYNOS_EINT_WAKEUP_MASK_DISABLED, + .eint_wake_mask_value = &eint_wake_mask_value, /* Only differences with exynos4210_wkup_irq_chip: */ .eint_wake_mask_reg = S5PV210_EINT_WAKEUP_MASK, .set_eint_wakeup_mask = s5pv210_pinctrl_set_eint_wakeup_mask, @@ -423,7 +432,7 @@ static const struct exynos_irq_chip exynos4210_wkup_irq_chip __initconst = { .eint_con = EXYNOS_WKUP_ECON_OFFSET, .eint_mask = EXYNOS_WKUP_EMASK_OFFSET, .eint_pend = EXYNOS_WKUP_EPEND_OFFSET, - .eint_wake_mask_value = EXYNOS_EINT_WAKEUP_MASK_DISABLED, + .eint_wake_mask_value = &eint_wake_mask_value, .eint_wake_mask_reg = EXYNOS_EINT_WAKEUP_MASK, .set_eint_wakeup_mask = exynos_pinctrl_set_eint_wakeup_mask, }; @@ -442,7 +451,7 @@ static const struct exynos_irq_chip exynos7_wkup_irq_chip __initconst = { .eint_con = EXYNOS7_WKUP_ECON_OFFSET, .eint_mask = EXYNOS7_WKUP_EMASK_OFFSET, .eint_pend = EXYNOS7_WKUP_EPEND_OFFSET, - .eint_wake_mask_value = EXYNOS_EINT_WAKEUP_MASK_DISABLED, + .eint_wake_mask_value = &eint_wake_mask_value, .eint_wake_mask_reg = EXYNOS5433_EINT_WAKEUP_MASK, .set_eint_wakeup_mask = exynos_pinctrl_set_eint_wakeup_mask, }; @@ -513,7 +522,7 @@ static void exynos_irq_demux_eint16_31(struct irq_desc *desc) * exynos_eint_wkup_init() - setup handling of external wakeup interrupts. * @d: driver data of samsung pinctrl driver. */ -int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d) +__init int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d) { struct device *dev = d->dev; struct device_node *wkup_np = NULL; @@ -521,7 +530,7 @@ int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d) struct samsung_pin_bank *bank; struct exynos_weint_data *weint_data; struct exynos_muxed_weint_data *muxed_data; - struct exynos_irq_chip *irq_chip; + const struct exynos_irq_chip *irq_chip; unsigned int muxed_banks = 0; unsigned int i; int idx, irq; @@ -531,12 +540,7 @@ int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d) match = of_match_node(exynos_wkup_irq_ids, np); if (match) { - irq_chip = kmemdup(match->data, - sizeof(*irq_chip), GFP_KERNEL); - if (!irq_chip) { - of_node_put(np); - return -ENOMEM; - } + irq_chip = match->data; wkup_np = np; break; } @@ -549,6 +553,14 @@ int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d) if (bank->eint_type != EINT_TYPE_WKUP) continue; + bank->irq_chip = devm_kmemdup(dev, irq_chip, sizeof(*irq_chip), + GFP_KERNEL); + if (!bank->irq_chip) { + of_node_put(wkup_np); + return -ENOMEM; + } + bank->irq_chip->chip.name = bank->name; + bank->irq_domain = irq_domain_add_linear(bank->of_node, bank->nr_pins, &exynos_eint_irqd_ops, bank); if (!bank->irq_domain) { @@ -557,8 +569,6 @@ int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d) return -ENXIO; } - bank->irq_chip = irq_chip; - if (!of_find_property(bank->of_node, "interrupts", NULL)) { bank->eint_type = EINT_TYPE_WKUP_MUX; ++muxed_banks; @@ -657,10 +667,6 @@ void exynos_pinctrl_suspend(struct samsung_pinctrl_drv_data *drvdata) irq_chip = bank->irq_chip; irq_chip->set_eint_wakeup_mask(drvdata, irq_chip); - } else if (bank->irq_chip != irq_chip) { - dev_warn(drvdata->dev, - "More than one external wakeup interrupt chip configured (bank: %s). This is not supported by hardware nor by driver.\n", - bank->name); } } } -- 2.17.1
On Mon, Jul 20, 2020 at 4:54 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> From: Marek Szyprowski <m.szyprowski@samsung.com>
>
> Use the bank name as the irqchip name. This name is later visible in
> /proc/interrupts, what makes it possible to easily identify each
> GPIO interrupt.
>
> /proc/interrupts before this patch:
> 143: 0 exynos4210_wkup_irq_chip 7 Edge hdmi
> 144: 0 exynos4210_wkup_irq_chip 6 Level wm8994
> 145: 1 exynos4210_wkup_irq_chip 7 Edge max77686-pmic, max77686-rtc
> 146: 1 exynos_gpio_irq_chip 3 Edge 3-0048
>
> /proc/interrupts after this patch:
> 143: 0 gpx3 7 Edge hdmi
> 144: 0 gpx3 6 Level wm8994
> 145: 1 gpx0 7 Edge max77686-pmic, max77686-rtc
> 146: 1 gpm2 3 Edge 3-0048
>
> Handling of the eint_wake_mask_value has been reworked, because each bank
> has now its own exynos_irq_chip structure allocated.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
>
> Hi Linus,
>
> I have only one patch queued for Samsung pinctrl. Can you apply it
> directly?
OK patch applied!
Yours,
Linus Walleij