On Thu, Aug 14, 2014 at 02:15:22AM +0200, Rafael J. Wysocki wrote:
> On Thursday, August 14, 2014 06:05:24 AM Greg Kroah-Hartman wrote:
> > On Wed, Aug 13, 2014 at 07:15:32PM +0200, Samuel Ortiz wrote:
> > > From: Faouaz TENOUTIT <faouaz.tenoutit@xxxxxxxxx>
> > >
> > > This patch exports a sysfs directory with ACPI _PLD information:
> > >
> > > $ ls -l /sys/bus/acpi/devices/ACPI-DEV/pld
> > > -r--r--r-- root root 4096 2014-05-30 08:39 panel
> > > -r--r--r-- root root 4096 2014-05-30 08:39 revision
> > > -r--r--r-- root root 4096 2014-05-30 08:39 rotation
> > > -r--r--r-- root root 4096 2014-05-30 08:39 shape
> > >
> > > This information can be used by user applications to:
> > > - Determine which specific connector or device input mechanism may be used
> > > for a given task.
> > > - Describes which panel surface of the system's housing the device
> > > connection point resides on (Front, Back, ...)
> > >
> > > More information about these _PLD fields can be found here:
> > > Documentation/ABI/testing/sysfs-bus-acpi
> > >
> > > Signed-off-by: Faouaz TENOUTIT <faouaz.tenoutit@xxxxxxxxx>
> > > Acked-by: Samuel Ortiz <samuel.ortiz@xxxxxxxxx>
> > > Acked-by: Andrew J Ross <andrew.j.ross@xxxxxxxxx>
> > > ---
> > > Documentation/ABI/testing/sysfs-bus-acpi | 60 ++++++++++++++++++++++++++++++++
> > > drivers/acpi/scan.c | 53 ++++++++++++++++++++++++----
> > > 2 files changed, 107 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/Documentation/ABI/testing/sysfs-bus-acpi b/Documentation/ABI/testing/sysfs-bus-acpi
> > > index 7fa9cbc..be58159 100644
> > > --- a/Documentation/ABI/testing/sysfs-bus-acpi
> > > +++ b/Documentation/ABI/testing/sysfs-bus-acpi
> > > @@ -56,3 +56,63 @@ Description:
> > > Writing 1 to this attribute will trigger hot removal of
> > > this device object. This file exists for every device
> > > object that has _EJ0 method.
> > > +
> > > +What: /sys/bus/acpi/devices/.../pld
> > > +Date: August 2014
> > > +Contact: Faouaz Tenoutit <faouaz.tenoutit@xxxxxxxxx>
> > > +Description:
> > > + This optional directory provides description of the physical
> > > + location, orientation and position of external devices.
> > > + It is for example used to describe how camera sensors are
> > > + physically mounted and allows user space software to rotate
> > > + images accordingly.
> > > +
> > > +What: /sys/bus/acpi/devices/.../pld/revision
> > > +Date: August 2014
> > > +Contact: Faouaz Tenoutit <faouaz.tenoutit@xxxxxxxxx>
> > > +Description:
> > > + The current Revision is 0x2
> > > +
> > > +What: /sys/bus/acpi/devices/.../pld/panel
> > > +Date: August 2014
> > > +Contact: Faouaz Tenoutit <faouaz.tenoutit@xxxxxxxxx>
> > > +Description:
> > > + Describes which panel surface of the system's housing the device
> > > + connection point resides on:
> > > + 0 Top
> > > + 1 Bottom
> > > + 2 Left
> > > + 3 Right
> > > + 4 Front
> > > + 5 Back
> > > + 6 Unknown
> > > +
> > > +What: /sys/bus/acpi/devices/.../pld/shape
> > > +Date: August 2014
> > > +Contact: Faouaz Tenoutit <faouaz.tenoutit@xxxxxxxxx>
> > > +Description:
> > > + Describes the shape of the device connection point:
> > > + 0 Round
> > > + 1 Oval
> > > + 2 Square
> > > + 3 Vertical Rectangle
> > > + 4 Horizontal Rectangle
> > > + 5 Vertical Trapezoid
> > > + 6 Horizontal Trapezoid
> > > + 7 Unknown - Shape rendered as a Rectangle with dotted lines
> > > + 8 Chamfered
> > > +
> > > +What: /sys/bus/acpi/devices/.../pld/rotation
> > > +Date: August 2014
> > > +Contact: Faouaz Tenoutit <faouaz.tenoutit@xxxxxxxxx>
> > > +Description:
> > > + Rotates the Shape clockwise in 45 degree steps around its
> > > + origin where:
> > > + 0 0°
> > > + 1 45°
> > > + 2 90°
> > > + 3 135°
> > > + 4 180°
> > > + 5 225°
> > > + 6 270°
> > > + 7 315°
> > > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> > > index 2ca42d5..19889a3 100644
> > > --- a/drivers/acpi/scan.c
> > > +++ b/drivers/acpi/scan.c
> > > @@ -686,6 +686,36 @@ static ssize_t status_show(struct device *dev, struct device_attribute *attr,
> > > }
> > > static DEVICE_ATTR_RO(status);
> > >
> > > +#define ACPI_SYSFS_PLD_PROP(prop) \
> > > + static ssize_t prop##_show(struct device *dev, \
> > > + struct device_attribute *attr, \
> > > + char *buf) { \
> > > + struct acpi_device *acpi_dev = to_acpi_device(dev); \
> > > + return sprintf(buf, "%d\n", acpi_dev->pld->prop); \
> > > +}; \
> > > +static DEVICE_ATTR_RO(prop)
> > > +
> > > +/*
> > > + * sysfs PLD parameters
> > > + */
> > > +ACPI_SYSFS_PLD_PROP(revision);
> > > +ACPI_SYSFS_PLD_PROP(panel);
> > > +ACPI_SYSFS_PLD_PROP(shape);
> > > +ACPI_SYSFS_PLD_PROP(rotation);
> > > +
> > > +static struct attribute *acpi_pld_attrs[] = {
> > > + &dev_attr_revision.attr,
> > > + &dev_attr_panel.attr,
> > > + &dev_attr_shape.attr,
> > > + &dev_attr_rotation.attr,
> > > + NULL,
> > > +};
> > > +
> > > +static const struct attribute_group acpi_pld_attr_group = {
> > > + .name = "pld",
> > > + .attrs = acpi_pld_attrs,
> > > +};
> > > +
> > > static int acpi_device_setup_files(struct acpi_device *dev)
> > > {
> > > struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
> > > @@ -768,11 +798,20 @@ static int acpi_device_setup_files(struct acpi_device *dev)
> > > }
> > >
> > > /*
> > > - * If device has _PLD, initialize the 'pld' struct
> > > + * If device has _PLD, 'pld' directory is created
> > > */
> > > - if (acpi_has_method(dev->handle, "_PLD"))
> > > - acpi_get_physical_device_location(dev->handle,
> > > - &dev->pld);
> > > + if (acpi_has_method(dev->handle, "_PLD")) {
> > > + status = acpi_get_physical_device_location(dev->handle,
> > > + &dev->pld);
> > > + if (ACPI_SUCCESS(status)) {
> > > + result = sysfs_create_group(&dev->dev.kobj,
> > > + &acpi_pld_attr_group);
> >
> > It's a device, why not call device_create_groups()?
> >
> >
> > > + if (result) {
> > > + ACPI_FREE(dev->pld);
> > > + dev->pld = NULL;
> > > + }
> > > + }
> > > + }
> >
> > Is this being called after or before the device in question is
> > registered with the driver core?
> >
> > Hint, I think I know the answer, but I want you to learn why this code
> > is wrong and will not work properly...
>
> It will work. There are other groups created in the same function and ACPI has
> a deferred KOBJ_ADD notification.
Ah, good, nevermind then.
Well, I'm still curious about the sysfs_create_group() call, please try
to use device_* functions when dealing with a struct device, if at all
possible.
thanks,
greg k-h
--
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
