Linux Input Archive on lore.kernel.org
 help / color / Atom feed
From: Bastien Nocera <hadess@hadess.net>
To: Hans de Goede <hdegoede@redhat.com>,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org, kbuild test robot <lkp@intel.com>
Subject: Re: [PATCH] Input: goodix - Fix compilation when ACPI support is disabled
Date: Wed, 25 Mar 2020 15:10:20 +0100
Message-ID: <a9c1e74e54abf7db4bb09e77e148831f42de9fa2.camel@hadess.net> (raw)
In-Reply-To: <3ed2db45-0fd6-694e-023d-427cb8854f81@redhat.com>

On Wed, 2020-03-25 at 15:05 +0100, Hans de Goede wrote:
> Hi,
> 
> On 3/25/20 3:02 PM, Bastien Nocera wrote:
> > On Wed, 2020-03-25 at 14:55 +0100, Hans de Goede wrote:
> > > We could do something like that, but TBH I'm not a fan of that
> > > 
> > > adding extra wrappers makes it harder to see what the code is
> > > 
> > > actually doing.
> > > 
> > > 
> > > 
> > > I understand your dislike for the extra braces I added and
> > > 
> > > I'm fine with fixing that by adding __maybe_unused to the
> > > 
> > > variable declarations at the top. I don't really see what
> > > 
> > > the problem with the #ifdef-s is given how clean they are,
> > > 
> > > with the braces thing fixed by using __maybe_unused things
> > > 
> > > would look like e.g. this:
> > 
> > It's not only the fact that there's extra #ifdef's, it's that the
> > ifdef's need to be just "that". It's not "#ifdef FOO", it's "#if
> > defined CONFIG_X86 && defined CONFIG_ACPI".
> 
> If that is the problem I would prefer adding:
> 
> /* Our special handling for GPIO accesses through ACPI is x86
> specific */
> #if defined CONFIG_X86 && defined CONFIG_ACPI
> #define ACPI_GPIO_SUPPORT
> #endif
> 
> And use:
> 
> #ifdef ACPI_GPIO_SUPPORT
> 
> Elsewhere.
> 
> Would that work for you?

That's slightly better, but I would still have preferred stubbing out
those ACPI calls directly. Right now, the fact that we expect half of
the commands to be stubbed out and the other half to not be called is
just weird.


  reply index

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-25 10:33 Hans de Goede
2020-03-25 10:47 ` Bastien Nocera
2020-03-25 11:49   ` Hans de Goede
2020-03-25 12:11     ` Bastien Nocera
2020-03-25 13:55       ` Hans de Goede
2020-03-25 14:02         ` Bastien Nocera
2020-03-25 14:05           ` Hans de Goede
2020-03-25 14:10             ` Bastien Nocera [this message]
2020-03-25 14:55               ` Hans de Goede

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=a9c1e74e54abf7db4bb09e77e148831f42de9fa2.camel@hadess.net \
    --to=hadess@hadess.net \
    --cc=dmitry.torokhov@gmail.com \
    --cc=hdegoede@redhat.com \
    --cc=linux-input@vger.kernel.org \
    --cc=lkp@intel.com \
    /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

Linux Input Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-input/0 linux-input/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-input linux-input/ https://lore.kernel.org/linux-input \
		linux-input@vger.kernel.org
	public-inbox-index linux-input

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-input


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git