linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Baruch Siach <baruch@tkos.co.il>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Chris Zankel <chris@zankel.net>,
	Max Filippov <jcmvbkbc@gmail.com>,
	"linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
	linux-xtensa@linux-xtensa.org
Subject: Re: [PATCH] gpio: xtensa: depend on HAVE_XTENSA_GPIO32
Date: Thu, 2 Jan 2014 15:33:33 +0200	[thread overview]
Message-ID: <20140102133333.GF6589@tarshish> (raw)
In-Reply-To: <CACRpkdakv+Ht+fvxWyAzbUqO2hfJKX=94FwA5SNakx5FcbA+yA@mail.gmail.com>

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 -

  reply	other threads:[~2014-01-02 13:33 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-16  7:05 [PATCH] gpio: xtensa: depend on HAVE_XTENSA_GPIO32 Baruch Siach
2013-12-17 14:24 ` Max Filippov
2014-01-02 12:07 ` Linus Walleij
2014-01-02 12:18   ` Baruch Siach
2014-01-02 13:21     ` Linus Walleij
2014-01-02 13:33       ` Baruch Siach [this message]
2014-01-02 17:04         ` Linus Walleij
2014-01-02 18:00           ` Baruch Siach

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140102133333.GF6589@tarshish \
    --to=baruch@tkos.co.il \
    --cc=chris@zankel.net \
    --cc=jcmvbkbc@gmail.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-xtensa@linux-xtensa.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).