Re: [PATCH v5 1/2] platform/x86: thinkpad_acpi: Add support to detect hardware damage detection capability.

[Nitin Joshi <nitjoshi@gmail.com>]

On 12/23/25 12:14, Ilpo Järvinen wrote:
> On Wed, 17 Dec 2025, Nitin Joshi wrote:
>
>> 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
>> Changes since v2:
>> -Control visibility of the sysfs attribute based upon ucdd_supported
>> Changes since v3:
>> -Fix documentation build warning
>> Changes since v4:
>> -Removed extra line
>> ---
>>   .../admin-guide/laptops/thinkpad-acpi.rst     |  21 ++++
>>   drivers/platform/x86/lenovo/thinkpad_acpi.c   | 105 ++++++++++++++++++
>>   2 files changed, 126 insertions(+)
>>
>> diff --git a/Documentation/admin-guide/laptops/thinkpad-acpi.rst b/Documentation/admin-guide/laptops/thinkpad-acpi.rst
>> index 4ab0fef7d440..2db05f718b11 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..cb1f6dae9334 100644
>> --- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
>> +++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
>> @@ -11080,6 +11080,106 @@ 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");
>> +		}
>> +	} else
>> +		return -ENODEV;
> The usual approach is to first check not supported + return errno, then
> you don't even need "else". The same comment to the other patch.

Ack , I will modify it and send next revision. Thank you for review.