2022-01-28 22:36:58

by Won Chung

[permalink] [raw]
Subject: [PATCH v2] ACPI: device_sysfs: Add sysfs support for _PLD

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 <[email protected]>
---
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 <[email protected]>
+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


2022-01-31 10:01:30

by Rafael J. Wysocki

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

On Fri, Jan 28, 2022 at 12:45 AM Won Chung <[email protected]> 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 <[email protected]>
> ---
> 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 <[email protected]>

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
>

2022-01-31 11:31:04

by Won Chung

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

On Fri, Jan 28, 2022 at 4:04 AM Rafael J. Wysocki <[email protected]> wrote:
>
> On Fri, Jan 28, 2022 at 12:45 AM Won Chung <[email protected]> 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 <[email protected]>
> > ---
> > 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 <[email protected]>
>
> 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
> >