Re: [PATCH v2] ACPI: device_sysfs: Add sysfs support for _PLD

[Won Chung]

On Fri, Jan 28, 2022 at 4:04 AM Rafael J. Wysocki <rafael@xxxxxxxxxx> wrote:
>
> On Fri, Jan 28, 2022 at 12:45 AM Won Chung <wonchung@xxxxxxxxxx> wrote:
> >
> > When ACPI table includes _PLD fields for a device, create a new file
> > (pld) in sysfs to share _PLD fields.
> >
> > Signed-off-by: Won Chung <wonchung@xxxxxxxxxx>
> > ---
> >  Documentation/ABI/testing/sysfs-bus-acpi | 53 ++++++++++++++++++++++++
> >  drivers/acpi/device_sysfs.c              | 42 +++++++++++++++++++
> >  2 files changed, 95 insertions(+)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-bus-acpi b/Documentation/ABI/testing/sysfs-bus-acpi
> > index 58abacf59b2a..7f4544c9d563 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-acpi
> > +++ b/Documentation/ABI/testing/sysfs-bus-acpi
> > @@ -96,3 +96,56 @@ Description:
> >                 hardware, if the _HRV control method is present.  It is mostly
> >                 useful for non-PCI devices because lspci can list the hardware
> >                 version for PCI devices.
> > +
> > +What:          /sys/bus/acpi/devices/.../pld
> > +Date:          Jan, 2022
> > +Contact:       Rafael J. Wysocki <rjw@xxxxxxxxxxxxx>
>
> Why do you think that it is appropriate to add contact information of
> someone to ABI documentation without asking them for their opinion?
>
> If you don't want to be the contact yourself, you can put linux-acpi in there.

Hi Rafael,

Sorry for that. I thought I had to put the maintainer's contact there
since everything else in this file has your name and it's being sent
to you for a review. I actually sent another follow up email in this
email thread with the contact information changed to my name, but I
think I also did that incorrectly since I did not change the version
number in the email title. I will correctly send a new one.

Won

>
> > +Description:
> > +               This attribute contains the output of the device object's
> > +               _PLD control method, if present. This information provides
> > +               details on physical location of a port.
> > +
> > +               Description on each _PLD field from ACPI specification:
> > +
> > +               =============== ============================================
> > +               GROUP_TOKEN     Unique numerical value identifying a group.
> > +               GROUP_POSITION  Identifies this device connection point’s
> > +                               position in the group.
> > +               USER_VISIBLE    Set if the device connection point can be
> > +                               seen by the user without disassembly.
> > +               DOCK            Set if the device connection point resides
> > +                               in a docking station or port replicator.
> > +               BAY             Set if describing a device in a bay or if
> > +                               device connection point is a bay.
> > +               LID             Set if this device connection point resides
> > +                               on the lid of laptop system.
> > +               PANEL           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 (Vertical Position and Horizontal
> > +                               Position will be ignored)
> > +               HORIZONTAL_     0 - Left
> > +               POSITION        1 - Center
> > +                               2 - Right
> > +               VERTICAL_       0 - Upper
> > +               POSITION        1 - Center
> > +                               2 - Lower
> > +               SHAPE           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
> > +                               15:9 - Reserved
> > +               =============== ===============================================
> > diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
> > index d5d6403ba07b..8d4df5fb1c45 100644
> > --- a/drivers/acpi/device_sysfs.c
> > +++ b/drivers/acpi/device_sysfs.c
> > @@ -509,6 +509,40 @@ static ssize_t status_show(struct device *dev, struct device_attribute *attr,
> >  }
> >  static DEVICE_ATTR_RO(status);
> >
> > +static ssize_t pld_show(struct device *dev, struct device_attribute *attr,
> > +                       char *buf)
> > +{
> > +       struct acpi_device *acpi_dev = to_acpi_device(dev);
> > +       acpi_status status;
> > +       struct acpi_pld_info *pld;
> > +
> > +       status = acpi_get_physical_device_location(acpi_dev->handle, &pld);
> > +       if (ACPI_FAILURE(status))
> > +               return -ENODEV;
> > +
> > +       return sprintf(buf, "GROUP_TOKEN=%u\n"
> > +               "GROUP_POSITION=%u\n"
> > +               "USER_VISIBLE=%u\n"
> > +               "DOCK=%u\n"
> > +               "BAY=%u\n"
> > +               "LID=%u\n"
> > +               "PANEL=%u\n"
> > +               "HORIZONTAL_POSITION=%u\n"
> > +               "VERTICAL_POSITION=%u\n"
> > +               "SHAPE=%u\n",
> > +               pld->group_token,
> > +               pld->group_position,
> > +               pld->user_visible,
> > +               pld->dock,
> > +               pld->bay,
> > +               pld->lid,
> > +               pld->panel,
> > +               pld->horizontal_position,
> > +               pld->vertical_position,
> > +               pld->shape);
> > +}
> > +static DEVICE_ATTR_RO(pld);
> > +
> >  /**
> >   * acpi_device_setup_files - Create sysfs attributes of an ACPI device.
> >   * @dev: ACPI device object.
> > @@ -595,6 +629,12 @@ int acpi_device_setup_files(struct acpi_device *dev)
> >                                                     &dev_attr_real_power_state);
> >         }
> >
> > +       if (acpi_has_method(dev->handle, "_PLD")) {
> > +               result = device_create_file(&dev->dev, &dev_attr_pld);
> > +               if (result)
> > +                       goto end;
> > +       }
> > +
> >         acpi_expose_nondev_subnodes(&dev->dev.kobj, &dev->data);
> >
> >  end:
> > @@ -645,4 +685,6 @@ void acpi_device_remove_files(struct acpi_device *dev)
> >                 device_remove_file(&dev->dev, &dev_attr_status);
> >         if (dev->handle)
> >                 device_remove_file(&dev->dev, &dev_attr_path);
> > +       if (acpi_has_method(dev->handle, "_PLD"))
> > +               device_remove_file(&dev->dev, &dev_attr_pld);
> >  }
> > --
> > 2.35.0.rc0.227.g00780c9af4-goog
> >