Hi Heikki,
Thank you for the review. It seems to be the convention to have a
separate attribute file for each field, as you pointed out. I have
made the change and sent v4.
Thanks,
Won
On Sun, Jan 30, 2022 at 11:33 PM Heikki Krogerus
<heikki.krogerus@xxxxxxxxxxxxxxx> wrote:
>
> On Fri, Jan 28, 2022 at 06:08:32PM +0000, Won Chung 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..3a9c6a4f4603 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: Won Chung <wonchung@xxxxxxxxxx>
> > +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);
>
> Why not have a pld group (directory) and a separate attribute file for
> each field?
>
>
> thanks,
>
> --
> heikki
