ACPI: install ACPI table handler before any dynamic tables being loaded

PATCH Applied Zhang Rui 3 participants 5 messages Oct 26, 2010 ACPI

lore

Share & tools

lore

Zhang Rui

author

2010-10-26 02:07 UTC #0 lore raw

PATCH 1/1 ACPI: install ACPI table handler before any dynamic tables being loaded

ACPI table sysfs I/F is broken by commit

78f1699659963fff97975df44db6d5dbe7218e55

Author: Alex Chiang <[email protected]>
Date:   Sun Dec 20 12:19:09 2009 -0700

ACPI: processor: call _PDC early

because dynamic SSDT tables may be loaded in _PDC, before installing the ACPI table handler.
As a result, the sysfs I/F of these dynamic tables are located at /sys/firmware/acpi/tables instead of /sys/firmware/acpi/tables/dynamic, which is not true.

Invoke acpi_sysfs_init() before acpi_early_processor_set_pdc(), so that the table handler is installed before any dynamic tables loaded.

https://bugzilla.kernel.org/show_bug.cgi?id=21142

CC Dennis Jansen <[email protected]>

CC Alex Chiang <[email protected]>

Signed-off-by Zhang Rui <[email protected]>

---
 drivers/acpi/bus.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

drivers/acpi/bus.c +6 -1

Index: linux-2.6/drivers/acpi/bus.c
===================================================================
@@ -935,6 +935,12 @@ static int __init acpi_bus_init(void)
 		goto error1;
 	}
 
+	/*
+	 * _PDC control method may load dynamic SSDT tables,
+	 * and we need to install the table handler before that.
+	 */
+	acpi_sysfs_init();
+
 	acpi_early_processor_set_pdc();
 
 	/*

@@ -1026,7 +1032,6 @@ static int __init acpi_init(void)
 	acpi_scan_init();
 	acpi_ec_init();
 	acpi_power_init();
-	acpi_sysfs_init();
 	acpi_debugfs_init();
 	acpi_sleep_proc_init();
 	acpi_wakeup_device_init();

Zhang Rui

author

2010-10-26 02:12 UTC #1 lore raw

cc Alex Chiang.

Quoted 56 lines · Zhang Rui

ACPI table sysfs I/F is broken by commit
78f1699659963fff97975df44db6d5dbe7218e55
Author: Alex Chiang <[email protected]>
Date:   Sun Dec 20 12:19:09 2009 -0700
ACPI: processor: call _PDC early
because dynamic SSDT tables may be loaded in _PDC, before installing the ACPI table handler.
As a result, the sysfs I/F of these dynamic tables are located at /sys/firmware/acpi/tables instead of /sys/firmware/acpi/tables/dynamic, which is not true.
Invoke acpi_sysfs_init() before acpi_early_processor_set_pdc(), so that the table handler is installed before any dynamic tables loaded.
https://bugzilla.kernel.org/show_bug.cgi?id=21142
CC Dennis Jansen <[email protected]>
CC Alex Chiang <[email protected]>
Signed-off-by Zhang Rui <[email protected]>
---
 drivers/acpi/bus.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
Index: linux-2.6/drivers/acpi/bus.c
===================================================================
drivers/acpi/bus.c +6 -2
@@ -935,6 +935,12 @@ static int __init acpi_bus_init(void)
 		goto error1;
 	}
 
+	/*
+	 * _PDC control method may load dynamic SSDT tables,
+	 * and we need to install the table handler before that.
+	 */
+	acpi_sysfs_init();
+
 	acpi_early_processor_set_pdc();
 
 	/*
@@ -1026,7 +1032,6 @@ static int __init acpi_init(void)
 	acpi_scan_init();
 	acpi_ec_init();
 	acpi_power_init();
-	acpi_sysfs_init();
 	acpi_debugfs_init();
 	acpi_sleep_proc_init();
 	acpi_wakeup_device_init();
 
 
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Alex Chiang

Acked-by

2010-10-26 05:06 UTC #2 lore raw

Hello Rui,

Thank you for adding me to the cc.

* Zhang Rui <[email protected]>:

Quoted 18 lines

cc Alex Chiang.
Quoted 15 lines · Zhang Rui
ACPI table sysfs I/F is broken by commit
78f1699659963fff97975df44db6d5dbe7218e55
Author: Alex Chiang <[email protected]>
Date:   Sun Dec 20 12:19:09 2009 -0700
ACPI: processor: call _PDC early
because dynamic SSDT tables may be loaded in _PDC, before installing the ACPI table handler.
As a result, the sysfs I/F of these dynamic tables are located at /sys/firmware/acpi/tables instead of /sys/firmware/acpi/tables/dynamic, which is not true.
Invoke acpi_sysfs_init() before acpi_early_processor_set_pdc(), so that the table handler is installed before any dynamic tables loaded.

You are right; I wasn't careful when I wrote 78f169965996, and forgot to verify that dynamically loaded tables should appear in /sys/firmware/acpi/tables/dynamic.

I see that the interface was introduced by 4658e4ef9d252 in February 2009, so I should have been aware of it.

I think it will be easier for future developers to be aware of the ACPI sysfs interfaces if they were documented. I see that the file Documentation/ABI/testing/sysfs-firmware-acpi exists, but does not describe many of the interfaces in sysfs.

Do you want to write a documentation patch, or shall I?

Also, I think Bjorn had some opinions on acpi_early_processor_set_pdc():

	https://bugzilla.kernel.org/show_bug.cgi?id=14824#c7

Your current patch is fine for fixing the immediate problem you notice, but perhaps you should explore the ideas that Bjorn wrote in his last comment, and then we can get rid of this ugly bit altogether.

Acked-by Alex Chiang <[email protected]>

Thanks,
/ac

Quoted 43 lines

Quoted 41 lines
https://bugzilla.kernel.org/show_bug.cgi?id=21142
CC Dennis Jansen <[email protected]>
CC Alex Chiang <[email protected]>
Signed-off-by Zhang Rui <[email protected]>
---
 drivers/acpi/bus.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
Index: linux-2.6/drivers/acpi/bus.c
===================================================================
drivers/acpi/bus.c +6 -2
@@ -935,6 +935,12 @@ static int __init acpi_bus_init(void)
 		goto error1;
 	}
 
+	/*
+	 * _PDC control method may load dynamic SSDT tables,
+	 * and we need to install the table handler before that.
+	 */
+	acpi_sysfs_init();
+
 	acpi_early_processor_set_pdc();
 
 	/*
@@ -1026,7 +1032,6 @@ static int __init acpi_init(void)
 	acpi_scan_init();
 	acpi_ec_init();
 	acpi_power_init();
-	acpi_sysfs_init();
 	acpi_debugfs_init();
 	acpi_sleep_proc_init();
 	acpi_wakeup_device_init();
 
 
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Zhang Rui

author

2010-10-26 05:45 UTC #3 lore raw

Quoted 31 lines · Alex Chiang

Hello Rui,
Thank you for adding me to the cc.
* Zhang Rui <[email protected]>:
Quoted 18 lines
cc Alex Chiang.
Quoted 15 lines · Zhang Rui
ACPI table sysfs I/F is broken by commit
78f1699659963fff97975df44db6d5dbe7218e55
Author: Alex Chiang <[email protected]>
Date:   Sun Dec 20 12:19:09 2009 -0700
ACPI: processor: call _PDC early
because dynamic SSDT tables may be loaded in _PDC, before installing the ACPI table handler.
As a result, the sysfs I/F of these dynamic tables are located at /sys/firmware/acpi/tables instead of /sys/firmware/acpi/tables/dynamic, which is not true.
Invoke acpi_sysfs_init() before acpi_early_processor_set_pdc(), so that the table handler is installed before any dynamic tables loaded.
You are right; I wasn't careful when I wrote 78f169965996, and forgot to verify that dynamically loaded tables should appear in /sys/firmware/acpi/tables/dynamic.
I see that the interface was introduced by 4658e4ef9d252 in February 2009, so I should have been aware of it.

Quoted 7 lines

I think it will be easier for future developers to be aware of the ACPI sysfs interfaces if they were documented. I see that the file Documentation/ABI/testing/sysfs-firmware-acpi exists, but does not describe many of the interfaces in sysfs.
Do you want to write a documentation patch, or shall I?

good point. Yes, I'll update the Documentation. :)

Quoted 8 lines

Also, I think Bjorn had some opinions on acpi_early_processor_set_pdc():
	https://bugzilla.kernel.org/show_bug.cgi?id=14824#c7
Your current patch is fine for fixing the immediate problem you notice, but perhaps you should explore the ideas that Bjorn wrote in his last comment, and then we can get rid of this ugly bit altogether.

agree. Making clear how EC works in windows is really helpful, not only for this problem but also for other EC bugs as well.
As Alexey may not longer work on ACPI EC driver, I'll take a look at the EC code and do some investigation on this area.

thanks,
rui

Quoted 48 lines

Acked-by Alex Chiang <[email protected]>
Thanks,
/ac
Quoted 43 lines
Quoted 41 lines
https://bugzilla.kernel.org/show_bug.cgi?id=21142
CC Dennis Jansen <[email protected]>
CC Alex Chiang <[email protected]>
Signed-off-by Zhang Rui <[email protected]>
---
 drivers/acpi/bus.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
Index: linux-2.6/drivers/acpi/bus.c
===================================================================
drivers/acpi/bus.c +6 -2
@@ -935,6 +935,12 @@ static int __init acpi_bus_init(void)
 		goto error1;
 	}
 
+	/*
+	 * _PDC control method may load dynamic SSDT tables,
+	 * and we need to install the table handler before that.
+	 */
+	acpi_sysfs_init();
+
 	acpi_early_processor_set_pdc();
 
 	/*
@@ -1026,7 +1032,6 @@ static int __init acpi_init(void)
 	acpi_scan_init();
 	acpi_ec_init();
 	acpi_power_init();
-	acpi_sysfs_init();
 	acpi_debugfs_init();
 	acpi_sleep_proc_init();
 	acpi_wakeup_device_init();
 
 
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Len Brown

reviewer Applied

2010-10-26 08:55 UTC #4 lore raw

In reply to Zhang Rui

applied

thanks,
Len Brown, Intel Open Source Technology Center