[PATCH] gpio: xtensa: depend on HAVE_XTENSA_GPIO32

[PATCH] gpio: xtensa: depend on HAVE_XTENSA_GPIO32

[Baruch Siach]

Prevent build failure when the selected variant does not support GPIO32.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
Chris, Max,

I assume that the gpio tree is the natural path for this patch as it carries 
the driver already. Please ack the arch/xtensa part.
---
 arch/xtensa/Kconfig  | 3 +++
 drivers/gpio/Kconfig | 4 ++++
 2 files changed, 7 insertions(+)

diff --git a/arch/xtensa/Kconfig b/arch/xtensa/Kconfig
index 65ffc09..6bd4ee3 100644
--- a/arch/xtensa/Kconfig
+++ b/arch/xtensa/Kconfig
@@ -74,16 +74,19 @@ choice
 config XTENSA_VARIANT_FSF
 	bool "fsf - default (not generic) configuration"
 	select MMU
+	select HAVE_XTENSA_GPIO32
 
 config XTENSA_VARIANT_DC232B
 	bool "dc232b - Diamond 232L Standard Core Rev.B (LE)"
 	select MMU
+	select HAVE_XTENSA_GPIO32
 	help
 	  This variant refers to Tensilica's Diamond 232L Standard core Rev.B (LE).
 
 config XTENSA_VARIANT_DC233C
 	bool "dc233c - Diamond 233L Standard Core Rev.C (LE)"
 	select MMU
+	select HAVE_XTENSA_GPIO32
 	help
 	  This variant refers to Tensilica's Diamond 233L Standard core Rev.C (LE).
 
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 43c45ea..766accb 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -290,9 +290,13 @@ config GPIO_XILINX
 	help
 	  Say yes here to support the Xilinx FPGA GPIO device
 
+config HAVE_XTENSA_GPIO32
+	bool
+
 config GPIO_XTENSA
 	bool "Xtensa GPIO32 support"
 	depends on XTENSA
+	depends on HAVE_XTENSA_GPIO32
 	depends on !SMP
 	help
 	  Say yes here to support the Xtensa internal GPIO32 IMPWIRE (input)
-- 
1.8.5.1

Re: [PATCH] gpio: xtensa: depend on HAVE_XTENSA_GPIO32

[Max Filippov]

Hi Baruch,

On Mon, Dec 16, 2013 at 11:05 AM, Baruch Siach <baruch@tkos.co.il> wrote:
> Prevent build failure when the selected variant does not support GPIO32.
>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
> Chris, Max,
>
> I assume that the gpio tree is the natural path for this patch as it carries
> the driver already. Please ack the arch/xtensa part.
> ---
>  arch/xtensa/Kconfig  | 3 +++
>  drivers/gpio/Kconfig | 4 ++++
>  2 files changed, 7 insertions(+)
>
> diff --git a/arch/xtensa/Kconfig b/arch/xtensa/Kconfig
> index 65ffc09..6bd4ee3 100644
> --- a/arch/xtensa/Kconfig
> +++ b/arch/xtensa/Kconfig
> @@ -74,16 +74,19 @@ choice
>  config XTENSA_VARIANT_FSF
>         bool "fsf - default (not generic) configuration"
>         select MMU
> +       select HAVE_XTENSA_GPIO32
>
>  config XTENSA_VARIANT_DC232B
>         bool "dc232b - Diamond 232L Standard Core Rev.B (LE)"
>         select MMU
> +       select HAVE_XTENSA_GPIO32
>         help
>           This variant refers to Tensilica's Diamond 232L Standard core Rev.B (LE).
>
>  config XTENSA_VARIANT_DC233C
>         bool "dc233c - Diamond 233L Standard Core Rev.C (LE)"
>         select MMU
> +       select HAVE_XTENSA_GPIO32
>         help
>           This variant refers to Tensilica's Diamond 233L Standard core Rev.C (LE).
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 43c45ea..766accb 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -290,9 +290,13 @@ config GPIO_XILINX
>         help
>           Say yes here to support the Xilinx FPGA GPIO device
>
> +config HAVE_XTENSA_GPIO32
> +       bool
> +

I'm not sure why this symbol should be defined in the drivers/gpio/Kconfig
and not in the arch/xtensa/Kconfig, after all it's a CPU property.
So if you agree please move it there. With that change

Acked-by: Max Filippov <jcmvbkbc@gmail.com>

>  config GPIO_XTENSA
>         bool "Xtensa GPIO32 support"
>         depends on XTENSA
> +       depends on HAVE_XTENSA_GPIO32
>         depends on !SMP
>         help
>           Say yes here to support the Xtensa internal GPIO32 IMPWIRE (input)
> --
> 1.8.5.1
>

-- 
Thanks.
-- Max

Re: [PATCH] gpio: xtensa: depend on HAVE_XTENSA_GPIO32

[Linus Walleij]

On Mon, Dec 16, 2013 at 8:05 AM, Baruch Siach <baruch@tkos.co.il> wrote:

> Prevent build failure when the selected variant does not support GPIO32.
>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
(...)

>  config XTENSA_VARIANT_FSF
>         bool "fsf - default (not generic) configuration"
>         select MMU
> +       select HAVE_XTENSA_GPIO32

(...)
> +config HAVE_XTENSA_GPIO32
> +       bool
> +
>  config GPIO_XTENSA
>         bool "Xtensa GPIO32 support"
>         depends on XTENSA
> +       depends on HAVE_XTENSA_GPIO32

This is just a tautology, you don't need a helper bool
for this.

Instead just select GPIO_XTENSA from your affected
platforms.

The patch only needs to hit arch/arm/mach-xtensa/Kconfig.

Yours,
Linus Walleij

Re: [PATCH] gpio: xtensa: depend on HAVE_XTENSA_GPIO32

[Baruch Siach]

Hi Linus,

On Thu, Jan 02, 2014 at 01:07:24PM +0100, Linus Walleij wrote:
> On Mon, Dec 16, 2013 at 8:05 AM, Baruch Siach <baruch@tkos.co.il> wrote:
> 
> > Prevent build failure when the selected variant does not support GPIO32.
> >
> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> (...)
> 
> >  config XTENSA_VARIANT_FSF
> >         bool "fsf - default (not generic) configuration"
> >         select MMU
> > +       select HAVE_XTENSA_GPIO32
> 
> (...)
> > +config HAVE_XTENSA_GPIO32
> > +       bool
> > +
> >  config GPIO_XTENSA
> >         bool "Xtensa GPIO32 support"
> >         depends on XTENSA
> > +       depends on HAVE_XTENSA_GPIO32
> 
> This is just a tautology, you don't need a helper bool
> for this.
> 
> Instead just select GPIO_XTENSA from your affected
> platforms.

But then you force this driver on all users. Why not leave the option to 
disable this driver when the user needs direct access to this hardware, or 
just wants to remove unused code?

> The patch only needs to hit arch/arm/mach-xtensa/Kconfig.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

Re: [PATCH] gpio: xtensa: depend on HAVE_XTENSA_GPIO32

[Linus Walleij]

On Thu, Jan 2, 2014 at 1:18 PM, Baruch Siach <baruch@tkos.co.il> wrote:
> On Thu, Jan 02, 2014 at 01:07:24PM +0100, Linus Walleij wrote:
>> On Mon, Dec 16, 2013 at 8:05 AM, Baruch Siach <baruch@tkos.co.il> wrote:
>>
>> > Prevent build failure when the selected variant does not support GPIO32.
>> >
>> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
>> (...)
>> > +config HAVE_XTENSA_GPIO32
>> > +       bool
>> > +
>> >  config GPIO_XTENSA
>> >         bool "Xtensa GPIO32 support"
>> >         depends on XTENSA
>> > +       depends on HAVE_XTENSA_GPIO32
>>
>> This is just a tautology, you don't need a helper bool
>> for this.
>>
>> Instead just select GPIO_XTENSA from your affected
>> platforms.
>
> But then you force this driver on all users. Why not leave the option to
> disable this driver when the user needs direct access to this hardware, or
> just wants to remove unused code?

Hmm you're right. But if the build fails on !XTENSA, can you not
put a

depends on XTENSA_VARIANT_FSF || XTENSA_VARIANT_DC232B || XTENSA_VARIANT_DC233C

Into drivers/gpio/Kconfig?

It's a simple oneliner...

Usually the only reason not to do this would be to get a wider compile
coverage but the patch says it only compiles on these platforms
anyway :-P

Yours,
Linus Walleij

Re: [PATCH] gpio: xtensa: depend on HAVE_XTENSA_GPIO32

[Baruch Siach]

Hi Linus,

On Thu, Jan 02, 2014 at 02:21:12PM +0100, Linus Walleij wrote:
> On Thu, Jan 2, 2014 at 1:18 PM, Baruch Siach <baruch@tkos.co.il> wrote:
> > On Thu, Jan 02, 2014 at 01:07:24PM +0100, Linus Walleij wrote:
> >> On Mon, Dec 16, 2013 at 8:05 AM, Baruch Siach <baruch@tkos.co.il> wrote:
> >>
> >> > Prevent build failure when the selected variant does not support GPIO32.
> >> >
> >> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> >> (...)
> >> > +config HAVE_XTENSA_GPIO32
> >> > +       bool
> >> > +
> >> >  config GPIO_XTENSA
> >> >         bool "Xtensa GPIO32 support"
> >> >         depends on XTENSA
> >> > +       depends on HAVE_XTENSA_GPIO32
> >>
> >> This is just a tautology, you don't need a helper bool
> >> for this.
> >>
> >> Instead just select GPIO_XTENSA from your affected
> >> platforms.
> >
> > But then you force this driver on all users. Why not leave the option to
> > disable this driver when the user needs direct access to this hardware, or
> > just wants to remove unused code?
> 
> Hmm you're right. But if the build fails on !XTENSA, can you not
> put a
> 
> depends on XTENSA_VARIANT_FSF || XTENSA_VARIANT_DC232B || XTENSA_VARIANT_DC233C
> 
> Into drivers/gpio/Kconfig?
> 
> It's a simple oneliner...

IMO, having a 'select HAVE_XTENSA_GPIO32' in arch/xtensa/Kconfig would makes 
it clear that this is something to consider when adding another Xtensa 
variant. Also, this dependency line might (hopefully will) become quite long. 
But if you object I'll change that.

> Usually the only reason not to do this would be to get a wider compile
> coverage but the patch says it only compiles on these platforms
> anyway :-P

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

Re: [PATCH] gpio: xtensa: depend on HAVE_XTENSA_GPIO32

[Linus Walleij]

On Thu, Jan 2, 2014 at 2:33 PM, Baruch Siach <baruch@tkos.co.il> wrote:
> On Thu, Jan 02, 2014 at 02:21:12PM +0100, Linus Walleij wrote:
>> On Thu, Jan 2, 2014 at 1:18 PM, Baruch Siach <baruch@tkos.co.il> wrote:
>> > On Thu, Jan 02, 2014 at 01:07:24PM +0100, Linus Walleij wrote:
>> >> On Mon, Dec 16, 2013 at 8:05 AM, Baruch Siach <baruch@tkos.co.il> wrote:
>> >>
>> >> > Prevent build failure when the selected variant does not support GPIO32.
>> >> >
>> >> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
>> >> (...)
>> >> > +config HAVE_XTENSA_GPIO32
>> >> > +       bool
>> >> > +
>> >> >  config GPIO_XTENSA
>> >> >         bool "Xtensa GPIO32 support"
>> >> >         depends on XTENSA
>> >> > +       depends on HAVE_XTENSA_GPIO32
>> >>
>> >> This is just a tautology, you don't need a helper bool
>> >> for this.
>> >>
>> >> Instead just select GPIO_XTENSA from your affected
>> >> platforms.
>> >
>> > But then you force this driver on all users. Why not leave the option to
>> > disable this driver when the user needs direct access to this hardware, or
>> > just wants to remove unused code?
>>
>> Hmm you're right. But if the build fails on !XTENSA, can you not
>> put a
>>
>> depends on XTENSA_VARIANT_FSF || XTENSA_VARIANT_DC232B || XTENSA_VARIANT_DC233C
>>
>> Into drivers/gpio/Kconfig?
>>
>> It's a simple oneliner...
>
> IMO, having a 'select HAVE_XTENSA_GPIO32' in arch/xtensa/Kconfig would makes
> it clear that this is something to consider when adding another Xtensa
> variant. Also, this dependency line might (hopefully will) become quite long.
> But if you object I'll change that.

I have no strong opinion on this, I would leave that decision to the
Xtensa maintainer.

Yours,
Linus Walleij

Re: [PATCH] gpio: xtensa: depend on HAVE_XTENSA_GPIO32

[Baruch Siach]

Hi Linus,

On Thu, Jan 02, 2014 at 06:04:57PM +0100, Linus Walleij wrote:
> On Thu, Jan 2, 2014 at 2:33 PM, Baruch Siach <baruch@tkos.co.il> wrote:
> > On Thu, Jan 02, 2014 at 02:21:12PM +0100, Linus Walleij wrote:
> >> On Thu, Jan 2, 2014 at 1:18 PM, Baruch Siach <baruch@tkos.co.il> wrote:
> >> > On Thu, Jan 02, 2014 at 01:07:24PM +0100, Linus Walleij wrote:
> >> >> On Mon, Dec 16, 2013 at 8:05 AM, Baruch Siach <baruch@tkos.co.il> wrote:
> >> >>
> >> >> > Prevent build failure when the selected variant does not support GPIO32.
> >> >> >
> >> >> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> >> >> (...)
> >> >> > +config HAVE_XTENSA_GPIO32
> >> >> > +       bool
> >> >> > +
> >> >> >  config GPIO_XTENSA
> >> >> >         bool "Xtensa GPIO32 support"
> >> >> >         depends on XTENSA
> >> >> > +       depends on HAVE_XTENSA_GPIO32
> >> >>
> >> >> This is just a tautology, you don't need a helper bool
> >> >> for this.
> >> >>
> >> >> Instead just select GPIO_XTENSA from your affected
> >> >> platforms.
> >> >
> >> > But then you force this driver on all users. Why not leave the option to
> >> > disable this driver when the user needs direct access to this hardware, or
> >> > just wants to remove unused code?
> >>
> >> Hmm you're right. But if the build fails on !XTENSA, can you not
> >> put a
> >>
> >> depends on XTENSA_VARIANT_FSF || XTENSA_VARIANT_DC232B || XTENSA_VARIANT_DC233C
> >>
> >> Into drivers/gpio/Kconfig?
> >>
> >> It's a simple oneliner...
> >
> > IMO, having a 'select HAVE_XTENSA_GPIO32' in arch/xtensa/Kconfig would makes
> > it clear that this is something to consider when adding another Xtensa
> > variant. Also, this dependency line might (hopefully will) become quite long.
> > But if you object I'll change that.
> 
> I have no strong opinion on this, I would leave that decision to the
> Xtensa maintainer.

The xtensa co-maintainer, Max Filippov, has already acked v2 of this patch.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -