Re: [Patch v5 7/7] acpi_memhotplug.c: auto bind the memory devicewhich is hotplugged before the driver is loaded

[Toshi Kani]

On Thu, 2012-11-15 at 14:59 +0800, Wen Congyang wrote:
> If the memory device is hotplugged before the driver is loaded, the user
> cannot see this device under the directory /sys/bus/acpi/devices/, and the
> user cannot bind it by hand after the driver is loaded.  This patch
> introduces a new feature to bind such device when the driver is being
> loaded.

Hi Wen,

While I can see the problem, I am a little concerned about this
approach.  If we go this way, all ACPI hotplug drivers need to do it on
their own way and walk the entire ACPI namespace when they are
installed.  Such common issue should be dealt by ACPI core.  I am
wondering if we can simply set MEMORY_HOTPLUG to default y for now.
Processor and Container are already set to y by default.

Thanks,
-Toshi


> CC: David Rientjes <rientjes@xxxxxxxxxx>
> CC: Jiang Liu <liuj97@xxxxxxxxx>
> CC: Len Brown <len.brown@xxxxxxxxx>
> CC: Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx>
> CC: Paul Mackerras <paulus@xxxxxxxxx>
> CC: Christoph Lameter <cl@xxxxxxxxx>
> Cc: Minchan Kim <minchan.kim@xxxxxxxxx>
> CC: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> CC: KOSAKI Motohiro <kosaki.motohiro@xxxxxxxxxxxxxx>
> CC: Yasuaki Ishimatsu <isimatu.yasuaki@xxxxxxxxxxxxxx>
> CC: Rafael J. Wysocki <rjw@xxxxxxx>
> CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> Signed-off-by: Wen Congyang <wency@xxxxxxxxxxxxxx>
> ---
>  drivers/acpi/acpi_memhotplug.c | 37 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
> index e0f7425..9f1d107 100644
> --- a/drivers/acpi/acpi_memhotplug.c
> +++ b/drivers/acpi/acpi_memhotplug.c
> @@ -52,6 +52,9 @@ MODULE_LICENSE("GPL");
>  #define MEMORY_POWER_ON_STATE	1
>  #define MEMORY_POWER_OFF_STATE	2
>  
> +static bool auto_probe;
> +module_param(auto_probe, bool, S_IRUGO | S_IWUSR);
> +
>  static int acpi_memory_device_add(struct acpi_device *device);
>  static int acpi_memory_device_remove(struct acpi_device *device, int type);
>  
> @@ -494,12 +497,44 @@ acpi_memory_register_notify_handler(acpi_handle handle,
>  				    u32 level, void *ctxt, void **retv)
>  {
>  	acpi_status status;
> -
> +	struct acpi_memory_device *mem_device = NULL;
> +	unsigned long long current_status;
>  
>  	status = is_memory_device(handle);
>  	if (ACPI_FAILURE(status))
>  		return AE_OK;	/* continue */
>  
> +	if (auto_probe) {
> +		/* Get device present/absent information from the _STA */
> +		status = acpi_evaluate_integer(handle, "_STA", NULL,
> +					       &current_status);
> +		if (ACPI_FAILURE(status))
> +			goto install;
> +
> +		/*
> +		 * Check for device status. Device should be
> +		 * present/enabled/functioning.
> +		 */
> +		if (!(current_status &
> +		      (ACPI_STA_DEVICE_PRESENT | ACPI_STA_DEVICE_ENABLED |
> +		       ACPI_STA_DEVICE_FUNCTIONING)))
> +			goto install;
> +
> +		if (acpi_memory_get_device(handle, &mem_device))
> +			goto install;
> +
> +		/* We have bound this device while we register the driver */
> +		if (mem_device->state == MEMORY_POWER_ON_STATE)
> +			goto install;
> +
> +		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
> +				  "\nauto probe memory device\n"));
> +
> +		if (acpi_memory_enable_device(mem_device))
> +			pr_err(PREFIX "Cannot enable memory device\n");
> +	}
> +
> +install:
>  	status = acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
>  					     acpi_memory_device_notify, NULL);
>  	/* continue */


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