All of lore.kernel.org
 help / color / mirror / Atom feed
* [gpio:devel 12/12] drivers/pinctrl/mvebu/pinctrl-armada-37xx.c:630:27: error: 'struct gpio_chip' has no member named 'irq_base'; did you mean 'base'?
@ 2017-10-15  1:52 kbuild test robot
  2017-10-15 14:21 ` Linus Walleij
  0 siblings, 1 reply; 10+ messages in thread
From: kbuild test robot @ 2017-10-15  1:52 UTC (permalink / raw)
  To: Grygorii Strashko; +Cc: kbuild-all, linux-gpio, Linus Walleij

[-- Attachment #1: Type: text/plain, Size: 2722 bytes --]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git devel
head:   be58c675d634e0b91edc0f721c77303ac2afa809
commit: be58c675d634e0b91edc0f721c77303ac2afa809 [12/12] gpiolib: drop irq_base field from gpio_chip struct
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        git checkout be58c675d634e0b91edc0f721c77303ac2afa809
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

   drivers/pinctrl/mvebu/pinctrl-armada-37xx.c: In function 'armada_37xx_irq_startup':
>> drivers/pinctrl/mvebu/pinctrl-armada-37xx.c:630:27: error: 'struct gpio_chip' has no member named 'irq_base'; did you mean 'base'?
     int irq = d->hwirq - chip->irq_base;
                              ^~

vim +630 drivers/pinctrl/mvebu/pinctrl-armada-37xx.c

2f2276053 Gregory CLEMENT 2017-04-28  626  
a9a1a4833 Gregory CLEMENT 2017-09-07  627  static unsigned int armada_37xx_irq_startup(struct irq_data *d)
a9a1a4833 Gregory CLEMENT 2017-09-07  628  {
a9a1a4833 Gregory CLEMENT 2017-09-07  629  	struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
a9a1a4833 Gregory CLEMENT 2017-09-07 @630  	int irq = d->hwirq - chip->irq_base;
a9a1a4833 Gregory CLEMENT 2017-09-07  631  	/*
a9a1a4833 Gregory CLEMENT 2017-09-07  632  	 * The mask field is a "precomputed bitmask for accessing the
a9a1a4833 Gregory CLEMENT 2017-09-07  633  	 * chip registers" which was introduced for the generic
a9a1a4833 Gregory CLEMENT 2017-09-07  634  	 * irqchip framework. As we don't use this framework, we can
a9a1a4833 Gregory CLEMENT 2017-09-07  635  	 * reuse this field for our own usage.
a9a1a4833 Gregory CLEMENT 2017-09-07  636  	 */
a9a1a4833 Gregory CLEMENT 2017-09-07  637  	d->mask = BIT(irq % GPIO_PER_REG);
a9a1a4833 Gregory CLEMENT 2017-09-07  638  
a9a1a4833 Gregory CLEMENT 2017-09-07  639  	armada_37xx_irq_unmask(d);
a9a1a4833 Gregory CLEMENT 2017-09-07  640  
a9a1a4833 Gregory CLEMENT 2017-09-07  641  	return 0;
a9a1a4833 Gregory CLEMENT 2017-09-07  642  }
a9a1a4833 Gregory CLEMENT 2017-09-07  643  

:::::: The code at line 630 was first introduced by commit
:::::: a9a1a4833613b8a2e8dd79f860ea1871f17da02c pinctrl: armada-37xx: Fix gpio interrupt setup

:::::: TO: Gregory CLEMENT <gregory.clement@free-electrons.com>
:::::: CC: Linus Walleij <linus.walleij@linaro.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36865 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [gpio:devel 12/12] drivers/pinctrl/mvebu/pinctrl-armada-37xx.c:630:27: error: 'struct gpio_chip' has no member named 'irq_base'; did you mean 'base'?
  2017-10-15  1:52 [gpio:devel 12/12] drivers/pinctrl/mvebu/pinctrl-armada-37xx.c:630:27: error: 'struct gpio_chip' has no member named 'irq_base'; did you mean 'base'? kbuild test robot
@ 2017-10-15 14:21 ` Linus Walleij
  2017-10-16  8:05   ` Thierry Reding
  0 siblings, 1 reply; 10+ messages in thread
From: Linus Walleij @ 2017-10-15 14:21 UTC (permalink / raw)
  To: kbuild test robot, Grygorii Strashko; +Cc: kbuild-all, linux-gpio

Grygorii, can you look at this?

Linus

On Sun, Oct 15, 2017 at 3:52 AM, kbuild test robot
<fengguang.wu@intel.com> wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git devel
> head:   be58c675d634e0b91edc0f721c77303ac2afa809
> commit: be58c675d634e0b91edc0f721c77303ac2afa809 [12/12] gpiolib: drop irq_base field from gpio_chip struct
> config: arm64-defconfig (attached as .config)
> compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         git checkout be58c675d634e0b91edc0f721c77303ac2afa809
>         # save the attached .config to linux build tree
>         make.cross ARCH=arm64
>
> All errors (new ones prefixed by >>):
>
>    drivers/pinctrl/mvebu/pinctrl-armada-37xx.c: In function 'armada_37xx_irq_startup':
>>> drivers/pinctrl/mvebu/pinctrl-armada-37xx.c:630:27: error: 'struct gpio_chip' has no member named 'irq_base'; did you mean 'base'?
>      int irq = d->hwirq - chip->irq_base;
>                               ^~
>
> vim +630 drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
>
> 2f2276053 Gregory CLEMENT 2017-04-28  626
> a9a1a4833 Gregory CLEMENT 2017-09-07  627  static unsigned int armada_37xx_irq_startup(struct irq_data *d)
> a9a1a4833 Gregory CLEMENT 2017-09-07  628  {
> a9a1a4833 Gregory CLEMENT 2017-09-07  629       struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
> a9a1a4833 Gregory CLEMENT 2017-09-07 @630       int irq = d->hwirq - chip->irq_base;
> a9a1a4833 Gregory CLEMENT 2017-09-07  631       /*
> a9a1a4833 Gregory CLEMENT 2017-09-07  632        * The mask field is a "precomputed bitmask for accessing the
> a9a1a4833 Gregory CLEMENT 2017-09-07  633        * chip registers" which was introduced for the generic
> a9a1a4833 Gregory CLEMENT 2017-09-07  634        * irqchip framework. As we don't use this framework, we can
> a9a1a4833 Gregory CLEMENT 2017-09-07  635        * reuse this field for our own usage.
> a9a1a4833 Gregory CLEMENT 2017-09-07  636        */
> a9a1a4833 Gregory CLEMENT 2017-09-07  637       d->mask = BIT(irq % GPIO_PER_REG);
> a9a1a4833 Gregory CLEMENT 2017-09-07  638
> a9a1a4833 Gregory CLEMENT 2017-09-07  639       armada_37xx_irq_unmask(d);
> a9a1a4833 Gregory CLEMENT 2017-09-07  640
> a9a1a4833 Gregory CLEMENT 2017-09-07  641       return 0;
> a9a1a4833 Gregory CLEMENT 2017-09-07  642  }
> a9a1a4833 Gregory CLEMENT 2017-09-07  643
>
> :::::: The code at line 630 was first introduced by commit
> :::::: a9a1a4833613b8a2e8dd79f860ea1871f17da02c pinctrl: armada-37xx: Fix gpio interrupt setup
>
> :::::: TO: Gregory CLEMENT <gregory.clement@free-electrons.com>
> :::::: CC: Linus Walleij <linus.walleij@linaro.org>
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [gpio:devel 12/12] drivers/pinctrl/mvebu/pinctrl-armada-37xx.c:630:27: error: 'struct gpio_chip' has no member named 'irq_base'; did you mean 'base'?
  2017-10-15 14:21 ` Linus Walleij
@ 2017-10-16  8:05   ` Thierry Reding
  2017-10-16  8:08     ` Thierry Reding
  0 siblings, 1 reply; 10+ messages in thread
From: Thierry Reding @ 2017-10-16  8:05 UTC (permalink / raw)
  To: Linus Walleij
  Cc: kbuild test robot, Grygorii Strashko, kbuild-all, linux-gpio

[-- Attachment #1: Type: text/plain, Size: 2588 bytes --]

On Sun, Oct 15, 2017 at 04:21:14PM +0200, Linus Walleij wrote:
> Grygorii, can you look at this?
> 
> Linus
> 
> On Sun, Oct 15, 2017 at 3:52 AM, kbuild test robot
> <fengguang.wu@intel.com> wrote:
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git devel
> > head:   be58c675d634e0b91edc0f721c77303ac2afa809
> > commit: be58c675d634e0b91edc0f721c77303ac2afa809 [12/12] gpiolib: drop irq_base field from gpio_chip struct
> > config: arm64-defconfig (attached as .config)
> > compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
> > reproduce:
> >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         git checkout be58c675d634e0b91edc0f721c77303ac2afa809
> >         # save the attached .config to linux build tree
> >         make.cross ARCH=arm64
> >
> > All errors (new ones prefixed by >>):
> >
> >    drivers/pinctrl/mvebu/pinctrl-armada-37xx.c: In function 'armada_37xx_irq_startup':
> >>> drivers/pinctrl/mvebu/pinctrl-armada-37xx.c:630:27: error: 'struct gpio_chip' has no member named 'irq_base'; did you mean 'base'?
> >      int irq = d->hwirq - chip->irq_base;

This looks like something you'd get if you merge my struct
gpio_irq_chip patches. However, the patch in my series will do the
conversion of the pinctrl-armada-37xx driver:

--- >8 ---
$ git log -p -- drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
commit 96e359456c942969fc3f0faa277fb27d1a830692
Author: Thierry Reding <treding@nvidia.com>
Date:   Mon Apr 3 11:40:03 2017 +0200

    gpio: Move irq_base to struct gpio_irq_chip

    In order to consolidate the multiple ways to associate an IRQ chip with
    a GPIO chip, move more fields into the new struct gpio_irq_chip.

    Signed-off-by: Thierry Reding <treding@nvidia.com>

diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
index c9851bd120b4..500238d898ea 100644
--- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
+++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
@@ -627,7 +627,7 @@ static void armada_37xx_irq_handler(struct irq_desc *desc)
 static unsigned int armada_37xx_irq_startup(struct irq_data *d)
 {
        struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
-       int irq = d->hwirq - chip->irq_base;
+       int irq = d->hwirq - chip->irq.first;
        /*
         * The mask field is a "precomputed bitmask for accessing the
         * chip registers" which was introduced for the generic

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [gpio:devel 12/12] drivers/pinctrl/mvebu/pinctrl-armada-37xx.c:630:27: error: 'struct gpio_chip' has no member named 'irq_base'; did you mean 'base'?
  2017-10-16  8:05   ` Thierry Reding
@ 2017-10-16  8:08     ` Thierry Reding
  2017-10-16 11:50       ` Linus Walleij
  0 siblings, 1 reply; 10+ messages in thread
From: Thierry Reding @ 2017-10-16  8:08 UTC (permalink / raw)
  To: Linus Walleij
  Cc: kbuild test robot, Grygorii Strashko, kbuild-all, linux-gpio

[-- Attachment #1: Type: text/plain, Size: 2883 bytes --]

On Mon, Oct 16, 2017 at 10:05:02AM +0200, Thierry Reding wrote:
> On Sun, Oct 15, 2017 at 04:21:14PM +0200, Linus Walleij wrote:
> > Grygorii, can you look at this?
> > 
> > Linus
> > 
> > On Sun, Oct 15, 2017 at 3:52 AM, kbuild test robot
> > <fengguang.wu@intel.com> wrote:
> > > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git devel
> > > head:   be58c675d634e0b91edc0f721c77303ac2afa809
> > > commit: be58c675d634e0b91edc0f721c77303ac2afa809 [12/12] gpiolib: drop irq_base field from gpio_chip struct
> > > config: arm64-defconfig (attached as .config)
> > > compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
> > > reproduce:
> > >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > >         chmod +x ~/bin/make.cross
> > >         git checkout be58c675d634e0b91edc0f721c77303ac2afa809
> > >         # save the attached .config to linux build tree
> > >         make.cross ARCH=arm64
> > >
> > > All errors (new ones prefixed by >>):
> > >
> > >    drivers/pinctrl/mvebu/pinctrl-armada-37xx.c: In function 'armada_37xx_irq_startup':
> > >>> drivers/pinctrl/mvebu/pinctrl-armada-37xx.c:630:27: error: 'struct gpio_chip' has no member named 'irq_base'; did you mean 'base'?
> > >      int irq = d->hwirq - chip->irq_base;
> 
> This looks like something you'd get if you merge my struct
> gpio_irq_chip patches. However, the patch in my series will do the
> conversion of the pinctrl-armada-37xx driver:
> 
> --- >8 ---
> $ git log -p -- drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> commit 96e359456c942969fc3f0faa277fb27d1a830692
> Author: Thierry Reding <treding@nvidia.com>
> Date:   Mon Apr 3 11:40:03 2017 +0200
> 
>     gpio: Move irq_base to struct gpio_irq_chip
> 
>     In order to consolidate the multiple ways to associate an IRQ chip with
>     a GPIO chip, move more fields into the new struct gpio_irq_chip.
> 
>     Signed-off-by: Thierry Reding <treding@nvidia.com>
> 
> diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> index c9851bd120b4..500238d898ea 100644
> --- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> +++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> @@ -627,7 +627,7 @@ static void armada_37xx_irq_handler(struct irq_desc *desc)
>  static unsigned int armada_37xx_irq_startup(struct irq_data *d)
>  {
>         struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
> -       int irq = d->hwirq - chip->irq_base;
> +       int irq = d->hwirq - chip->irq.first;
>         /*
>          * The mask field is a "precomputed bitmask for accessing the
>          * chip registers" which was introduced for the generic

Nevermind, I see there is now yet another conflict that would require
yet another rebase of that series.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [gpio:devel 12/12] drivers/pinctrl/mvebu/pinctrl-armada-37xx.c:630:27: error: 'struct gpio_chip' has no member named 'irq_base'; did you mean 'base'?
  2017-10-16  8:08     ` Thierry Reding
@ 2017-10-16 11:50       ` Linus Walleij
  2017-10-16 12:46         ` Thierry Reding
  0 siblings, 1 reply; 10+ messages in thread
From: Linus Walleij @ 2017-10-16 11:50 UTC (permalink / raw)
  To: Thierry Reding
  Cc: kbuild test robot, Grygorii Strashko, kbuild-all, linux-gpio

On Mon, Oct 16, 2017 at 10:08 AM, Thierry Reding
<thierry.reding@gmail.com> wrote:

> Nevermind, I see there is now yet another conflict that would require
> yet another rebase of that series.

Sorry about this :(

It sucks with moving targets, there was some fallout from Grygorii's patch
to map IRQs dynamically and remove irq_base so we need to clean that
up first.

This particular bug from the autobuilder seems to be constrained to the
armada driver though.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [gpio:devel 12/12] drivers/pinctrl/mvebu/pinctrl-armada-37xx.c:630:27: error: 'struct gpio_chip' has no member named 'irq_base'; did you mean 'base'?
  2017-10-16 11:50       ` Linus Walleij
@ 2017-10-16 12:46         ` Thierry Reding
  2017-10-16 14:53           ` Gregory CLEMENT
  2017-10-19 13:17           ` Linus Walleij
  0 siblings, 2 replies; 10+ messages in thread
From: Thierry Reding @ 2017-10-16 12:46 UTC (permalink / raw)
  To: Linus Walleij
  Cc: kbuild test robot, Grygorii Strashko, Gregory CLEMENT,
	kbuild-all, linux-gpio

[-- Attachment #1: Type: text/plain, Size: 3168 bytes --]

On Mon, Oct 16, 2017 at 01:50:08PM +0200, Linus Walleij wrote:
> On Mon, Oct 16, 2017 at 10:08 AM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
> 
> > Nevermind, I see there is now yet another conflict that would require
> > yet another rebase of that series.
> 
> Sorry about this :(
> 
> It sucks with moving targets, there was some fallout from Grygorii's patch
> to map IRQs dynamically and remove irq_base so we need to clean that
> up first.
> 
> This particular bug from the autobuilder seems to be constrained to the
> armada driver though.

Well, the problem here is that Grygorii's patch removes the field but
with there still being one user left. So we'd have to make sure that the
real last user goes away before that patch is applied.

Looking at the code it seems a little phony. d->hwirq - chip->irq_base
seems like a very risky thing to do because you never really know what
irq_base will end up being (in the general case where it's dynamically
allocated). But the driver passes 0 to gpiochip_irqchip_add() as the
first_irq parameter, therefore the operation becomes a noop and so it
should be completely safe to remove that line.

Something like the below should do. Cc += Gregory.

Thierry

--- >8 ---
From fcd87329cf4edf6a6f5d491a1893af401be7dedf Mon Sep 17 00:00:00 2001
From: Thierry Reding <treding@nvidia.com>
Date: Mon, 16 Oct 2017 14:40:23 +0200
Subject: [PATCH] pinctrl: armada-37xx: Stop using struct gpio_chip.irq_base

The Armada 37xx driver always initializes the IRQ base to 0, hence the
subtraction is a no-op. Remove the subtraction and thereby the last user
of struct gpio_chip's .irq_base field.

Note that this was also actually a bug and only worked because of the
above assumption. If the IRQ base had been dynamically allocated, the
subtraction would've caused the wrong mask to be generated since the
struct irq_data.hwirq field is an index local to the IRQ domain. As a
result, it should now be safe to also allocate this chip's IRQ base
dynamically, unless there are consumers left that refer to the IRQs by
their global number.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
index 71b944748304..ac299a6cdfd6 100644
--- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
+++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
@@ -627,14 +627,14 @@ static void armada_37xx_irq_handler(struct irq_desc *desc)
 static unsigned int armada_37xx_irq_startup(struct irq_data *d)
 {
 	struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
-	int irq = d->hwirq - chip->irq_base;
+
 	/*
 	 * The mask field is a "precomputed bitmask for accessing the
 	 * chip registers" which was introduced for the generic
 	 * irqchip framework. As we don't use this framework, we can
 	 * reuse this field for our own usage.
 	 */
-	d->mask = BIT(irq % GPIO_PER_REG);
+	d->mask = BIT(d->hwirq % GPIO_PER_REG);
 
 	armada_37xx_irq_unmask(d);
 
-- 
2.14.1


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [gpio:devel 12/12] drivers/pinctrl/mvebu/pinctrl-armada-37xx.c:630:27: error: 'struct gpio_chip' has no member named 'irq_base'; did you mean 'base'?
  2017-10-16 12:46         ` Thierry Reding
@ 2017-10-16 14:53           ` Gregory CLEMENT
  2017-10-16 21:15             ` Linus Walleij
  2017-10-19 13:17           ` Linus Walleij
  1 sibling, 1 reply; 10+ messages in thread
From: Gregory CLEMENT @ 2017-10-16 14:53 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Linus Walleij, kbuild test robot, Grygorii Strashko, kbuild-all,
	linux-gpio

Hi Thierry,


 On lun., oct. 16 2017, Thierry Reding <thierry.reding@gmail.com> wrote:

> On Mon, Oct 16, 2017 at 01:50:08PM +0200, Linus Walleij wrote:
>> On Mon, Oct 16, 2017 at 10:08 AM, Thierry Reding
>> <thierry.reding@gmail.com> wrote:
>> 
>> > Nevermind, I see there is now yet another conflict that would require
>> > yet another rebase of that series.
>> 
>> Sorry about this :(
>> 
>> It sucks with moving targets, there was some fallout from Grygorii's patch
>> to map IRQs dynamically and remove irq_base so we need to clean that
>> up first.
>> 
>> This particular bug from the autobuilder seems to be constrained to the
>> armada driver though.
>
> Well, the problem here is that Grygorii's patch removes the field but
> with there still being one user left. So we'd have to make sure that the
> real last user goes away before that patch is applied.
>
> Looking at the code it seems a little phony. d->hwirq - chip->irq_base
> seems like a very risky thing to do because you never really know what
> irq_base will end up being (in the general case where it's dynamically
> allocated). But the driver passes 0 to gpiochip_irqchip_add() as the
> first_irq parameter, therefore the operation becomes a noop and so it
> should be completely safe to remove that line.
>
> Something like the below should do. Cc += Gregory.

Thanks for adding me in CC I wonder why I was not in CC in the original
series whereas we properly fill the MAINTAINER file for this.

About your patch it looks good. The reason to use irq_base was that at
the beginning I didn't use dynamic irq at all, and according to the test I
did while developing the driver I could have an irq_base which started
from the last irq for the first gpio controller (on these SoC we can
two different controller using the same driver). Now I wonder if there
was something wrong when I saw that, but as now I moved on dynamic
allocation for irq, then we don't have to worry about it.

As I said your patch looks OK but I would like to test it on a Armada
37xx based board to be sure. I planed to do it on Wednesday.

Thanks,

Gregory


>
> Thierry
>
> --- >8 ---
> From fcd87329cf4edf6a6f5d491a1893af401be7dedf Mon Sep 17 00:00:00 2001
> From: Thierry Reding <treding@nvidia.com>
> Date: Mon, 16 Oct 2017 14:40:23 +0200
> Subject: [PATCH] pinctrl: armada-37xx: Stop using struct gpio_chip.irq_base
>
> The Armada 37xx driver always initializes the IRQ base to 0, hence the
> subtraction is a no-op. Remove the subtraction and thereby the last user
> of struct gpio_chip's .irq_base field.
>
> Note that this was also actually a bug and only worked because of the
> above assumption. If the IRQ base had been dynamically allocated, the
> subtraction would've caused the wrong mask to be generated since the
> struct irq_data.hwirq field is an index local to the IRQ domain. As a
> result, it should now be safe to also allocate this chip's IRQ base
> dynamically, unless there are consumers left that refer to the IRQs by
> their global number.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> index 71b944748304..ac299a6cdfd6 100644
> --- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> +++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> @@ -627,14 +627,14 @@ static void armada_37xx_irq_handler(struct irq_desc *desc)
>  static unsigned int armada_37xx_irq_startup(struct irq_data *d)
>  {
>  	struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
> -	int irq = d->hwirq - chip->irq_base;
> +
>  	/*
>  	 * The mask field is a "precomputed bitmask for accessing the
>  	 * chip registers" which was introduced for the generic
>  	 * irqchip framework. As we don't use this framework, we can
>  	 * reuse this field for our own usage.
>  	 */
> -	d->mask = BIT(irq % GPIO_PER_REG);
> +	d->mask = BIT(d->hwirq % GPIO_PER_REG);
>  
>  	armada_37xx_irq_unmask(d);
>  
> -- 
> 2.14.1
>

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [gpio:devel 12/12] drivers/pinctrl/mvebu/pinctrl-armada-37xx.c:630:27: error: 'struct gpio_chip' has no member named 'irq_base'; did you mean 'base'?
  2017-10-16 14:53           ` Gregory CLEMENT
@ 2017-10-16 21:15             ` Linus Walleij
  2017-10-19 11:16               ` Gregory CLEMENT
  0 siblings, 1 reply; 10+ messages in thread
From: Linus Walleij @ 2017-10-16 21:15 UTC (permalink / raw)
  To: Gregory CLEMENT
  Cc: Thierry Reding, kbuild test robot, Grygorii Strashko, kbuild-all,
	linux-gpio

On Mon, Oct 16, 2017 at 4:53 PM, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:

> Thanks for adding me in CC I wonder why I was not in CC in the original
> series whereas we properly fill the MAINTAINER file for this.

Because the change causing this was not touching any Armada
files.

It is a fallout from the driver using gpiolib internals. (chip->irq_base).

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [gpio:devel 12/12] drivers/pinctrl/mvebu/pinctrl-armada-37xx.c:630:27: error: 'struct gpio_chip' has no member named 'irq_base'; did you mean 'base'?
  2017-10-16 21:15             ` Linus Walleij
@ 2017-10-19 11:16               ` Gregory CLEMENT
  0 siblings, 0 replies; 10+ messages in thread
From: Gregory CLEMENT @ 2017-10-19 11:16 UTC (permalink / raw)
  To: Linus Walleij, Thierry Reding
  Cc: kbuild test robot, Grygorii Strashko, kbuild-all, linux-gpio

Hi Linus and Thierry,
 
 On lun., oct. 16 2017, Linus Walleij <linus.walleij@linaro.org> wrote:

> On Mon, Oct 16, 2017 at 4:53 PM, Gregory CLEMENT
> <gregory.clement@free-electrons.com> wrote:
>
>> Thanks for adding me in CC I wonder why I was not in CC in the original
>> series whereas we properly fill the MAINTAINER file for this.
>
> Because the change causing this was not touching any Armada
> files.
>
> It is a fallout from the driver using gpiolib
> internals. (chip->irq_base).

OK I see.

About the patch itself I tested it on the Armada 3720 DB and it's OK for
me:

Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com>

Thanks,

Gregory

>
> Yours,
> Linus Walleij

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [gpio:devel 12/12] drivers/pinctrl/mvebu/pinctrl-armada-37xx.c:630:27: error: 'struct gpio_chip' has no member named 'irq_base'; did you mean 'base'?
  2017-10-16 12:46         ` Thierry Reding
  2017-10-16 14:53           ` Gregory CLEMENT
@ 2017-10-19 13:17           ` Linus Walleij
  1 sibling, 0 replies; 10+ messages in thread
From: Linus Walleij @ 2017-10-19 13:17 UTC (permalink / raw)
  To: Thierry Reding
  Cc: kbuild test robot, Grygorii Strashko, Gregory CLEMENT,
	kbuild-all, linux-gpio

On Mon, Oct 16, 2017 at 2:46 PM, Thierry Reding
<thierry.reding@gmail.com> wrote:

> Something like the below should do. Cc += Gregory.

Thanks for helping fixing this!

Patch applied with Gregory's review tag.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-10-19 13:17 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-15  1:52 [gpio:devel 12/12] drivers/pinctrl/mvebu/pinctrl-armada-37xx.c:630:27: error: 'struct gpio_chip' has no member named 'irq_base'; did you mean 'base'? kbuild test robot
2017-10-15 14:21 ` Linus Walleij
2017-10-16  8:05   ` Thierry Reding
2017-10-16  8:08     ` Thierry Reding
2017-10-16 11:50       ` Linus Walleij
2017-10-16 12:46         ` Thierry Reding
2017-10-16 14:53           ` Gregory CLEMENT
2017-10-16 21:15             ` Linus Walleij
2017-10-19 11:16               ` Gregory CLEMENT
2017-10-19 13:17           ` Linus Walleij

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.