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

[Rafael J. Wysocki]

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.

> +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
>