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.wysocki@xxxxxxxxx>
:
> > > 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.
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.
Otherwise the changes look very good.
Reviewed-by: Toshi Kani <toshi.kani@xxxxxx>
Thanks,
-Toshi
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
