On Friday, May 03, 2013 12:27:54 PM Toshi Kani wrote:
> On Fri, 2013-05-03 at 14:05 +0200, Rafael J. Wysocki wrote:
> > On Thursday, May 02, 2013 05:20:12 PM Toshi Kani wrote:
> > > On Thu, 2013-05-02 at 14:31 +0200, Rafael J. Wysocki wrote:
> > > > From: Rafael J. Wysocki <rafael.j.wyso...@intel.com>
> :
> > > > Index: linux-pm/drivers/base/cpu.c
> > > > ===================================================================
> > > > --- linux-pm.orig/drivers/base/cpu.c
> > > > +++ linux-pm/drivers/base/cpu.c
> > > > @@ -13,11 +13,21 @@
> > > > #include <linux/gfp.h>
> > > > #include <linux/slab.h>
> > > > #include <linux/percpu.h>
> > > > +#include <linux/acpi.h>
> > > >
> > > > #include "base.h"
> > > >
> > > > static DEFINE_PER_CPU(struct device *, cpu_sys_devices);
> > > >
> > > > +static int cpu_subsys_match(struct device *dev, struct device_driver
> > > > *drv)
> > > > +{
> > > > + /* ACPI style match is the only one that may succeed. */
> > > > + if (acpi_driver_match_device(dev, drv))
> > >
> > > Can you explain why this change is needed?
> >
> > This is the mechanism by which the driver core determines which driver to
> > use
> > with a processor device passed to device_attach().
> >
> > Basically, it walks the list of drivers whose bus type is cpu_subsys and
> > calls cpu_subsys->match(), which points to cpu_subsys_match(), for the
> > device
> > and each of the drivers. The result of that tell is whether or not to use
> > the given driver with the device.
> >
> > Now, acpi_driver_match_device() returns 'true' if (a) the device has an ACPI
> > handle and (b) at least one of the IDs of the struct acpi_device associated
> > with that handle is in the driver's .acpi_match_table table. Since the ACPI
> > processor's .acpi_match_table contains the same set of IDs as the table
> > of device IDs of processor_handler, this guarantees that the ACPI processor
> > driver will be used for the devices prepared by acpi_processor_add().
> >
> > What it boils down to is that acpi_processor_start() is going to be called
> > for every device whose ACPI handle is populated by acpi_processor_add().
> >
> > > Do CPU devices still behave the same on non-ACPI systems?
> >
> > Yes, they do. The whole driver matching/binding is irrelevant to them,
> > because
> > the ACPI processor driver is the only one registering itself under
> > cpu_subsys.
>
> Thanks for the detailed explanation! I missed that the new processor
> driver is registered to cpu_subsys. I now see what you did. This is
> clever.
Well, thanks! :-)
> One minor comment.
>
> +static __cpuinit int __acpi_processor_start(struct acpi_device *device)
> > {
> > - struct acpi_device *device = per_cpu(processor_device_array,
> pr->id);
> > + struct acpi_processor *pr = acpi_driver_data(device);
> > + acpi_status status;
> > int result = 0;
> >
> > + if (!pr)
> > + return -ENODEV;
> > +
> > + if (pr->flags.need_hotplug_init)
> > + return 0;
> > +
>
> I felt the name of "need_hotplug_init" is a bit misleading since the
> func actually skips when the need-flag is set. It may be nice to rename
> it to defer_online_init, offline or something like that.
I just wanted to avoid making too many non-essential changes in one patch.
We can change the name of that field at any time later.
> Otherwise the changes look very good.
>
> Reviewed-by: Toshi Kani <toshi.k...@hp.com>
Thank you!
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/
