diff mbox series

[v3] ACPI: device_sysfs: Add sysfs support for _PLD

Message ID 20220128180832.2329149-1-wonchung@google.com
State Superseded
Headers show
Series [v3] ACPI: device_sysfs: Add sysfs support for _PLD | expand

Commit Message

Won Chung Jan. 28, 2022, 6:08 p.m. UTC 
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@google.com>
---
 Documentation/ABI/testing/sysfs-bus-acpi | 53 ++++++++++++++++++++++++
 drivers/acpi/device_sysfs.c              | 42 +++++++++++++++++++
 2 files changed, 95 insertions(+)

Comments

Heikki Krogerus Jan. 31, 2022, 7:33 a.m. UTC | #1 
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@google.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@google.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,
Won Chung Feb. 1, 2022, 1:50 a.m. UTC | #2 
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@linux.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@google.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@google.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
Won Chung Feb. 1, 2022, 1:52 a.m. UTC | #3 
On Sun, Jan 30, 2022 at 11:33 PM Heikki Krogerus
<heikki.krogerus@linux.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@google.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@google.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

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
diff mbox series

Patch

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@google.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);
+
 /**
  * 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);
 }