Messages in this thread | | | From | Won Chung <> | Date | Fri, 4 Mar 2022 10:11:50 -0800 | Subject | Re: [PATCH] driver core: Add sysfs support for physical location of a device |
| |
On Wed, Mar 2, 2022 at 5:37 PM Won Chung <wonchung@google.com> 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 <wonchung@google.com> > --- > .../ABI/testing/sysfs-devices-location | 42 ++++++ > drivers/base/core.c | 138 ++++++++++++++++++ > include/linux/device.h | 73 +++++++++ > 3 files changed, 253 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-devices-location > > diff --git a/Documentation/ABI/testing/sysfs-devices-location b/Documentation/ABI/testing/sysfs-devices-location > new file mode 100644 > index 000000000000..bde81ce97a61 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-devices-location > @@ -0,0 +1,42 @@ > +What: /sys/devices/.../location > +Date: March 2022 > +Contact: Won Chung <wonchung@google.com> > +Description: > + This directory contains information on physical location of > + the device connection point with respect to the system's > + housing. > + > +What: /sys/devices/.../location/panel > +Date: March 2022 > +Contact: Won Chung <wonchung@google.com> > +Description: > + Describes which panel surface of the system’s housing the > + device connection point resides on. > + > +What: /sys/devices/.../location/vertical_position > +Date: March 2022 > +Contact: Won Chung <wonchung@google.com> > +Description: > + Describes vertical position of the device connection point on > + the panel surface. > + > +What: /sys/devices/.../location/horizontal_position > +Date: March 2022 > +Contact: Won Chung <wonchung@google.com> > +Description: > + Describes horizontal position of the device connection point on > + the panel surface. > + > +What: /sys/devices/.../location/dock > +Date: March 2022 > +Contact: Won Chung <wonchung@google.com> > +Description: > + "Yes" if the device connection point resides in a docking > + station or a port replicator. "No" otherwise. > + > +What: /sys/devices/.../location/lid > +Date: March 2022 > +Contact: Won Chung <wonchung@google.com> > +Description: > + "Yes" if the device connection point resides on the lid of > + laptop system. "No" otherwise. > diff --git a/drivers/base/core.c b/drivers/base/core.c > index 7bb957b11861..6b30007a319a 100644 > --- a/drivers/base/core.c > +++ b/drivers/base/core.c > @@ -2466,6 +2466,136 @@ static ssize_t removable_show(struct device *dev, struct device_attribute *attr, > } > static DEVICE_ATTR_RO(removable); > > +static int dev_get_and_add_location(struct device *dev) > +{ > +#if defined(CONFIG_ACPI) > + struct acpi_pld_info *pld; > + acpi_status status; > + > + if (!has_acpi_companion(dev)) > + return 0; > + > + status = acpi_get_physical_device_location(ACPI_HANDLE(dev), &pld); > + if (ACPI_FAILURE(status)) > + return 0; > + > + dev->location = (struct device_location) { > + .panel = pld->panel, > + .vertical_position = pld->vertical_position, > + .horizontal_position = pld->horizontal_position, > + .dock = pld->dock, > + .lid = pld->lid, > + }; > + > + return 1; > +#else > + return 0; > +#endif > +} > + > +static ssize_t panel_show(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + const char *panel; > + > + switch (dev->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; > + case DEVICE_PANEL_BACK: > + panel = "back"; > + break; > + default: > + panel = "unknown"; > + } > + return sprintf(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->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 sprintf(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->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 sprintf(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 sprintf(buf, "%s\n", dev->location.dock ? "yes" : "no"); > +} > +static DEVICE_ATTR_RO(dock); > + > +static ssize_t lid_show(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + return sprintf(buf, "%s\n", dev->location.lid ? "yes" : "no"); > +} > +static DEVICE_ATTR_RO(lid); > + > +static struct attribute *dev_attr_location[] = { > + &dev_attr_panel.attr, > + &dev_attr_vertical_position.attr, > + &dev_attr_horizontal_position.attr, > + &dev_attr_dock.attr, > + &dev_attr_lid.attr, > + NULL, > +}; > + > +static const struct attribute_group dev_attr_location_group = { > + .name = "physical_location", > + .attrs = dev_attr_location, > +}; > + > int device_add_groups(struct device *dev, const struct attribute_group **groups) > { > return sysfs_create_groups(&dev->kobj, groups); > @@ -2649,8 +2779,16 @@ static int device_add_attrs(struct device *dev) > goto err_remove_dev_waiting_for_supplier; > } > > + if (dev_get_and_add_location(dev)) { > + error = device_add_group(dev, &dev_attr_location_group); > + if (error) > + goto err_remove_dev_location; > + } > + > return 0; > > + err_remove_dev_location: > + device_remove_group(dev, &dev_attr_location_group); > err_remove_dev_waiting_for_supplier: > device_remove_file(dev, &dev_attr_waiting_for_supplier); > err_remove_dev_online: > diff --git a/include/linux/device.h b/include/linux/device.h > index 93459724dcde..424be9cb735e 100644 > --- a/include/linux/device.h > +++ b/include/linux/device.h > @@ -386,6 +386,75 @@ struct dev_msi_info { > #endif > }; > > +/** > + * enum device_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_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_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_location_vertical_position { > + DEVICE_VERT_POS_UPPER, > + DEVICE_VERT_POS_CENTER, > + DEVICE_VERT_POS_LOWER, > +}; > + > +/** > + * enum device_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_location_horizontal_position { > + DEVICE_HORI_POS_LEFT, > + DEVICE_HORI_POS_CENTER, > + DEVICE_HORI_POS_RIGHT, > +}; > + > +/** > + * struct device_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_location { > + enum device_location_panel panel; > + enum device_location_vertical_position vertical_position; > + enum device_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. > @@ -456,6 +525,8 @@ struct dev_msi_info { > * @removable: Whether the device can be removed from the system. This > * should be set by the subsystem / bus driver that discovered > * the device. > + * @location: Describes physical location of the device connection point in > + * the system housing. > * > * @offline_disabled: If set, the device is permanently online. > * @offline: Set after successful invocation of bus type's .offline(). > @@ -569,6 +640,8 @@ struct device { > > enum device_removable removable; > > + struct device_location location; > + > bool offline_disabled:1; > bool offline:1; > bool of_node_reused:1; > -- > 2.35.1.574.g5d30c73bfb-goog >
Hi,
I will send a new patch v2 to correct directory names in Documentation and also clarify some naming in core.c.
Won
| |