Re: [RFC PATCH 2/8] PNPACPI: use whilte list for pnpacpi device enumeration

[Zhang Rui <rui.zhang@intel.com>]

On Sun, 2014-03-09 at 18:49 +0100, Rafael J. Wysocki wrote:
> On Sunday, March 09, 2014 01:29:30 PM Zhang Rui wrote:
> > On Fri, 2014-03-07 at 02:44 +0100, Rafael J. Wysocki wrote:
> > > On Wednesday, February 26, 2014 05:11:08 PM Zhang Rui wrote:
> > > > +
> > > > +static int __init acpi_pnp_scan_handler_attach(struct acpi_device *adev,
> > > 
> > > This can't be __init.
> > > 
> > > > +                                   const struct acpi_device_id *id)
> > > > +{
> > > > +	return 1;
> > > > +}
> > > > +
> > > > +static int __init acpi_pnp_scan_handler_match(char *devid, char *handlerid)
> > > 
> > > And this too.
> > > 
> > > > +{
> > > > +	int i;
> > > > +
> > > > +	if (!ispnpidacpi(devid))
> > > > +		return 0;
> > > > +
> > > > +	if (memcmp(devid, handlerid, 3))
> > > > +		return 0;
> > > > +
> > > > +        for (i = 3; i < 7; i++) {
> > > > +                if (handlerid[i] != 'X' &&
> > > > +		    toupper(devid[i]) != toupper(handlerid[i]))
> > > > +                        return 0;
> > > > +        }
> > > > +	return 1;
> > > > +}
> > > > +
> > > > +static struct acpi_scan_handler pnpacpi_handler __initdata = {
> > > 
> > > And this cannot be __initdata, because the list of ACPI scan handlers is
> > > walked during hotplug.
> > > 
> > right. will update it in next version.
> > 
> > > > +	.ids = acpi_pnp_device_ids,
> > > > +	.match = acpi_pnp_scan_handler_match,
> > > > +	.attach = acpi_pnp_scan_handler_attach,
> > > > +};
> > > > +
> > > > +void __init acpi_pnp_init(void)
> > > > +{
> > > > +	acpi_scan_add_handler(&pnpacpi_handler);
> > > > +}
> > > > +
> > > > +static acpi_status __init pnpacpi_add_device_handler(acpi_handle handle,
> > > > +						     u32 lvl, void *context,
> > > > +						     void **rv)
> > > > +{
> > > > +	struct acpi_device *device;
> > > > +
> > > > +	if (acpi_bus_get_device(handle, &device))
> > > > +		return AE_CTRL_DEPTH;
> > > > +	if (device->handler == &pnpacpi_handler)
> > > > +		pnpacpi_add_device(device);
> > > 
> > > Why don't you do that in acpi_pnp_scan_handler_attach() ?
> > > 
> > because the PNP bus is not ready at that time.
> 
> Do you mean it hasn't been initialized yet?
> 
right.
both acpi_init() and pnp_init() are subsys_initcall, but pnp_init() is
invoked later because of the link order.

> But we can initialize it before the scan handler initialization, can't we?
> 
well, I'm not sure, here is what I get from drivers/Makefile
obj-$(CONFIG_ACPI)              += acpi/
obj-$(CONFIG_SFI)               += sfi/
# PnP must come after ACPI since it will eventually need to check ifacpi
# was used and do nothing if so
obj-$(CONFIG_PNP)               += pnp/

this comment was added long time ago and I do not know the background of
it, maybe Len knows the reason of this?

thanks,
rui