All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lukas Wunner <lukas at wunner.de>
To: devel@acpica.org
Subject: Re: [Devel] [PATCH v2 2/2] ACPI / utils: Add acpi_dev_present()
Date: Tue, 01 Dec 2015 13:58:14 +0100	[thread overview]
Message-ID: <20151201125814.GA28094@wunner.de> (raw)
In-Reply-To: 565C086B.4000000@huawei.com

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

Hi Hanjun Guo,

thank you for taking a look at the patches.

On Mon, Nov 30, 2015 at 04:27:23PM +0800, Hanjun Guo wrote:
> If the driver can't pass the "dev" as the argument form acpi device or
> platform device, this is the way to match HIDs.
> 
> But if we get the pointer to acpi device or platform device structure
> before calling this function, I think acpi_match_device_ids() can match
> what you need, and we don't need to invent another function.

Of the 7 users,

* 4 detect the presence of a particular HID to activate platform-
  specific quirks (acer-wmi.c, eeepc-wmi.c, thinkpad_helper.c,
  cht_bsw_max98090_ti.c). The drivers never obtain a pointer to the
  struct acpi_device. This is also the reason why I need to detect
  the presence of apple-gmux in several DRM drivers, to activate
  quirks specific to MacBook Pros with dual GPUs, and the one thing
  they have in common is the presence of the gmux controller.
  The DRM drivers likewise never have a need to hold a pointer to
  apple-gmux' struct acpi_device.

* 2 detect the presence of a particular HID and *afterwards*
  instantiate a platform_device (atom/sst/sst_acpi.c and
  common/sst-acpi.c). So we can't use acpi_match_device_ids()
  there either.

* 1 is a driver for a platform_device (cht_bsw_rt5645.c) which was
  instantiated by atom/sst/sst_acpi.c. The driver is responsible
  for two chips and differentiates between the two by detecting the
  presence of a particular HID. It would be possible to refactor the
  code so that atom/sst/sst_acpi.c passes down the matched HID to
  cht_bsw_rt5645.c, then it wouldn't be necessary to match for a
  second time. Also, the only difference between the two chipsets
  seems to be a minute change in struct snd_soc_dapm_route, so I'm
  wondering if it's necessary to differentiate between the chipsets
  at all. Mark Brown may want to poke the developers of the driver to
  refactor the code.

TL;DR: 6 of the 7 users can't use acpi_match_device_ids() and the
7th could be refactored so that it doesn't have to detect the presence
of a specific HID at all.

Best regards,

Lukas

WARNING: multiple messages have this Message-ID (diff)
From: Lukas Wunner <lukas@wunner.de>
To: Hanjun Guo <guohanjun@huawei.com>
Cc: linux-acpi@vger.kernel.org,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Len Brown <lenb@kernel.org>,
	devel@acpica.org, Robert Moore <robert.moore@intel.com>,
	Mark Brown <broonie@kernel.org>,
	Hui Wang <hui.wang@canonical.com>,
	Darren Hart <dvhart@infradead.org>
Subject: Re: [PATCH v2 2/2] ACPI / utils: Add acpi_dev_present()
Date: Tue, 1 Dec 2015 13:58:14 +0100	[thread overview]
Message-ID: <20151201125814.GA28094@wunner.de> (raw)
In-Reply-To: <565C086B.4000000@huawei.com>

Hi Hanjun Guo,

thank you for taking a look at the patches.

On Mon, Nov 30, 2015 at 04:27:23PM +0800, Hanjun Guo wrote:
> If the driver can't pass the "dev" as the argument form acpi device or
> platform device, this is the way to match HIDs.
> 
> But if we get the pointer to acpi device or platform device structure
> before calling this function, I think acpi_match_device_ids() can match
> what you need, and we don't need to invent another function.

Of the 7 users,

* 4 detect the presence of a particular HID to activate platform-
  specific quirks (acer-wmi.c, eeepc-wmi.c, thinkpad_helper.c,
  cht_bsw_max98090_ti.c). The drivers never obtain a pointer to the
  struct acpi_device. This is also the reason why I need to detect
  the presence of apple-gmux in several DRM drivers, to activate
  quirks specific to MacBook Pros with dual GPUs, and the one thing
  they have in common is the presence of the gmux controller.
  The DRM drivers likewise never have a need to hold a pointer to
  apple-gmux' struct acpi_device.

* 2 detect the presence of a particular HID and *afterwards*
  instantiate a platform_device (atom/sst/sst_acpi.c and
  common/sst-acpi.c). So we can't use acpi_match_device_ids()
  there either.

* 1 is a driver for a platform_device (cht_bsw_rt5645.c) which was
  instantiated by atom/sst/sst_acpi.c. The driver is responsible
  for two chips and differentiates between the two by detecting the
  presence of a particular HID. It would be possible to refactor the
  code so that atom/sst/sst_acpi.c passes down the matched HID to
  cht_bsw_rt5645.c, then it wouldn't be necessary to match for a
  second time. Also, the only difference between the two chipsets
  seems to be a minute change in struct snd_soc_dapm_route, so I'm
  wondering if it's necessary to differentiate between the chipsets
  at all. Mark Brown may want to poke the developers of the driver to
  refactor the code.

TL;DR: 6 of the 7 users can't use acpi_match_device_ids() and the
7th could be refactored so that it doesn't have to detect the presence
of a specific HID at all.

Best regards,

Lukas

         reply	other threads:[~2015-12-01 12:58 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-25 20:19 [Devel] [PATCH v2 0/2] Add acpi_dev_present Lukas Wunner
2015-11-25 20:19 ` Lukas Wunner
2015-11-25 20:19 ` [Devel] [PATCH v2 2/2] ACPI / utils: Add acpi_dev_present() Lukas Wunner
2015-11-25 20:19   ` Lukas Wunner
2015-11-30  8:27   ` Hanjun Guo
2015-12-01 12:58     ` Lukas Wunner [this message]
2015-12-01 12:58       ` Lukas Wunner
2015-12-02  8:26       ` Hanjun Guo
2015-11-25 20:19 ` [Devel] [PATCH v2 1/2] ACPI / scan: Fix acpi_bus_id_list bookkeeping Lukas Wunner
2015-11-25 20:19   ` Lukas Wunner
2015-11-30  6:27   ` Hanjun Guo
2015-11-30  8:14     ` Hanjun Guo
2015-12-01 13:08       ` [Devel] " Lukas Wunner
2015-12-01 13:08         ` Lukas Wunner
2015-12-02  8:56         ` Hanjun Guo
2015-12-06 21:09           ` [Devel] " Lukas Wunner
2015-12-06 21:09             ` Lukas Wunner
2015-12-07  1:39             ` Rafael J. Wysocki

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=20151201125814.GA28094@wunner.de \
    --to=devel@acpica.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 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.