When ACPI table includes _PLD fields for a device, create a new
directory (physical_location) in sysfs to share _PLD fields.
Currently without PLD information, when there are multiple of same
devices, it is hard to distinguish which device corresponds to which
physical device at which location. For example, when there are two Type
C connectors, it is hard to find out which connector corresponds to the
Type C port on the left panel versus the Type C port on the right panel.
With PLD information provided, we can determine which specific device at
which location is doing what.
_PLD output includes much more fields, but only generic fields are added
and exposed to sysfs, so that non-ACPI devices can also support it in
the future. The minimal generic fields needed for locating a device are
the following.
- panel
- vertical_position
- horizontal_position
- dock
- lid
Signed-off-by: Won Chung <[email protected]>
---
Changes from v5
- Remove physical_location directory only if physical_location is present.
- Free memory for physical_location in device when device is removed.
Changes from v4
- Remove physical_location directory when device is deleted.
- Correctly handle error from adding physical_location in
device_add_attrs().
Changes from v3
- Move dev_add_physical_location() and dev_attr_physical_location_group
to driver/base/physical_location.h.
- Use pointer and reorder physical_location in struct device to pack its
bytes. (checked using pahole)
- Unify naming to physical_location since the name location is used in
some places like USB port for different information.
Changes from v2
- Use sysfs_emit to create files.
- Correct mix of spaces and tabs.
Changes from v1
- Correct directory names in Documentation.
- Clarify namings in core.c
.../testing/sysfs-devices-physical_location | 42 ++++++
drivers/base/Makefile | 1 +
drivers/base/core.c | 15 ++
drivers/base/physical_location.c | 137 ++++++++++++++++++
drivers/base/physical_location.h | 16 ++
include/linux/device.h | 73 ++++++++++
6 files changed, 284 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-devices-physical_location
create mode 100644 drivers/base/physical_location.c
create mode 100644 drivers/base/physical_location.h
diff --git a/Documentation/ABI/testing/sysfs-devices-physical_location b/Documentation/ABI/testing/sysfs-devices-physical_location
new file mode 100644
index 000000000000..202324b87083
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-devices-physical_location
@@ -0,0 +1,42 @@
+What: /sys/devices/.../physical_location
+Date: March 2022
+Contact: Won Chung <[email protected]>
+Description:
+ This directory contains information on physical location of
+ the device connection point with respect to the system's
+ housing.
+
+What: /sys/devices/.../physical_location/panel
+Date: March 2022
+Contact: Won Chung <[email protected]>
+Description:
+ Describes which panel surface of the system’s housing the
+ device connection point resides on.
+
+What: /sys/devices/.../physical_location/vertical_position
+Date: March 2022
+Contact: Won Chung <[email protected]>
+Description:
+ Describes vertical position of the device connection point on
+ the panel surface.
+
+What: /sys/devices/.../physical_location/horizontal_position
+Date: March 2022
+Contact: Won Chung <[email protected]>
+Description:
+ Describes horizontal position of the device connection point on
+ the panel surface.
+
+What: /sys/devices/.../physical_location/dock
+Date: March 2022
+Contact: Won Chung <[email protected]>
+Description:
+ "Yes" if the device connection point resides in a docking
+ station or a port replicator. "No" otherwise.
+
+What: /sys/devices/.../physical_location/lid
+Date: March 2022
+Contact: Won Chung <[email protected]>
+Description:
+ "Yes" if the device connection point resides on the lid of
+ laptop system. "No" otherwise.
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 02f7f1358e86..83217d243c25 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_DEV_COREDUMP) += devcoredump.o
obj-$(CONFIG_GENERIC_MSI_IRQ_DOMAIN) += platform-msi.o
obj-$(CONFIG_GENERIC_ARCH_TOPOLOGY) += arch_topology.o
obj-$(CONFIG_GENERIC_ARCH_NUMA) += arch_numa.o
+obj-$(CONFIG_ACPI) += physical_location.o
obj-y += test/
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 7bb957b11861..64d4d8646c12 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -32,6 +32,7 @@
#include <linux/dma-map-ops.h> /* for dma_default_coherent */
#include "base.h"
+#include "physical_location.h"
#include "power/power.h"
#ifdef CONFIG_SYSFS_DEPRECATED
@@ -2649,8 +2650,17 @@ static int device_add_attrs(struct device *dev)
goto err_remove_dev_waiting_for_supplier;
}
+ if (dev_add_physical_location(dev)) {
+ error = device_add_group(dev,
+ &dev_attr_physical_location_group);
+ if (error)
+ goto err_remove_dev_removable;
+ }
+
return 0;
+ err_remove_dev_removable:
+ device_remove_file(dev, &dev_attr_removable);
err_remove_dev_waiting_for_supplier:
device_remove_file(dev, &dev_attr_waiting_for_supplier);
err_remove_dev_online:
@@ -2672,6 +2682,11 @@ static void device_remove_attrs(struct device *dev)
struct class *class = dev->class;
const struct device_type *type = dev->type;
+ if (dev->physical_location) {
+ device_remove_group(dev, &dev_attr_physical_location_group);
+ kfree(dev->physical_location);
+ }
+
device_remove_file(dev, &dev_attr_removable);
device_remove_file(dev, &dev_attr_waiting_for_supplier);
device_remove_file(dev, &dev_attr_online);
diff --git a/drivers/base/physical_location.c b/drivers/base/physical_location.c
new file mode 100644
index 000000000000..4c1a52ecd7f6
--- /dev/null
+++ b/drivers/base/physical_location.c
@@ -0,0 +1,137 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Device physical location support
+ *
+ * Author: Won Chung <[email protected]>
+ */
+
+#include <linux/acpi.h>
+#include <linux/sysfs.h>
+
+#include "physical_location.h"
+
+bool dev_add_physical_location(struct device *dev)
+{
+ struct acpi_pld_info *pld;
+ acpi_status status;
+
+ if (!has_acpi_companion(dev))
+ return false;
+
+ status = acpi_get_physical_device_location(ACPI_HANDLE(dev), &pld);
+ if (ACPI_FAILURE(status))
+ return false;
+
+ dev->physical_location =
+ kzalloc(sizeof(*dev->physical_location), GFP_KERNEL);
+ dev->physical_location->panel = pld->panel;
+ dev->physical_location->vertical_position = pld->vertical_position;
+ dev->physical_location->horizontal_position = pld->horizontal_position;
+ dev->physical_location->dock = pld->dock;
+ dev->physical_location->lid = pld->lid;
+
+ return true;
+}
+
+static ssize_t panel_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ const char *panel;
+
+ switch (dev->physical_location->panel) {
+ case DEVICE_PANEL_TOP:
+ panel = "top";
+ break;
+ case DEVICE_PANEL_BOTTOM:
+ panel = "bottom";
+ break;
+ case DEVICE_PANEL_LEFT:
+ panel = "left";
+ break;
+ case DEVICE_PANEL_RIGHT:
+ panel = "right";
+ break;
+ case DEVICE_PANEL_FRONT:
+ panel = "front";
+ break;
+ default:
+ panel = "unknown";
+ }
+ return sysfs_emit(buf, "%s\n", panel);
+}
+static DEVICE_ATTR_RO(panel);
+
+static ssize_t vertical_position_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ const char *vertical_position;
+
+ switch (dev->physical_location->vertical_position) {
+ case DEVICE_VERT_POS_UPPER:
+ vertical_position = "upper";
+ break;
+ case DEVICE_VERT_POS_CENTER:
+ vertical_position = "center";
+ break;
+ case DEVICE_VERT_POS_LOWER:
+ vertical_position = "lower";
+ break;
+ default:
+ vertical_position = "unknown";
+ }
+ return sysfs_emit(buf, "%s\n", vertical_position);
+}
+static DEVICE_ATTR_RO(vertical_position);
+
+static ssize_t horizontal_position_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ const char *horizontal_position;
+
+ switch (dev->physical_location->horizontal_position) {
+ case DEVICE_HORI_POS_LEFT:
+ horizontal_position = "left";
+ break;
+ case DEVICE_HORI_POS_CENTER:
+ horizontal_position = "center";
+ break;
+ case DEVICE_HORI_POS_RIGHT:
+ horizontal_position = "right";
+ break;
+ default:
+ horizontal_position = "unknown";
+ }
+ return sysfs_emit(buf, "%s\n", horizontal_position);
+}
+static DEVICE_ATTR_RO(horizontal_position);
+
+static ssize_t dock_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ return sysfs_emit(buf, "%s\n",
+ dev->physical_location->dock ? "yes" : "no");
+}
+static DEVICE_ATTR_RO(dock);
+
+static ssize_t lid_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ return sysfs_emit(buf, "%s\n",
+ dev->physical_location->lid ? "yes" : "no");
+}
+static DEVICE_ATTR_RO(lid);
+
+static struct attribute *dev_attr_physical_location[] = {
+ &dev_attr_panel.attr,
+ &dev_attr_vertical_position.attr,
+ &dev_attr_horizontal_position.attr,
+ &dev_attr_dock.attr,
+ &dev_attr_lid.attr,
+ NULL,
+};
+
+const struct attribute_group dev_attr_physical_location_group = {
+ .name = "physical_location",
+ .attrs = dev_attr_physical_location,
+};
+
diff --git a/drivers/base/physical_location.h b/drivers/base/physical_location.h
new file mode 100644
index 000000000000..82cde9f1b161
--- /dev/null
+++ b/drivers/base/physical_location.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Device physical location support
+ *
+ * Author: Won Chung <[email protected]>
+ */
+
+#include <linux/device.h>
+
+#ifdef CONFIG_ACPI
+extern bool dev_add_physical_location(struct device *dev);
+extern const struct attribute_group dev_attr_physical_location_group;
+#else
+static inline bool dev_add_physical_location(struct device *dev) { return false; };
+static const struct attribute_group dev_attr_physical_location_group = {};
+#endif
diff --git a/include/linux/device.h b/include/linux/device.h
index 93459724dcde..766fbea6ca83 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -386,6 +386,75 @@ struct dev_msi_info {
#endif
};
+/**
+ * enum device_physical_location_panel - Describes which panel surface of the
+ * system's housing the device connection point resides on.
+ * @DEVICE_PANEL_TOP: Device connection point is on the top panel.
+ * @DEVICE_PANEL_BOTTOM: Device connection point is on the bottom panel.
+ * @DEVICE_PANEL_LEFT: Device connection point is on the left panel.
+ * @DEVICE_PANEL_RIGHT: Device connection point is on the right panel.
+ * @DEVICE_PANEL_FRONT: Device connection point is on the front panel.
+ * @DEVICE_PANEL_BACK: Device connection point is on the back panel.
+ * @DEVICE_PANEL_UNKNOWN: The panel with device connection point is unknown.
+ */
+enum device_physical_location_panel {
+ DEVICE_PANEL_TOP,
+ DEVICE_PANEL_BOTTOM,
+ DEVICE_PANEL_LEFT,
+ DEVICE_PANEL_RIGHT,
+ DEVICE_PANEL_FRONT,
+ DEVICE_PANEL_BACK,
+ DEVICE_PANEL_UNKNOWN,
+};
+
+/**
+ * enum device_physical_location_vertical_position - Describes vertical
+ * position of the device connection point on the panel surface.
+ * @DEVICE_VERT_POS_UPPER: Device connection point is at upper part of panel.
+ * @DEVICE_VERT_POS_CENTER: Device connection point is at center part of panel.
+ * @DEVICE_VERT_POS_LOWER: Device connection point is at lower part of panel.
+ */
+enum device_physical_location_vertical_position {
+ DEVICE_VERT_POS_UPPER,
+ DEVICE_VERT_POS_CENTER,
+ DEVICE_VERT_POS_LOWER,
+};
+
+/**
+ * enum device_physical_location_horizontal_position - Describes horizontal
+ * position of the device connection point on the panel surface.
+ * @DEVICE_HORI_POS_LEFT: Device connection point is at left part of panel.
+ * @DEVICE_HORI_POS_CENTER: Device connection point is at center part of panel.
+ * @DEVICE_HORI_POS_RIGHT: Device connection point is at right part of panel.
+ */
+enum device_physical_location_horizontal_position {
+ DEVICE_HORI_POS_LEFT,
+ DEVICE_HORI_POS_CENTER,
+ DEVICE_HORI_POS_RIGHT,
+};
+
+/**
+ * struct device_physical_location - Device data related to physical location
+ * of the device connection point.
+ * @panel: Panel surface of the system's housing that the device connection
+ * point resides on.
+ * @vertical_position: Vertical position of the device connection point within
+ * the panel.
+ * @horizontal_position: Horizontal position of the device connection point
+ * within the panel.
+ * @dock: Set if the device connection point resides in a docking station or
+ * port replicator.
+ * @lid: Set if this device connection point resides on the lid of laptop
+ * system.
+ */
+struct device_physical_location {
+ enum device_physical_location_panel panel;
+ enum device_physical_location_vertical_position vertical_position;
+ enum device_physical_location_horizontal_position horizontal_position;
+ bool dock;
+ bool lid;
+};
+
/**
* struct device - The basic device structure
* @parent: The device's "parent" device, the device to which it is attached.
@@ -453,6 +522,8 @@ struct dev_msi_info {
* device (i.e. the bus driver that discovered the device).
* @iommu_group: IOMMU group the device belongs to.
* @iommu: Per device generic IOMMU runtime data
+ * @physical_location: Describes physical location of the device connection
+ * point in the system housing.
* @removable: Whether the device can be removed from the system. This
* should be set by the subsystem / bus driver that discovered
* the device.
@@ -567,6 +638,8 @@ struct device {
struct iommu_group *iommu_group;
struct dev_iommu *iommu;
+ struct device_physical_location *physical_location;
+
enum device_removable removable;
bool offline_disabled:1;
--
2.35.1.723.g4982287a31-goog
On Mon, Mar 14, 2022 at 12:55 PM Won Chung <[email protected]> wrote:
>
> When ACPI table includes _PLD fields for a device, create a new
> directory (physical_location) in sysfs to share _PLD fields.
>
> Currently without PLD information, when there are multiple of same
> devices, it is hard to distinguish which device corresponds to which
> physical device at which location. For example, when there are two Type
> C connectors, it is hard to find out which connector corresponds to the
> Type C port on the left panel versus the Type C port on the right panel.
> With PLD information provided, we can determine which specific device at
> which location is doing what.
>
> _PLD output includes much more fields, but only generic fields are added
> and exposed to sysfs, so that non-ACPI devices can also support it in
> the future. The minimal generic fields needed for locating a device are
> the following.
> - panel
> - vertical_position
> - horizontal_position
> - dock
> - lid
>
> Signed-off-by: Won Chung <[email protected]>
> ---
>
> Changes from v5
> - Remove physical_location directory only if physical_location is present.
> - Free memory for physical_location in device when device is removed.
>
> Changes from v4
> - Remove physical_location directory when device is deleted.
> - Correctly handle error from adding physical_location in
> device_add_attrs().
>
> Changes from v3
> - Move dev_add_physical_location() and dev_attr_physical_location_group
> to driver/base/physical_location.h.
> - Use pointer and reorder physical_location in struct device to pack its
> bytes. (checked using pahole)
> - Unify naming to physical_location since the name location is used in
> some places like USB port for different information.
>
> Changes from v2
> - Use sysfs_emit to create files.
> - Correct mix of spaces and tabs.
>
> Changes from v1
> - Correct directory names in Documentation.
> - Clarify namings in core.c
>
> .../testing/sysfs-devices-physical_location | 42 ++++++
> drivers/base/Makefile | 1 +
> drivers/base/core.c | 15 ++
> drivers/base/physical_location.c | 137 ++++++++++++++++++
> drivers/base/physical_location.h | 16 ++
> include/linux/device.h | 73 ++++++++++
> 6 files changed, 284 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-devices-physical_location
> create mode 100644 drivers/base/physical_location.c
> create mode 100644 drivers/base/physical_location.h
>
> diff --git a/Documentation/ABI/testing/sysfs-devices-physical_location b/Documentation/ABI/testing/sysfs-devices-physical_location
> new file mode 100644
> index 000000000000..202324b87083
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-devices-physical_location
> @@ -0,0 +1,42 @@
> +What: /sys/devices/.../physical_location
> +Date: March 2022
> +Contact: Won Chung <[email protected]>
> +Description:
> + This directory contains information on physical location of
> + the device connection point with respect to the system's
> + housing.
> +
> +What: /sys/devices/.../physical_location/panel
> +Date: March 2022
> +Contact: Won Chung <[email protected]>
> +Description:
> + Describes which panel surface of the system’s housing the
> + device connection point resides on.
> +
> +What: /sys/devices/.../physical_location/vertical_position
> +Date: March 2022
> +Contact: Won Chung <[email protected]>
> +Description:
> + Describes vertical position of the device connection point on
> + the panel surface.
> +
> +What: /sys/devices/.../physical_location/horizontal_position
> +Date: March 2022
> +Contact: Won Chung <[email protected]>
> +Description:
> + Describes horizontal position of the device connection point on
> + the panel surface.
> +
> +What: /sys/devices/.../physical_location/dock
> +Date: March 2022
> +Contact: Won Chung <[email protected]>
> +Description:
> + "Yes" if the device connection point resides in a docking
> + station or a port replicator. "No" otherwise.
> +
> +What: /sys/devices/.../physical_location/lid
> +Date: March 2022
> +Contact: Won Chung <[email protected]>
> +Description:
> + "Yes" if the device connection point resides on the lid of
> + laptop system. "No" otherwise.
> diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> index 02f7f1358e86..83217d243c25 100644
> --- a/drivers/base/Makefile
> +++ b/drivers/base/Makefile
> @@ -25,6 +25,7 @@ obj-$(CONFIG_DEV_COREDUMP) += devcoredump.o
> obj-$(CONFIG_GENERIC_MSI_IRQ_DOMAIN) += platform-msi.o
> obj-$(CONFIG_GENERIC_ARCH_TOPOLOGY) += arch_topology.o
> obj-$(CONFIG_GENERIC_ARCH_NUMA) += arch_numa.o
> +obj-$(CONFIG_ACPI) += physical_location.o
>
> obj-y += test/
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 7bb957b11861..64d4d8646c12 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -32,6 +32,7 @@
> #include <linux/dma-map-ops.h> /* for dma_default_coherent */
>
> #include "base.h"
> +#include "physical_location.h"
> #include "power/power.h"
>
> #ifdef CONFIG_SYSFS_DEPRECATED
> @@ -2649,8 +2650,17 @@ static int device_add_attrs(struct device *dev)
> goto err_remove_dev_waiting_for_supplier;
> }
>
> + if (dev_add_physical_location(dev)) {
> + error = device_add_group(dev,
> + &dev_attr_physical_location_group);
> + if (error)
> + goto err_remove_dev_removable;
> + }
> +
> return 0;
>
> + err_remove_dev_removable:
> + device_remove_file(dev, &dev_attr_removable);
> err_remove_dev_waiting_for_supplier:
> device_remove_file(dev, &dev_attr_waiting_for_supplier);
> err_remove_dev_online:
> @@ -2672,6 +2682,11 @@ static void device_remove_attrs(struct device *dev)
> struct class *class = dev->class;
> const struct device_type *type = dev->type;
>
> + if (dev->physical_location) {
> + device_remove_group(dev, &dev_attr_physical_location_group);
> + kfree(dev->physical_location);
> + }
> +
> device_remove_file(dev, &dev_attr_removable);
> device_remove_file(dev, &dev_attr_waiting_for_supplier);
> device_remove_file(dev, &dev_attr_online);
> diff --git a/drivers/base/physical_location.c b/drivers/base/physical_location.c
> new file mode 100644
> index 000000000000..4c1a52ecd7f6
> --- /dev/null
> +++ b/drivers/base/physical_location.c
> @@ -0,0 +1,137 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Device physical location support
> + *
> + * Author: Won Chung <[email protected]>
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/sysfs.h>
> +
> +#include "physical_location.h"
> +
> +bool dev_add_physical_location(struct device *dev)
> +{
> + struct acpi_pld_info *pld;
> + acpi_status status;
> +
> + if (!has_acpi_companion(dev))
> + return false;
> +
> + status = acpi_get_physical_device_location(ACPI_HANDLE(dev), &pld);
> + if (ACPI_FAILURE(status))
> + return false;
> +
> + dev->physical_location =
> + kzalloc(sizeof(*dev->physical_location), GFP_KERNEL);
> + dev->physical_location->panel = pld->panel;
> + dev->physical_location->vertical_position = pld->vertical_position;
> + dev->physical_location->horizontal_position = pld->horizontal_position;
> + dev->physical_location->dock = pld->dock;
> + dev->physical_location->lid = pld->lid;
> +
> + return true;
> +}
> +
> +static ssize_t panel_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + const char *panel;
> +
> + switch (dev->physical_location->panel) {
> + case DEVICE_PANEL_TOP:
> + panel = "top";
> + break;
> + case DEVICE_PANEL_BOTTOM:
> + panel = "bottom";
> + break;
> + case DEVICE_PANEL_LEFT:
> + panel = "left";
> + break;
> + case DEVICE_PANEL_RIGHT:
> + panel = "right";
> + break;
> + case DEVICE_PANEL_FRONT:
> + panel = "front";
> + break;
> + default:
> + panel = "unknown";
> + }
> + return sysfs_emit(buf, "%s\n", panel);
> +}
> +static DEVICE_ATTR_RO(panel);
> +
> +static ssize_t vertical_position_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + const char *vertical_position;
> +
> + switch (dev->physical_location->vertical_position) {
> + case DEVICE_VERT_POS_UPPER:
> + vertical_position = "upper";
> + break;
> + case DEVICE_VERT_POS_CENTER:
> + vertical_position = "center";
> + break;
> + case DEVICE_VERT_POS_LOWER:
> + vertical_position = "lower";
> + break;
> + default:
> + vertical_position = "unknown";
> + }
> + return sysfs_emit(buf, "%s\n", vertical_position);
> +}
> +static DEVICE_ATTR_RO(vertical_position);
> +
> +static ssize_t horizontal_position_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + const char *horizontal_position;
> +
> + switch (dev->physical_location->horizontal_position) {
> + case DEVICE_HORI_POS_LEFT:
> + horizontal_position = "left";
> + break;
> + case DEVICE_HORI_POS_CENTER:
> + horizontal_position = "center";
> + break;
> + case DEVICE_HORI_POS_RIGHT:
> + horizontal_position = "right";
> + break;
> + default:
> + horizontal_position = "unknown";
> + }
> + return sysfs_emit(buf, "%s\n", horizontal_position);
> +}
> +static DEVICE_ATTR_RO(horizontal_position);
> +
> +static ssize_t dock_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + return sysfs_emit(buf, "%s\n",
> + dev->physical_location->dock ? "yes" : "no");
> +}
> +static DEVICE_ATTR_RO(dock);
> +
> +static ssize_t lid_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + return sysfs_emit(buf, "%s\n",
> + dev->physical_location->lid ? "yes" : "no");
> +}
> +static DEVICE_ATTR_RO(lid);
> +
> +static struct attribute *dev_attr_physical_location[] = {
> + &dev_attr_panel.attr,
> + &dev_attr_vertical_position.attr,
> + &dev_attr_horizontal_position.attr,
> + &dev_attr_dock.attr,
> + &dev_attr_lid.attr,
> + NULL,
> +};
> +
> +const struct attribute_group dev_attr_physical_location_group = {
> + .name = "physical_location",
> + .attrs = dev_attr_physical_location,
> +};
> +
> diff --git a/drivers/base/physical_location.h b/drivers/base/physical_location.h
> new file mode 100644
> index 000000000000..82cde9f1b161
> --- /dev/null
> +++ b/drivers/base/physical_location.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Device physical location support
> + *
> + * Author: Won Chung <[email protected]>
> + */
> +
> +#include <linux/device.h>
> +
> +#ifdef CONFIG_ACPI
> +extern bool dev_add_physical_location(struct device *dev);
> +extern const struct attribute_group dev_attr_physical_location_group;
> +#else
> +static inline bool dev_add_physical_location(struct device *dev) { return false; };
> +static const struct attribute_group dev_attr_physical_location_group = {};
> +#endif
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 93459724dcde..766fbea6ca83 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -386,6 +386,75 @@ struct dev_msi_info {
> #endif
> };
>
> +/**
> + * enum device_physical_location_panel - Describes which panel surface of the
> + * system's housing the device connection point resides on.
> + * @DEVICE_PANEL_TOP: Device connection point is on the top panel.
> + * @DEVICE_PANEL_BOTTOM: Device connection point is on the bottom panel.
> + * @DEVICE_PANEL_LEFT: Device connection point is on the left panel.
> + * @DEVICE_PANEL_RIGHT: Device connection point is on the right panel.
> + * @DEVICE_PANEL_FRONT: Device connection point is on the front panel.
> + * @DEVICE_PANEL_BACK: Device connection point is on the back panel.
> + * @DEVICE_PANEL_UNKNOWN: The panel with device connection point is unknown.
> + */
> +enum device_physical_location_panel {
> + DEVICE_PANEL_TOP,
> + DEVICE_PANEL_BOTTOM,
> + DEVICE_PANEL_LEFT,
> + DEVICE_PANEL_RIGHT,
> + DEVICE_PANEL_FRONT,
> + DEVICE_PANEL_BACK,
> + DEVICE_PANEL_UNKNOWN,
> +};
> +
> +/**
> + * enum device_physical_location_vertical_position - Describes vertical
> + * position of the device connection point on the panel surface.
> + * @DEVICE_VERT_POS_UPPER: Device connection point is at upper part of panel.
> + * @DEVICE_VERT_POS_CENTER: Device connection point is at center part of panel.
> + * @DEVICE_VERT_POS_LOWER: Device connection point is at lower part of panel.
> + */
> +enum device_physical_location_vertical_position {
> + DEVICE_VERT_POS_UPPER,
> + DEVICE_VERT_POS_CENTER,
> + DEVICE_VERT_POS_LOWER,
> +};
> +
> +/**
> + * enum device_physical_location_horizontal_position - Describes horizontal
> + * position of the device connection point on the panel surface.
> + * @DEVICE_HORI_POS_LEFT: Device connection point is at left part of panel.
> + * @DEVICE_HORI_POS_CENTER: Device connection point is at center part of panel.
> + * @DEVICE_HORI_POS_RIGHT: Device connection point is at right part of panel.
> + */
> +enum device_physical_location_horizontal_position {
> + DEVICE_HORI_POS_LEFT,
> + DEVICE_HORI_POS_CENTER,
> + DEVICE_HORI_POS_RIGHT,
> +};
> +
> +/**
> + * struct device_physical_location - Device data related to physical location
> + * of the device connection point.
> + * @panel: Panel surface of the system's housing that the device connection
> + * point resides on.
> + * @vertical_position: Vertical position of the device connection point within
> + * the panel.
> + * @horizontal_position: Horizontal position of the device connection point
> + * within the panel.
> + * @dock: Set if the device connection point resides in a docking station or
> + * port replicator.
> + * @lid: Set if this device connection point resides on the lid of laptop
> + * system.
> + */
> +struct device_physical_location {
> + enum device_physical_location_panel panel;
> + enum device_physical_location_vertical_position vertical_position;
> + enum device_physical_location_horizontal_position horizontal_position;
> + bool dock;
> + bool lid;
> +};
> +
> /**
> * struct device - The basic device structure
> * @parent: The device's "parent" device, the device to which it is attached.
> @@ -453,6 +522,8 @@ struct dev_msi_info {
> * device (i.e. the bus driver that discovered the device).
> * @iommu_group: IOMMU group the device belongs to.
> * @iommu: Per device generic IOMMU runtime data
> + * @physical_location: Describes physical location of the device connection
> + * point in the system housing.
> * @removable: Whether the device can be removed from the system. This
> * should be set by the subsystem / bus driver that discovered
> * the device.
> @@ -567,6 +638,8 @@ struct device {
> struct iommu_group *iommu_group;
> struct dev_iommu *iommu;
>
> + struct device_physical_location *physical_location;
> +
> enum device_removable removable;
>
> bool offline_disabled:1;
> --
> 2.35.1.723.g4982287a31-goog
>
Hi Greg,
I am sorry to keep bothering you with this, but can you take a look at
this patch for a review when you have time? Thank you very much!
Won
On Wed, Mar 23, 2022 at 12:21 AM Greg Kroah-Hartman
<[email protected]> wrote:
>
> On Tue, Mar 22, 2022 at 09:36:02AM -0700, Won Chung wrote:
> > On Mon, Mar 14, 2022 at 12:55 PM Won Chung <[email protected]> wrote:
> > I am sorry to keep bothering you with this, but can you take a look at
> > this patch for a review when you have time? Thank you very much!
>
> It is the middle of the merge window now, and I will look at this after
> 5.18-rc1 is out. Due to all of the constant churn in this (and you only
> fixing the memory leak in the last submission), I had put this on the
> bottom of my review queue.
>
> Also, I don't see an actual user of these new attributes anywhere, so
> even if this was accepted, who is going to use it?
>
> thanks,
>
> greg k-h
Hi Greg,
Thank you for the information.
As a start, we are planning to use this in Chrome OS to specify the
physical location of each USB device since Coreboot adds _PLD fields
to usb port and typec connector. Currently, if there is one USB device
connected on the left panel and another USB device connected on the
right panel, the userspace cannot distinguish which device is on the
left or the right panel. Once each USB device exposes its location
info, we can gather more data on users' preference on port usage.
Also, it can apply to not just USB devices, but also other types like
HDMI ports in the future.
Would this answer your question?
Hey Benson, can you take a look at this if I am not mistaken or
missing some details?
Thanks,
Won
On Tue, Mar 22, 2022 at 09:36:02AM -0700, Won Chung wrote:
> On Mon, Mar 14, 2022 at 12:55 PM Won Chung <[email protected]> wrote:
> I am sorry to keep bothering you with this, but can you take a look at
> this patch for a review when you have time? Thank you very much!
It is the middle of the merge window now, and I will look at this after
5.18-rc1 is out. Due to all of the constant churn in this (and you only
fixing the memory leak in the last submission), I had put this on the
bottom of my review queue.
Also, I don't see an actual user of these new attributes anywhere, so
even if this was accepted, who is going to use it?
thanks,
greg k-h