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

[Nitin Joshi]

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@xxxxxxxxx>
> > Signed-off-by: Nitin Joshi<nitjoshi@xxxxxxxxx>
> > ---
> > 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.