Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1310181 > unrolled thread

Re: [PATCH 10/11] acpi: Export acpi_bus_type

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 10/11] acpi: Export acpi_bus_type "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-01-15 16:00 +0100
    Re: [PATCH 10/11] acpi: Export acpi_bus_type Ankitprasad Sharma <ankitprasad.r.sharma@intel.com> - 2016-01-18 10:30 +0100
      Re: [PATCH 10/11] acpi: Export acpi_bus_type "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-01-18 16:00 +0100
        Re: [PATCH 10/11] acpi: Export acpi_bus_type Lukas Wunner <lukas@wunner.de> - 2016-01-18 19:40 +0100
          Re: [PATCH 10/11] acpi: Export acpi_bus_type Ankitprasad Sharma <ankitprasad.r.sharma@intel.com> - 2016-01-19 09:40 +0100
        Re: [PATCH 10/11] acpi: Export acpi_bus_type "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-01-18 23:30 +0100
          Re: [PATCH 10/11] acpi: Export acpi_bus_type Lukas Wunner <lukas@wunner.de> - 2016-01-18 23:40 +0100
            Re: [PATCH 10/11] acpi: Export acpi_bus_type "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-01-18 23:50 +0100
              Re: [PATCH 10/11] acpi: Export acpi_bus_type Lukas Wunner <lukas@wunner.de> - 2016-01-19 00:10 +0100
                Re: [PATCH 10/11] acpi: Export acpi_bus_type "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-01-19 01:00 +0100
                  Re: [PATCH 10/11] acpi: Export acpi_bus_type Lukas Wunner <lukas@wunner.de> - 2016-01-19 17:40 +0100
                    Re: [PATCH 10/11] acpi: Export acpi_bus_type "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-01-19 23:10 +0100

#1310181 — Re: [PATCH 10/11] acpi: Export acpi_bus_type

On Thursday, January 14, 2016 11:46:46 AM ankitprasad.r.sharma@intel.com wrote:
> From: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> 
> Some modules, like i915.ko, needs to detect when certain ACPI features
> are active inorder to prevent corruption on contended resources.
> In particular, use of BIOS RapidStart Technology may corrupt the contents
> of the reserved graphics memory, due to unalarmed hibernation. In which
> case i915.ko cannot assume that it (reserved gfx memory) remains
> unmodified and must recreate teh contents and importantly not use it to
> store unrecoverable user data.
> 
> Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Len Brown <lenb@kernel.org>
> Cc: linux-acpi@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  drivers/acpi/bus.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index a212cef..69509c7 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -814,6 +814,7 @@ struct bus_type acpi_bus_type = {
>  	.remove		= acpi_device_remove,
>  	.uevent		= acpi_device_uevent,
>  };
> +EXPORT_SYMBOL_GPL(acpi_bus_type);
>  
>  /* --------------------------------------------------------------------------
>                               Initialization/Cleanup
> 

No.

I see no reason whatsoever for doing this.

Thanks,
Rafael

[toc] | [next] | [standalone]

#1311410

On Fri, 2016-01-15 at 15:51 +0100, Rafael J. Wysocki wrote:
> On Thursday, January 14, 2016 11:46:46 AM ankitprasad.r.sharma@intel.com wrote:
> > From: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> > 
> > Some modules, like i915.ko, needs to detect when certain ACPI features
> > are active inorder to prevent corruption on contended resources.
> > In particular, use of BIOS RapidStart Technology may corrupt the contents
> > of the reserved graphics memory, due to unalarmed hibernation. In which
> > case i915.ko cannot assume that it (reserved gfx memory) remains
> > unmodified and must recreate teh contents and importantly not use it to
> > store unrecoverable user data.
> > 
> > Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > Cc: Len Brown <lenb@kernel.org>
> > Cc: linux-acpi@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > ---
> >  drivers/acpi/bus.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> > index a212cef..69509c7 100644
> > --- a/drivers/acpi/bus.c
> > +++ b/drivers/acpi/bus.c
> > @@ -814,6 +814,7 @@ struct bus_type acpi_bus_type = {
> >  	.remove		= acpi_device_remove,
> >  	.uevent		= acpi_device_uevent,
> >  };
> > +EXPORT_SYMBOL_GPL(acpi_bus_type);
> >  
> >  /* --------------------------------------------------------------------------
> >                               Initialization/Cleanup
> > 
> 
> No.
> 
> I see no reason whatsoever for doing this.
> 
> Thanks,
> Rafael
Hi Rafael,

Thanks for the response.

Can you please help me with, how to detect the presence of a certain
acpi device using its id (for example, INT3392 for Intel RST device)? 

As you might have seen (in the next patch in this series), that we use
this symbol (acpi_bus_type) to iterate over all the devices registered
on acpi bus, to check if there is a device with id INT3392 present or
not.

Thanks,
Ankit

[toc] | [prev] | [next] | [standalone]

#1311597

On Monday, January 18, 2016 02:31:00 PM Ankitprasad Sharma wrote:
> On Fri, 2016-01-15 at 15:51 +0100, Rafael J. Wysocki wrote:
> > On Thursday, January 14, 2016 11:46:46 AM ankitprasad.r.sharma@intel.com wrote:
> > > From: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> > > 
> > > Some modules, like i915.ko, needs to detect when certain ACPI features
> > > are active inorder to prevent corruption on contended resources.
> > > In particular, use of BIOS RapidStart Technology may corrupt the contents
> > > of the reserved graphics memory, due to unalarmed hibernation. In which
> > > case i915.ko cannot assume that it (reserved gfx memory) remains
> > > unmodified and must recreate teh contents and importantly not use it to
> > > store unrecoverable user data.
> > > 
> > > Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> > > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > > Cc: Len Brown <lenb@kernel.org>
> > > Cc: linux-acpi@vger.kernel.org
> > > Cc: linux-kernel@vger.kernel.org
> > > ---
> > >  drivers/acpi/bus.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> > > index a212cef..69509c7 100644
> > > --- a/drivers/acpi/bus.c
> > > +++ b/drivers/acpi/bus.c
> > > @@ -814,6 +814,7 @@ struct bus_type acpi_bus_type = {
> > >  	.remove		= acpi_device_remove,
> > >  	.uevent		= acpi_device_uevent,
> > >  };
> > > +EXPORT_SYMBOL_GPL(acpi_bus_type);
> > >  
> > >  /* --------------------------------------------------------------------------
> > >                               Initialization/Cleanup
> > > 
> > 
> > No.
> > 
> > I see no reason whatsoever for doing this.
> > 
> > Thanks,
> > Rafael
> Hi Rafael,
> 
> Thanks for the response.
> 
> Can you please help me with, how to detect the presence of a certain
> acpi device using its id (for example, INT3392 for Intel RST device)? 

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).

> As you might have seen (in the next patch in this series), that we use
> this symbol (acpi_bus_type) to iterate over all the devices registered
> on acpi bus, to check if there is a device with id INT3392 present or
> not.

Please don't do that this way.

I'll have a look at the other patch later.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1311743

Hi,

On Mon, Jan 18, 2016 at 03:57:29PM +0100, Rafael J. Wysocki wrote:
> On Monday, January 18, 2016 02:31:00 PM Ankitprasad Sharma wrote:
> > On Fri, 2016-01-15 at 15:51 +0100, Rafael J. Wysocki wrote:
> > > On Thursday, January 14, 2016 11:46:46 AM ankitprasad.r.sharma@intel.com wrote:
> > > > From: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> > > > 
> > > > Some modules, like i915.ko, needs to detect when certain ACPI features
> > > > are active inorder to prevent corruption on contended resources.
> > > > In particular, use of BIOS RapidStart Technology may corrupt the contents
> > > > of the reserved graphics memory, due to unalarmed hibernation. In which
> > > > case i915.ko cannot assume that it (reserved gfx memory) remains
> > > > unmodified and must recreate teh contents and importantly not use it to
> > > > store unrecoverable user data.
> > > > 
> > > > Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> > > > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > > > Cc: Len Brown <lenb@kernel.org>
> > > > Cc: linux-acpi@vger.kernel.org
> > > > Cc: linux-kernel@vger.kernel.org
> > > > ---
> > > >  drivers/acpi/bus.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> > > > index a212cef..69509c7 100644
> > > > --- a/drivers/acpi/bus.c
> > > > +++ b/drivers/acpi/bus.c
> > > > @@ -814,6 +814,7 @@ struct bus_type acpi_bus_type = {
> > > >  	.remove		= acpi_device_remove,
> > > >  	.uevent		= acpi_device_uevent,
> > > >  };
> > > > +EXPORT_SYMBOL_GPL(acpi_bus_type);
> > > >  
> > > >  /* --------------------------------------------------------------------------
> > > >                               Initialization/Cleanup
> > > > 
> > > 
> > > No.
> > > 
> > > I see no reason whatsoever for doing this.
> > > 
> > > Thanks,
> > > Rafael
> > Hi Rafael,
> > 
> > Thanks for the response.
> > 
> > Can you please help me with, how to detect the presence of a certain
> > acpi device using its id (for example, INT3392 for Intel RST device)? 
> 
> 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).

acpi_dev_present() is exported, so can be used in drivers just fine:
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id=2d12b6b381ba059d5f92798f5ea739672a2f5fcf


> > As you might have seen (in the next patch in this series), that we use
> > this symbol (acpi_bus_type) to iterate over all the devices registered
> > on acpi bus, to check if there is a device with id INT3392 present or
> > not.

Ankitprasad, just change your patch [11/11] thusly:

-       if (intel_detect_acpi_rst()) {
+       if (acpi_dev_present("INT3392")) {


Using bus_for_each_dev() was the wrong approach, most drivers call
acpi_get_devices() to detect the presence of a particular HID,
however that necessitates the definition of a callback in each driver,
leading to lots of duplicate code. Hence the introduction of
acpi_dev_present() which is also faster because it just iterates over
a list instead of walking the namespace.

This new API landed in Linus' tree last Tuesday (PST), so you need
to merge Linus' tree back into yours or wait until it gets merged
into drm-intel-nightly.

Best regards,

Lukas

[toc] | [prev] | [next] | [standalone]

#1312004

On Mon, 2016-01-18 at 19:26 +0100, Lukas Wunner wrote:
Hi,
> Hi,
> 
> On Mon, Jan 18, 2016 at 03:57:29PM +0100, Rafael J. Wysocki wrote:
> > On Monday, January 18, 2016 02:31:00 PM Ankitprasad Sharma wrote:
> > > On Fri, 2016-01-15 at 15:51 +0100, Rafael J. Wysocki wrote:
> > > > On Thursday, January 14, 2016 11:46:46 AM ankitprasad.r.sharma@intel.com wrote:
> > > > > From: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> > > > > 
> > > > > Some modules, like i915.ko, needs to detect when certain ACPI features
> > > > > are active inorder to prevent corruption on contended resources.
> > > > > In particular, use of BIOS RapidStart Technology may corrupt the contents
> > > > > of the reserved graphics memory, due to unalarmed hibernation. In which
> > > > > case i915.ko cannot assume that it (reserved gfx memory) remains
> > > > > unmodified and must recreate teh contents and importantly not use it to
> > > > > store unrecoverable user data.
> > > > > 
> > > > > Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> > > > > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > > > > Cc: Len Brown <lenb@kernel.org>
> > > > > Cc: linux-acpi@vger.kernel.org
> > > > > Cc: linux-kernel@vger.kernel.org
> > > > > ---
> > > > >  drivers/acpi/bus.c | 1 +
> > > > >  1 file changed, 1 insertion(+)
> > > > > 
> > > > > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> > > > > index a212cef..69509c7 100644
> > > > > --- a/drivers/acpi/bus.c
> > > > > +++ b/drivers/acpi/bus.c
> > > > > @@ -814,6 +814,7 @@ struct bus_type acpi_bus_type = {
> > > > >  	.remove		= acpi_device_remove,
> > > > >  	.uevent		= acpi_device_uevent,
> > > > >  };
> > > > > +EXPORT_SYMBOL_GPL(acpi_bus_type);
> > > > >  
> > > > >  /* --------------------------------------------------------------------------
> > > > >                               Initialization/Cleanup
> > > > > 
> > > > 
> > > > No.
> > > > 
> > > > I see no reason whatsoever for doing this.
> > > > 
> > > > Thanks,
> > > > Rafael
> > > Hi Rafael,
> > > 
> > > Thanks for the response.
> > > 
> > > Can you please help me with, how to detect the presence of a certain
> > > acpi device using its id (for example, INT3392 for Intel RST device)? 
> > 
> > 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).
> 
> acpi_dev_present() is exported, so can be used in drivers just fine:
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id=2d12b6b381ba059d5f92798f5ea739672a2f5fcf
> 
> 
> > > As you might have seen (in the next patch in this series), that we use
> > > this symbol (acpi_bus_type) to iterate over all the devices registered
> > > on acpi bus, to check if there is a device with id INT3392 present or
> > > not.
> 
> Ankitprasad, just change your patch [11/11] thusly:
> 
> -       if (intel_detect_acpi_rst()) {
> +       if (acpi_dev_present("INT3392")) {
> 
> 
> Using bus_for_each_dev() was the wrong approach, most drivers call
> acpi_get_devices() to detect the presence of a particular HID,
> however that necessitates the definition of a callback in each driver,
> leading to lots of duplicate code. Hence the introduction of
> acpi_dev_present() which is also faster because it just iterates over
> a list instead of walking the namespace.
> 
> This new API landed in Linus' tree last Tuesday (PST), so you need
> to merge Linus' tree back into yours or wait until it gets merged
> into drm-intel-nightly.
> 
> Best regards,
> 
> Lukas

Thank you, Lukas/Rafael.

-Ankit

[toc] | [prev] | [next] | [standalone]

#1311830

On Monday, January 18, 2016 03:57:29 PM Rafael J. Wysocki wrote:
> On Monday, January 18, 2016 02:31:00 PM Ankitprasad Sharma wrote:
> > On Fri, 2016-01-15 at 15:51 +0100, Rafael J. Wysocki wrote:
> > > On Thursday, January 14, 2016 11:46:46 AM ankitprasad.r.sharma@intel.com wrote:
> > > > From: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> > > > 
> > > > Some modules, like i915.ko, needs to detect when certain ACPI features
> > > > are active inorder to prevent corruption on contended resources.
> > > > In particular, use of BIOS RapidStart Technology may corrupt the contents
> > > > of the reserved graphics memory, due to unalarmed hibernation. In which
> > > > case i915.ko cannot assume that it (reserved gfx memory) remains
> > > > unmodified and must recreate teh contents and importantly not use it to
> > > > store unrecoverable user data.
> > > > 
> > > > Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> > > > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > > > Cc: Len Brown <lenb@kernel.org>
> > > > Cc: linux-acpi@vger.kernel.org
> > > > Cc: linux-kernel@vger.kernel.org
> > > > ---
> > > >  drivers/acpi/bus.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> > > > index a212cef..69509c7 100644
> > > > --- a/drivers/acpi/bus.c
> > > > +++ b/drivers/acpi/bus.c
> > > > @@ -814,6 +814,7 @@ struct bus_type acpi_bus_type = {
> > > >  	.remove		= acpi_device_remove,
> > > >  	.uevent		= acpi_device_uevent,
> > > >  };
> > > > +EXPORT_SYMBOL_GPL(acpi_bus_type);
> > > >  
> > > >  /* --------------------------------------------------------------------------
> > > >                               Initialization/Cleanup
> > > > 
> > > 
> > > No.
> > > 
> > > I see no reason whatsoever for doing this.
> > > 
> > > Thanks,
> > > Rafael
> > Hi Rafael,
> > 
> > Thanks for the response.
> > 
> > Can you please help me with, how to detect the presence of a certain
> > acpi device using its id (for example, INT3392 for Intel RST device)? 
> 
> 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.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1311835

Hi,

On Mon, Jan 18, 2016 at 11:28:27PM +0100, Rafael J. Wysocki wrote:
> On Monday, January 18, 2016 03:57:29 PM Rafael J. Wysocki wrote:
> > On Monday, January 18, 2016 02:31:00 PM Ankitprasad Sharma wrote:
> > > On Fri, 2016-01-15 at 15:51 +0100, Rafael J. Wysocki wrote:
> > > > On Thursday, January 14, 2016 11:46:46 AM ankitprasad.r.sharma@intel.com wrote:
> > > > > From: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> > > > > 
> > > > > Some modules, like i915.ko, needs to detect when certain ACPI features
> > > > > are active inorder to prevent corruption on contended resources.
> > > > > In particular, use of BIOS RapidStart Technology may corrupt the contents
> > > > > of the reserved graphics memory, due to unalarmed hibernation. In which
> > > > > case i915.ko cannot assume that it (reserved gfx memory) remains
> > > > > unmodified and must recreate teh contents and importantly not use it to
> > > > > store unrecoverable user data.
> > > > > 
> > > > > Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> > > > > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > > > > Cc: Len Brown <lenb@kernel.org>
> > > > > Cc: linux-acpi@vger.kernel.org
> > > > > Cc: linux-kernel@vger.kernel.org
> > > > > ---
> > > > >  drivers/acpi/bus.c | 1 +
> > > > >  1 file changed, 1 insertion(+)
> > > > > 
> > > > > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> > > > > index a212cef..69509c7 100644
> > > > > --- a/drivers/acpi/bus.c
> > > > > +++ b/drivers/acpi/bus.c
> > > > > @@ -814,6 +814,7 @@ struct bus_type acpi_bus_type = {
> > > > >  	.remove		= acpi_device_remove,
> > > > >  	.uevent		= acpi_device_uevent,
> > > > >  };
> > > > > +EXPORT_SYMBOL_GPL(acpi_bus_type);
> > > > >  
> > > > >  /* --------------------------------------------------------------------------
> > > > >                               Initialization/Cleanup
> > > > > 
> > > > 
> > > > No.
> > > > 
> > > > I see no reason whatsoever for doing this.
> > > > 
> > > > Thanks,
> > > > Rafael
> > > Hi Rafael,
> > > 
> > > Thanks for the response.
> > > 
> > > Can you please help me with, how to detect the presence of a certain
> > > acpi device using its id (for example, INT3392 for Intel RST device)? 
> > 
> > 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.

Best regards,

Lukas

[toc] | [prev] | [next] | [standalone]

#1311837

On Monday, January 18, 2016 11:39:07 PM Lukas Wunner wrote:
> Hi,
> 
> On Mon, Jan 18, 2016 at 11:28:27PM +0100, Rafael J. Wysocki wrote:
> > On Monday, January 18, 2016 03:57:29 PM Rafael J. Wysocki wrote:
> > > On Monday, January 18, 2016 02:31:00 PM Ankitprasad Sharma wrote:
> > > > On Fri, 2016-01-15 at 15:51 +0100, Rafael J. Wysocki wrote:
> > > > > On Thursday, January 14, 2016 11:46:46 AM ankitprasad.r.sharma@intel.com wrote:
> > > > > > From: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> > > > > > 
> > > > > > Some modules, like i915.ko, needs to detect when certain ACPI features
> > > > > > are active inorder to prevent corruption on contended resources.
> > > > > > In particular, use of BIOS RapidStart Technology may corrupt the contents
> > > > > > of the reserved graphics memory, due to unalarmed hibernation. In which
> > > > > > case i915.ko cannot assume that it (reserved gfx memory) remains
> > > > > > unmodified and must recreate teh contents and importantly not use it to
> > > > > > store unrecoverable user data.
> > > > > > 
> > > > > > Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> > > > > > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > > > > > Cc: Len Brown <lenb@kernel.org>
> > > > > > Cc: linux-acpi@vger.kernel.org
> > > > > > Cc: linux-kernel@vger.kernel.org
> > > > > > ---
> > > > > >  drivers/acpi/bus.c | 1 +
> > > > > >  1 file changed, 1 insertion(+)
> > > > > > 
> > > > > > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> > > > > > index a212cef..69509c7 100644
> > > > > > --- a/drivers/acpi/bus.c
> > > > > > +++ b/drivers/acpi/bus.c
> > > > > > @@ -814,6 +814,7 @@ struct bus_type acpi_bus_type = {
> > > > > >  	.remove		= acpi_device_remove,
> > > > > >  	.uevent		= acpi_device_uevent,
> > > > > >  };
> > > > > > +EXPORT_SYMBOL_GPL(acpi_bus_type);
> > > > > >  
> > > > > >  /* --------------------------------------------------------------------------
> > > > > >                               Initialization/Cleanup
> > > > > > 
> > > > > 
> > > > > No.
> > > > > 
> > > > > I see no reason whatsoever for doing this.
> > > > > 
> > > > > Thanks,
> > > > > Rafael
> > > > Hi Rafael,
> > > > 
> > > > Thanks for the response.
> > > > 
> > > > Can you please help me with, how to detect the presence of a certain
> > > > acpi device using its id (for example, INT3392 for Intel RST device)? 
> > > 
> > > 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.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1311841

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:
> > Hi,
> > 
> > On Mon, Jan 18, 2016 at 11:28:27PM +0100, Rafael J. Wysocki wrote:
> > > On Monday, January 18, 2016 03:57:29 PM Rafael J. Wysocki wrote:
> > > > On Monday, January 18, 2016 02:31:00 PM Ankitprasad Sharma wrote:
> > > > > On Fri, 2016-01-15 at 15:51 +0100, Rafael J. Wysocki wrote:
> > > > > > On Thursday, January 14, 2016 11:46:46 AM ankitprasad.r.sharma@intel.com wrote:
> > > > > > > From: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> > > > > > > 
> > > > > > > Some modules, like i915.ko, needs to detect when certain ACPI features
> > > > > > > are active inorder to prevent corruption on contended resources.
> > > > > > > In particular, use of BIOS RapidStart Technology may corrupt the contents
> > > > > > > of the reserved graphics memory, due to unalarmed hibernation. In which
> > > > > > > case i915.ko cannot assume that it (reserved gfx memory) remains
> > > > > > > unmodified and must recreate teh contents and importantly not use it to
> > > > > > > store unrecoverable user data.
> > > > > > > 
> > > > > > > Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> > > > > > > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > > > > > > Cc: Len Brown <lenb@kernel.org>
> > > > > > > Cc: linux-acpi@vger.kernel.org
> > > > > > > Cc: linux-kernel@vger.kernel.org
> > > > > > > ---
> > > > > > >  drivers/acpi/bus.c | 1 +
> > > > > > >  1 file changed, 1 insertion(+)
> > > > > > > 
> > > > > > > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> > > > > > > index a212cef..69509c7 100644
> > > > > > > --- a/drivers/acpi/bus.c
> > > > > > > +++ b/drivers/acpi/bus.c
> > > > > > > @@ -814,6 +814,7 @@ struct bus_type acpi_bus_type = {
> > > > > > >  	.remove		= acpi_device_remove,
> > > > > > >  	.uevent		= acpi_device_uevent,
> > > > > > >  };
> > > > > > > +EXPORT_SYMBOL_GPL(acpi_bus_type);
> > > > > > >  
> > > > > > >  /* --------------------------------------------------------------------------
> > > > > > >                               Initialization/Cleanup
> > > > > > > 
> > > > > > 
> > > > > > No.
> > > > > > 
> > > > > > I see no reason whatsoever for doing this.
> > > > > > 
> > > > > > Thanks,
> > > > > > Rafael
> > > > > Hi Rafael,
> > > > > 
> > > > > Thanks for the response.
> > > > > 
> > > > > Can you please help me with, how to detect the presence of a certain
> > > > > acpi device using its id (for example, INT3392 for Intel RST device)?
> > > > 
> > > > 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.

I've posted 5 patches over the last days which use acpi_dev_present():

http://lists.freedesktop.org/archives/dri-devel/2016-January/098403.html
http://mailman.alsa-project.org/pipermail/alsa-devel/2016-January/103056.html
http://mailman.alsa-project.org/pipermail/alsa-devel/2016-January/103058.html
http://thread.gmane.org/gmane.linux.drivers.platform.x86.devel/8474/focus=8475
http://thread.gmane.org/gmane.linux.drivers.platform.x86.devel/8474/focus=8476

When considering a rename of the function, please bear in mind that it
will cause breakage for anyone testing or merging these patches.
(Postponing a rename until these patches have landed would avoid that.)

Thanks,

Lukas

[toc] | [prev] | [next] | [standalone]

#1311856

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.

> 
> I've posted 5 patches over the last days which use acpi_dev_present():
> 
> http://lists.freedesktop.org/archives/dri-devel/2016-January/098403.html
> http://mailman.alsa-project.org/pipermail/alsa-devel/2016-January/103056.html
> http://mailman.alsa-project.org/pipermail/alsa-devel/2016-January/103058.html
> http://thread.gmane.org/gmane.linux.drivers.platform.x86.devel/8474/focus=8475
> http://thread.gmane.org/gmane.linux.drivers.platform.x86.devel/8474/focus=8476
> 
> When considering a rename of the function, please bear in mind that it
> will cause breakage for anyone testing or merging these patches.
> (Postponing a rename until these patches have landed would avoid that.)

No problem with that.  Please let me know when these patches are merged.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1312372

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)

[toc] | [prev] | [next] | [standalone]

#1312551

On Tuesday, January 19, 2016 05:31:04 PM Lukas Wunner wrote:
> 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.

I know, but it doesn't matter too much.  I don't want people to wonder
what the difference between acpi_dev_present() and acpi_device_is_present()
is and when to use which of them.

> 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.

Again, I know, but the name of the function should be different.

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

That's OK, but it's not enough.

I guess it won't be a big deal to change the function name and rebase
the patches depending on it on top of that change, will it?

Thanks,
Rafael

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web