Re: [PATCH] ACPI / scan: Always call acpi_bus_scan() for bus check notifications

2013-07-11 Thread Toshi Kani
On Thu, 2013-07-11 at 02:39 +0200, Rafael J. Wysocki wrote:
> On Wednesday, July 10, 2013 05:48:26 PM Toshi Kani wrote:
> > On Thu, 2013-07-11 at 00:45 +0200, Rafael J. Wysocki wrote:
> > > On Wednesday, July 10, 2013 02:11:05 AM Rafael J. Wysocki wrote:
> > > > On Tuesday, July 09, 2013 01:32:42 PM Toshi Kani wrote:
> > > > > On Mon, 2013-07-08 at 02:10 +0200, Rafael J. Wysocki wrote:
> > > > > > From: Rafael J. Wysocki 
> > > > > > 
> > > > > > An ACPI_NOTIFY_BUS_CHECK notification means that we should scan the
> > > > > > entire namespace starting from the given handle even if the device
> > > > > > represented by that handle is present (other devices below it may
> > > > > > just have been added).
> > > > > > 
> > > > > > For this reason, modify acpi_scan_bus_device_check() to always run
> > > > > > acpi_bus_scan() if the notification being handled is of type
> > > > > > ACPI_NOTIFY_BUS_CHECK.
> > > > > > 
> > > > > > Signed-off-by: Rafael J. Wysocki 
> > > > > > Cc: 3.10+ 
> > > > > 
> > > > > Acked-by: Toshi Kani 
> > > > > 
> > > > > But, I think we need the additional patch below.
> > > > 
> > > > Yes, I think you're right.
> > > 
> > > That said I'd prefer to put the check into acpi_bus_device_attach() like 
> > > in
> > > the appended patch.
> > 
> > That's fine by me.  
> > 
> > Acked-by: Toshi Kani 
> > 
> > Just a minor point, though.  Isn't it a bit inconsistent with
> > device_attach(), which checks dev->driver inside the function?
> 
> Well, device_attach() may be called from different places while this is
> the only place where acpi_scan_attach_handler() is called.
> 
> The check in acpi_bus_device_attach() is easier to follow to me, because
> it clearly means "we don't need to do anything more if there's a handler",
> while the check in acpi_scan_attach_handler() makes you wonder "why do we
> need to return 1 in that case?" and then you need to go to the caller and
> look at the check of the return value to see "ah, because we don't want
> that device_attach() to be called then!".

Sounds good to me.

Thanks,
-Toshi


> 
> > That said, I am OK with either way.
> 
> Cool. :-)
> 
> Thanks,
> Rafael
> 
> 
> > > ---
> > > From: Rafael J. Wysocki 
> > > Subject: ACPI / scan: Do not try to attach scan handlers to devices 
> > > having them
> > > 
> > > In acpi_bus_device_attach(), if there is an ACPI device object
> > > for the given handle and that device object has a scan handler
> > > attached to it already, there's nothing more to do for that handle
> > > and the function should just return success immediately.  Make
> > > that happen.
> > > 
> > > Reported-by: Toshi Kani 
> > > Signed-off-by: Rafael J. Wysocki 
> > > ---
> > >  drivers/acpi/scan.c |3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > Index: linux-pm/drivers/acpi/scan.c
> > > ===
> > > --- linux-pm.orig/drivers/acpi/scan.c
> > > +++ linux-pm/drivers/acpi/scan.c
> > > @@ -1984,6 +1984,9 @@ static acpi_status acpi_bus_device_attac
> > >   if (acpi_bus_get_device(handle, ))
> > >   return AE_CTRL_DEPTH;
> > >  
> > > + if (device->handler)
> > > + return AE_OK;
> > > +
> > >   ret = acpi_scan_attach_handler(device);
> > >   if (ret)
> > >   return ret > 0 ? AE_OK : AE_CTRL_DEPTH;
> > > 
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > > the body of a message to majord...@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ACPI / scan: Always call acpi_bus_scan() for bus check notifications

2013-07-11 Thread Toshi Kani
On Thu, 2013-07-11 at 02:39 +0200, Rafael J. Wysocki wrote:
 On Wednesday, July 10, 2013 05:48:26 PM Toshi Kani wrote:
  On Thu, 2013-07-11 at 00:45 +0200, Rafael J. Wysocki wrote:
   On Wednesday, July 10, 2013 02:11:05 AM Rafael J. Wysocki wrote:
On Tuesday, July 09, 2013 01:32:42 PM Toshi Kani wrote:
 On Mon, 2013-07-08 at 02:10 +0200, Rafael J. Wysocki wrote:
  From: Rafael J. Wysocki rafael.j.wyso...@intel.com
  
  An ACPI_NOTIFY_BUS_CHECK notification means that we should scan the
  entire namespace starting from the given handle even if the device
  represented by that handle is present (other devices below it may
  just have been added).
  
  For this reason, modify acpi_scan_bus_device_check() to always run
  acpi_bus_scan() if the notification being handled is of type
  ACPI_NOTIFY_BUS_CHECK.
  
  Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
  Cc: 3.10+ sta...@vger.kernel.org
 
 Acked-by: Toshi Kani toshi.k...@hp.com
 
 But, I think we need the additional patch below.

Yes, I think you're right.
   
   That said I'd prefer to put the check into acpi_bus_device_attach() like 
   in
   the appended patch.
  
  That's fine by me.  
  
  Acked-by: Toshi Kani toshi.k...@hp.com
  
  Just a minor point, though.  Isn't it a bit inconsistent with
  device_attach(), which checks dev-driver inside the function?
 
 Well, device_attach() may be called from different places while this is
 the only place where acpi_scan_attach_handler() is called.
 
 The check in acpi_bus_device_attach() is easier to follow to me, because
 it clearly means we don't need to do anything more if there's a handler,
 while the check in acpi_scan_attach_handler() makes you wonder why do we
 need to return 1 in that case? and then you need to go to the caller and
 look at the check of the return value to see ah, because we don't want
 that device_attach() to be called then!.

Sounds good to me.

Thanks,
-Toshi


 
  That said, I am OK with either way.
 
 Cool. :-)
 
 Thanks,
 Rafael
 
 
   ---
   From: Rafael J. Wysocki rafael.j.wyso...@intel.com
   Subject: ACPI / scan: Do not try to attach scan handlers to devices 
   having them
   
   In acpi_bus_device_attach(), if there is an ACPI device object
   for the given handle and that device object has a scan handler
   attached to it already, there's nothing more to do for that handle
   and the function should just return success immediately.  Make
   that happen.
   
   Reported-by: Toshi Kani toshi.k...@hp.com
   Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
   ---
drivers/acpi/scan.c |3 +++
1 file changed, 3 insertions(+)
   
   Index: linux-pm/drivers/acpi/scan.c
   ===
   --- linux-pm.orig/drivers/acpi/scan.c
   +++ linux-pm/drivers/acpi/scan.c
   @@ -1984,6 +1984,9 @@ static acpi_status acpi_bus_device_attac
 if (acpi_bus_get_device(handle, device))
 return AE_CTRL_DEPTH;

   + if (device-handler)
   + return AE_OK;
   +
 ret = acpi_scan_attach_handler(device);
 if (ret)
 return ret  0 ? AE_OK : AE_CTRL_DEPTH;
   
   --
   To unsubscribe from this list: send the line unsubscribe linux-acpi in
   the body of a message to majord...@vger.kernel.org
   More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
  
  --
  To unsubscribe from this list: send the line unsubscribe linux-acpi in
  the body of a message to majord...@vger.kernel.org
  More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ACPI / scan: Always call acpi_bus_scan() for bus check notifications

2013-07-10 Thread Rafael J. Wysocki
On Wednesday, July 10, 2013 05:48:26 PM Toshi Kani wrote:
> On Thu, 2013-07-11 at 00:45 +0200, Rafael J. Wysocki wrote:
> > On Wednesday, July 10, 2013 02:11:05 AM Rafael J. Wysocki wrote:
> > > On Tuesday, July 09, 2013 01:32:42 PM Toshi Kani wrote:
> > > > On Mon, 2013-07-08 at 02:10 +0200, Rafael J. Wysocki wrote:
> > > > > From: Rafael J. Wysocki 
> > > > > 
> > > > > An ACPI_NOTIFY_BUS_CHECK notification means that we should scan the
> > > > > entire namespace starting from the given handle even if the device
> > > > > represented by that handle is present (other devices below it may
> > > > > just have been added).
> > > > > 
> > > > > For this reason, modify acpi_scan_bus_device_check() to always run
> > > > > acpi_bus_scan() if the notification being handled is of type
> > > > > ACPI_NOTIFY_BUS_CHECK.
> > > > > 
> > > > > Signed-off-by: Rafael J. Wysocki 
> > > > > Cc: 3.10+ 
> > > > 
> > > > Acked-by: Toshi Kani 
> > > > 
> > > > But, I think we need the additional patch below.
> > > 
> > > Yes, I think you're right.
> > 
> > That said I'd prefer to put the check into acpi_bus_device_attach() like in
> > the appended patch.
> 
> That's fine by me.  
> 
> Acked-by: Toshi Kani 
> 
> Just a minor point, though.  Isn't it a bit inconsistent with
> device_attach(), which checks dev->driver inside the function?

Well, device_attach() may be called from different places while this is
the only place where acpi_scan_attach_handler() is called.

The check in acpi_bus_device_attach() is easier to follow to me, because
it clearly means "we don't need to do anything more if there's a handler",
while the check in acpi_scan_attach_handler() makes you wonder "why do we
need to return 1 in that case?" and then you need to go to the caller and
look at the check of the return value to see "ah, because we don't want
that device_attach() to be called then!".

> That said, I am OK with either way.

Cool. :-)

Thanks,
Rafael


> > ---
> > From: Rafael J. Wysocki 
> > Subject: ACPI / scan: Do not try to attach scan handlers to devices having 
> > them
> > 
> > In acpi_bus_device_attach(), if there is an ACPI device object
> > for the given handle and that device object has a scan handler
> > attached to it already, there's nothing more to do for that handle
> > and the function should just return success immediately.  Make
> > that happen.
> > 
> > Reported-by: Toshi Kani 
> > Signed-off-by: Rafael J. Wysocki 
> > ---
> >  drivers/acpi/scan.c |3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > Index: linux-pm/drivers/acpi/scan.c
> > ===
> > --- linux-pm.orig/drivers/acpi/scan.c
> > +++ linux-pm/drivers/acpi/scan.c
> > @@ -1984,6 +1984,9 @@ static acpi_status acpi_bus_device_attac
> > if (acpi_bus_get_device(handle, ))
> > return AE_CTRL_DEPTH;
> >  
> > +   if (device->handler)
> > +   return AE_OK;
> > +
> > ret = acpi_scan_attach_handler(device);
> > if (ret)
> > return ret > 0 ? AE_OK : AE_CTRL_DEPTH;
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ACPI / scan: Always call acpi_bus_scan() for bus check notifications

2013-07-10 Thread Toshi Kani
On Thu, 2013-07-11 at 00:45 +0200, Rafael J. Wysocki wrote:
> On Wednesday, July 10, 2013 02:11:05 AM Rafael J. Wysocki wrote:
> > On Tuesday, July 09, 2013 01:32:42 PM Toshi Kani wrote:
> > > On Mon, 2013-07-08 at 02:10 +0200, Rafael J. Wysocki wrote:
> > > > From: Rafael J. Wysocki 
> > > > 
> > > > An ACPI_NOTIFY_BUS_CHECK notification means that we should scan the
> > > > entire namespace starting from the given handle even if the device
> > > > represented by that handle is present (other devices below it may
> > > > just have been added).
> > > > 
> > > > For this reason, modify acpi_scan_bus_device_check() to always run
> > > > acpi_bus_scan() if the notification being handled is of type
> > > > ACPI_NOTIFY_BUS_CHECK.
> > > > 
> > > > Signed-off-by: Rafael J. Wysocki 
> > > > Cc: 3.10+ 
> > > 
> > > Acked-by: Toshi Kani 
> > > 
> > > But, I think we need the additional patch below.
> > 
> > Yes, I think you're right.
> 
> That said I'd prefer to put the check into acpi_bus_device_attach() like in
> the appended patch.

That's fine by me.  

Acked-by: Toshi Kani 

Just a minor point, though.  Isn't it a bit inconsistent with
device_attach(), which checks dev->driver inside the function?  That
said, I am OK with either way.

Thanks,
-Toshi


> 
> Thanks,
> Rafael
> 
> 
> ---
> From: Rafael J. Wysocki 
> Subject: ACPI / scan: Do not try to attach scan handlers to devices having 
> them
> 
> In acpi_bus_device_attach(), if there is an ACPI device object
> for the given handle and that device object has a scan handler
> attached to it already, there's nothing more to do for that handle
> and the function should just return success immediately.  Make
> that happen.
> 
> Reported-by: Toshi Kani 
> Signed-off-by: Rafael J. Wysocki 
> ---
>  drivers/acpi/scan.c |3 +++
>  1 file changed, 3 insertions(+)
> 
> Index: linux-pm/drivers/acpi/scan.c
> ===
> --- linux-pm.orig/drivers/acpi/scan.c
> +++ linux-pm/drivers/acpi/scan.c
> @@ -1984,6 +1984,9 @@ static acpi_status acpi_bus_device_attac
>   if (acpi_bus_get_device(handle, ))
>   return AE_CTRL_DEPTH;
>  
> + if (device->handler)
> + return AE_OK;
> +
>   ret = acpi_scan_attach_handler(device);
>   if (ret)
>   return ret > 0 ? AE_OK : AE_CTRL_DEPTH;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ACPI / scan: Always call acpi_bus_scan() for bus check notifications

2013-07-10 Thread Rafael J. Wysocki
On Wednesday, July 10, 2013 02:11:05 AM Rafael J. Wysocki wrote:
> On Tuesday, July 09, 2013 01:32:42 PM Toshi Kani wrote:
> > On Mon, 2013-07-08 at 02:10 +0200, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki 
> > > 
> > > An ACPI_NOTIFY_BUS_CHECK notification means that we should scan the
> > > entire namespace starting from the given handle even if the device
> > > represented by that handle is present (other devices below it may
> > > just have been added).
> > > 
> > > For this reason, modify acpi_scan_bus_device_check() to always run
> > > acpi_bus_scan() if the notification being handled is of type
> > > ACPI_NOTIFY_BUS_CHECK.
> > > 
> > > Signed-off-by: Rafael J. Wysocki 
> > > Cc: 3.10+ 
> > 
> > Acked-by: Toshi Kani 
> > 
> > But, I think we need the additional patch below.
> 
> Yes, I think you're right.

That said I'd prefer to put the check into acpi_bus_device_attach() like in
the appended patch.

Thanks,
Rafael


---
From: Rafael J. Wysocki 
Subject: ACPI / scan: Do not try to attach scan handlers to devices having them

In acpi_bus_device_attach(), if there is an ACPI device object
for the given handle and that device object has a scan handler
attached to it already, there's nothing more to do for that handle
and the function should just return success immediately.  Make
that happen.

Reported-by: Toshi Kani 
Signed-off-by: Rafael J. Wysocki 
---
 drivers/acpi/scan.c |3 +++
 1 file changed, 3 insertions(+)

Index: linux-pm/drivers/acpi/scan.c
===
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -1984,6 +1984,9 @@ static acpi_status acpi_bus_device_attac
if (acpi_bus_get_device(handle, ))
return AE_CTRL_DEPTH;
 
+   if (device->handler)
+   return AE_OK;
+
ret = acpi_scan_attach_handler(device);
if (ret)
return ret > 0 ? AE_OK : AE_CTRL_DEPTH;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ACPI / scan: Always call acpi_bus_scan() for bus check notifications

2013-07-10 Thread Rafael J. Wysocki
On Wednesday, July 10, 2013 02:11:05 AM Rafael J. Wysocki wrote:
 On Tuesday, July 09, 2013 01:32:42 PM Toshi Kani wrote:
  On Mon, 2013-07-08 at 02:10 +0200, Rafael J. Wysocki wrote:
   From: Rafael J. Wysocki rafael.j.wyso...@intel.com
   
   An ACPI_NOTIFY_BUS_CHECK notification means that we should scan the
   entire namespace starting from the given handle even if the device
   represented by that handle is present (other devices below it may
   just have been added).
   
   For this reason, modify acpi_scan_bus_device_check() to always run
   acpi_bus_scan() if the notification being handled is of type
   ACPI_NOTIFY_BUS_CHECK.
   
   Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
   Cc: 3.10+ sta...@vger.kernel.org
  
  Acked-by: Toshi Kani toshi.k...@hp.com
  
  But, I think we need the additional patch below.
 
 Yes, I think you're right.

That said I'd prefer to put the check into acpi_bus_device_attach() like in
the appended patch.

Thanks,
Rafael


---
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
Subject: ACPI / scan: Do not try to attach scan handlers to devices having them

In acpi_bus_device_attach(), if there is an ACPI device object
for the given handle and that device object has a scan handler
attached to it already, there's nothing more to do for that handle
and the function should just return success immediately.  Make
that happen.

Reported-by: Toshi Kani toshi.k...@hp.com
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---
 drivers/acpi/scan.c |3 +++
 1 file changed, 3 insertions(+)

Index: linux-pm/drivers/acpi/scan.c
===
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -1984,6 +1984,9 @@ static acpi_status acpi_bus_device_attac
if (acpi_bus_get_device(handle, device))
return AE_CTRL_DEPTH;
 
+   if (device-handler)
+   return AE_OK;
+
ret = acpi_scan_attach_handler(device);
if (ret)
return ret  0 ? AE_OK : AE_CTRL_DEPTH;

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ACPI / scan: Always call acpi_bus_scan() for bus check notifications

2013-07-10 Thread Toshi Kani
On Thu, 2013-07-11 at 00:45 +0200, Rafael J. Wysocki wrote:
 On Wednesday, July 10, 2013 02:11:05 AM Rafael J. Wysocki wrote:
  On Tuesday, July 09, 2013 01:32:42 PM Toshi Kani wrote:
   On Mon, 2013-07-08 at 02:10 +0200, Rafael J. Wysocki wrote:
From: Rafael J. Wysocki rafael.j.wyso...@intel.com

An ACPI_NOTIFY_BUS_CHECK notification means that we should scan the
entire namespace starting from the given handle even if the device
represented by that handle is present (other devices below it may
just have been added).

For this reason, modify acpi_scan_bus_device_check() to always run
acpi_bus_scan() if the notification being handled is of type
ACPI_NOTIFY_BUS_CHECK.

Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
Cc: 3.10+ sta...@vger.kernel.org
   
   Acked-by: Toshi Kani toshi.k...@hp.com
   
   But, I think we need the additional patch below.
  
  Yes, I think you're right.
 
 That said I'd prefer to put the check into acpi_bus_device_attach() like in
 the appended patch.

That's fine by me.  

Acked-by: Toshi Kani toshi.k...@hp.com

Just a minor point, though.  Isn't it a bit inconsistent with
device_attach(), which checks dev-driver inside the function?  That
said, I am OK with either way.

Thanks,
-Toshi


 
 Thanks,
 Rafael
 
 
 ---
 From: Rafael J. Wysocki rafael.j.wyso...@intel.com
 Subject: ACPI / scan: Do not try to attach scan handlers to devices having 
 them
 
 In acpi_bus_device_attach(), if there is an ACPI device object
 for the given handle and that device object has a scan handler
 attached to it already, there's nothing more to do for that handle
 and the function should just return success immediately.  Make
 that happen.
 
 Reported-by: Toshi Kani toshi.k...@hp.com
 Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
 ---
  drivers/acpi/scan.c |3 +++
  1 file changed, 3 insertions(+)
 
 Index: linux-pm/drivers/acpi/scan.c
 ===
 --- linux-pm.orig/drivers/acpi/scan.c
 +++ linux-pm/drivers/acpi/scan.c
 @@ -1984,6 +1984,9 @@ static acpi_status acpi_bus_device_attac
   if (acpi_bus_get_device(handle, device))
   return AE_CTRL_DEPTH;
  
 + if (device-handler)
 + return AE_OK;
 +
   ret = acpi_scan_attach_handler(device);
   if (ret)
   return ret  0 ? AE_OK : AE_CTRL_DEPTH;
 
 --
 To unsubscribe from this list: send the line unsubscribe linux-acpi in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ACPI / scan: Always call acpi_bus_scan() for bus check notifications

2013-07-10 Thread Rafael J. Wysocki
On Wednesday, July 10, 2013 05:48:26 PM Toshi Kani wrote:
 On Thu, 2013-07-11 at 00:45 +0200, Rafael J. Wysocki wrote:
  On Wednesday, July 10, 2013 02:11:05 AM Rafael J. Wysocki wrote:
   On Tuesday, July 09, 2013 01:32:42 PM Toshi Kani wrote:
On Mon, 2013-07-08 at 02:10 +0200, Rafael J. Wysocki wrote:
 From: Rafael J. Wysocki rafael.j.wyso...@intel.com
 
 An ACPI_NOTIFY_BUS_CHECK notification means that we should scan the
 entire namespace starting from the given handle even if the device
 represented by that handle is present (other devices below it may
 just have been added).
 
 For this reason, modify acpi_scan_bus_device_check() to always run
 acpi_bus_scan() if the notification being handled is of type
 ACPI_NOTIFY_BUS_CHECK.
 
 Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
 Cc: 3.10+ sta...@vger.kernel.org

Acked-by: Toshi Kani toshi.k...@hp.com

But, I think we need the additional patch below.
   
   Yes, I think you're right.
  
  That said I'd prefer to put the check into acpi_bus_device_attach() like in
  the appended patch.
 
 That's fine by me.  
 
 Acked-by: Toshi Kani toshi.k...@hp.com
 
 Just a minor point, though.  Isn't it a bit inconsistent with
 device_attach(), which checks dev-driver inside the function?

Well, device_attach() may be called from different places while this is
the only place where acpi_scan_attach_handler() is called.

The check in acpi_bus_device_attach() is easier to follow to me, because
it clearly means we don't need to do anything more if there's a handler,
while the check in acpi_scan_attach_handler() makes you wonder why do we
need to return 1 in that case? and then you need to go to the caller and
look at the check of the return value to see ah, because we don't want
that device_attach() to be called then!.

 That said, I am OK with either way.

Cool. :-)

Thanks,
Rafael


  ---
  From: Rafael J. Wysocki rafael.j.wyso...@intel.com
  Subject: ACPI / scan: Do not try to attach scan handlers to devices having 
  them
  
  In acpi_bus_device_attach(), if there is an ACPI device object
  for the given handle and that device object has a scan handler
  attached to it already, there's nothing more to do for that handle
  and the function should just return success immediately.  Make
  that happen.
  
  Reported-by: Toshi Kani toshi.k...@hp.com
  Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
  ---
   drivers/acpi/scan.c |3 +++
   1 file changed, 3 insertions(+)
  
  Index: linux-pm/drivers/acpi/scan.c
  ===
  --- linux-pm.orig/drivers/acpi/scan.c
  +++ linux-pm/drivers/acpi/scan.c
  @@ -1984,6 +1984,9 @@ static acpi_status acpi_bus_device_attac
  if (acpi_bus_get_device(handle, device))
  return AE_CTRL_DEPTH;
   
  +   if (device-handler)
  +   return AE_OK;
  +
  ret = acpi_scan_attach_handler(device);
  if (ret)
  return ret  0 ? AE_OK : AE_CTRL_DEPTH;
  
  --
  To unsubscribe from this list: send the line unsubscribe linux-acpi in
  the body of a message to majord...@vger.kernel.org
  More majordomo info at  http://vger.kernel.org/majordomo-info.html
 
 
 --
 To unsubscribe from this list: send the line unsubscribe linux-acpi in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ACPI / scan: Always call acpi_bus_scan() for bus check notifications

2013-07-09 Thread Rafael J. Wysocki
On Tuesday, July 09, 2013 01:32:42 PM Toshi Kani wrote:
> On Mon, 2013-07-08 at 02:10 +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki 
> > 
> > An ACPI_NOTIFY_BUS_CHECK notification means that we should scan the
> > entire namespace starting from the given handle even if the device
> > represented by that handle is present (other devices below it may
> > just have been added).
> > 
> > For this reason, modify acpi_scan_bus_device_check() to always run
> > acpi_bus_scan() if the notification being handled is of type
> > ACPI_NOTIFY_BUS_CHECK.
> > 
> > Signed-off-by: Rafael J. Wysocki 
> > Cc: 3.10+ 
> 
> Acked-by: Toshi Kani 
> 
> But, I think we need the additional patch below.

Yes, I think you're right.

Thanks,
Rafael



> =
> From: Toshi Kani 
> Subject: [PATCH] ACPI: Do not call attach() if device is attached
> 
> attach() of ACPI scan handlers does not expect to be called multiple
> times on a same device.  Also, the attached handler may not be changed
> without calling its detach().  Change acpi_scan_attach_handler() not
> to call attach() when the given device is already attached.
> 
> Signed-off-by: Toshi Kani 
> ---
>  drivers/acpi/scan.c |3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 20757e0..2b9e867 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -1885,6 +1885,9 @@ static int acpi_scan_attach_handler(struct
> acpi_device *device)
>   struct acpi_hardware_id *hwid;
>   int ret = 0;
>  
> + if (device->handler)
> + return 1;
> +
>   list_for_each_entry(hwid, >pnp.ids, list) {
>   const struct acpi_device_id *devid;
>   struct acpi_scan_handler *handler;
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ACPI / scan: Always call acpi_bus_scan() for bus check notifications

2013-07-09 Thread Toshi Kani
On Mon, 2013-07-08 at 02:10 +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki 
> 
> An ACPI_NOTIFY_BUS_CHECK notification means that we should scan the
> entire namespace starting from the given handle even if the device
> represented by that handle is present (other devices below it may
> just have been added).
> 
> For this reason, modify acpi_scan_bus_device_check() to always run
> acpi_bus_scan() if the notification being handled is of type
> ACPI_NOTIFY_BUS_CHECK.
> 
> Signed-off-by: Rafael J. Wysocki 
> Cc: 3.10+ 

Acked-by: Toshi Kani 

But, I think we need the additional patch below.

Thanks,
-Toshi

=
From: Toshi Kani 
Subject: [PATCH] ACPI: Do not call attach() if device is attached

attach() of ACPI scan handlers does not expect to be called multiple
times on a same device.  Also, the attached handler may not be changed
without calling its detach().  Change acpi_scan_attach_handler() not
to call attach() when the given device is already attached.

Signed-off-by: Toshi Kani 
---
 drivers/acpi/scan.c |3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 20757e0..2b9e867 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1885,6 +1885,9 @@ static int acpi_scan_attach_handler(struct
acpi_device *device)
struct acpi_hardware_id *hwid;
int ret = 0;
 
+   if (device->handler)
+   return 1;
+
list_for_each_entry(hwid, >pnp.ids, list) {
const struct acpi_device_id *devid;
struct acpi_scan_handler *handler;


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ACPI / scan: Always call acpi_bus_scan() for bus check notifications

2013-07-09 Thread Toshi Kani
On Mon, 2013-07-08 at 02:10 +0200, Rafael J. Wysocki wrote:
 From: Rafael J. Wysocki rafael.j.wyso...@intel.com
 
 An ACPI_NOTIFY_BUS_CHECK notification means that we should scan the
 entire namespace starting from the given handle even if the device
 represented by that handle is present (other devices below it may
 just have been added).
 
 For this reason, modify acpi_scan_bus_device_check() to always run
 acpi_bus_scan() if the notification being handled is of type
 ACPI_NOTIFY_BUS_CHECK.
 
 Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
 Cc: 3.10+ sta...@vger.kernel.org

Acked-by: Toshi Kani toshi.k...@hp.com

But, I think we need the additional patch below.

Thanks,
-Toshi

=
From: Toshi Kani toshi.k...@hp.com
Subject: [PATCH] ACPI: Do not call attach() if device is attached

attach() of ACPI scan handlers does not expect to be called multiple
times on a same device.  Also, the attached handler may not be changed
without calling its detach().  Change acpi_scan_attach_handler() not
to call attach() when the given device is already attached.

Signed-off-by: Toshi Kani toshi.k...@hp.com
---
 drivers/acpi/scan.c |3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 20757e0..2b9e867 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1885,6 +1885,9 @@ static int acpi_scan_attach_handler(struct
acpi_device *device)
struct acpi_hardware_id *hwid;
int ret = 0;
 
+   if (device-handler)
+   return 1;
+
list_for_each_entry(hwid, device-pnp.ids, list) {
const struct acpi_device_id *devid;
struct acpi_scan_handler *handler;


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ACPI / scan: Always call acpi_bus_scan() for bus check notifications

2013-07-09 Thread Rafael J. Wysocki
On Tuesday, July 09, 2013 01:32:42 PM Toshi Kani wrote:
 On Mon, 2013-07-08 at 02:10 +0200, Rafael J. Wysocki wrote:
  From: Rafael J. Wysocki rafael.j.wyso...@intel.com
  
  An ACPI_NOTIFY_BUS_CHECK notification means that we should scan the
  entire namespace starting from the given handle even if the device
  represented by that handle is present (other devices below it may
  just have been added).
  
  For this reason, modify acpi_scan_bus_device_check() to always run
  acpi_bus_scan() if the notification being handled is of type
  ACPI_NOTIFY_BUS_CHECK.
  
  Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
  Cc: 3.10+ sta...@vger.kernel.org
 
 Acked-by: Toshi Kani toshi.k...@hp.com
 
 But, I think we need the additional patch below.

Yes, I think you're right.

Thanks,
Rafael



 =
 From: Toshi Kani toshi.k...@hp.com
 Subject: [PATCH] ACPI: Do not call attach() if device is attached
 
 attach() of ACPI scan handlers does not expect to be called multiple
 times on a same device.  Also, the attached handler may not be changed
 without calling its detach().  Change acpi_scan_attach_handler() not
 to call attach() when the given device is already attached.
 
 Signed-off-by: Toshi Kani toshi.k...@hp.com
 ---
  drivers/acpi/scan.c |3 +++
  1 file changed, 3 insertions(+)
 
 diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
 index 20757e0..2b9e867 100644
 --- a/drivers/acpi/scan.c
 +++ b/drivers/acpi/scan.c
 @@ -1885,6 +1885,9 @@ static int acpi_scan_attach_handler(struct
 acpi_device *device)
   struct acpi_hardware_id *hwid;
   int ret = 0;
  
 + if (device-handler)
 + return 1;
 +
   list_for_each_entry(hwid, device-pnp.ids, list) {
   const struct acpi_device_id *devid;
   struct acpi_scan_handler *handler;
 
 
 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] ACPI / scan: Always call acpi_bus_scan() for bus check notifications

2013-07-07 Thread Rafael J. Wysocki
From: Rafael J. Wysocki 

An ACPI_NOTIFY_BUS_CHECK notification means that we should scan the
entire namespace starting from the given handle even if the device
represented by that handle is present (other devices below it may
just have been added).

For this reason, modify acpi_scan_bus_device_check() to always run
acpi_bus_scan() if the notification being handled is of type
ACPI_NOTIFY_BUS_CHECK.

Signed-off-by: Rafael J. Wysocki 
Cc: 3.10+ 
---
 drivers/acpi/scan.c |   10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

Index: linux-pm/drivers/acpi/scan.c
===
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -353,10 +353,12 @@ static void acpi_scan_bus_device_check(a
mutex_lock(_scan_lock);
lock_device_hotplug();
 
-   acpi_bus_get_device(handle, );
-   if (device) {
-   dev_warn(>dev, "Attempt to re-insert\n");
-   goto out;
+   if (ost_source != ACPI_NOTIFY_BUS_CHECK) {
+   acpi_bus_get_device(handle, );
+   if (device) {
+   dev_warn(>dev, "Attempt to re-insert\n");
+   goto out;
+   }
}
acpi_evaluate_hotplug_ost(handle, ost_source,
  ACPI_OST_SC_INSERT_IN_PROGRESS, NULL);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] ACPI / scan: Always call acpi_bus_scan() for bus check notifications

2013-07-07 Thread Rafael J. Wysocki
From: Rafael J. Wysocki rafael.j.wyso...@intel.com

An ACPI_NOTIFY_BUS_CHECK notification means that we should scan the
entire namespace starting from the given handle even if the device
represented by that handle is present (other devices below it may
just have been added).

For this reason, modify acpi_scan_bus_device_check() to always run
acpi_bus_scan() if the notification being handled is of type
ACPI_NOTIFY_BUS_CHECK.

Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
Cc: 3.10+ sta...@vger.kernel.org
---
 drivers/acpi/scan.c |   10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

Index: linux-pm/drivers/acpi/scan.c
===
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -353,10 +353,12 @@ static void acpi_scan_bus_device_check(a
mutex_lock(acpi_scan_lock);
lock_device_hotplug();
 
-   acpi_bus_get_device(handle, device);
-   if (device) {
-   dev_warn(device-dev, Attempt to re-insert\n);
-   goto out;
+   if (ost_source != ACPI_NOTIFY_BUS_CHECK) {
+   acpi_bus_get_device(handle, device);
+   if (device) {
+   dev_warn(device-dev, Attempt to re-insert\n);
+   goto out;
+   }
}
acpi_evaluate_hotplug_ost(handle, ost_source,
  ACPI_OST_SC_INSERT_IN_PROGRESS, NULL);

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/