On Tuesday, January 22, 2013 03:47:18 PM Greg Kroah-Hartman wrote:
> On Tue, Jan 22, 2013 at 03:25:15AM +0100, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> >
> > Make it possible to retrieve the current power state of a device with
> > ACPI power management from user space via sysfs by adding two new
> > attributes, power_state and real_power_state, to the sysfs directory
> > associated with the struct acpi_device object representing the
> > device's ACPI node.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> > ---
> > Documentation/ABI/testing/sysfs-devices-power_state | 20 ++++++
> > Documentation/ABI/testing/sysfs-devices-real_power_state | 23 +++++++
> > drivers/acpi/scan.c | 49 ++++++++++++++-
> > 3 files changed, 91 insertions(+), 1 deletion(-)
> >
> > Index: linux-pm/drivers/acpi/scan.c
> > ===================================================================
> > --- linux-pm.orig/drivers/acpi/scan.c
> > +++ linux-pm/drivers/acpi/scan.c
> > @@ -178,6 +178,32 @@ err_out:
> > }
> > EXPORT_SYMBOL(acpi_bus_hot_remove_device);
> >
> > +static ssize_t real_power_state_show(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct acpi_device *adev = to_acpi_device(dev);
> > + int state;
> > + int ret;
> > +
> > + ret = acpi_device_get_power(adev, &state);
> > + if (ret)
> > + return ret;
> > +
> > + return sprintf(buf, "%s\n", acpi_power_state_string(state));
> > +}
> > +
> > +static DEVICE_ATTR(real_power_state, 0444, real_power_state_show, NULL);
> > +
> > +static ssize_t power_state_show(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct acpi_device *adev = to_acpi_device(dev);
> > +
> > + return sprintf(buf, "%s\n", acpi_power_state_string(adev->power.state));
> > +}
> > +
> > +static DEVICE_ATTR(power_state, 0444, power_state_show, NULL);
> > +
> > static ssize_t
> > acpi_eject_store(struct device *d, struct device_attribute *attr,
> > const char *buf, size_t count)
> > @@ -369,8 +395,22 @@ static int acpi_device_setup_files(struc
> > * hot-removal function from userland.
> > */
> > status = acpi_get_handle(dev->handle, "_EJ0", &temp);
> > - if (ACPI_SUCCESS(status))
> > + if (ACPI_SUCCESS(status)) {
> > result = device_create_file(&dev->dev, &dev_attr_eject);
> > + if (result)
> > + return result;
> > + }
> > +
> > + if (dev->flags.power_manageable) {
> > + result = device_create_file(&dev->dev, &dev_attr_power_state);
> > + if (result)
> > + return result;
> > +
> > + if (dev->power.flags.power_resources)
> > + result = device_create_file(&dev->dev,
> > + &dev_attr_real_power_state);
> > + }
> > +
>
> It isn't your fault, but I just noticed this when reading patch 2/4, but
> I think you are racing userspace here. The device is registered,
> userspace is notified that the device is present, and _then_ the acpi
> core creates the sysfs files for the device.
>
> That's not good, and needs to be fixed, especially now that you are
> adding more and more sysfs files for these ACPI devices.
>
> To do that, you need to set up "default" files for ACPI devices, they
> can be enabled or not by the driver core, and will be created before the
> event that the device is created is sent to userspace.
>
> Care to fix that all up in a follow-on patchset before some tools start
> to get confused?
I will.
That's kind of has been happening for quite a long time, though, as far as
I can say, so confusion would have ensued already before. Not that I think
it's not worth fixing, to be sure. :-)
So acpi_device_setup_files() is called too late already, right?
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
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
