public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] platform/x86: thinkpad_acpi: Add support to detect hardware damage detection capability.
@ 2025-12-10 15:11 Nitin Joshi
  2025-12-10 15:11 ` [PATCH v2 2/2] platform/x86: thinkpad_acpi: Add sysfs to display details of damaged device Nitin Joshi
  2025-12-16  1:19 ` [PATCH v2 1/2] platform/x86: thinkpad_acpi: Add support to detect hardware damage detection capability kernel test robot
  0 siblings, 2 replies; 11+ messages in thread
From: Nitin Joshi @ 2025-12-10 15:11 UTC (permalink / raw)
  To: hansg, ilpo.jarvinen
  Cc: platform-driver-x86, linux-kernel, njoshi1, Nitin Joshi,
	Mark Pearson

Thinkpads are adding the ability to detect and report hardware damage
status. Add new sysfs interface to identify whether hardware damage
is detected or not.

Initial support is available for the USB-C replaceable connector.

Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Signed-off-by: Nitin Joshi<nitjoshi@gmail.com>
---
Changes since v1:
-Split patch between hwdd_status and hwdd_detail
-Incorporated review comments
---
 .../admin-guide/laptops/thinkpad-acpi.rst     |  21 ++++
 drivers/platform/x86/lenovo/thinkpad_acpi.c   | 104 ++++++++++++++++++
 2 files changed, 125 insertions(+)

diff --git a/Documentation/admin-guide/laptops/thinkpad-acpi.rst b/Documentation/admin-guide/laptops/thinkpad-acpi.rst
index 4ab0fef7d440..94349e5f1298 100644
--- a/Documentation/admin-guide/laptops/thinkpad-acpi.rst
+++ b/Documentation/admin-guide/laptops/thinkpad-acpi.rst
@@ -54,6 +54,7 @@ detailed description):
 	- Setting keyboard language
 	- WWAN Antenna type
 	- Auxmac
+	- Hardware damage detection capability
 
 A compatibility table by model and feature is maintained on the web
 site, http://ibm-acpi.sf.net/. I appreciate any success or failure
@@ -1576,6 +1577,26 @@ percentage level, above which charging will stop.
 The exact semantics of the attributes may be found in
 Documentation/ABI/testing/sysfs-class-power.
 
+Hardware damage detection capability
+-----------------
+
+sysfs attributes: hwdd_status
+
+Thinkpads are adding the ability to detect and report hardware damage.
+Add new sysfs interface to identify the damaged device status.
+Initial support is available for the USB-C replaceable connector.
+
+The command to check device damaged status is::
+
+        cat /sys/devices/platform/thinkpad_acpi/hwdd_status
+
+This value displays status of device damaged
+- 0 = Not Damaged
+- 1 = Damaged
+
+The property is read-only. If feature is not supported then sysfs
+attribute is not created.
+
 Multiple Commands, Module Parameters
 ------------------------------------
 
diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c b/drivers/platform/x86/lenovo/thinkpad_acpi.c
index cc19fe520ea9..4cf365550bcb 100644
--- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
+++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
@@ -11080,6 +11080,105 @@ static const struct attribute_group auxmac_attr_group = {
 	.attrs = auxmac_attributes,
 };
 
+/*************************************************************************
+ * HWDD subdriver, for the Lenovo Hardware Damage Detection feature.
+ */
+
+#define HWDD_GET_DMG_USBC	0x80000001
+#define HWDD_GET_CAP		0
+#define HWDD_NOT_SUPPORTED	BIT(31)
+#define HWDD_SUPPORT_USBC	BIT(0)
+
+#define PORT_STATUS		GENMASK(7, 4)
+#define NUM_PORTS		4
+
+static bool hwdd_support_available;
+static bool ucdd_supported;
+
+static int hwdd_command(int command, int *output)
+{
+	acpi_handle hwdd_handle;
+
+	if (ACPI_FAILURE(acpi_get_handle(hkey_handle, "HWDD", &hwdd_handle)))
+		return -ENODEV;
+
+	if (!acpi_evalf(hwdd_handle, output, NULL, "dd", command))
+		return -EIO;
+
+	return 0;
+}
+
+/* sysfs type-c damage detection capability */
+static ssize_t hwdd_status_show(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	unsigned int damage_status, port_status;
+	int err, i;
+
+	if (ucdd_supported) {
+		/* Get USB TYPE-C damage status */
+		err = hwdd_command(HWDD_GET_DMG_USBC, &damage_status);
+		if (err)
+			return err;
+
+		port_status = FIELD_GET(PORT_STATUS, damage_status);
+		for (i = 0; i < NUM_PORTS; i++) {
+			if (!(damage_status & BIT(i)))
+				continue;
+			if (port_status & BIT(i))
+				return sysfs_emit(buf, "1\n");
+		}
+	}
+
+	return sysfs_emit(buf, "0\n");
+}
+static DEVICE_ATTR_RO(hwdd_status);
+
+static struct attribute *hwdd_attributes[] = {
+	&dev_attr_hwdd_status.attr,
+	NULL
+};
+
+static umode_t hwdd_attr_is_visible(struct kobject *kobj,
+				struct attribute *attr, int n)
+{
+	return hwdd_support_available ? attr->mode : 0;
+}
+
+static const struct attribute_group hwdd_attr_group = {
+	.is_visible = hwdd_attr_is_visible,
+	.attrs = hwdd_attributes,
+};
+
+static int tpacpi_hwdd_init(struct ibm_init_struct *iibm)
+{
+	int err, output;
+
+	/* Below command checks the HWDD damage capability */
+	err = hwdd_command(HWDD_GET_CAP, &output);
+	if (err)
+		return err;
+
+	if (!(output & HWDD_NOT_SUPPORTED))
+		return -ENODEV;
+
+	hwdd_support_available = true;
+
+	/*
+	 * BIT(0) is assigned to check capability of damage detection is
+	 * supported for USB Type-C port or not.
+	 */
+	if (output & HWDD_SUPPORT_USBC)
+		ucdd_supported = true;
+
+	return err;
+}
+
+static struct ibm_struct hwdd_driver_data = {
+	.name = "hwdd",
+};
+
 /* --------------------------------------------------------------------- */
 
 static struct attribute *tpacpi_driver_attributes[] = {
@@ -11139,6 +11238,7 @@ static const struct attribute_group *tpacpi_groups[] = {
 	&kbdlang_attr_group,
 	&dprc_attr_group,
 	&auxmac_attr_group,
+	&hwdd_attr_group,
 	NULL,
 };
 
@@ -11752,6 +11852,10 @@ static struct ibm_init_struct ibms_init[] __initdata = {
 		.init = auxmac_init,
 		.data = &auxmac_data,
 	},
+	{
+		.init = tpacpi_hwdd_init,
+		.data = &hwdd_driver_data,
+	},
 };
 
 static int __init set_ibm_param(const char *val, const struct kernel_param *kp)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/2] platform/x86: thinkpad_acpi: Add sysfs to display details of damaged device.
  2025-12-10 15:11 [PATCH v2 1/2] platform/x86: thinkpad_acpi: Add support to detect hardware damage detection capability Nitin Joshi
@ 2025-12-10 15:11 ` Nitin Joshi
  2025-12-10 15:43   ` Mario Limonciello
  2025-12-16  4:42   ` kernel test robot
  2025-12-16  1:19 ` [PATCH v2 1/2] platform/x86: thinkpad_acpi: Add support to detect hardware damage detection capability kernel test robot
  1 sibling, 2 replies; 11+ messages in thread
From: Nitin Joshi @ 2025-12-10 15:11 UTC (permalink / raw)
  To: hansg, ilpo.jarvinen
  Cc: platform-driver-x86, linux-kernel, njoshi1, Nitin Joshi,
	Mark Pearson

Add new sysfs interface to identify the impacted component with location of
device.

Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Signed-off-by: Nitin Joshi<nitjoshi@gmail.com>
---
 .../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;
+	}
+
+	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
 };
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/2] platform/x86: thinkpad_acpi: Add sysfs to display details of damaged device.
  2025-12-10 15:11 ` [PATCH v2 2/2] platform/x86: thinkpad_acpi: Add sysfs to display details of damaged device Nitin Joshi
@ 2025-12-10 15:43   ` Mario Limonciello
  2025-12-10 16:27     ` Nitin
  2025-12-16  4:42   ` kernel test robot
  1 sibling, 1 reply; 11+ messages in thread
From: Mario Limonciello @ 2025-12-10 15:43 UTC (permalink / raw)
  To: Nitin Joshi, hansg, ilpo.jarvinen
  Cc: platform-driver-x86, linux-kernel, njoshi1, Mark Pearson

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@squebb.ca>
> Signed-off-by: Nitin Joshi<nitjoshi@gmail.com>
> ---
>   .../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.

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 (!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
>   };
>   


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/2] platform/x86: thinkpad_acpi: Add sysfs to display details of damaged device.
  2025-12-10 15:43   ` Mario Limonciello
@ 2025-12-10 16:27     ` Nitin
  2025-12-10 16:44       ` Mario Limonciello
  0 siblings, 1 reply; 11+ messages in thread
From: Nitin @ 2025-12-10 16:27 UTC (permalink / raw)
  To: Mario Limonciello, hansg, ilpo.jarvinen
  Cc: platform-driver-x86, linux-kernel, njoshi1, Mark Pearson

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@squebb.ca>
>> Signed-off-by: Nitin Joshi<nitjoshi@gmail.com>
>> ---
>>   .../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 ?

>
>
>> +
>> +    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 !

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/2] platform/x86: thinkpad_acpi: Add sysfs to display details of damaged device.
  2025-12-10 16:27     ` Nitin
@ 2025-12-10 16:44       ` Mario Limonciello
  2025-12-10 16:52         ` Nitin
  0 siblings, 1 reply; 11+ messages in thread
From: Mario Limonciello @ 2025-12-10 16:44 UTC (permalink / raw)
  To: Nitin, hansg, ilpo.jarvinen
  Cc: platform-driver-x86, linux-kernel, njoshi1, Mark Pearson

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@squebb.ca>
>>> Signed-off-by: Nitin Joshi<nitjoshi@gmail.com>
>>> ---
>>>   .../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 !


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/2] platform/x86: thinkpad_acpi: Add sysfs to display details of damaged device.
  2025-12-10 16:44       ` Mario Limonciello
@ 2025-12-10 16:52         ` Nitin
  0 siblings, 0 replies; 11+ messages in thread
From: Nitin @ 2025-12-10 16:52 UTC (permalink / raw)
  To: Mario Limonciello, hansg, ilpo.jarvinen
  Cc: platform-driver-x86, linux-kernel, njoshi1, Mark Pearson


On 12/11/25 01:44, Mario Limonciello wrote:
> 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@squebb.ca>
>>>> Signed-off-by: Nitin Joshi<nitjoshi@gmail.com>
>>>> ---
>>>>   .../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().

Ack !

  I will include case for !ucdd_supported in next version. Thank you !

>>
>>>
>>>
>>>> +
>>>> +    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 !
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] platform/x86: thinkpad_acpi: Add support to detect hardware damage detection capability.
  2025-12-10 15:11 [PATCH v2 1/2] platform/x86: thinkpad_acpi: Add support to detect hardware damage detection capability Nitin Joshi
  2025-12-10 15:11 ` [PATCH v2 2/2] platform/x86: thinkpad_acpi: Add sysfs to display details of damaged device Nitin Joshi
@ 2025-12-16  1:19 ` kernel test robot
  2025-12-16  2:29   ` Nitin
  1 sibling, 1 reply; 11+ messages in thread
From: kernel test robot @ 2025-12-16  1:19 UTC (permalink / raw)
  To: Nitin Joshi, hansg, ilpo.jarvinen
  Cc: oe-kbuild-all, platform-driver-x86, linux-kernel, njoshi1,
	Nitin Joshi, Mark Pearson

Hi Nitin,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.19-rc1 next-20251215]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Nitin-Joshi/platform-x86-thinkpad_acpi-Add-sysfs-to-display-details-of-damaged-device/20251210-231409
base:   linus/master
patch link:    https://lore.kernel.org/r/20251210151133.7933-1-nitjoshi%40gmail.com
patch subject: [PATCH v2 1/2] platform/x86: thinkpad_acpi: Add support to detect hardware damage detection capability.
reproduce: (https://download.01.org/0day-ci/archive/20251216/202512160219.94nMjvxO-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202512160219.94nMjvxO-lkp@intel.com/

All warnings (new ones prefixed by >>):

   WARNING: No kernel-doc for file ./include/linux/pci.h
   ERROR: Cannot find file ./include/linux/mod_devicetable.h
   WARNING: No kernel-doc for file ./include/linux/mod_devicetable.h
   ERROR: Cannot find file ./include/linux/bootconfig.h
   WARNING: No kernel-doc for file ./include/linux/bootconfig.h
>> Documentation/admin-guide/laptops/thinkpad-acpi.rst:1581: WARNING: Title underline too short.
--
   Hardware damage detection capability
   ----------------- [docutils]
>> Documentation/admin-guide/laptops/thinkpad-acpi.rst:1581: WARNING: Title underline too short.


vim +1581 Documentation/admin-guide/laptops/thinkpad-acpi.rst

  1579	
  1580	Hardware damage detection capability
> 1581	-----------------
  1582	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] platform/x86: thinkpad_acpi: Add support to detect hardware damage detection capability.
  2025-12-16  1:19 ` [PATCH v2 1/2] platform/x86: thinkpad_acpi: Add support to detect hardware damage detection capability kernel test robot
@ 2025-12-16  2:29   ` Nitin
  2025-12-16 10:19     ` Ilpo Järvinen
  0 siblings, 1 reply; 11+ messages in thread
From: Nitin @ 2025-12-16  2:29 UTC (permalink / raw)
  To: kernel test robot, hansg, ilpo.jarvinen
  Cc: oe-kbuild-all, platform-driver-x86, linux-kernel, njoshi1,
	Mark Pearson

Hello ,

On 12/16/25 10:19, kernel test robot wrote:
> Hi Nitin,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on linus/master]
> [also build test WARNING on v6.19-rc1 next-20251215]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Nitin-Joshi/platform-x86-thinkpad_acpi-Add-sysfs-to-display-details-of-damaged-device/20251210-231409
> base:   linus/master
> patch link:    https://lore.kernel.org/r/20251210151133.7933-1-nitjoshi%40gmail.com
> patch subject: [PATCH v2 1/2] platform/x86: thinkpad_acpi: Add support to detect hardware damage detection capability.
> reproduce: (https://download.01.org/0day-ci/archive/20251216/202512160219.94nMjvxO-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202512160219.94nMjvxO-lkp@intel.com/

Thank you for reporting this issue and I will fix this issue .

I have already send patch v3 for this and will fix this warning in next 
patch version i.e v4 .

May i know, if its OK to add this Reported-by and closes tag in patch 
version v4 ?

>
> All warnings (new ones prefixed by >>):
>
>     WARNING: No kernel-doc for file ./include/linux/pci.h
>     ERROR: Cannot find file ./include/linux/mod_devicetable.h
>     WARNING: No kernel-doc for file ./include/linux/mod_devicetable.h
>     ERROR: Cannot find file ./include/linux/bootconfig.h
>     WARNING: No kernel-doc for file ./include/linux/bootconfig.h
>>> Documentation/admin-guide/laptops/thinkpad-acpi.rst:1581: WARNING: Title underline too short.
> --
>     Hardware damage detection capability
>     ----------------- [docutils]
>>> Documentation/admin-guide/laptops/thinkpad-acpi.rst:1581: WARNING: Title underline too short.

Ack. Thank you !

>
> vim +1581 Documentation/admin-guide/laptops/thinkpad-acpi.rst
>
>    1579	
>    1580	Hardware damage detection capability
>> 1581	-----------------
>    1582	
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/2] platform/x86: thinkpad_acpi: Add sysfs to display details of damaged device.
  2025-12-10 15:11 ` [PATCH v2 2/2] platform/x86: thinkpad_acpi: Add sysfs to display details of damaged device Nitin Joshi
  2025-12-10 15:43   ` Mario Limonciello
@ 2025-12-16  4:42   ` kernel test robot
  1 sibling, 0 replies; 11+ messages in thread
From: kernel test robot @ 2025-12-16  4:42 UTC (permalink / raw)
  To: Nitin Joshi, hansg, ilpo.jarvinen
  Cc: oe-kbuild-all, platform-driver-x86, linux-kernel, njoshi1,
	Nitin Joshi, Mark Pearson

Hi Nitin,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.19-rc1 next-20251216]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Nitin-Joshi/platform-x86-thinkpad_acpi-Add-sysfs-to-display-details-of-damaged-device/20251210-231409
base:   linus/master
patch link:    https://lore.kernel.org/r/20251210151133.7933-2-nitjoshi%40gmail.com
patch subject: [PATCH v2 2/2] platform/x86: thinkpad_acpi: Add sysfs to display details of damaged device.
reproduce: (https://download.01.org/0day-ci/archive/20251216/202512160557.9va36P2F-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202512160557.9va36P2F-lkp@intel.com/

All warnings (new ones prefixed by >>):

   Hardware damage detection capability
   ----------------- [docutils]
   Documentation/admin-guide/laptops/thinkpad-acpi.rst:1604: ERROR: Unexpected indentation. [docutils]
>> Documentation/admin-guide/laptops/thinkpad-acpi.rst:1605: WARNING: Block quote ends without a blank line; unexpected unindent. [docutils]
   ERROR: Cannot find file ./include/linux/pstore_zone.h
   ERROR: Cannot find file ./include/linux/pstore_zone.h
   WARNING: No kernel-doc for file ./include/linux/pstore_zone.h
   ERROR: Cannot find file ./include/linux/pstore_blk.h
   ERROR: Cannot find file ./include/linux/pstore_blk.h


vim +1605 Documentation/admin-guide/laptops/thinkpad-acpi.rst

  1598	
  1599	        cat /sys/devices/platform/thinkpad_acpi/hwdd_detail
  1600	
  1601	This value displays location of damaged device having 1 line per damaged "item".
  1602	For example:
  1603	if no damage is detected:
  1604	  No damage detected
> 1605	if damage detected:
  1606	  TYPE-C: Base, Right side, Center port
  1607	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] platform/x86: thinkpad_acpi: Add support to detect hardware damage detection capability.
  2025-12-16  2:29   ` Nitin
@ 2025-12-16 10:19     ` Ilpo Järvinen
  2025-12-16 11:15       ` Nitin
  0 siblings, 1 reply; 11+ messages in thread
From: Ilpo Järvinen @ 2025-12-16 10:19 UTC (permalink / raw)
  To: Nitin
  Cc: kernel test robot, Hans de Goede, oe-kbuild-all,
	platform-driver-x86, LKML, njoshi1, Mark Pearson

On Tue, 16 Dec 2025, Nitin wrote:

> Hello ,
> 
> On 12/16/25 10:19, kernel test robot wrote:
> > Hi Nitin,
> > 
> > kernel test robot noticed the following build warnings:
> > 
> > [auto build test WARNING on linus/master]
> > [also build test WARNING on v6.19-rc1 next-20251215]
> > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > And when submitting patch, we suggest to use '--base' as documented in
> > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> > 
> > url:
> > https://github.com/intel-lab-lkp/linux/commits/Nitin-Joshi/platform-x86-thinkpad_acpi-Add-sysfs-to-display-details-of-damaged-device/20251210-231409
> > base:   linus/master
> > patch link:
> > https://lore.kernel.org/r/20251210151133.7933-1-nitjoshi%40gmail.com
> > patch subject: [PATCH v2 1/2] platform/x86: thinkpad_acpi: Add support to
> > detect hardware damage detection capability.
> > reproduce:
> > (https://download.01.org/0day-ci/archive/20251216/202512160219.94nMjvxO-lkp@intel.com/reproduce)
> > 
> > If you fix the issue in a separate patch/commit (i.e. not just a new version
> > of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Closes:
> > https://lore.kernel.org/oe-kbuild-all/202512160219.94nMjvxO-lkp@intel.com/
> 
> Thank you for reporting this issue and I will fix this issue .
> 
> I have already send patch v3 for this and will fix this warning in next patch
> version i.e v4 .
> 
> May i know, if its OK to add this Reported-by and closes tag in patch version
> v4 ?

Hi,

Please only add those tags when you're fixing code that exists in a repo 
already. For new work, we don't normally add lkp report tags despite the 
email suggesting you to do so (I know it's a bit confusing but it's sort 
of same as we don't normally mention all reviewers who gave improvement 
suggestions either).

> > All warnings (new ones prefixed by >>):
> > 
> >     WARNING: No kernel-doc for file ./include/linux/pci.h
> >     ERROR: Cannot find file ./include/linux/mod_devicetable.h
> >     WARNING: No kernel-doc for file ./include/linux/mod_devicetable.h
> >     ERROR: Cannot find file ./include/linux/bootconfig.h
> >     WARNING: No kernel-doc for file ./include/linux/bootconfig.h
> > > > Documentation/admin-guide/laptops/thinkpad-acpi.rst:1581: WARNING: Title
> > > > underline too short.
> > --
> >     Hardware damage detection capability
> >     ----------------- [docutils]
> > > > Documentation/admin-guide/laptops/thinkpad-acpi.rst:1581: WARNING: Title
> > > > underline too short.
> 
> Ack. Thank you !
> 
> > 
> > vim +1581 Documentation/admin-guide/laptops/thinkpad-acpi.rst
> > 
> >    1579	
> >    1580	Hardware damage detection capability
> > > 1581	-----------------
> >    1582	
> > 
> 

-- 
 i.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] platform/x86: thinkpad_acpi: Add support to detect hardware damage detection capability.
  2025-12-16 10:19     ` Ilpo Järvinen
@ 2025-12-16 11:15       ` Nitin
  0 siblings, 0 replies; 11+ messages in thread
From: Nitin @ 2025-12-16 11:15 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: kernel test robot, Hans de Goede, oe-kbuild-all,
	platform-driver-x86, LKML, njoshi1, Mark Pearson


On 12/16/25 19:19, Ilpo Järvinen wrote:
> On Tue, 16 Dec 2025, Nitin wrote:
>
>> Hello ,
>>
>> On 12/16/25 10:19, kernel test robot wrote:
>>> Hi Nitin,
>>>
>>> kernel test robot noticed the following build warnings:
>>>
>>> [auto build test WARNING on linus/master]
>>> [also build test WARNING on v6.19-rc1 next-20251215]
>>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>>> And when submitting patch, we suggest to use '--base' as documented in
>>> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>>>
>>> url:
>>> https://github.com/intel-lab-lkp/linux/commits/Nitin-Joshi/platform-x86-thinkpad_acpi-Add-sysfs-to-display-details-of-damaged-device/20251210-231409
>>> base:   linus/master
>>> patch link:
>>> https://lore.kernel.org/r/20251210151133.7933-1-nitjoshi%40gmail.com
>>> patch subject: [PATCH v2 1/2] platform/x86: thinkpad_acpi: Add support to
>>> detect hardware damage detection capability.
>>> reproduce:
>>> (https://download.01.org/0day-ci/archive/20251216/202512160219.94nMjvxO-lkp@intel.com/reproduce)
>>>
>>> If you fix the issue in a separate patch/commit (i.e. not just a new version
>>> of
>>> the same patch/commit), kindly add following tags
>>> | Reported-by: kernel test robot <lkp@intel.com>
>>> | Closes:
>>> https://lore.kernel.org/oe-kbuild-all/202512160219.94nMjvxO-lkp@intel.com/
>> Thank you for reporting this issue and I will fix this issue .
>>
>> I have already send patch v3 for this and will fix this warning in next patch
>> version i.e v4 .
>>
>> May i know, if its OK to add this Reported-by and closes tag in patch version
>> v4 ?
> Hi,
>
> Please only add those tags when you're fixing code that exists in a repo
> already. For new work, we don't normally add lkp report tags despite the
> email suggesting you to do so (I know it's a bit confusing but it's sort
> of same as we don't normally mention all reviewers who gave improvement
> suggestions either).

Noted, Thank you for clarification . I will send next patch revision soon.

Thank you !

>>> All warnings (new ones prefixed by >>):
>>>
>>>      WARNING: No kernel-doc for file ./include/linux/pci.h
>>>      ERROR: Cannot find file ./include/linux/mod_devicetable.h
>>>      WARNING: No kernel-doc for file ./include/linux/mod_devicetable.h
>>>      ERROR: Cannot find file ./include/linux/bootconfig.h
>>>      WARNING: No kernel-doc for file ./include/linux/bootconfig.h
>>>>> Documentation/admin-guide/laptops/thinkpad-acpi.rst:1581: WARNING: Title
>>>>> underline too short.
>>> --
>>>      Hardware damage detection capability
>>>      ----------------- [docutils]
>>>>> Documentation/admin-guide/laptops/thinkpad-acpi.rst:1581: WARNING: Title
>>>>> underline too short.
>> Ack. Thank you !
>>
>>> vim +1581 Documentation/admin-guide/laptops/thinkpad-acpi.rst
>>>
>>>     1579	
>>>     1580	Hardware damage detection capability
>>>> 1581	-----------------
>>>     1582	
>>>

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-12-16 11:15 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-10 15:11 [PATCH v2 1/2] platform/x86: thinkpad_acpi: Add support to detect hardware damage detection capability Nitin Joshi
2025-12-10 15:11 ` [PATCH v2 2/2] platform/x86: thinkpad_acpi: Add sysfs to display details of damaged device Nitin Joshi
2025-12-10 15:43   ` Mario Limonciello
2025-12-10 16:27     ` Nitin
2025-12-10 16:44       ` Mario Limonciello
2025-12-10 16:52         ` Nitin
2025-12-16  4:42   ` kernel test robot
2025-12-16  1:19 ` [PATCH v2 1/2] platform/x86: thinkpad_acpi: Add support to detect hardware damage detection capability kernel test robot
2025-12-16  2:29   ` Nitin
2025-12-16 10:19     ` Ilpo Järvinen
2025-12-16 11:15       ` Nitin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox