intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Lukas Wunner <lukas@wunner.de>
To: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>,
	intel-gfx@lists.freedesktop.org, akash.goel@intel.com,
	shashidhar.hiremath@intel.com, tvrtko.ursulin@linux.intel.com,
	Len Brown <lenb@kernel.org>,
	linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 10/11] acpi: Export acpi_bus_type
Date: Tue, 19 Jan 2016 17:31:04 +0100	[thread overview]
Message-ID: <20160119163104.GA9469@wunner.de> (raw)
In-Reply-To: <5484189.RBRMZdsTjH@vostro.rjw.lan>

Hi Rafael,

On Tue, Jan 19, 2016 at 12:59:13AM +0100, Rafael J. Wysocki wrote:
> On Tuesday, January 19, 2016 12:00:47 AM Lukas Wunner wrote:
> > Hi,
> > 
> > On Mon, Jan 18, 2016 at 11:46:18PM +0100, Rafael J. Wysocki wrote:
> > > On Monday, January 18, 2016 11:39:07 PM Lukas Wunner wrote:
> 
> [cut]
> 
> > > > > > If you want to check if the device ir present at all, you cen use
> > > > > > acpi_device_is_present() introduced recently (although that would need
> > > > > > to be exported if you want to use it from a driver).
> > > > > 
> > > > > I meant acpi_dev_present(), sorry about the mistake.
> > > > > 
> > > > > I guess we should rename it to acpi_device_found() or something similar
> > > > > to avoid such confusion in the future.
> > > > 
> > > > The name was chosen because the PCI equivalent is called pci_dev_present()
> > > > and I assumed that name already stuck in developers' heads, so if they're
> > > > looking for an ACPI presence detection function, that's what they'd look
> > > > for first.
> > > 
> > > But "present" in ACPI really means something different.  There may be ACPI
> > > device objects in the namespace for devices that are not *actually* present.
> > 
> > You mean synthesized devices like LNXSYBUS?
> > Don't think anyone is going to test for the presence of that.
> 
> No, I mean real devices, where the corresponding ACPI object has _STA that
> returns 0.
> 
> There may be a couple of reasons for that.  The device the ACPI object
> corresponds to may not be physically present (eg. it may possible to
> hot-add it) or the device may depend on something else for functionality
> and that thing hasn't been set up yet etc.
> 
> The presence of an ACPI device object in the namespace means that the
> platform firmware knows about the device, but it need not mean that
> the device is really there.  _STA returns that piece of information.

Thank you for the clarification, these are very good points.

The drivers in question use acpi_get_devices() merely to probe for
presence of a device in the namespace. They do not invoke _STA,
nor do they even hold a pointer to the acpi_device or acpi_handle
when detecting presence. Mostly this is about activating quirks
if a certain ACPI device is detected.

Currently about 50% of the calls to acpi_get_devices() in the drivers
fit this pattern and the point of acpi_dev_present() is to give
developers a simple, lightweight tool as an alternative.

However the kernel-doc should be amended to clarify that _STA is not
invoked. The patch below is a suggestion, feel free to rephrase.

Thanks & best regards,

Lukas

-- >8 --
Subject: [PATCH] ACPI / utils: Clarify appropriate usage of acpi_dev_present()

Rafael J. Wysocki pointed out that even though a device is present
in the namespace, its _STA control method might still return 0 in the
"device is present" bit. Amend the documentation accordingly.

Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/acpi/utils.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
index f2f9873..99af3bc 100644
--- a/drivers/acpi/utils.c
+++ b/drivers/acpi/utils.c
@@ -716,6 +716,8 @@ EXPORT_SYMBOL(acpi_check_dsm);
  *
  * Return %true if the device was present at the moment of invocation.
  * Note that if the device is pluggable, it may since have disappeared.
+ * Also, this merely checks presence in the namespace but does not
+ * invoke the _STA control method.
  *
  * For this function to work, acpi_bus_scan() must have been executed
  * which happens in the subsys_initcall() subsection. Hence, do not
-- 
1.8.5.2 (Apple Git-48)

  reply	other threads:[~2016-01-19 16:31 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-14  6:16 [PATCH v14 0/11] Support for creating/using Stolen memory backed objects ankitprasad.r.sharma
2016-01-14  6:16 ` [PATCH 01/11] drm/i915: Add support for mapping an object page by page ankitprasad.r.sharma
2016-01-14 10:32   ` Tvrtko Ursulin
2016-01-14 13:34     ` Chris Wilson
2016-01-15  9:53       ` Tvrtko Ursulin
2016-01-15 11:44         ` Chris Wilson
2016-01-15 15:15           ` Tvrtko Ursulin
2016-01-14  6:16 ` [PATCH 02/11] drm/i915: Introduce i915_gem_object_get_dma_address() ankitprasad.r.sharma
2016-01-14  6:16 ` [PATCH 03/11] drm/i915: Use insert_page for pwrite_fast ankitprasad.r.sharma
2016-01-14  9:46   ` Chris Wilson
2016-01-14  6:16 ` [PATCH 04/11] drm/i915: Clearing buffer objects via CPU/GTT ankitprasad.r.sharma
2016-01-14 10:06   ` Chris Wilson
2016-01-14  6:16 ` [PATCH 05/11] drm/i915: Support for creating Stolen memory backed objects ankitprasad.r.sharma
2016-01-14 10:24   ` Chris Wilson
2016-01-14 10:46     ` Tvrtko Ursulin
2016-01-14 11:14       ` Chris Wilson
2016-01-14 11:27         ` Tvrtko Ursulin
2016-01-14 11:57           ` Chris Wilson
2016-01-14  6:16 ` [PATCH 06/11] drm/i915: Propagating correct error codes to the userspace ankitprasad.r.sharma
2016-01-14 10:25   ` Chris Wilson
2016-01-14  6:16 ` [PATCH 07/11] drm/i915: Add support for stealing purgable stolen pages ankitprasad.r.sharma
2016-01-14  6:16 ` [PATCH 08/11] drm/i915: Support for pread/pwrite from/to non shmem backed objects ankitprasad.r.sharma
2016-01-14 10:51   ` Chris Wilson
2016-01-14  6:16 ` [PATCH 09/11] drm/i915: Migrate stolen objects before hibernation ankitprasad.r.sharma
2016-01-14 10:53   ` Chris Wilson
2016-01-14  6:16 ` [PATCH 10/11] acpi: Export acpi_bus_type ankitprasad.r.sharma
2016-01-15 14:51   ` Rafael J. Wysocki
2016-01-18  9:01     ` Ankitprasad Sharma
2016-01-18 14:57       ` Rafael J. Wysocki
2016-01-18 18:26         ` Lukas Wunner
2016-01-19  8:15           ` Ankitprasad Sharma
2016-01-18 22:28         ` Rafael J. Wysocki
2016-01-18 22:39           ` Lukas Wunner
2016-01-18 22:46             ` Rafael J. Wysocki
2016-01-18 23:00               ` Lukas Wunner
2016-01-18 23:59                 ` Rafael J. Wysocki
2016-01-19 16:31                   ` Lukas Wunner [this message]
2016-01-19 22:03                     ` Rafael J. Wysocki
2016-01-14  6:16 ` [PATCH 11/11] drm/i915: Disable use of stolen area by User when Intel RST is present ankitprasad.r.sharma
2016-01-14 11:20 ` ✗ failure: Fi.CI.BAT Patchwork

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=20160119163104.GA9469@wunner.de \
    --to=lukas@wunner.de \
    --cc=akash.goel@intel.com \
    --cc=ankitprasad.r.sharma@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rjw@rjwysocki.net \
    --cc=shashidhar.hiremath@intel.com \
    --cc=tvrtko.ursulin@linux.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
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).