Re: [PATCH v2 2/2] platform/x86: thinkpad_acpi: Add sysfs to display details of damaged device.
From: Mario Limonciello
Date: Wed Dec 10 2025 - 11:45:53 EST
On 12/10/25 10:27 AM, Nitin wrote:
Hi Mario,
Thank you for your comments.
On 12/11/25 00:43, Mario Limonciello wrote:
On 12/10/25 9:11 AM, Nitin Joshi wrote:
Add new sysfs interface to identify the impacted component with
location of
device.
Reviewed-by: Mark Pearson <mpearson-lenovo@xxxxxxxxx>
Signed-off-by: Nitin Joshi<nitjoshi@xxxxxxxxx>
---
.../admin-guide/laptops/thinkpad-acpi.rst | 13 +-
drivers/platform/x86/lenovo/thinkpad_acpi.c | 112 +++++++++++++++++-
2 files changed, 121 insertions(+), 4 deletions(-)
diff --git a/Documentation/admin-guide/laptops/thinkpad-acpi.rst b/
Documentation/admin-guide/laptops/thinkpad-acpi.rst
index 94349e5f1298..3a9190ac47d0 100644
--- a/Documentation/admin-guide/laptops/thinkpad-acpi.rst
+++ b/Documentation/admin-guide/laptops/thinkpad-acpi.rst
@@ -1580,7 +1580,7 @@ Documentation/ABI/testing/sysfs-class-power.
Hardware damage detection capability
-----------------
-sysfs attributes: hwdd_status
+sysfs attributes: hwdd_status, hwdd_detail
Thinkpads are adding the ability to detect and report hardware
damage.
Add new sysfs interface to identify the damaged device status.
@@ -1594,6 +1594,17 @@ This value displays status of device damaged
- 0 = Not Damaged
- 1 = Damaged
+The command to check location of damaged device is::
+
+ cat /sys/devices/platform/thinkpad_acpi/hwdd_detail
+
+This value displays location of damaged device having 1 line per
damaged "item".
+For example:
+if no damage is detected:
+ No damage detected
+if damage detected:
+ TYPE-C: Base, Right side, Center port
+
The property is read-only. If feature is not supported then sysfs
attribute is not created.
diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c b/drivers/
platform/x86/lenovo/thinkpad_acpi.c
index 4cf365550bcb..a092d57d995d 100644
--- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
+++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
@@ -11089,8 +11089,24 @@ static const struct attribute_group
auxmac_attr_group = {
#define HWDD_NOT_SUPPORTED BIT(31)
#define HWDD_SUPPORT_USBC BIT(0)
-#define PORT_STATUS GENMASK(7, 4)
-#define NUM_PORTS 4
+#define PORT_STATUS GENMASK(7, 4)
+#define LID_STATUS GENMASK(11, 8)
+#define BASE_STATUS GENMASK(15, 12)
+#define POS_STATUS GENMASK(3, 2)
+#define PANEL_STATUS GENMASK(1, 0)
+
+#define PORT_DETAIL_OFFSET 16
+
+#define PANEL_TOP 0
+#define PANEL_BASE 1
+#define PANEL_LEFT 2
+#define PANEL_RIGHT 3
+
+#define POS_LEFT 0
+#define POS_CENTER 1
+#define POS_RIGHT 2
+
+#define NUM_PORTS 4
static bool hwdd_support_available;
static bool ucdd_supported;
@@ -11108,7 +11124,95 @@ static int hwdd_command(int command, int
*output)
return 0;
}
-/* sysfs type-c damage detection capability */
+static bool display_damage(char *buf, int *count, char *type,
unsigned int dmg_status)
+{
+ unsigned char lid_status, base_status, port_status;
+ unsigned char loc_status, pos_status, panel_status;
+ bool damage_detected = false;
+ int i;
+
+ port_status = FIELD_GET(PORT_STATUS, dmg_status);
+ lid_status = FIELD_GET(LID_STATUS, dmg_status);
+ base_status = FIELD_GET(BASE_STATUS, dmg_status);
+ for (i = 0; i < NUM_PORTS; i++) {
+ if (!(dmg_status & BIT(i)))
+ continue;
+
+ if (port_status & BIT(i)) {
+ *count += sysfs_emit_at(buf, *count, "%s: ", type);
+ loc_status = (dmg_status >> (PORT_DETAIL_OFFSET + (4 *
i))) & 0xF;
+ pos_status = FIELD_GET(POS_STATUS, loc_status);
+ panel_status = FIELD_GET(PANEL_STATUS, loc_status);
+
+ if (lid_status & BIT(i))
+ *count += sysfs_emit_at(buf, *count, "Lid, ");
+ if (base_status & BIT(i))
+ *count += sysfs_emit_at(buf, *count, "Base, ");
+
+ switch (pos_status) {
+ case PANEL_TOP:
+ *count += sysfs_emit_at(buf, *count, "Top, ");
+ break;
+ case PANEL_BASE:
+ *count += sysfs_emit_at(buf, *count, "Bottom, ");
+ break;
+ case PANEL_LEFT:
+ *count += sysfs_emit_at(buf, *count, "Left, ");
+ break;
+ case PANEL_RIGHT:
+ *count += sysfs_emit_at(buf, *count, "Right, ");
+ break;
+ default:
+ pr_err("Unexpected value %d in switch statement\n",
pos_status);
+ };
+
+ switch (panel_status) {
+ case POS_LEFT:
+ *count += sysfs_emit_at(buf, *count, "Left port\n");
+ break;
+ case POS_CENTER:
+ *count += sysfs_emit_at(buf, *count, "Center port\n");
+ break;
+ case POS_RIGHT:
+ *count += sysfs_emit_at(buf, *count, "Right port\n");
+ break;
+ default:
+ *count += sysfs_emit_at(buf, *count, "Undefined\n");
+ break;
+ };
+ damage_detected = true;
+ }
+ }
+ return damage_detected;
+}
+
+/* sysfs type-c damage detection detail */
+static ssize_t hwdd_detail_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ bool damage_detected = false;
+ unsigned int damage_status;
+ int err, count = 0;
+
+
+ if (ucdd_supported) {
+ /* Get USB TYPE-C damage status */
+ err = hwdd_command(HWDD_GET_DMG_USBC, &damage_status);
+ if (err)
+ return err;
+
+ if (display_damage(buf, &count, "Type-C", damage_status))
+ damage_detected = true;
+ }
Since this is always visible aren't you missing a case for !
ucdd_supported? I would think you should be returning -ENODEV.
In actual, this condition should never occur as only USB Type-C is
supported in this ASL method but i think it's ok to add this check, if
there is any benefit.
In this case, is it recommended to add such case like !ucdd_supported?
Also, if new device id like type-a etc.. is added in future then we
need to include corresponding device id supported also in this check to
make sysfs visible.
Although arguably it would be better to control visibility of the
sysfs attribute based upon ucdd_supported. You can simplify
hwdd_detail_show() too then.
If new device id is added in future then we need to add additional flag
to control visibility of sysfs .
At this moment , i cant see anything obvious to be simplified
in hwdd_detail_show() . Did i missed something ?
Well my comment was specifically upon visibility. If you avoid attribute
being visible conditional on ucdd_supported, you don't need to actually
check this in *_show().
+
+ if (!damage_detected)
+ count += sysfs_emit_at(buf, count, "No damage detected\n");
+
+ return count;
+}
+
+/* sysfs typc damage detection capability */
static ssize_t hwdd_status_show(struct device *dev,
struct device_attribute *attr,
char *buf)
@@ -11134,9 +11238,11 @@ static ssize_t hwdd_status_show(struct
device *dev,
return sysfs_emit(buf, "0\n");
}
static DEVICE_ATTR_RO(hwdd_status);
+static DEVICE_ATTR_RO(hwdd_detail);
static struct attribute *hwdd_attributes[] = {
&dev_attr_hwdd_status.attr,
+ &dev_attr_hwdd_detail.attr,
NULL
};
Thank you !