[PATCH] gpio: dwapb: enable for ARC

[PATCH] gpio: dwapb: enable for ARC

[Alexey Brodkin]

From: Vineet Gupta <vgupta@synopsys.com>

Synopsys SDP platform uses DW GPIO controller in design with
ARC cores. So adding ARC to architectures that may select this
GPIO controller.

Even though support for Synopsys SDP is yet to be submitted we'll need
this tiny option enabled at least for properly working interrupts (DW
GPIO controller is used as interrupt controller).

Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpio/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 4b3284b..83d2d0b 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -148,7 +148,7 @@ config GPIO_GENERIC_PLATFORM
 
 config GPIO_DWAPB
 	tristate "Synopsys DesignWare APB GPIO driver"
-	depends on ((ARM || ARM64) && OF_GPIO) || X86_INTEL_QUARK
+	depends on ((ARC || ARM || ARM64) && OF_GPIO) || X86_INTEL_QUARK
 	select GPIO_GENERIC
 	select GENERIC_IRQ_CHIP
 	help
-- 
2.1.0

Re: [PATCH] gpio: dwapb: enable for ARC

[Alexey Brodkin]

Hi Linus,

On Tue, 2015-03-31 at 11:30 +0300, Alexey Brodkin wrote:
> From: Vineet Gupta <vgupta@synopsys.com>
> 
> Synopsys SDP platform uses DW GPIO controller in design with
> ARC cores. So adding ARC to architectures that may select this
> GPIO controller.
> 
> Even though support for Synopsys SDP is yet to be submitted we'll need
> this tiny option enabled at least for properly working interrupts (DW
> GPIO controller is used as interrupt controller).
> 
> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
> Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/gpio/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 4b3284b..83d2d0b 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -148,7 +148,7 @@ config GPIO_GENERIC_PLATFORM
>  
>  config GPIO_DWAPB
>  	tristate "Synopsys DesignWare APB GPIO driver"
> -	depends on ((ARM || ARM64) && OF_GPIO) || X86_INTEL_QUARK
> +	depends on ((ARC || ARM || ARM64) && OF_GPIO) || X86_INTEL_QUARK
>  	select GPIO_GENERIC
>  	select GENERIC_IRQ_CHIP
>  	help

I'm wondering if there's a chance for this patch to be reviewed and/or
applied?

-Alexey

Re: [PATCH] gpio: dwapb: enable for ARC

[Vineet Gupta]

On Tuesday 07 April 2015 07:25 PM, Alexey Brodkin wrote:

Hi Linus,

On Tue, 2015-03-31 at 11:30 +0300, Alexey Brodkin wrote:


> From: Vineet Gupta <vgupta@synopsys.com><mailto:vgupta@synopsys.com>
>
> Synopsys SDP platform uses DW GPIO controller in design with
> ARC cores. So adding ARC to architectures that may select this
> GPIO controller.
>
> Even though support for Synopsys SDP is yet to be submitted we'll need
> this tiny option enabled at least for properly working interrupts (DW
> GPIO controller is used as interrupt controller).
>
> Signed-off-by: Vineet Gupta <vgupta@synopsys.com><mailto:vgupta@synopsys.com>
> Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com><mailto:abrodkin@synopsys.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com><mailto:andriy.shevchenko@linux.intel.com>
> Cc: Linus Walleij <linus.walleij@linaro.org><mailto:linus.walleij@linaro.org>
> ---
>  drivers/gpio/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 4b3284b..83d2d0b 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -148,7 +148,7 @@ config GPIO_GENERIC_PLATFORM
>
>  config GPIO_DWAPB
>       tristate "Synopsys DesignWare APB GPIO driver"
> -     depends on ((ARM || ARM64) && OF_GPIO) || X86_INTEL_QUARK
> +     depends on ((ARC || ARM || ARM64) && OF_GPIO) || X86_INTEL_QUARK
>       select GPIO_GENERIC
>       select GENERIC_IRQ_CHIP
>       help


I'm wondering if there's a chance for this patch to be reviewed and/or
applied?


Actually, given that ARC SDP support merge in upstream hinges on this one, Linus can u please provide ur ACK on this one so I could pick it up instead and route via the ARC tree for next window. That way we could lalso avoid any cross branch merge issues etc.

Thx,
-Vineet

Re: [PATCH] gpio: dwapb: enable for ARC

[Linus Walleij]

On Tue, Mar 31, 2015 at 10:30 AM, Alexey Brodkin
<Alexey.Brodkin@synopsys.com> wrote:

> From: Vineet Gupta <vgupta@synopsys.com>
>
> Synopsys SDP platform uses DW GPIO controller in design with
> ARC cores. So adding ARC to architectures that may select this
> GPIO controller.
>
> Even though support for Synopsys SDP is yet to be submitted we'll need
> this tiny option enabled at least for properly working interrupts (DW
> GPIO controller is used as interrupt controller).
>
> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
> Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>

Patch applied.

But ...

> -       depends on ((ARM || ARM64) && OF_GPIO) || X86_INTEL_QUARK
> +       depends on ((ARC || ARM || ARM64) && OF_GPIO) || X86_INTEL_QUARK

This is getting a bit silly. What stops us from actually just enabling
it for any architecture?

Yours,
Linus Walleij

Re: [PATCH] gpio: dwapb: enable for ARC

[Alexey Brodkin]

On Wed, 2015-04-08 at 17:03 +0200, Linus Walleij wrote:
> On Tue, Mar 31, 2015 at 10:30 AM, Alexey Brodkin
> <Alexey.Brodkin@synopsys.com> wrote:
> 
> > From: Vineet Gupta <vgupta@synopsys.com>
> >
> > Synopsys SDP platform uses DW GPIO controller in design with
> > ARC cores. So adding ARC to architectures that may select this
> > GPIO controller.
> >
> > Even though support for Synopsys SDP is yet to be submitted we'll need
> > this tiny option enabled at least for properly working interrupts (DW
> > GPIO controller is used as interrupt controller).
> >
> > Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
> > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Cc: Linus Walleij <linus.walleij@linaro.org>
> 
> Patch applied.
> 
> But ...
> 
> > -       depends on ((ARM || ARM64) && OF_GPIO) || X86_INTEL_QUARK
> > +       depends on ((ARC || ARM || ARM64) && OF_GPIO) || X86_INTEL_QUARK
> 
> This is getting a bit silly. What stops us from actually just enabling
> it for any architecture?

Agree. That looks like nonsense now.
Origin of arch-limitation is
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/gpio/Kconfig?id=1972c97db5b0c125918f662cd084c7d5370674c0

--->8---
gpio: dwapb: fix compile errors
Whereas the DWAPB driver does not really depend on the ARM
architecture, it uses [readl|writel]_relaxed() not found on
arch such as Blackfin, so restrict this to ARM until there is
another architecture that can make use of it.

It is also using the of_node of the gpiochip, so fix this
too by requiring OF_GPIO.

All error/warnings:

make.cross ARCH=blackfin
drivers/gpio/gpio-dwapb.c: In function 'dwapb_irq_handler':
drivers/gpio/gpio-dwapb.c:91:2: error: implicit declaration of function
'readl_relaxed' [-Werror=implicit-function-declaration]
drivers/gpio/gpio-dwapb.c: In function 'dwapb_configure_irqs':
drivers/gpio/gpio-dwapb.c:212:32: error: 'struct gpio_chip' has no
member named 'of_node'
drivers/gpio/gpio-dwapb.c:221:16: error: 'struct gpio_chip' has no
member named 'of_node'
drivers/gpio/gpio-dwapb.c: In function 'dwapb_gpio_add_port':
drivers/gpio/gpio-dwapb.c:331:14: error: 'struct gpio_chip' has no
member named 'of_node'
cc1: some warnings being treated as errors
--->8---

Probably better fix is to make dwgpio depend on !BLACKFIN because we
know it won't be built for it. If there're other arches that don't
define [readl|writel]_relaxed() we may add them as well.

-Alexey

Re: [PATCH] gpio: dwapb: enable for ARC

[Linus Walleij]

Better ask Steven, Arnd et al...

On Thu, Apr 9, 2015 at 9:48 AM, Alexey Brodkin
<Alexey.Brodkin@synopsys.com> wrote:
> On Wed, 2015-04-08 at 17:03 +0200, Linus Walleij wrote:
>> On Tue, Mar 31, 2015 at 10:30 AM, Alexey Brodkin
>> <Alexey.Brodkin@synopsys.com> wrote:
>>
>> > From: Vineet Gupta <vgupta@synopsys.com>
>> >
>> > Synopsys SDP platform uses DW GPIO controller in design with
>> > ARC cores. So adding ARC to architectures that may select this
>> > GPIO controller.
>> >
>> > Even though support for Synopsys SDP is yet to be submitted we'll need
>> > this tiny option enabled at least for properly working interrupts (DW
>> > GPIO controller is used as interrupt controller).
>> >
>> > Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
>> > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
>> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> > Cc: Linus Walleij <linus.walleij@linaro.org>
>>
>> Patch applied.
>>
>> But ...
>>
>> > -       depends on ((ARM || ARM64) && OF_GPIO) || X86_INTEL_QUARK
>> > +       depends on ((ARC || ARM || ARM64) && OF_GPIO) || X86_INTEL_QUARK
>>
>> This is getting a bit silly. What stops us from actually just enabling
>> it for any architecture?
>
> Agree. That looks like nonsense now.
> Origin of arch-limitation is
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/gpio/Kconfig?id=1972c97db5b0c125918f662cd084c7d5370674c0
>
> --->8---
> gpio: dwapb: fix compile errors
> Whereas the DWAPB driver does not really depend on the ARM
> architecture, it uses [readl|writel]_relaxed() not found on
> arch such as Blackfin, so restrict this to ARM until there is
> another architecture that can make use of it.
>
> It is also using the of_node of the gpiochip, so fix this
> too by requiring OF_GPIO.
>
> All error/warnings:
>
> make.cross ARCH=blackfin
> drivers/gpio/gpio-dwapb.c: In function 'dwapb_irq_handler':
> drivers/gpio/gpio-dwapb.c:91:2: error: implicit declaration of function
> 'readl_relaxed' [-Werror=implicit-function-declaration]
> drivers/gpio/gpio-dwapb.c: In function 'dwapb_configure_irqs':
> drivers/gpio/gpio-dwapb.c:212:32: error: 'struct gpio_chip' has no
> member named 'of_node'
> drivers/gpio/gpio-dwapb.c:221:16: error: 'struct gpio_chip' has no
> member named 'of_node'
> drivers/gpio/gpio-dwapb.c: In function 'dwapb_gpio_add_port':
> drivers/gpio/gpio-dwapb.c:331:14: error: 'struct gpio_chip' has no
> member named 'of_node'
> cc1: some warnings being treated as errors
> --->8---
>
> Probably better fix is to make dwgpio depend on !BLACKFIN because we
> know it won't be built for it. If there're other arches that don't
> define [readl|writel]_relaxed() we may add them as well.

This restrictions should be gone after commit
9439eb3ab9d1ece6e4ad7baaa4a7f534f9b9dab0
"asm-generic: io: implement relaxed accessor macros as conditional wrappers"

I will make a patch removing the dependencies and see what happens.

Yours,
Linus Walleij

Re: [PATCH] gpio: dwapb: enable for ARC

[Arnd Bergmann]

On Thursday 09 April 2015 10:29:59 Linus Walleij wrote:
> >
> > make.cross ARCH=blackfin
> > drivers/gpio/gpio-dwapb.c: In function 'dwapb_irq_handler':
> > drivers/gpio/gpio-dwapb.c:91:2: error: implicit declaration of function
> > 'readl_relaxed' [-Werror=implicit-function-declaration]
> > drivers/gpio/gpio-dwapb.c: In function 'dwapb_configure_irqs':
> > drivers/gpio/gpio-dwapb.c:212:32: error: 'struct gpio_chip' has no
> > member named 'of_node'
> > drivers/gpio/gpio-dwapb.c:221:16: error: 'struct gpio_chip' has no
> > member named 'of_node'
> > drivers/gpio/gpio-dwapb.c: In function 'dwapb_gpio_add_port':
> > drivers/gpio/gpio-dwapb.c:331:14: error: 'struct gpio_chip' has no
> > member named 'of_node'
> > cc1: some warnings being treated as errors
> > --->8---
> >
> > Probably better fix is to make dwgpio depend on !BLACKFIN because we
> > know it won't be built for it. If there're other arches that don't
> > define [readl|writel]_relaxed() we may add them as well.
> 
> This restrictions should be gone after commit
> 9439eb3ab9d1ece6e4ad7baaa4a7f534f9b9dab0
> "asm-generic: io: implement relaxed accessor macros as conditional wrappers"
> 
> I will make a patch removing the dependencies and see what happens.

Yes, makes sense. I think we don't need anything other than

	depends on OF_GPIO || X86_INTEL_QUARK

	Arnd

Re: [PATCH] gpio: dwapb: enable for ARC

[Linus Walleij]

On Thu, Apr 9, 2015 at 12:31 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 09 April 2015 10:29:59 Linus Walleij wrote:

>> I will make a patch removing the dependencies and see what happens.
>
> Yes, makes sense. I think we don't need anything other than
>
>         depends on OF_GPIO || X86_INTEL_QUARK

I didn't even keep that. Just available for everyone and its dog,
this hardware is so clearly generic.

I also rearranged the Kconfig menus in GPIO so users are not
confused (hopefully) as much by strange GPIO stuff anymore.

Yours,
Linus Walleij