lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 31 Jan 2022 17:50:27 -0800
From:   Won Chung <wonchung@...gle.com>
To:     Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Cc:     "Rafael J . Wysocki" <rafael@...nel.org>,
        Len Brown <lenb@...nel.org>,
        Benson Leung <bleung@...omium.org>,
        Prashant Malani <pmalani@...omium.org>,
        linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] ACPI: device_sysfs: Add sysfs support for _PLD

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@...ux.intel.com> 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@...gle.com>
> > ---
> >  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@...gle.com>
> > +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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ