(2014/09/04 6:41), Rafael J. Wysocki wrote:
On Wednesday, September 03, 2014 01:39:13 PM Yasuaki Ishimatsu wrote:
By this commit, "202317a : ACPI / scan: Add acpi_device objects for all
device nodes in the namespace", all device nodes in the namespace are
shown under /sys/bus/acpi/devices directory even if the devices are not
present and not functional. And if a device node has _SUN method, it is
decoded and the sun value is cached to acpi_device_pnp->sun.
But when device is not present and not functional, some firmware returns
wrong sun value. And the wrong value is cached to acpi_device_pnp->sun.
Therefore, even if a device is hot added and firmware returns correct
sun value of the device, the value is not reflected to
acpi_device_pnp->sun. So user cannot know correct sun value of the
device.
By not caching sun value, the patch fixes the issue.
Queued up for 3.17-rc4, but with a different changelog, because in my opinion
the main problem is that _SUN is not guaranteed to return the same value
every time it is evaluated, so either it should not be cached, or we should
update the cached value whenever the device status changes.
I see. I have no objection to changing the changelog.
Please change it.
Thanks,
Yasuaki Ishimatsu
Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@xxxxxxxxxxxxxx>
Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
---
drivers/acpi/scan.c | 15 ++++++++-------
include/acpi/acpi_bus.h | 1 -
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 9a92989..3bf7764 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -667,8 +667,14 @@ static ssize_t
acpi_device_sun_show(struct device *dev, struct device_attribute *attr,
char *buf) {
struct acpi_device *acpi_dev = to_acpi_device(dev);
+ acpi_status status;
+ unsigned long long sun;
+
+ status = acpi_evaluate_integer(acpi_dev->handle, "_SUN", NULL, &sun);
+ if (ACPI_FAILURE(status))
+ return -ENODEV;
- return sprintf(buf, "%lu\n", acpi_dev->pnp.sun);
+ return sprintf(buf, "%llu\n", sun);
}
static DEVICE_ATTR(sun, 0444, acpi_device_sun_show, NULL);
@@ -690,7 +696,6 @@ static int acpi_device_setup_files(struct acpi_device *dev)
{
struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
acpi_status status;
- unsigned long long sun;
int result = 0;
/*
@@ -731,14 +736,10 @@ static int acpi_device_setup_files(struct acpi_device *dev)
if (dev->pnp.unique_id)
result = device_create_file(&dev->dev, &dev_attr_uid);
- status = acpi_evaluate_integer(dev->handle, "_SUN", NULL, &sun);
- if (ACPI_SUCCESS(status)) {
- dev->pnp.sun = (unsigned long)sun;
+ if (acpi_has_method(dev->handle, "_SUN")) {
result = device_create_file(&dev->dev, &dev_attr_sun);
if (result)
goto end;
- } else {
- dev->pnp.sun = (unsigned long)-1;
}
if (acpi_has_method(dev->handle, "_STA")) {
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index bcfd808..c1c9de1 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -246,7 +246,6 @@ struct acpi_device_pnp {
acpi_device_name device_name; /* Driver-determined */
acpi_device_class device_class; /* " */
union acpi_object *str_obj; /* unicode string for _STR method */
- unsigned long sun; /* _SUN */
};
#define acpi_device_bid(d) ((d)->pnp.bus_id)
--
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
--
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
