subject:"Re\: \[PATCH 5\/6\] ACPI\: Replace struct acpi_bus

Hi Gerry,

On Tuesday, December 11, 2012 11:09:06 PM Jiang Liu wrote:
> Hi Rafael,
>   I have worked out a patch set to clean up ACPI/PCI related 
> notifications,
> please refer to
> http://www.spinics.net/lists/linux-pci/msg17822.html
>   The patchset doesn't apply cleanly to Bjorn's latest pci-next tree. I 
> will
> help to rebase it if needed.

I have reviewed the patches and I think they make sense overall.  However,
the statement that acpi_pci_bind()/acpi_pci_unbind() are used to maintain
PCI-ACPI binding relationships is quite inaccurate, because all what these
functions do is to (1) add/remove ACPI PM notifiers to/from PCI devices and
(2) retrieve the _PRT information for bridges from ACPI tables.  In fact,
the *binding* itself is managed by the code in drivers/acpi/glue.c.

Also, please have a look at my suggestion in the last reply to Yinghai:

http://marc.info/?l=linux-pci=135522965707752=2

In fact, I think we can go even further than that.  Namely, if we populate the
ACPI handle of the device in pci_scan_device(), then we can just move the PM
notifier and wakeup setup to platform_pci_wakeup_init(), where it should be
(we'll also need to add a corresponding _exit() function, then, but that'll be
much cleaner anyway).

Then, the remaining thing would be to ensure that _PRT entries are parsed
as appropriate somewhere around pci_init_capabilities().

Also, I wonder if you can help test the $subject patchset on a system with
hardware PCI hotplug (preferably on top of the linux-pm.git/master branch)?

Rafael

-- 
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 5/6] ACPI: Replace struct acpi_bus_ops with enum type

2012-12-11 Thread Jiang Liu

Hi Rafael,
I have worked out a patch set to clean up ACPI/PCI related 
notifications,
please refer to
http://www.spinics.net/lists/linux-pci/msg17822.html
The patchset doesn't apply cleanly to Bjorn's latest pci-next tree. I 
will
help to rebase it if needed.
Regards!
Gerry

On 12/11/2012 10:26 AM, Yinghai Lu wrote:
> On Mon, Dec 10, 2012 at 5:28 PM, Rafael J. Wysocki  wrote:
>>>
>>> OK, thanks for the pointers.  I actually see more differences between our
>>> patchsets.  For one example, you seem to have left the parent->ops.bind()
>>> stuff in acpi_add_single_object() which calls it even drivers_autoprobe is
>>> set.
>>
>> Sorry, that should have been "which calls it even when drivers_autoprobe is
>> not set".  I need to be more careful.
>>
> 
> oh,  Jiang Liu had one patch to remove that workaround.
> 
> http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=b40dba80c2b8395570d8357e6b3f417c27c84504
> 
> ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops
> 
> Maybe you can review that patches in my for-pci-next2...
> those are ACPI related anyway.
> 
> those patches have been there for a while, and Bjorn did not have time
> to digest them.
> 
> or you prefer I resend updated version as huge whole patchset?
> 
> Thanks
> 
> Yinghai
> --
> 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 5/6] ACPI: Replace struct acpi_bus_ops with enum type

On Monday, December 10, 2012 06:26:08 PM Yinghai Lu wrote:
> On Mon, Dec 10, 2012 at 5:28 PM, Rafael J. Wysocki  wrote:
> >>
> >> OK, thanks for the pointers.  I actually see more differences between our
> >> patchsets.  For one example, you seem to have left the parent->ops.bind()
> >> stuff in acpi_add_single_object() which calls it even drivers_autoprobe is
> >> set.
> >
> > Sorry, that should have been "which calls it even when drivers_autoprobe is
> > not set".  I need to be more careful.
> >
> 
> oh,  Jiang Liu had one patch to remove that workaround.
> 
> http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=b40dba80c2b8395570d8357e6b3f417c27c84504
> 
> ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops

OK, so I'm looking at the current code, which for me is the master branch of
the linux-pm.git tree (the linux-next branch is the same ATM), and I'm not
seeing acpi_pci_unbind_cb() in there.  So surely this patch applies to
something different, right?  In which case I wonder what reason there is for
me to look at it at all?

Besides, I think it may be done differently and in a more straightforward
way.  Namely, on top of my current patchset, it is guaranteed that not only
struct pci_dev objects will always be registered after the companion struct
acpi_device ones, but also they always will be *created* after those
companion objects have been registered.  So in principle we can populate
a new struct pci_dev's ACPI handle as soon as in pci_scan_device(),
next to pci_set_of_node().  Then, we can do something like acpi_pci_bind(),
although without the whole acpi_get_pci_dev() nonsense, in pci_setup_device(),
in which case we won't need to do it anywhere else.

As an added benefit, acpi_platform_notify() would then see a populated ACPI
handle in that struct pci_dev when finally registering the PCI device, so it
wouldn't need to do the whole acpi_find_bridge_device() and type->find_device()
dance.

> Maybe you can review that patches in my for-pci-next2...
> those are ACPI related anyway.

I can, provided that (1) they are based on top of my tree or v3.7 and (2)
they don't conflict with patches we're currently discussing.

> those patches have been there for a while, and Bjorn did not have time
> to digest them.

Well, Bjorn's review bandwidth is limited and we need to take that into
account.

> or you prefer I resend updated version as huge whole patchset?

No, no huge patchsets, please.  Let's take one step at a time, so that
everyone involved/interested can understand what's going on, OK?

My review capacity also is not unlimited, mind you.  I can't promise I'll
have the time to review more than a few patches a day (where "a few" is
rather less than "several").

Thanks,
Rafael

-- 
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 5/6] ACPI: Replace struct acpi_bus_ops with enum type

On Monday, December 10, 2012 06:26:08 PM Yinghai Lu wrote:
On Mon, Dec 10, 2012 at 5:28 PM, Rafael J. Wysocki r...@sisk.pl wrote:

OK, thanks for the pointers. I actually see more differences between our
patchsets. For one example, you seem to have left the parent-ops.bind()
stuff in acpi_add_single_object() which calls it even drivers_autoprobe is
set.

Sorry, that should have been which calls it even when drivers_autoprobe is
not set. I need to be more careful.

oh, Jiang Liu had one patch to remove that workaround.

http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=b40dba80c2b8395570d8357e6b3f417c27c84504

ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops

OK, so I'm looking at the current code, which for me is the master branch of
the linux-pm.git tree (the linux-next branch is the same ATM), and I'm not
seeing acpi_pci_unbind_cb() in there. So surely this patch applies to
something different, right? In which case I wonder what reason there is for
me to look at it at all?

Besides, I think it may be done differently and in a more straightforward
way. Namely, on top of my current patchset, it is guaranteed that not only
struct pci_dev objects will always be registered after the companion struct
acpi_device ones, but also they always will be *created* after those
companion objects have been registered. So in principle we can populate
a new struct pci_dev's ACPI handle as soon as in pci_scan_device(),
next to pci_set_of_node(). Then, we can do something like acpi_pci_bind(),
although without the whole acpi_get_pci_dev() nonsense, in pci_setup_device(),
in which case we won't need to do it anywhere else.

As an added benefit, acpi_platform_notify() would then see a populated ACPI
handle in that struct pci_dev when finally registering the PCI device, so it
wouldn't need to do the whole acpi_find_bridge_device() and type-find_device()
dance.

Maybe you can review that patches in my for-pci-next2...
those are ACPI related anyway.

I can, provided that (1) they are based on top of my tree or v3.7 and (2)
they don't conflict with patches we're currently discussing.

those patches have been there for a while, and Bjorn did not have time
to digest them.

Well, Bjorn's review bandwidth is limited and we need to take that into
account.

or you prefer I resend updated version as huge whole patchset?

No, no huge patchsets, please. Let's take one step at a time, so that
everyone involved/interested can understand what's going on, OK?

My review capacity also is not unlimited, mind you. I can't promise I'll
have the time to review more than a few patches a day (where a few is
rather less than several).

Thanks,
Rafael

--
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 5/6] ACPI: Replace struct acpi_bus_ops with enum type

2012-12-11 Thread Jiang Liu

Hi Rafael,
I have worked out a patch set to clean up ACPI/PCI related
notifications,
please refer to
http://www.spinics.net/lists/linux-pci/msg17822.html
The patchset doesn't apply cleanly to Bjorn's latest pci-next tree. I
will
help to rebase it if needed.
Regards!
Gerry

On 12/11/2012 10:26 AM, Yinghai Lu wrote:
On Mon, Dec 10, 2012 at 5:28 PM, Rafael J. Wysocki r...@sisk.pl wrote:

Sorry, that should have been which calls it even when drivers_autoprobe is
not set. I need to be more careful.

oh, Jiang Liu had one patch to remove that workaround.

http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=b40dba80c2b8395570d8357e6b3f417c27c84504

ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops

Maybe you can review that patches in my for-pci-next2...
those are ACPI related anyway.

those patches have been there for a while, and Bjorn did not have time
to digest them.

or you prefer I resend updated version as huge whole patchset?

Thanks

Yinghai
--
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 5/6] ACPI: Replace struct acpi_bus_ops with enum type

Hi Gerry,

On Tuesday, December 11, 2012 11:09:06 PM Jiang Liu wrote:
 Hi Rafael,
   I have worked out a patch set to clean up ACPI/PCI related 
 notifications,
 please refer to
 http://www.spinics.net/lists/linux-pci/msg17822.html
   The patchset doesn't apply cleanly to Bjorn's latest pci-next tree. I 
 will
 help to rebase it if needed.

I have reviewed the patches and I think they make sense overall.  However,
the statement that acpi_pci_bind()/acpi_pci_unbind() are used to maintain
PCI-ACPI binding relationships is quite inaccurate, because all what these
functions do is to (1) add/remove ACPI PM notifiers to/from PCI devices and
(2) retrieve the _PRT information for bridges from ACPI tables.  In fact,
the *binding* itself is managed by the code in drivers/acpi/glue.c.

Also, please have a look at my suggestion in the last reply to Yinghai:

http://marc.info/?l=linux-pcim=135522965707752w=2

In fact, I think we can go even further than that.  Namely, if we populate the
ACPI handle of the device in pci_scan_device(), then we can just move the PM
notifier and wakeup setup to platform_pci_wakeup_init(), where it should be
(we'll also need to add a corresponding _exit() function, then, but that'll be
much cleaner anyway).

Then, the remaining thing would be to ensure that _PRT entries are parsed
as appropriate somewhere around pci_init_capabilities().

Also, I wonder if you can help test the $subject patchset on a system with
hardware PCI hotplug (preferably on top of the linux-pm.git/master branch)?

Rafael


-- 
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 5/6] ACPI: Replace struct acpi_bus_ops with enum type

On Mon, Dec 10, 2012 at 5:28 PM, Rafael J. Wysocki  wrote:
>>
>> OK, thanks for the pointers.  I actually see more differences between our
>> patchsets.  For one example, you seem to have left the parent->ops.bind()
>> stuff in acpi_add_single_object() which calls it even drivers_autoprobe is
>> set.
>
> Sorry, that should have been "which calls it even when drivers_autoprobe is
> not set".  I need to be more careful.
>

oh,  Jiang Liu had one patch to remove that workaround.

http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=b40dba80c2b8395570d8357e6b3f417c27c84504

ACPI/pci-bind: remove bind/unbind callbacks from acpi_device_ops

Maybe you can review that patches in my for-pci-next2...
those are ACPI related anyway.

those patches have been there for a while, and Bjorn did not have time
to digest them.

or you prefer I resend updated version as huge whole patchset?

Thanks

Yinghai
--
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 5/6] ACPI: Replace struct acpi_bus_ops with enum type

On Tuesday, December 11, 2012 02:02:14 AM Rafael J. Wysocki wrote:
> On Monday, December 10, 2012 03:14:32 PM Yinghai Lu wrote:
> > On Mon, Dec 10, 2012 at 3:09 PM, Rafael J. Wysocki  wrote:
> > > On Monday, December 10, 2012 11:47:27 PM Rafael J. Wysocki wrote:
> > >> On Monday, December 10, 2012 09:07:06 AM Yinghai Lu wrote:
> > >> > On Mon, Dec 10, 2012 at 6:46 AM, Rafael J. Wysocki  
> > >> > wrote:
> > >> > > On Sunday, December 09, 2012 09:34:42 PM Yinghai Lu wrote:
> > >> > >>
> > >> > >> Can we expand the BUS_ADD_* concept to other devices instead of just
> > >> > >> acpi_device?
> > >> > >>
> > >> > >> aka we should let struct device has this add_type field.
> > >> > >
> > >> > > Having done that in ACPI to cover our use case here, we can try to 
> > >> > > move it
> > >> > > into struct device if there are use cases beyond ACPI that can't be 
> > >> > > covered
> > >> > > by using deferred driver probing.
> > >> >
> > >> > pci device for hotplug have same problem. need to delay driver attach
> > >> > for them too.
> > >>
> > >> OK, I'll take a look.  Any pointers to speed that up?
> > >>
> > >> > also BUS_ADD_MATCH and BUS_ADD_START are duplicated.
> > >>
> > >> Not at the moment, they do different things as code goes.
> > >>
> > >> > old add are separated to adding all devices to tree and then matching
> > >> > work to load the drivers.
> > >> >
> > >> > so _START is not needed anymore, only user.start in pci_root driver
> > >> > should be removed.
> > >> > code in .start could be moved .add without problem.
> > >>
> > >> Yes, I'm going to do that as the next step.  I didn't want this 
> > >> particular
> > >> patchset to grow too big.  I'll post another one on top of it if people
> > >> don't have problems with this one.
> > >
> > > By the way, can you please remind me where you wanted to put the
> > > pci_bus_add_devices() and why?
> > >
> > 
> > please check my for-pci-next branch at
> > http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=shortlog;h=refs/heads/for-pci-next
> > 
> > that includes delay loading acpi driver and pci driver.
> > 
> > http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=8c031eabbdc83dd4d93933b82d96b55d038bcb64
> >  PCI: prepare to use device drivers_autoprobe to delay attach drivers
> > 
> > http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=40a12dc8942a8ed02bfbf75ee1ffbfbdf1511b45
> > PCI: Use device_add for device and bus early
> > 
> > http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=8af9b4c250091c30afedeb2e7f14fca06997c811
> > ACPI: add drivers_autoprobe in struct acpi_device
> > 
> > http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=ae498e157e9dc8794932562b2f885ddc3a1a229a
> >  ACPI: use device drivers_autoprobe to delay loading acpi drivers
> > 
> > http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=6bee785b563a0b0e311e188321b1160593d5e6ee
> >  PCI, ACPI: Remove not used acpi_pci_root_start()
> > 
> > http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=f467a1cd18a07a250be8527b94612fd4a654fbd1
> > ACPI: remove acpi_op_start workaround
> 
> OK, thanks for the pointers.  I actually see more differences between our
> patchsets.  For one example, you seem to have left the parent->ops.bind()
> stuff in acpi_add_single_object() which calls it even drivers_autoprobe is
> set.

Sorry, that should have been "which calls it even when drivers_autoprobe is
not set".  I need to be more careful.


> Is that the case, or am I missing anything?

Thanks,
Rafael


-- 
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 5/6] ACPI: Replace struct acpi_bus_ops with enum type

On Monday, December 10, 2012 03:14:32 PM Yinghai Lu wrote:
> On Mon, Dec 10, 2012 at 3:09 PM, Rafael J. Wysocki  wrote:
> > On Monday, December 10, 2012 11:47:27 PM Rafael J. Wysocki wrote:
> >> On Monday, December 10, 2012 09:07:06 AM Yinghai Lu wrote:
> >> > On Mon, Dec 10, 2012 at 6:46 AM, Rafael J. Wysocki  wrote:
> >> > > On Sunday, December 09, 2012 09:34:42 PM Yinghai Lu wrote:
> >> > >>
> >> > >> Can we expand the BUS_ADD_* concept to other devices instead of just
> >> > >> acpi_device?
> >> > >>
> >> > >> aka we should let struct device has this add_type field.
> >> > >
> >> > > Having done that in ACPI to cover our use case here, we can try to 
> >> > > move it
> >> > > into struct device if there are use cases beyond ACPI that can't be 
> >> > > covered
> >> > > by using deferred driver probing.
> >> >
> >> > pci device for hotplug have same problem. need to delay driver attach
> >> > for them too.
> >>
> >> OK, I'll take a look.  Any pointers to speed that up?
> >>
> >> > also BUS_ADD_MATCH and BUS_ADD_START are duplicated.
> >>
> >> Not at the moment, they do different things as code goes.
> >>
> >> > old add are separated to adding all devices to tree and then matching
> >> > work to load the drivers.
> >> >
> >> > so _START is not needed anymore, only user.start in pci_root driver
> >> > should be removed.
> >> > code in .start could be moved .add without problem.
> >>
> >> Yes, I'm going to do that as the next step.  I didn't want this particular
> >> patchset to grow too big.  I'll post another one on top of it if people
> >> don't have problems with this one.
> >
> > By the way, can you please remind me where you wanted to put the
> > pci_bus_add_devices() and why?
> >
> 
> please check my for-pci-next branch at
> http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=shortlog;h=refs/heads/for-pci-next
> 
> that includes delay loading acpi driver and pci driver.
> 
> http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=8c031eabbdc83dd4d93933b82d96b55d038bcb64
>  PCI: prepare to use device drivers_autoprobe to delay attach drivers
> 
> http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=40a12dc8942a8ed02bfbf75ee1ffbfbdf1511b45
> PCI: Use device_add for device and bus early
> 
> http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=8af9b4c250091c30afedeb2e7f14fca06997c811
> ACPI: add drivers_autoprobe in struct acpi_device
> 
> http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=ae498e157e9dc8794932562b2f885ddc3a1a229a
>  ACPI: use device drivers_autoprobe to delay loading acpi drivers
> 
> http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=6bee785b563a0b0e311e188321b1160593d5e6ee
>  PCI, ACPI: Remove not used acpi_pci_root_start()
> 
> http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=f467a1cd18a07a250be8527b94612fd4a654fbd1
> ACPI: remove acpi_op_start workaround

OK, thanks for the pointers.  I actually see more differences between our
patchsets.  For one example, you seem to have left the parent->ops.bind()
stuff in acpi_add_single_object() which calls it even drivers_autoprobe is
set.  Is that the case, or am I missing anything?

Rafael


-- 
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 5/6] ACPI: Replace struct acpi_bus_ops with enum type

On Monday, December 10, 2012 03:22:48 PM Yinghai Lu wrote:
> On Mon, Dec 10, 2012 at 2:47 PM, Rafael J. Wysocki  wrote:
> > On Monday, December 10, 2012 09:07:06 AM Yinghai Lu wrote:
> >> On Mon, Dec 10, 2012 at 6:46 AM, Rafael J. Wysocki  wrote:
> >> > On Sunday, December 09, 2012 09:34:42 PM Yinghai Lu wrote:
> >> >>
> >> >> Can we expand the BUS_ADD_* concept to other devices instead of just
> >> >> acpi_device?
> >> >>
> >> >> aka we should let struct device has this add_type field.
> >> >
> >> > Having done that in ACPI to cover our use case here, we can try to move 
> >> > it
> >> > into struct device if there are use cases beyond ACPI that can't be 
> >> > covered
> >> > by using deferred driver probing.
> >>
> >> pci device for hotplug have same problem. need to delay driver attach
> >> for them too.
> >
> > OK, I'll take a look.  Any pointers to speed that up?
> >
> >> also BUS_ADD_MATCH and BUS_ADD_START are duplicated.
> >
> > Not at the moment, they do different things as code goes.
> 
> I do think that is same problem which is in the driver/base core.
> 
> it should support delay loading driver for the hotplug case at the first 
> point.
> that is reason that .start is introduced for acpi driver..to workround
> the problem
> in driver core.

OK, but let's eliminate .start() first and then go make changes to the
driver core.  That is, let's take one step at a time. :-)

> >> old add are separated to adding all devices to tree and then matching
> >> work to load the drivers.
> >>
> >> so _START is not needed anymore, only user.start in pci_root driver
> >> should be removed.
> >> code in .start could be moved .add without problem.
> >
> > Yes, I'm going to do that as the next step.  I didn't want this particular
> 
> after you remove that .start, that will the same as my two patches.
> except that you patches toggle that per-device add_type for every device.
> 
> and my patches is only toggle that per device drivers_autoprobe for
> hot-add devices.

I'm not 100% sure of that, but quite likely that's the case.  I'd like to avoid
special-casing hotplug in any way, if possible.  Also, I have other reasons to
do it for all devices (like the resources management for one example).

> > patchset to grow too big.  I'll post another one on top of it if people
> > don't have problems with this one.
> 
> yesterday, I replace my two acpi driver delay loading patches with your 
> patches.
> 
> during pci root bus hot remove/add test, got the panic...

I don't see what the reason may be at the moment, it would be good to get more
info (like a screenshot of the panic).

> did not have time to look at it yet.

OK

Thanks,
Rafael


-- 
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 5/6] ACPI: Replace struct acpi_bus_ops with enum type

On Mon, Dec 10, 2012 at 2:47 PM, Rafael J. Wysocki  wrote:
> On Monday, December 10, 2012 09:07:06 AM Yinghai Lu wrote:
>> On Mon, Dec 10, 2012 at 6:46 AM, Rafael J. Wysocki  wrote:
>> > On Sunday, December 09, 2012 09:34:42 PM Yinghai Lu wrote:
>> >>
>> >> Can we expand the BUS_ADD_* concept to other devices instead of just
>> >> acpi_device?
>> >>
>> >> aka we should let struct device has this add_type field.
>> >
>> > Having done that in ACPI to cover our use case here, we can try to move it
>> > into struct device if there are use cases beyond ACPI that can't be covered
>> > by using deferred driver probing.
>>
>> pci device for hotplug have same problem. need to delay driver attach
>> for them too.
>
> OK, I'll take a look.  Any pointers to speed that up?
>
>> also BUS_ADD_MATCH and BUS_ADD_START are duplicated.
>
> Not at the moment, they do different things as code goes.

I do think that is same problem which is in the driver/base core.

it should support delay loading driver for the hotplug case at the first point.
that is reason that .start is introduced for acpi driver..to workround
the problem
in driver core.

>
>> old add are separated to adding all devices to tree and then matching
>> work to load the drivers.
>>
>> so _START is not needed anymore, only user.start in pci_root driver
>> should be removed.
>> code in .start could be moved .add without problem.
>
> Yes, I'm going to do that as the next step.  I didn't want this particular

after you remove that .start, that will the same as my two patches.
except that you patches toggle that per-device add_type for every device.

and my patches is only toggle that per device drivers_autoprobe for
hot-add devices.

> patchset to grow too big.  I'll post another one on top of it if people
> don't have problems with this one.

yesterday, I replace my two acpi driver delay loading patches with your patches.

during pci root bus hot remove/add test, got the panic...

did not have time to look at it yet.

Yinghai
--
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 5/6] ACPI: Replace struct acpi_bus_ops with enum type

On Mon, Dec 10, 2012 at 3:09 PM, Rafael J. Wysocki  wrote:
> On Monday, December 10, 2012 11:47:27 PM Rafael J. Wysocki wrote:
>> On Monday, December 10, 2012 09:07:06 AM Yinghai Lu wrote:
>> > On Mon, Dec 10, 2012 at 6:46 AM, Rafael J. Wysocki  wrote:
>> > > On Sunday, December 09, 2012 09:34:42 PM Yinghai Lu wrote:
>> > >>
>> > >> Can we expand the BUS_ADD_* concept to other devices instead of just
>> > >> acpi_device?
>> > >>
>> > >> aka we should let struct device has this add_type field.
>> > >
>> > > Having done that in ACPI to cover our use case here, we can try to move 
>> > > it
>> > > into struct device if there are use cases beyond ACPI that can't be 
>> > > covered
>> > > by using deferred driver probing.
>> >
>> > pci device for hotplug have same problem. need to delay driver attach
>> > for them too.
>>
>> OK, I'll take a look.  Any pointers to speed that up?
>>
>> > also BUS_ADD_MATCH and BUS_ADD_START are duplicated.
>>
>> Not at the moment, they do different things as code goes.
>>
>> > old add are separated to adding all devices to tree and then matching
>> > work to load the drivers.
>> >
>> > so _START is not needed anymore, only user.start in pci_root driver
>> > should be removed.
>> > code in .start could be moved .add without problem.
>>
>> Yes, I'm going to do that as the next step.  I didn't want this particular
>> patchset to grow too big.  I'll post another one on top of it if people
>> don't have problems with this one.
>
> By the way, can you please remind me where you wanted to put the
> pci_bus_add_devices() and why?
>

please check my for-pci-next branch at
http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=shortlog;h=refs/heads/for-pci-next

that includes delay loading acpi driver and pci driver.

http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=8c031eabbdc83dd4d93933b82d96b55d038bcb64
 PCI: prepare to use device drivers_autoprobe to delay attach drivers

http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=40a12dc8942a8ed02bfbf75ee1ffbfbdf1511b45
PCI: Use device_add for device and bus early

http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=8af9b4c250091c30afedeb2e7f14fca06997c811
ACPI: add drivers_autoprobe in struct acpi_device

http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=ae498e157e9dc8794932562b2f885ddc3a1a229a
 ACPI: use device drivers_autoprobe to delay loading acpi drivers

http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=6bee785b563a0b0e311e188321b1160593d5e6ee
 PCI, ACPI: Remove not used acpi_pci_root_start()

http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=f467a1cd18a07a250be8527b94612fd4a654fbd1
ACPI: remove acpi_op_start workaround
--
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 5/6] ACPI: Replace struct acpi_bus_ops with enum type

On Monday, December 10, 2012 11:47:27 PM Rafael J. Wysocki wrote:
> On Monday, December 10, 2012 09:07:06 AM Yinghai Lu wrote:
> > On Mon, Dec 10, 2012 at 6:46 AM, Rafael J. Wysocki  wrote:
> > > On Sunday, December 09, 2012 09:34:42 PM Yinghai Lu wrote:
> > >>
> > >> Can we expand the BUS_ADD_* concept to other devices instead of just
> > >> acpi_device?
> > >>
> > >> aka we should let struct device has this add_type field.
> > >
> > > Having done that in ACPI to cover our use case here, we can try to move it
> > > into struct device if there are use cases beyond ACPI that can't be 
> > > covered
> > > by using deferred driver probing.
> > 
> > pci device for hotplug have same problem. need to delay driver attach
> > for them too.
> 
> OK, I'll take a look.  Any pointers to speed that up?
> 
> > also BUS_ADD_MATCH and BUS_ADD_START are duplicated.
> 
> Not at the moment, they do different things as code goes.
> 
> > old add are separated to adding all devices to tree and then matching
> > work to load the drivers.
> > 
> > so _START is not needed anymore, only user.start in pci_root driver
> > should be removed.
> > code in .start could be moved .add without problem.
> 
> Yes, I'm going to do that as the next step.  I didn't want this particular
> patchset to grow too big.  I'll post another one on top of it if people
> don't have problems with this one.

By the way, can you please remind me where you wanted to put the
pci_bus_add_devices() and why?

Rafael


-- 
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 5/6] ACPI: Replace struct acpi_bus_ops with enum type

On Monday, December 10, 2012 09:07:06 AM Yinghai Lu wrote:
> On Mon, Dec 10, 2012 at 6:46 AM, Rafael J. Wysocki  wrote:
> > On Sunday, December 09, 2012 09:34:42 PM Yinghai Lu wrote:
> >>
> >> Can we expand the BUS_ADD_* concept to other devices instead of just
> >> acpi_device?
> >>
> >> aka we should let struct device has this add_type field.
> >
> > Having done that in ACPI to cover our use case here, we can try to move it
> > into struct device if there are use cases beyond ACPI that can't be covered
> > by using deferred driver probing.
> 
> pci device for hotplug have same problem. need to delay driver attach
> for them too.

OK, I'll take a look.  Any pointers to speed that up?

> also BUS_ADD_MATCH and BUS_ADD_START are duplicated.

Not at the moment, they do different things as code goes.

> old add are separated to adding all devices to tree and then matching
> work to load the drivers.
> 
> so _START is not needed anymore, only user.start in pci_root driver
> should be removed.
> code in .start could be moved .add without problem.

Yes, I'm going to do that as the next step.  I didn't want this particular
patchset to grow too big.  I'll post another one on top of it if people
don't have problems with this one.

Thanks,
Rafael


-- 
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 5/6] ACPI: Replace struct acpi_bus_ops with enum type

On Mon, Dec 10, 2012 at 6:46 AM, Rafael J. Wysocki  wrote:
> On Sunday, December 09, 2012 09:34:42 PM Yinghai Lu wrote:
>>
>> Can we expand the BUS_ADD_* concept to other devices instead of just
>> acpi_device?
>>
>> aka we should let struct device has this add_type field.
>
> Having done that in ACPI to cover our use case here, we can try to move it
> into struct device if there are use cases beyond ACPI that can't be covered
> by using deferred driver probing.

pci device for hotplug have same problem. need to delay driver attach
for them too.

also BUS_ADD_MATCH and BUS_ADD_START are duplicated.

old add are separated to adding all devices to tree and then matching
work to load the drivers.

so _START is not needed anymore, only user.start in pci_root driver
should be removed.
code in .start could be moved .add without problem.
--
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 5/6] ACPI: Replace struct acpi_bus_ops with enum type

On Sunday, December 09, 2012 09:34:42 PM Yinghai Lu wrote:
> On Sun, Dec 9, 2012 at 3:03 PM, Rafael J. Wysocki  wrote:
> > From: Rafael J. Wysocki 
> >
> > Notice that one member of struct acpi_bus_ops, acpi_op_add, is not
> > used anywhere any more and the relationship between its remaining
> > members, acpi_op_match and acpi_op_start, is such that it doesn't
> > make sense to set the latter without setting the former at the same
> > time.  Therefore, replace struct acpi_bus_ops with new a enum type,
> > enum acpi_bus_add_type, with three values, ACPI_BUS_ADD_BASIC,
> > ACPI_BUS_ADD_MATCH, ACPI_BUS_ADD_START, corresponding to
> > both acpi_op_match and acpi_op_start unset, acpi_op_match set and
> > acpi_op_start unset, and both acpi_op_match and acpi_op_start set,
> > respectively.
> >
> 
> Can we expand the BUS_ADD_* concept to other devices instead of just
> acpi_device?
> 
> aka we should let struct device has this add_type field.

Having done that in ACPI to cover our use case here, we can try to move it
into struct device if there are use cases beyond ACPI that can't be covered
by using deferred driver probing.

Thanks,
Rafael


-- 
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 5/6] ACPI: Replace struct acpi_bus_ops with enum type

On Sunday, December 09, 2012 09:34:42 PM Yinghai Lu wrote:
 On Sun, Dec 9, 2012 at 3:03 PM, Rafael J. Wysocki r...@sisk.pl wrote:
  From: Rafael J. Wysocki rafael.j.wyso...@intel.com
 
  Notice that one member of struct acpi_bus_ops, acpi_op_add, is not
  used anywhere any more and the relationship between its remaining
  members, acpi_op_match and acpi_op_start, is such that it doesn't
  make sense to set the latter without setting the former at the same
  time.  Therefore, replace struct acpi_bus_ops with new a enum type,
  enum acpi_bus_add_type, with three values, ACPI_BUS_ADD_BASIC,
  ACPI_BUS_ADD_MATCH, ACPI_BUS_ADD_START, corresponding to
  both acpi_op_match and acpi_op_start unset, acpi_op_match set and
  acpi_op_start unset, and both acpi_op_match and acpi_op_start set,
  respectively.
 
 
 Can we expand the BUS_ADD_* concept to other devices instead of just
 acpi_device?
 
 aka we should let struct device has this add_type field.

Having done that in ACPI to cover our use case here, we can try to move it
into struct device if there are use cases beyond ACPI that can't be covered
by using deferred driver probing.

Thanks,
Rafael


-- 
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 5/6] ACPI: Replace struct acpi_bus_ops with enum type

On Mon, Dec 10, 2012 at 6:46 AM, Rafael J. Wysocki r...@sisk.pl wrote:
 On Sunday, December 09, 2012 09:34:42 PM Yinghai Lu wrote:

 Can we expand the BUS_ADD_* concept to other devices instead of just
 acpi_device?

 aka we should let struct device has this add_type field.

 Having done that in ACPI to cover our use case here, we can try to move it
 into struct device if there are use cases beyond ACPI that can't be covered
 by using deferred driver probing.

pci device for hotplug have same problem. need to delay driver attach
for them too.

also BUS_ADD_MATCH and BUS_ADD_START are duplicated.

old add are separated to adding all devices to tree and then matching
work to load the drivers.

so _START is not needed anymore, only user.start in pci_root driver
should be removed.
code in .start could be moved .add without problem.
--
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 5/6] ACPI: Replace struct acpi_bus_ops with enum type

On Monday, December 10, 2012 09:07:06 AM Yinghai Lu wrote:
 On Mon, Dec 10, 2012 at 6:46 AM, Rafael J. Wysocki r...@sisk.pl wrote:
  On Sunday, December 09, 2012 09:34:42 PM Yinghai Lu wrote:
 
  Can we expand the BUS_ADD_* concept to other devices instead of just
  acpi_device?
 
  aka we should let struct device has this add_type field.
 
  Having done that in ACPI to cover our use case here, we can try to move it
  into struct device if there are use cases beyond ACPI that can't be covered
  by using deferred driver probing.
 
 pci device for hotplug have same problem. need to delay driver attach
 for them too.

OK, I'll take a look.  Any pointers to speed that up?

 also BUS_ADD_MATCH and BUS_ADD_START are duplicated.

Not at the moment, they do different things as code goes.

 old add are separated to adding all devices to tree and then matching
 work to load the drivers.
 
 so _START is not needed anymore, only user.start in pci_root driver
 should be removed.
 code in .start could be moved .add without problem.

Yes, I'm going to do that as the next step.  I didn't want this particular
patchset to grow too big.  I'll post another one on top of it if people
don't have problems with this one.

Thanks,
Rafael


-- 
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 5/6] ACPI: Replace struct acpi_bus_ops with enum type

On Monday, December 10, 2012 11:47:27 PM Rafael J. Wysocki wrote:
 On Monday, December 10, 2012 09:07:06 AM Yinghai Lu wrote:
  On Mon, Dec 10, 2012 at 6:46 AM, Rafael J. Wysocki r...@sisk.pl wrote:
   On Sunday, December 09, 2012 09:34:42 PM Yinghai Lu wrote:
  
   Can we expand the BUS_ADD_* concept to other devices instead of just
   acpi_device?
  
   aka we should let struct device has this add_type field.
  
   Having done that in ACPI to cover our use case here, we can try to move it
   into struct device if there are use cases beyond ACPI that can't be 
   covered
   by using deferred driver probing.
  
  pci device for hotplug have same problem. need to delay driver attach
  for them too.
 
 OK, I'll take a look.  Any pointers to speed that up?
 
  also BUS_ADD_MATCH and BUS_ADD_START are duplicated.
 
 Not at the moment, they do different things as code goes.
 
  old add are separated to adding all devices to tree and then matching
  work to load the drivers.
  
  so _START is not needed anymore, only user.start in pci_root driver
  should be removed.
  code in .start could be moved .add without problem.
 
 Yes, I'm going to do that as the next step.  I didn't want this particular
 patchset to grow too big.  I'll post another one on top of it if people
 don't have problems with this one.

By the way, can you please remind me where you wanted to put the
pci_bus_add_devices() and why?

Rafael


-- 
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 5/6] ACPI: Replace struct acpi_bus_ops with enum type

On Mon, Dec 10, 2012 at 3:09 PM, Rafael J. Wysocki r...@sisk.pl wrote:
 On Monday, December 10, 2012 11:47:27 PM Rafael J. Wysocki wrote:
 On Monday, December 10, 2012 09:07:06 AM Yinghai Lu wrote:
  On Mon, Dec 10, 2012 at 6:46 AM, Rafael J. Wysocki r...@sisk.pl wrote:
   On Sunday, December 09, 2012 09:34:42 PM Yinghai Lu wrote:
  
   Can we expand the BUS_ADD_* concept to other devices instead of just
   acpi_device?
  
   aka we should let struct device has this add_type field.
  
   Having done that in ACPI to cover our use case here, we can try to move 
   it
   into struct device if there are use cases beyond ACPI that can't be 
   covered
   by using deferred driver probing.
 
  pci device for hotplug have same problem. need to delay driver attach
  for them too.

 OK, I'll take a look.  Any pointers to speed that up?

  also BUS_ADD_MATCH and BUS_ADD_START are duplicated.

 Not at the moment, they do different things as code goes.

  old add are separated to adding all devices to tree and then matching
  work to load the drivers.
 
  so _START is not needed anymore, only user.start in pci_root driver
  should be removed.
  code in .start could be moved .add without problem.

 Yes, I'm going to do that as the next step.  I didn't want this particular
 patchset to grow too big.  I'll post another one on top of it if people
 don't have problems with this one.

 By the way, can you please remind me where you wanted to put the
 pci_bus_add_devices() and why?


please check my for-pci-next branch at
http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=shortlog;h=refs/heads/for-pci-next

that includes delay loading acpi driver and pci driver.

http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=8c031eabbdc83dd4d93933b82d96b55d038bcb64
 PCI: prepare to use device drivers_autoprobe to delay attach drivers

http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=40a12dc8942a8ed02bfbf75ee1ffbfbdf1511b45
PCI: Use device_add for device and bus early

http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=8af9b4c250091c30afedeb2e7f14fca06997c811
ACPI: add drivers_autoprobe in struct acpi_device

http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=ae498e157e9dc8794932562b2f885ddc3a1a229a
 ACPI: use device drivers_autoprobe to delay loading acpi drivers

http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=6bee785b563a0b0e311e188321b1160593d5e6ee
 PCI, ACPI: Remove not used acpi_pci_root_start()

http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=f467a1cd18a07a250be8527b94612fd4a654fbd1
ACPI: remove acpi_op_start workaround
--
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 5/6] ACPI: Replace struct acpi_bus_ops with enum type

On Mon, Dec 10, 2012 at 2:47 PM, Rafael J. Wysocki r...@sisk.pl wrote:
 On Monday, December 10, 2012 09:07:06 AM Yinghai Lu wrote:
 On Mon, Dec 10, 2012 at 6:46 AM, Rafael J. Wysocki r...@sisk.pl wrote:
  On Sunday, December 09, 2012 09:34:42 PM Yinghai Lu wrote:
 
  Can we expand the BUS_ADD_* concept to other devices instead of just
  acpi_device?
 
  aka we should let struct device has this add_type field.
 
  Having done that in ACPI to cover our use case here, we can try to move it
  into struct device if there are use cases beyond ACPI that can't be covered
  by using deferred driver probing.

 pci device for hotplug have same problem. need to delay driver attach
 for them too.

 OK, I'll take a look.  Any pointers to speed that up?

 also BUS_ADD_MATCH and BUS_ADD_START are duplicated.

 Not at the moment, they do different things as code goes.

I do think that is same problem which is in the driver/base core.

it should support delay loading driver for the hotplug case at the first point.
that is reason that .start is introduced for acpi driver..to workround
the problem
in driver core.


 old add are separated to adding all devices to tree and then matching
 work to load the drivers.

 so _START is not needed anymore, only user.start in pci_root driver
 should be removed.
 code in .start could be moved .add without problem.

 Yes, I'm going to do that as the next step.  I didn't want this particular

after you remove that .start, that will the same as my two patches.
except that you patches toggle that per-device add_type for every device.

and my patches is only toggle that per device drivers_autoprobe for
hot-add devices.

 patchset to grow too big.  I'll post another one on top of it if people
 don't have problems with this one.

yesterday, I replace my two acpi driver delay loading patches with your patches.

during pci root bus hot remove/add test, got the panic...

did not have time to look at it yet.

Yinghai
--
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 5/6] ACPI: Replace struct acpi_bus_ops with enum type

On Monday, December 10, 2012 03:22:48 PM Yinghai Lu wrote:
 On Mon, Dec 10, 2012 at 2:47 PM, Rafael J. Wysocki r...@sisk.pl wrote:
  On Monday, December 10, 2012 09:07:06 AM Yinghai Lu wrote:
  On Mon, Dec 10, 2012 at 6:46 AM, Rafael J. Wysocki r...@sisk.pl wrote:
   On Sunday, December 09, 2012 09:34:42 PM Yinghai Lu wrote:
  
   Can we expand the BUS_ADD_* concept to other devices instead of just
   acpi_device?
  
   aka we should let struct device has this add_type field.
  
   Having done that in ACPI to cover our use case here, we can try to move 
   it
   into struct device if there are use cases beyond ACPI that can't be 
   covered
   by using deferred driver probing.
 
  pci device for hotplug have same problem. need to delay driver attach
  for them too.
 
  OK, I'll take a look.  Any pointers to speed that up?
 
  also BUS_ADD_MATCH and BUS_ADD_START are duplicated.
 
  Not at the moment, they do different things as code goes.
 
 I do think that is same problem which is in the driver/base core.
 
 it should support delay loading driver for the hotplug case at the first 
 point.
 that is reason that .start is introduced for acpi driver..to workround
 the problem
 in driver core.

OK, but let's eliminate .start() first and then go make changes to the
driver core.  That is, let's take one step at a time. :-)

  old add are separated to adding all devices to tree and then matching
  work to load the drivers.
 
  so _START is not needed anymore, only user.start in pci_root driver
  should be removed.
  code in .start could be moved .add without problem.
 
  Yes, I'm going to do that as the next step.  I didn't want this particular
 
 after you remove that .start, that will the same as my two patches.
 except that you patches toggle that per-device add_type for every device.
 
 and my patches is only toggle that per device drivers_autoprobe for
 hot-add devices.

I'm not 100% sure of that, but quite likely that's the case.  I'd like to avoid
special-casing hotplug in any way, if possible.  Also, I have other reasons to
do it for all devices (like the resources management for one example).

  patchset to grow too big.  I'll post another one on top of it if people
  don't have problems with this one.
 
 yesterday, I replace my two acpi driver delay loading patches with your 
 patches.
 
 during pci root bus hot remove/add test, got the panic...

I don't see what the reason may be at the moment, it would be good to get more
info (like a screenshot of the panic).

 did not have time to look at it yet.

OK

Thanks,
Rafael


-- 
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 5/6] ACPI: Replace struct acpi_bus_ops with enum type

On Monday, December 10, 2012 03:14:32 PM Yinghai Lu wrote:
 On Mon, Dec 10, 2012 at 3:09 PM, Rafael J. Wysocki r...@sisk.pl wrote:
  On Monday, December 10, 2012 11:47:27 PM Rafael J. Wysocki wrote:
  On Monday, December 10, 2012 09:07:06 AM Yinghai Lu wrote:
   On Mon, Dec 10, 2012 at 6:46 AM, Rafael J. Wysocki r...@sisk.pl wrote:
On Sunday, December 09, 2012 09:34:42 PM Yinghai Lu wrote:
   
Can we expand the BUS_ADD_* concept to other devices instead of just
acpi_device?
   
aka we should let struct device has this add_type field.
   
Having done that in ACPI to cover our use case here, we can try to 
move it
into struct device if there are use cases beyond ACPI that can't be 
covered
by using deferred driver probing.
  
   pci device for hotplug have same problem. need to delay driver attach
   for them too.
 
  OK, I'll take a look.  Any pointers to speed that up?
 
   also BUS_ADD_MATCH and BUS_ADD_START are duplicated.
 
  Not at the moment, they do different things as code goes.
 
   old add are separated to adding all devices to tree and then matching
   work to load the drivers.
  
   so _START is not needed anymore, only user.start in pci_root driver
   should be removed.
   code in .start could be moved .add without problem.
 
  Yes, I'm going to do that as the next step.  I didn't want this particular
  patchset to grow too big.  I'll post another one on top of it if people
  don't have problems with this one.
 
  By the way, can you please remind me where you wanted to put the
  pci_bus_add_devices() and why?
 
 
 please check my for-pci-next branch at
 http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=shortlog;h=refs/heads/for-pci-next
 
 that includes delay loading acpi driver and pci driver.
 
 http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=8c031eabbdc83dd4d93933b82d96b55d038bcb64
  PCI: prepare to use device drivers_autoprobe to delay attach drivers
 
 http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=40a12dc8942a8ed02bfbf75ee1ffbfbdf1511b45
 PCI: Use device_add for device and bus early
 
 http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=8af9b4c250091c30afedeb2e7f14fca06997c811
 ACPI: add drivers_autoprobe in struct acpi_device
 
 http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=ae498e157e9dc8794932562b2f885ddc3a1a229a
  ACPI: use device drivers_autoprobe to delay loading acpi drivers
 
 http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=6bee785b563a0b0e311e188321b1160593d5e6ee
  PCI, ACPI: Remove not used acpi_pci_root_start()
 
 http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=f467a1cd18a07a250be8527b94612fd4a654fbd1
 ACPI: remove acpi_op_start workaround

OK, thanks for the pointers.  I actually see more differences between our
patchsets.  For one example, you seem to have left the parent-ops.bind()
stuff in acpi_add_single_object() which calls it even drivers_autoprobe is
set.  Is that the case, or am I missing anything?

Rafael


-- 
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 5/6] ACPI: Replace struct acpi_bus_ops with enum type

On Tuesday, December 11, 2012 02:02:14 AM Rafael J. Wysocki wrote:
 On Monday, December 10, 2012 03:14:32 PM Yinghai Lu wrote:
  On Mon, Dec 10, 2012 at 3:09 PM, Rafael J. Wysocki r...@sisk.pl wrote:
   On Monday, December 10, 2012 11:47:27 PM Rafael J. Wysocki wrote:
   On Monday, December 10, 2012 09:07:06 AM Yinghai Lu wrote:
On Mon, Dec 10, 2012 at 6:46 AM, Rafael J. Wysocki r...@sisk.pl 
wrote:
 On Sunday, December 09, 2012 09:34:42 PM Yinghai Lu wrote:

 Can we expand the BUS_ADD_* concept to other devices instead of just
 acpi_device?

 aka we should let struct device has this add_type field.

 Having done that in ACPI to cover our use case here, we can try to 
 move it
 into struct device if there are use cases beyond ACPI that can't be 
 covered
 by using deferred driver probing.
   
pci device for hotplug have same problem. need to delay driver attach
for them too.
  
   OK, I'll take a look.  Any pointers to speed that up?
  
also BUS_ADD_MATCH and BUS_ADD_START are duplicated.
  
   Not at the moment, they do different things as code goes.
  
old add are separated to adding all devices to tree and then matching
work to load the drivers.
   
so _START is not needed anymore, only user.start in pci_root driver
should be removed.
code in .start could be moved .add without problem.
  
   Yes, I'm going to do that as the next step.  I didn't want this 
   particular
   patchset to grow too big.  I'll post another one on top of it if people
   don't have problems with this one.
  
   By the way, can you please remind me where you wanted to put the
   pci_bus_add_devices() and why?
  
  
  please check my for-pci-next branch at
  http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=shortlog;h=refs/heads/for-pci-next
  
  that includes delay loading acpi driver and pci driver.
  
  http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=8c031eabbdc83dd4d93933b82d96b55d038bcb64
   PCI: prepare to use device drivers_autoprobe to delay attach drivers
  
  http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=40a12dc8942a8ed02bfbf75ee1ffbfbdf1511b45
  PCI: Use device_add for device and bus early
  
  http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=8af9b4c250091c30afedeb2e7f14fca06997c811
  ACPI: add drivers_autoprobe in struct acpi_device
  
  http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=ae498e157e9dc8794932562b2f885ddc3a1a229a
   ACPI: use device drivers_autoprobe to delay loading acpi drivers
  
  http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=6bee785b563a0b0e311e188321b1160593d5e6ee
   PCI, ACPI: Remove not used acpi_pci_root_start()
  
  http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=f467a1cd18a07a250be8527b94612fd4a654fbd1
  ACPI: remove acpi_op_start workaround
 
 OK, thanks for the pointers.  I actually see more differences between our
 patchsets.  For one example, you seem to have left the parent-ops.bind()
 stuff in acpi_add_single_object() which calls it even drivers_autoprobe is
 set.

Sorry, that should have been which calls it even when drivers_autoprobe is
not set.  I need to be more careful.


 Is that the case, or am I missing anything?

Thanks,
Rafael


-- 
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 5/6] ACPI: Replace struct acpi_bus_ops with enum type