[PATCH] platform/x86: thinkpad_acpi: Add USB-C Security (USCS) support

[PATCH] platform/x86: thinkpad_acpi: Add USB-C Security (USCS) support

[Vishnu Sankar]

Newer ThinkPad systems expose a USB-C Security (Restricted Mode) feature.
When active, USB-C data connections are disabled while power delivery is
preserved. This is useful for kiosk and physically-secured deployments.

Hardware interface:

The HKEY device exposes a read-only ACPI method USCS():

  Return value bit layout:
    Bit 16 : Capability flag (1 = feature present on this SKU)
    Bit  0 : Current state  (0 = security OFF, 1 = security ON)

The sysfs attribute is read-only.

The Fn+U followed by Fn+S hotkey chord is the only way to toggle the
hardware state.

Hotkey:

Fn+U followed by Fn+S generates HKEY event 0x131e.

sysfs interface:

  /sys/devices/platform/thinkpad_acpi/usb_c_security  (read-only)
  "enabled\n"  -- data connections are currently blocked
  "disabled\n" -- data connections are currently allowed

  The attribute is hidden on SKUs where the USCS capability bit (bit 16)
  is not set, so there is no ABI impact on unsupported hardware.

Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Signed-off-by: Vishnu Sankar <vishnuocv@gmail.com>
---
 .../admin-guide/laptops/thinkpad-acpi.rst     |  24 ++++
 drivers/platform/x86/lenovo/thinkpad_acpi.c   | 115 ++++++++++++++++++
 2 files changed, 139 insertions(+)

diff --git a/Documentation/admin-guide/laptops/thinkpad-acpi.rst b/Documentation/admin-guide/laptops/thinkpad-acpi.rst
index f874db31801d..db4588af0278 100644
--- a/Documentation/admin-guide/laptops/thinkpad-acpi.rst
+++ b/Documentation/admin-guide/laptops/thinkpad-acpi.rst
@@ -1543,6 +1543,30 @@ Values:
 
 	This setting can also be toggled via the Fn+doubletap hotkey.
 
+USB-C Security
+--------------
+
+sysfs: usb_c_security
+
+Reports the current state of the USB-C Security (Restricted Mode) feature
+on supported ThinkPad systems. When enabled, USB-C data connections are
+disabled while power delivery is preserved.
+
+The available command is::
+
+        cat /sys/devices/platform/thinkpad_acpi/usb_c_security
+
+Values:
+
+	* ``enabled``  - USB-C data connections are currently blocked
+	* ``disabled`` - USB-C data connections are currently allowed
+
+The attribute is read-only. The USB-C Security state can only be toggled
+via the Fn+U followed by Fn+S hotkey chord.
+
+The sysfs attribute is not created on platforms that do not support this
+feature.
+
 Auxmac
 ------
 
diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c b/drivers/platform/x86/lenovo/thinkpad_acpi.c
index e1cee42a1683..889db802185a 100644
--- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
+++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
@@ -185,6 +185,7 @@ enum tpacpi_hkey_event_t {
 	TP_HKEY_EV_AMT_TOGGLE		= 0x131a, /* Toggle AMT on/off */
 	TP_HKEY_EV_CAMERASHUTTER_TOGGLE = 0x131b, /* Toggle Camera Shutter */
 	TP_HKEY_EV_DOUBLETAP_TOGGLE	= 0x131c, /* Toggle trackpoint doubletap on/off */
+	TP_HKEY_EV_USB_C_SECURITY	= 0x131e, /* Toggle USB C Security ON/OFF */
 	TP_HKEY_EV_PROFILE_TOGGLE	= 0x131f, /* Toggle platform profile in 2024 systems */
 	TP_HKEY_EV_PROFILE_TOGGLE2	= 0x1401, /* Toggle platform profile in 2025 + systems */
 
@@ -373,6 +374,8 @@ static struct {
 	u32 has_adaptive_kbd:1;
 	u32 kbd_lang:1;
 	u32 trackpoint_doubletap_enable:1;
+	u32 usbc_security_supported:1;
+	u32 usbc_security_enabled:1;
 	struct quirk_entry *quirks;
 } tp_features;
 
@@ -11265,6 +11268,111 @@ static struct ibm_struct hwdd_driver_data = {
 	.name = "hwdd",
 };
 
+/*************************************************************************
+ * USB-C Security subdriver
+ *
+ * HKEY.USCS(0) is a read-only ACPI method; its argument is ignored.
+ * It always returns:
+ *   bit 16 - USB-C security capability present on this SKU or not
+ *   bit  0 - USB-C Security state (enable or disable)
+ *
+ * Hotkey
+ * ------
+ * 0x131e (Fn+U, Fn+S): firmware toggles USBS before firing the event.
+ * The driver reads back the new state and notifies the sysfs attribute.
+ *
+ */
+
+/* USCS() return word bit layout */
+#define USCS_CAP_BIT		BIT(16)	/* capability: feature present on SKU */
+#define USCS_STATUS_BIT		BIT(0)	/* current security state */
+
+static DEFINE_MUTEX(usbc_security_mutex);
+
+/*
+ * usbc_security_query - read current USB-C security state via USCS()
+ * @enabled: out - true when security is ON (data connections blocked)
+ *
+ * Returns true if the feature is supported and query succeeded,
+ * false otherwise (feature absent or ACPI call failed).
+ */
+static bool usbc_security_query(bool *enabled)
+{
+	int status;
+
+	mutex_lock(&usbc_security_mutex);
+	if (!acpi_evalf(hkey_handle, &status, "USCS", "dd", 0)) {
+		mutex_unlock(&usbc_security_mutex);
+		return false;
+	}
+	mutex_unlock(&usbc_security_mutex);
+
+	if (!(status & USCS_CAP_BIT)) {
+		pr_debug("USCS cap bit absent (raw=0x%x)\n", status);
+		return false;
+	}
+
+	*enabled = !!(status & USCS_STATUS_BIT);
+	return true;
+}
+
+/* sysfs: /sys/devices/platform/thinkpad_acpi/usb_c_security ---------- */
+static ssize_t usb_c_security_show(struct device *dev,
+				   struct device_attribute *attr,
+				   char *buf)
+{
+	return sysfs_emit(buf, "%s\n",
+			  tp_features.usbc_security_enabled ? "enabled" : "disabled");
+}
+
+static DEVICE_ATTR_RO(usb_c_security);
+
+static struct attribute *usbc_security_attributes[] = {
+	&dev_attr_usb_c_security.attr,
+	NULL,
+};
+
+static umode_t usbc_security_attr_is_visible(struct kobject *kobj,
+					     struct attribute *attr, int n)
+{
+	return tp_features.usbc_security_supported ? attr->mode : 0;
+}
+
+static const struct attribute_group usbc_security_attr_group = {
+	.is_visible = usbc_security_attr_is_visible,
+	.attrs = usbc_security_attributes,
+};
+
+static int tpacpi_usbc_security_init(struct ibm_init_struct *iibm)
+{
+	bool enabled;
+
+	tp_features.usbc_security_supported =
+		usbc_security_query(&enabled);
+	tp_features.usbc_security_enabled = enabled;
+	return 0;
+}
+
+/* tpacpi_usbc_security_hotkey - handle Fn+U Fn+S hotkey (0x131e) */
+static bool tpacpi_usbc_security_hotkey(void)
+{
+	bool enabled;
+
+	if (!tp_features.usbc_security_supported)
+		return false;
+
+	if (!usbc_security_query(&enabled))
+		return false;
+
+	tp_features.usbc_security_enabled = enabled;
+	sysfs_notify(&tpacpi_pdev->dev.kobj, NULL, "usb_c_security");
+	return true;
+}
+
+static struct ibm_struct usbc_security_driver_data = {
+	.name = "usbc_security",
+};
+
 /* --------------------------------------------------------------------- */
 
 static struct attribute *tpacpi_driver_attributes[] = {
@@ -11325,6 +11433,7 @@ static const struct attribute_group *tpacpi_groups[] = {
 	&dprc_attr_group,
 	&auxmac_attr_group,
 	&hwdd_attr_group,
+	&usbc_security_attr_group,
 	NULL,
 };
 
@@ -11479,6 +11588,8 @@ static bool tpacpi_driver_event(const unsigned int hkey_event)
 	case TP_HKEY_EV_PROFILE_TOGGLE2:
 		platform_profile_cycle();
 		return true;
+	case TP_HKEY_EV_USB_C_SECURITY:
+		return tpacpi_usbc_security_hotkey();
 	}
 
 	return false;
@@ -11930,6 +12041,10 @@ static struct ibm_init_struct ibms_init[] __initdata = {
 		.init = tpacpi_hwdd_init,
 		.data = &hwdd_driver_data,
 	},
+	{
+		.init = tpacpi_usbc_security_init,
+		.data = &usbc_security_driver_data,
+	},
 };
 
 static int __init set_ibm_param(const char *val, const struct kernel_param *kp)
-- 
2.51.0

Re: [PATCH] platform/x86: thinkpad_acpi: Add USB-C Security (USCS) support

[Ilpo Järvinen]

On Fri, 22 May 2026, Vishnu Sankar wrote:

> Newer ThinkPad systems expose a USB-C Security (Restricted Mode) feature.
> When active, USB-C data connections are disabled while power delivery is
> preserved. This is useful for kiosk and physically-secured deployments.
> 
> Hardware interface:
> 
> The HKEY device exposes a read-only ACPI method USCS():
> 
>   Return value bit layout:
>     Bit 16 : Capability flag (1 = feature present on this SKU)
>     Bit  0 : Current state  (0 = security OFF, 1 = security ON)
> 
> The sysfs attribute is read-only.
> 
> The Fn+U followed by Fn+S hotkey chord is the only way to toggle the
> hardware state.
> 
> Hotkey:
> 
> Fn+U followed by Fn+S generates HKEY event 0x131e.
> 
> sysfs interface:
> 
>   /sys/devices/platform/thinkpad_acpi/usb_c_security  (read-only)
>   "enabled\n"  -- data connections are currently blocked
>   "disabled\n" -- data connections are currently allowed
> 
>   The attribute is hidden on SKUs where the USCS capability bit (bit 16)
>   is not set, so there is no ABI impact on unsupported hardware.
> 
> Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> Signed-off-by: Vishnu Sankar <vishnuocv@gmail.com>
> ---
>  .../admin-guide/laptops/thinkpad-acpi.rst     |  24 ++++
>  drivers/platform/x86/lenovo/thinkpad_acpi.c   | 115 ++++++++++++++++++
>  2 files changed, 139 insertions(+)
> 
> diff --git a/Documentation/admin-guide/laptops/thinkpad-acpi.rst b/Documentation/admin-guide/laptops/thinkpad-acpi.rst
> index f874db31801d..db4588af0278 100644
> --- a/Documentation/admin-guide/laptops/thinkpad-acpi.rst
> +++ b/Documentation/admin-guide/laptops/thinkpad-acpi.rst
> @@ -1543,6 +1543,30 @@ Values:
>  
>  	This setting can also be toggled via the Fn+doubletap hotkey.
>  
> +USB-C Security
> +--------------
> +
> +sysfs: usb_c_security
> +
> +Reports the current state of the USB-C Security (Restricted Mode) feature
> +on supported ThinkPad systems. When enabled, USB-C data connections are
> +disabled while power delivery is preserved.
> +
> +The available command is::
> +
> +        cat /sys/devices/platform/thinkpad_acpi/usb_c_security
> +
> +Values:
> +
> +	* ``enabled``  - USB-C data connections are currently blocked
> +	* ``disabled`` - USB-C data connections are currently allowed
> +
> +The attribute is read-only. The USB-C Security state can only be toggled
> +via the Fn+U followed by Fn+S hotkey chord.
> +
> +The sysfs attribute is not created on platforms that do not support this
> +feature.
> +
>  Auxmac
>  ------
>  
> diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c b/drivers/platform/x86/lenovo/thinkpad_acpi.c
> index e1cee42a1683..889db802185a 100644
> --- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
> +++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
> @@ -185,6 +185,7 @@ enum tpacpi_hkey_event_t {
>  	TP_HKEY_EV_AMT_TOGGLE		= 0x131a, /* Toggle AMT on/off */
>  	TP_HKEY_EV_CAMERASHUTTER_TOGGLE = 0x131b, /* Toggle Camera Shutter */
>  	TP_HKEY_EV_DOUBLETAP_TOGGLE	= 0x131c, /* Toggle trackpoint doubletap on/off */
> +	TP_HKEY_EV_USB_C_SECURITY	= 0x131e, /* Toggle USB C Security ON/OFF */
>  	TP_HKEY_EV_PROFILE_TOGGLE	= 0x131f, /* Toggle platform profile in 2024 systems */
>  	TP_HKEY_EV_PROFILE_TOGGLE2	= 0x1401, /* Toggle platform profile in 2025 + systems */
>  
> @@ -373,6 +374,8 @@ static struct {
>  	u32 has_adaptive_kbd:1;
>  	u32 kbd_lang:1;
>  	u32 trackpoint_doubletap_enable:1;
> +	u32 usbc_security_supported:1;
> +	u32 usbc_security_enabled:1;
>  	struct quirk_entry *quirks;
>  } tp_features;
>  
> @@ -11265,6 +11268,111 @@ static struct ibm_struct hwdd_driver_data = {
>  	.name = "hwdd",
>  };
>  
> +/*************************************************************************
> + * USB-C Security subdriver
> + *
> + * HKEY.USCS(0) is a read-only ACPI method; its argument is ignored.
> + * It always returns:
> + *   bit 16 - USB-C security capability present on this SKU or not
> + *   bit  0 - USB-C Security state (enable or disable)
> + *
> + * Hotkey
> + * ------
> + * 0x131e (Fn+U, Fn+S): firmware toggles USBS before firing the event.
> + * The driver reads back the new state and notifies the sysfs attribute.
> + *
> + */
> +
> +/* USCS() return word bit layout */
> +#define USCS_CAP_BIT		BIT(16)	/* capability: feature present on SKU */
> +#define USCS_STATUS_BIT		BIT(0)	/* current security state */
> +
> +static DEFINE_MUTEX(usbc_security_mutex);
> +
> +/*
> + * usbc_security_query - read current USB-C security state via USCS()
> + * @enabled: out - true when security is ON (data connections blocked)
> + *
> + * Returns true if the feature is supported and query succeeded,
> + * false otherwise (feature absent or ACPI call failed).
> + */
> +static bool usbc_security_query(bool *enabled)
> +{
> +	int status;
> +
> +	mutex_lock(&usbc_security_mutex);
> +	if (!acpi_evalf(hkey_handle, &status, "USCS", "dd", 0)) {
> +		mutex_unlock(&usbc_security_mutex);
> +		return false;
> +	}
> +	mutex_unlock(&usbc_security_mutex);

Please use cleanup.h.

> +
> +	if (!(status & USCS_CAP_BIT)) {
> +		pr_debug("USCS cap bit absent (raw=0x%x)\n", status);
> +		return false;
> +	}
> +
> +	*enabled = !!(status & USCS_STATUS_BIT);

No need to do !! when assigning to bool.

> +	return true;
> +}
> +
> +/* sysfs: /sys/devices/platform/thinkpad_acpi/usb_c_security ---------- */
> +static ssize_t usb_c_security_show(struct device *dev,
> +				   struct device_attribute *attr,
> +				   char *buf)
> +{
> +	return sysfs_emit(buf, "%s\n",
> +			  tp_features.usbc_security_enabled ? "enabled" : "disabled");

Please use string_choices.h.

> +}
> +
> +static DEVICE_ATTR_RO(usb_c_security);
> +
> +static struct attribute *usbc_security_attributes[] = {
> +	&dev_attr_usb_c_security.attr,
> +	NULL,
> +};
> +
> +static umode_t usbc_security_attr_is_visible(struct kobject *kobj,
> +					     struct attribute *attr, int n)
> +{
> +	return tp_features.usbc_security_supported ? attr->mode : 0;
> +}
> +
> +static const struct attribute_group usbc_security_attr_group = {
> +	.is_visible = usbc_security_attr_is_visible,
> +	.attrs = usbc_security_attributes,
> +};
> +
> +static int tpacpi_usbc_security_init(struct ibm_init_struct *iibm)
> +{
> +	bool enabled;
> +
> +	tp_features.usbc_security_supported =
> +		usbc_security_query(&enabled);

Fits to one line.

> +	tp_features.usbc_security_enabled = enabled;

I'm not sure I follow the logic here as you always seem set it enabled 
disregarding even support or not?

And that hotkey enabling seems deadcode with it already enabled here.

I feel like I must blind to something obvious but cannot just find 
what that would be.

> +	return 0;
> +}
> +
> +/* tpacpi_usbc_security_hotkey - handle Fn+U Fn+S hotkey (0x131e) */
> +static bool tpacpi_usbc_security_hotkey(void)
> +{
> +	bool enabled;
> +
> +	if (!tp_features.usbc_security_supported)
> +		return false;
> +
> +	if (!usbc_security_query(&enabled))
> +		return false;
> +
> +	tp_features.usbc_security_enabled = enabled;
> +	sysfs_notify(&tpacpi_pdev->dev.kobj, NULL, "usb_c_security");
> +	return true;
> +}
> +
> +static struct ibm_struct usbc_security_driver_data = {
> +	.name = "usbc_security",
> +};
> +
>  /* --------------------------------------------------------------------- */
>  
>  static struct attribute *tpacpi_driver_attributes[] = {
> @@ -11325,6 +11433,7 @@ static const struct attribute_group *tpacpi_groups[] = {
>  	&dprc_attr_group,
>  	&auxmac_attr_group,
>  	&hwdd_attr_group,
> +	&usbc_security_attr_group,
>  	NULL,
>  };
>  
> @@ -11479,6 +11588,8 @@ static bool tpacpi_driver_event(const unsigned int hkey_event)
>  	case TP_HKEY_EV_PROFILE_TOGGLE2:
>  		platform_profile_cycle();
>  		return true;
> +	case TP_HKEY_EV_USB_C_SECURITY:
> +		return tpacpi_usbc_security_hotkey();
>  	}
>  
>  	return false;
> @@ -11930,6 +12041,10 @@ static struct ibm_init_struct ibms_init[] __initdata = {
>  		.init = tpacpi_hwdd_init,
>  		.data = &hwdd_driver_data,
>  	},
> +	{
> +		.init = tpacpi_usbc_security_init,
> +		.data = &usbc_security_driver_data,
> +	},
>  };
>  
>  static int __init set_ibm_param(const char *val, const struct kernel_param *kp)
> 

-- 
 i.

Re: [PATCH] platform/x86: thinkpad_acpi: Add USB-C Security (USCS) support

[Vishnu Sankar]

Hi Ilpo,

Thank you for the comments.

On Tue, May 26, 2026 at 2:19 AM Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Fri, 22 May 2026, Vishnu Sankar wrote:
>
> > Newer ThinkPad systems expose a USB-C Security (Restricted Mode) feature.
> > When active, USB-C data connections are disabled while power delivery is
> > preserved. This is useful for kiosk and physically-secured deployments.
> >
> > Hardware interface:
> >
> > The HKEY device exposes a read-only ACPI method USCS():
> >
> >   Return value bit layout:
> >     Bit 16 : Capability flag (1 = feature present on this SKU)
> >     Bit  0 : Current state  (0 = security OFF, 1 = security ON)
> >
> > The sysfs attribute is read-only.
> >
> > The Fn+U followed by Fn+S hotkey chord is the only way to toggle the
> > hardware state.
> >
> > Hotkey:
> >
> > Fn+U followed by Fn+S generates HKEY event 0x131e.
> >
> > sysfs interface:
> >
> >   /sys/devices/platform/thinkpad_acpi/usb_c_security  (read-only)
> >   "enabled\n"  -- data connections are currently blocked
> >   "disabled\n" -- data connections are currently allowed
> >
> >   The attribute is hidden on SKUs where the USCS capability bit (bit 16)
> >   is not set, so there is no ABI impact on unsupported hardware.
> >
> > Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> > Signed-off-by: Vishnu Sankar <vishnuocv@gmail.com>
> > ---
> >  .../admin-guide/laptops/thinkpad-acpi.rst     |  24 ++++
> >  drivers/platform/x86/lenovo/thinkpad_acpi.c   | 115 ++++++++++++++++++
> >  2 files changed, 139 insertions(+)
> >
> > diff --git a/Documentation/admin-guide/laptops/thinkpad-acpi.rst b/Documentation/admin-guide/laptops/thinkpad-acpi.rst
> > index f874db31801d..db4588af0278 100644
> > --- a/Documentation/admin-guide/laptops/thinkpad-acpi.rst
> > +++ b/Documentation/admin-guide/laptops/thinkpad-acpi.rst
> > @@ -1543,6 +1543,30 @@ Values:
> >
> >       This setting can also be toggled via the Fn+doubletap hotkey.
> >
> > +USB-C Security
> > +--------------
> > +
> > +sysfs: usb_c_security
> > +
> > +Reports the current state of the USB-C Security (Restricted Mode) feature
> > +on supported ThinkPad systems. When enabled, USB-C data connections are
> > +disabled while power delivery is preserved.
> > +
> > +The available command is::
> > +
> > +        cat /sys/devices/platform/thinkpad_acpi/usb_c_security
> > +
> > +Values:
> > +
> > +     * ``enabled``  - USB-C data connections are currently blocked
> > +     * ``disabled`` - USB-C data connections are currently allowed
> > +
> > +The attribute is read-only. The USB-C Security state can only be toggled
> > +via the Fn+U followed by Fn+S hotkey chord.
> > +
> > +The sysfs attribute is not created on platforms that do not support this
> > +feature.
> > +
> >  Auxmac
> >  ------
> >
> > diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c b/drivers/platform/x86/lenovo/thinkpad_acpi.c
> > index e1cee42a1683..889db802185a 100644
> > --- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
> > +++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
> > @@ -185,6 +185,7 @@ enum tpacpi_hkey_event_t {
> >       TP_HKEY_EV_AMT_TOGGLE           = 0x131a, /* Toggle AMT on/off */
> >       TP_HKEY_EV_CAMERASHUTTER_TOGGLE = 0x131b, /* Toggle Camera Shutter */
> >       TP_HKEY_EV_DOUBLETAP_TOGGLE     = 0x131c, /* Toggle trackpoint doubletap on/off */
> > +     TP_HKEY_EV_USB_C_SECURITY       = 0x131e, /* Toggle USB C Security ON/OFF */
> >       TP_HKEY_EV_PROFILE_TOGGLE       = 0x131f, /* Toggle platform profile in 2024 systems */
> >       TP_HKEY_EV_PROFILE_TOGGLE2      = 0x1401, /* Toggle platform profile in 2025 + systems */
> >
> > @@ -373,6 +374,8 @@ static struct {
> >       u32 has_adaptive_kbd:1;
> >       u32 kbd_lang:1;
> >       u32 trackpoint_doubletap_enable:1;
> > +     u32 usbc_security_supported:1;
> > +     u32 usbc_security_enabled:1;
> >       struct quirk_entry *quirks;
> >  } tp_features;
> >
> > @@ -11265,6 +11268,111 @@ static struct ibm_struct hwdd_driver_data = {
> >       .name = "hwdd",
> >  };
> >
> > +/*************************************************************************
> > + * USB-C Security subdriver
> > + *
> > + * HKEY.USCS(0) is a read-only ACPI method; its argument is ignored.
> > + * It always returns:
> > + *   bit 16 - USB-C security capability present on this SKU or not
> > + *   bit  0 - USB-C Security state (enable or disable)
> > + *
> > + * Hotkey
> > + * ------
> > + * 0x131e (Fn+U, Fn+S): firmware toggles USBS before firing the event.
> > + * The driver reads back the new state and notifies the sysfs attribute.
> > + *
> > + */
> > +
> > +/* USCS() return word bit layout */
> > +#define USCS_CAP_BIT         BIT(16) /* capability: feature present on SKU */
> > +#define USCS_STATUS_BIT              BIT(0)  /* current security state */
> > +
> > +static DEFINE_MUTEX(usbc_security_mutex);
> > +
> > +/*
> > + * usbc_security_query - read current USB-C security state via USCS()
> > + * @enabled: out - true when security is ON (data connections blocked)
> > + *
> > + * Returns true if the feature is supported and query succeeded,
> > + * false otherwise (feature absent or ACPI call failed).
> > + */
> > +static bool usbc_security_query(bool *enabled)
> > +{
> > +     int status;
> > +
> > +     mutex_lock(&usbc_security_mutex);
> > +     if (!acpi_evalf(hkey_handle, &status, "USCS", "dd", 0)) {
> > +             mutex_unlock(&usbc_security_mutex);
> > +             return false;
> > +     }
> > +     mutex_unlock(&usbc_security_mutex);
>
> Please use cleanup.h.
Will do.
Will replace the manual mutex_lock/unlock pattern with guard(mutex).
>
> > +
> > +     if (!(status & USCS_CAP_BIT)) {
> > +             pr_debug("USCS cap bit absent (raw=0x%x)\n", status);
> > +             return false;
> > +     }
> > +
> > +     *enabled = !!(status & USCS_STATUS_BIT);
>
> No need to do !! when assigning to bool.
Agreed, will remove the !!.
>
> > +     return true;
> > +}
> > +
> > +/* sysfs: /sys/devices/platform/thinkpad_acpi/usb_c_security ---------- */
> > +static ssize_t usb_c_security_show(struct device *dev,
> > +                                struct device_attribute *attr,
> > +                                char *buf)
> > +{
> > +     return sysfs_emit(buf, "%s\n",
> > +                       tp_features.usbc_security_enabled ? "enabled" : "disabled");
>
> Please use string_choices.h.
Will use str_enabled_disabled() which is already used in
thinkpad_acpi.c for similar pattern.
>
> > +}
> > +
> > +static DEVICE_ATTR_RO(usb_c_security);
> > +
> > +static struct attribute *usbc_security_attributes[] = {
> > +     &dev_attr_usb_c_security.attr,
> > +     NULL,
> > +};
> > +
> > +static umode_t usbc_security_attr_is_visible(struct kobject *kobj,
> > +                                          struct attribute *attr, int n)
> > +{
> > +     return tp_features.usbc_security_supported ? attr->mode : 0;
> > +}
> > +
> > +static const struct attribute_group usbc_security_attr_group = {
> > +     .is_visible = usbc_security_attr_is_visible,
> > +     .attrs = usbc_security_attributes,
> > +};
> > +
> > +static int tpacpi_usbc_security_init(struct ibm_init_struct *iibm)
> > +{
> > +     bool enabled;
> > +
> > +     tp_features.usbc_security_supported =
> > +             usbc_security_query(&enabled);
>
> Fits to one line.
Acked.
>
> > +     tp_features.usbc_security_enabled = enabled;
>
> I'm not sure I follow the logic here as you always seem set it enabled
> disregarding even support or not?
>
> And that hotkey enabling seems deadcode with it already enabled here.
>
> I feel like I must blind to something obvious but cannot just find
> what that would be.
I'm sorry, this is a bug.
I will change to the following:
    tp_features.usbc_security_supported = usbc_security_query(&enabled);
    if (tp_features.usbc_security_supported)
        tp_features.usbc_security_enabled = enabled;

The hotkey path is not dead code — it updates usbc_security_enabled
when the user presses Fn+U Fn+S, since the EC firmware toggles the
state and fires the event. The init only captures the state at boot;
subsequent toggles come through the hotkey. Sorry for the confusion in
the init code.
>
> > +     return 0;
> > +}
> > +
> > +/* tpacpi_usbc_security_hotkey - handle Fn+U Fn+S hotkey (0x131e) */
> > +static bool tpacpi_usbc_security_hotkey(void)
> > +{
> > +     bool enabled;
> > +
> > +     if (!tp_features.usbc_security_supported)
> > +             return false;
> > +
> > +     if (!usbc_security_query(&enabled))
> > +             return false;
> > +
> > +     tp_features.usbc_security_enabled = enabled;
> > +     sysfs_notify(&tpacpi_pdev->dev.kobj, NULL, "usb_c_security");
> > +     return true;
> > +}
> > +
> > +static struct ibm_struct usbc_security_driver_data = {
> > +     .name = "usbc_security",
> > +};
> > +
> >  /* --------------------------------------------------------------------- */
> >
> >  static struct attribute *tpacpi_driver_attributes[] = {
> > @@ -11325,6 +11433,7 @@ static const struct attribute_group *tpacpi_groups[] = {
> >       &dprc_attr_group,
> >       &auxmac_attr_group,
> >       &hwdd_attr_group,
> > +     &usbc_security_attr_group,
> >       NULL,
> >  };
> >
> > @@ -11479,6 +11588,8 @@ static bool tpacpi_driver_event(const unsigned int hkey_event)
> >       case TP_HKEY_EV_PROFILE_TOGGLE2:
> >               platform_profile_cycle();
> >               return true;
> > +     case TP_HKEY_EV_USB_C_SECURITY:
> > +             return tpacpi_usbc_security_hotkey();
> >       }
> >
> >       return false;
> > @@ -11930,6 +12041,10 @@ static struct ibm_init_struct ibms_init[] __initdata = {
> >               .init = tpacpi_hwdd_init,
> >               .data = &hwdd_driver_data,
> >       },
> > +     {
> > +             .init = tpacpi_usbc_security_init,
> > +             .data = &usbc_security_driver_data,
> > +     },
> >  };
> >
> >  static int __init set_ibm_param(const char *val, const struct kernel_param *kp)
> >
>
> --
>  i.
>

I will update the v2 with all these changes soon, if no further comments.
-- 

Regards,

      Vishnu Sankar

Re: [PATCH] platform/x86: thinkpad_acpi: Add USB-C Security (USCS) support

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 11657 bytes --]

On Tue, 26 May 2026, Vishnu Sankar wrote:

> Hi Ilpo,
> 
> Thank you for the comments.
> 
> On Tue, May 26, 2026 at 2:19 AM Ilpo Järvinen
> <ilpo.jarvinen@linux.intel.com> wrote:
> >
> > On Fri, 22 May 2026, Vishnu Sankar wrote:
> >
> > > Newer ThinkPad systems expose a USB-C Security (Restricted Mode) feature.
> > > When active, USB-C data connections are disabled while power delivery is
> > > preserved. This is useful for kiosk and physically-secured deployments.
> > >
> > > Hardware interface:
> > >
> > > The HKEY device exposes a read-only ACPI method USCS():
> > >
> > >   Return value bit layout:
> > >     Bit 16 : Capability flag (1 = feature present on this SKU)
> > >     Bit  0 : Current state  (0 = security OFF, 1 = security ON)
> > >
> > > The sysfs attribute is read-only.
> > >
> > > The Fn+U followed by Fn+S hotkey chord is the only way to toggle the
> > > hardware state.
> > >
> > > Hotkey:
> > >
> > > Fn+U followed by Fn+S generates HKEY event 0x131e.
> > >
> > > sysfs interface:
> > >
> > >   /sys/devices/platform/thinkpad_acpi/usb_c_security  (read-only)
> > >   "enabled\n"  -- data connections are currently blocked
> > >   "disabled\n" -- data connections are currently allowed
> > >
> > >   The attribute is hidden on SKUs where the USCS capability bit (bit 16)
> > >   is not set, so there is no ABI impact on unsupported hardware.
> > >
> > > Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> > > Signed-off-by: Vishnu Sankar <vishnuocv@gmail.com>
> > > ---
> > >  .../admin-guide/laptops/thinkpad-acpi.rst     |  24 ++++
> > >  drivers/platform/x86/lenovo/thinkpad_acpi.c   | 115 ++++++++++++++++++
> > >  2 files changed, 139 insertions(+)
> > >
> > > diff --git a/Documentation/admin-guide/laptops/thinkpad-acpi.rst b/Documentation/admin-guide/laptops/thinkpad-acpi.rst
> > > index f874db31801d..db4588af0278 100644
> > > --- a/Documentation/admin-guide/laptops/thinkpad-acpi.rst
> > > +++ b/Documentation/admin-guide/laptops/thinkpad-acpi.rst
> > > @@ -1543,6 +1543,30 @@ Values:
> > >
> > >       This setting can also be toggled via the Fn+doubletap hotkey.
> > >
> > > +USB-C Security
> > > +--------------
> > > +
> > > +sysfs: usb_c_security
> > > +
> > > +Reports the current state of the USB-C Security (Restricted Mode) feature
> > > +on supported ThinkPad systems. When enabled, USB-C data connections are
> > > +disabled while power delivery is preserved.
> > > +
> > > +The available command is::
> > > +
> > > +        cat /sys/devices/platform/thinkpad_acpi/usb_c_security
> > > +
> > > +Values:
> > > +
> > > +     * ``enabled``  - USB-C data connections are currently blocked
> > > +     * ``disabled`` - USB-C data connections are currently allowed
> > > +
> > > +The attribute is read-only. The USB-C Security state can only be toggled
> > > +via the Fn+U followed by Fn+S hotkey chord.
> > > +
> > > +The sysfs attribute is not created on platforms that do not support this
> > > +feature.
> > > +
> > >  Auxmac
> > >  ------
> > >
> > > diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c b/drivers/platform/x86/lenovo/thinkpad_acpi.c
> > > index e1cee42a1683..889db802185a 100644
> > > --- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
> > > +++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
> > > @@ -185,6 +185,7 @@ enum tpacpi_hkey_event_t {
> > >       TP_HKEY_EV_AMT_TOGGLE           = 0x131a, /* Toggle AMT on/off */
> > >       TP_HKEY_EV_CAMERASHUTTER_TOGGLE = 0x131b, /* Toggle Camera Shutter */
> > >       TP_HKEY_EV_DOUBLETAP_TOGGLE     = 0x131c, /* Toggle trackpoint doubletap on/off */
> > > +     TP_HKEY_EV_USB_C_SECURITY       = 0x131e, /* Toggle USB C Security ON/OFF */
> > >       TP_HKEY_EV_PROFILE_TOGGLE       = 0x131f, /* Toggle platform profile in 2024 systems */
> > >       TP_HKEY_EV_PROFILE_TOGGLE2      = 0x1401, /* Toggle platform profile in 2025 + systems */
> > >
> > > @@ -373,6 +374,8 @@ static struct {
> > >       u32 has_adaptive_kbd:1;
> > >       u32 kbd_lang:1;
> > >       u32 trackpoint_doubletap_enable:1;
> > > +     u32 usbc_security_supported:1;
> > > +     u32 usbc_security_enabled:1;
> > >       struct quirk_entry *quirks;
> > >  } tp_features;
> > >
> > > @@ -11265,6 +11268,111 @@ static struct ibm_struct hwdd_driver_data = {
> > >       .name = "hwdd",
> > >  };
> > >
> > > +/*************************************************************************
> > > + * USB-C Security subdriver
> > > + *
> > > + * HKEY.USCS(0) is a read-only ACPI method; its argument is ignored.
> > > + * It always returns:
> > > + *   bit 16 - USB-C security capability present on this SKU or not
> > > + *   bit  0 - USB-C Security state (enable or disable)
> > > + *
> > > + * Hotkey
> > > + * ------
> > > + * 0x131e (Fn+U, Fn+S): firmware toggles USBS before firing the event.
> > > + * The driver reads back the new state and notifies the sysfs attribute.
> > > + *
> > > + */
> > > +
> > > +/* USCS() return word bit layout */
> > > +#define USCS_CAP_BIT         BIT(16) /* capability: feature present on SKU */
> > > +#define USCS_STATUS_BIT              BIT(0)  /* current security state */
> > > +
> > > +static DEFINE_MUTEX(usbc_security_mutex);
> > > +
> > > +/*
> > > + * usbc_security_query - read current USB-C security state via USCS()
> > > + * @enabled: out - true when security is ON (data connections blocked)
> > > + *
> > > + * Returns true if the feature is supported and query succeeded,
> > > + * false otherwise (feature absent or ACPI call failed).
> > > + */
> > > +static bool usbc_security_query(bool *enabled)
> > > +{
> > > +     int status;
> > > +
> > > +     mutex_lock(&usbc_security_mutex);
> > > +     if (!acpi_evalf(hkey_handle, &status, "USCS", "dd", 0)) {
> > > +             mutex_unlock(&usbc_security_mutex);
> > > +             return false;
> > > +     }
> > > +     mutex_unlock(&usbc_security_mutex);
> >
> > Please use cleanup.h.
> Will do.
> Will replace the manual mutex_lock/unlock pattern with guard(mutex).
> >
> > > +
> > > +     if (!(status & USCS_CAP_BIT)) {
> > > +             pr_debug("USCS cap bit absent (raw=0x%x)\n", status);
> > > +             return false;
> > > +     }
> > > +
> > > +     *enabled = !!(status & USCS_STATUS_BIT);
> >
> > No need to do !! when assigning to bool.
> Agreed, will remove the !!.
> >
> > > +     return true;
> > > +}
> > > +
> > > +/* sysfs: /sys/devices/platform/thinkpad_acpi/usb_c_security ---------- */
> > > +static ssize_t usb_c_security_show(struct device *dev,
> > > +                                struct device_attribute *attr,
> > > +                                char *buf)
> > > +{
> > > +     return sysfs_emit(buf, "%s\n",
> > > +                       tp_features.usbc_security_enabled ? "enabled" : "disabled");
> >
> > Please use string_choices.h.
> Will use str_enabled_disabled() which is already used in
> thinkpad_acpi.c for similar pattern.
> >
> > > +}
> > > +
> > > +static DEVICE_ATTR_RO(usb_c_security);
> > > +
> > > +static struct attribute *usbc_security_attributes[] = {
> > > +     &dev_attr_usb_c_security.attr,
> > > +     NULL,
> > > +};
> > > +
> > > +static umode_t usbc_security_attr_is_visible(struct kobject *kobj,
> > > +                                          struct attribute *attr, int n)
> > > +{
> > > +     return tp_features.usbc_security_supported ? attr->mode : 0;
> > > +}
> > > +
> > > +static const struct attribute_group usbc_security_attr_group = {
> > > +     .is_visible = usbc_security_attr_is_visible,
> > > +     .attrs = usbc_security_attributes,
> > > +};
> > > +
> > > +static int tpacpi_usbc_security_init(struct ibm_init_struct *iibm)
> > > +{
> > > +     bool enabled;
> > > +
> > > +     tp_features.usbc_security_supported =
> > > +             usbc_security_query(&enabled);
> >
> > Fits to one line.
> Acked.
> >
> > > +     tp_features.usbc_security_enabled = enabled;
> >
> > I'm not sure I follow the logic here as you always seem set it enabled
> > disregarding even support or not?
> >
> > And that hotkey enabling seems deadcode with it already enabled here.
> >
> > I feel like I must blind to something obvious but cannot just find
> > what that would be.
> I'm sorry, this is a bug.
> I will change to the following:
>     tp_features.usbc_security_supported = usbc_security_query(&enabled);
>     if (tp_features.usbc_security_supported)
>         tp_features.usbc_security_enabled = enabled;
>
> The hotkey path is not dead code — it updates usbc_security_enabled
> when the user presses Fn+U Fn+S, since the EC firmware toggles the
> state and fires the event. The init only captures the state at boot;
> subsequent toggles come through the hotkey. Sorry for the confusion in
> the init code.

Oh, I see now what I was missing and it's just as obvious I was I 
thinking... I, for some reason, read these as:

	tp_features.usbc_security_enabled = true;

...but you had = enabled there. I'm sorry about my confusion.

I find the the way usbc_security_query() is architected a bit confusing 
though, it returns supported/not supported in return value and another 
boolean through the bool pointer. And that looks broken, if 
usbc_security_query() returns early, it won't set *enabled which then 
remains uninitialized (contains pseudogarbage from stack so your testing 
might have been luck and had it always working the way you wanted).

> > > +     return 0;
> > > +}
> > > +
> > > +/* tpacpi_usbc_security_hotkey - handle Fn+U Fn+S hotkey (0x131e) */
> > > +static bool tpacpi_usbc_security_hotkey(void)
> > > +{
> > > +     bool enabled;
> > > +
> > > +     if (!tp_features.usbc_security_supported)
> > > +             return false;
> > > +
> > > +     if (!usbc_security_query(&enabled))
> > > +             return false;
> > > +
> > > +     tp_features.usbc_security_enabled = enabled;
> > > +     sysfs_notify(&tpacpi_pdev->dev.kobj, NULL, "usb_c_security");
> > > +     return true;
> > > +}
> > > +
> > > +static struct ibm_struct usbc_security_driver_data = {
> > > +     .name = "usbc_security",
> > > +};
> > > +
> > >  /* --------------------------------------------------------------------- */
> > >
> > >  static struct attribute *tpacpi_driver_attributes[] = {
> > > @@ -11325,6 +11433,7 @@ static const struct attribute_group *tpacpi_groups[] = {
> > >       &dprc_attr_group,
> > >       &auxmac_attr_group,
> > >       &hwdd_attr_group,
> > > +     &usbc_security_attr_group,
> > >       NULL,
> > >  };
> > >
> > > @@ -11479,6 +11588,8 @@ static bool tpacpi_driver_event(const unsigned int hkey_event)
> > >       case TP_HKEY_EV_PROFILE_TOGGLE2:
> > >               platform_profile_cycle();
> > >               return true;
> > > +     case TP_HKEY_EV_USB_C_SECURITY:
> > > +             return tpacpi_usbc_security_hotkey();
> > >       }
> > >
> > >       return false;
> > > @@ -11930,6 +12041,10 @@ static struct ibm_init_struct ibms_init[] __initdata = {
> > >               .init = tpacpi_hwdd_init,
> > >               .data = &hwdd_driver_data,
> > >       },
> > > +     {
> > > +             .init = tpacpi_usbc_security_init,
> > > +             .data = &usbc_security_driver_data,
> > > +     },
> > >  };
> > >
> > >  static int __init set_ibm_param(const char *val, const struct kernel_param *kp)
> > >
> >
> > --
> >  i.
> >
> 
> I will update the v2 with all these changes soon, if no further comments.
> 

-- 
 i.

Re: [PATCH] platform/x86: thinkpad_acpi: Add USB-C Security (USCS) support

[Vishnu Sankar]

On Tue, May 26, 2026 at 6:16 PM Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Tue, 26 May 2026, Vishnu Sankar wrote:
>
> > Hi Ilpo,
> >
> > Thank you for the comments.
> >
> > On Tue, May 26, 2026 at 2:19 AM Ilpo Järvinen
> > <ilpo.jarvinen@linux.intel.com> wrote:
> > >
> > > On Fri, 22 May 2026, Vishnu Sankar wrote:
> > >
> > > > Newer ThinkPad systems expose a USB-C Security (Restricted Mode) feature.
> > > > When active, USB-C data connections are disabled while power delivery is
> > > > preserved. This is useful for kiosk and physically-secured deployments.
> > > >
> > > > Hardware interface:
> > > >
> > > > The HKEY device exposes a read-only ACPI method USCS():
> > > >
> > > >   Return value bit layout:
> > > >     Bit 16 : Capability flag (1 = feature present on this SKU)
> > > >     Bit  0 : Current state  (0 = security OFF, 1 = security ON)
> > > >
> > > > The sysfs attribute is read-only.
> > > >
> > > > The Fn+U followed by Fn+S hotkey chord is the only way to toggle the
> > > > hardware state.
> > > >
> > > > Hotkey:
> > > >
> > > > Fn+U followed by Fn+S generates HKEY event 0x131e.
> > > >
> > > > sysfs interface:
> > > >
> > > >   /sys/devices/platform/thinkpad_acpi/usb_c_security  (read-only)
> > > >   "enabled\n"  -- data connections are currently blocked
> > > >   "disabled\n" -- data connections are currently allowed
> > > >
> > > >   The attribute is hidden on SKUs where the USCS capability bit (bit 16)
> > > >   is not set, so there is no ABI impact on unsupported hardware.
> > > >
> > > > Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> > > > Signed-off-by: Vishnu Sankar <vishnuocv@gmail.com>
> > > > ---
> > > >  .../admin-guide/laptops/thinkpad-acpi.rst     |  24 ++++
> > > >  drivers/platform/x86/lenovo/thinkpad_acpi.c   | 115 ++++++++++++++++++
> > > >  2 files changed, 139 insertions(+)
> > > >
> > > > diff --git a/Documentation/admin-guide/laptops/thinkpad-acpi.rst b/Documentation/admin-guide/laptops/thinkpad-acpi.rst
> > > > index f874db31801d..db4588af0278 100644
> > > > --- a/Documentation/admin-guide/laptops/thinkpad-acpi.rst
> > > > +++ b/Documentation/admin-guide/laptops/thinkpad-acpi.rst
> > > > @@ -1543,6 +1543,30 @@ Values:
> > > >
> > > >       This setting can also be toggled via the Fn+doubletap hotkey.
> > > >
> > > > +USB-C Security
> > > > +--------------
> > > > +
> > > > +sysfs: usb_c_security
> > > > +
> > > > +Reports the current state of the USB-C Security (Restricted Mode) feature
> > > > +on supported ThinkPad systems. When enabled, USB-C data connections are
> > > > +disabled while power delivery is preserved.
> > > > +
> > > > +The available command is::
> > > > +
> > > > +        cat /sys/devices/platform/thinkpad_acpi/usb_c_security
> > > > +
> > > > +Values:
> > > > +
> > > > +     * ``enabled``  - USB-C data connections are currently blocked
> > > > +     * ``disabled`` - USB-C data connections are currently allowed
> > > > +
> > > > +The attribute is read-only. The USB-C Security state can only be toggled
> > > > +via the Fn+U followed by Fn+S hotkey chord.
> > > > +
> > > > +The sysfs attribute is not created on platforms that do not support this
> > > > +feature.
> > > > +
> > > >  Auxmac
> > > >  ------
> > > >
> > > > diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c b/drivers/platform/x86/lenovo/thinkpad_acpi.c
> > > > index e1cee42a1683..889db802185a 100644
> > > > --- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
> > > > +++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
> > > > @@ -185,6 +185,7 @@ enum tpacpi_hkey_event_t {
> > > >       TP_HKEY_EV_AMT_TOGGLE           = 0x131a, /* Toggle AMT on/off */
> > > >       TP_HKEY_EV_CAMERASHUTTER_TOGGLE = 0x131b, /* Toggle Camera Shutter */
> > > >       TP_HKEY_EV_DOUBLETAP_TOGGLE     = 0x131c, /* Toggle trackpoint doubletap on/off */
> > > > +     TP_HKEY_EV_USB_C_SECURITY       = 0x131e, /* Toggle USB C Security ON/OFF */
> > > >       TP_HKEY_EV_PROFILE_TOGGLE       = 0x131f, /* Toggle platform profile in 2024 systems */
> > > >       TP_HKEY_EV_PROFILE_TOGGLE2      = 0x1401, /* Toggle platform profile in 2025 + systems */
> > > >
> > > > @@ -373,6 +374,8 @@ static struct {
> > > >       u32 has_adaptive_kbd:1;
> > > >       u32 kbd_lang:1;
> > > >       u32 trackpoint_doubletap_enable:1;
> > > > +     u32 usbc_security_supported:1;
> > > > +     u32 usbc_security_enabled:1;
> > > >       struct quirk_entry *quirks;
> > > >  } tp_features;
> > > >
> > > > @@ -11265,6 +11268,111 @@ static struct ibm_struct hwdd_driver_data = {
> > > >       .name = "hwdd",
> > > >  };
> > > >
> > > > +/*************************************************************************
> > > > + * USB-C Security subdriver
> > > > + *
> > > > + * HKEY.USCS(0) is a read-only ACPI method; its argument is ignored.
> > > > + * It always returns:
> > > > + *   bit 16 - USB-C security capability present on this SKU or not
> > > > + *   bit  0 - USB-C Security state (enable or disable)
> > > > + *
> > > > + * Hotkey
> > > > + * ------
> > > > + * 0x131e (Fn+U, Fn+S): firmware toggles USBS before firing the event.
> > > > + * The driver reads back the new state and notifies the sysfs attribute.
> > > > + *
> > > > + */
> > > > +
> > > > +/* USCS() return word bit layout */
> > > > +#define USCS_CAP_BIT         BIT(16) /* capability: feature present on SKU */
> > > > +#define USCS_STATUS_BIT              BIT(0)  /* current security state */
> > > > +
> > > > +static DEFINE_MUTEX(usbc_security_mutex);
> > > > +
> > > > +/*
> > > > + * usbc_security_query - read current USB-C security state via USCS()
> > > > + * @enabled: out - true when security is ON (data connections blocked)
> > > > + *
> > > > + * Returns true if the feature is supported and query succeeded,
> > > > + * false otherwise (feature absent or ACPI call failed).
> > > > + */
> > > > +static bool usbc_security_query(bool *enabled)
> > > > +{
> > > > +     int status;
> > > > +
> > > > +     mutex_lock(&usbc_security_mutex);
> > > > +     if (!acpi_evalf(hkey_handle, &status, "USCS", "dd", 0)) {
> > > > +             mutex_unlock(&usbc_security_mutex);
> > > > +             return false;
> > > > +     }
> > > > +     mutex_unlock(&usbc_security_mutex);
> > >
> > > Please use cleanup.h.
> > Will do.
> > Will replace the manual mutex_lock/unlock pattern with guard(mutex).
> > >
> > > > +
> > > > +     if (!(status & USCS_CAP_BIT)) {
> > > > +             pr_debug("USCS cap bit absent (raw=0x%x)\n", status);
> > > > +             return false;
> > > > +     }
> > > > +
> > > > +     *enabled = !!(status & USCS_STATUS_BIT);
> > >
> > > No need to do !! when assigning to bool.
> > Agreed, will remove the !!.
> > >
> > > > +     return true;
> > > > +}
> > > > +
> > > > +/* sysfs: /sys/devices/platform/thinkpad_acpi/usb_c_security ---------- */
> > > > +static ssize_t usb_c_security_show(struct device *dev,
> > > > +                                struct device_attribute *attr,
> > > > +                                char *buf)
> > > > +{
> > > > +     return sysfs_emit(buf, "%s\n",
> > > > +                       tp_features.usbc_security_enabled ? "enabled" : "disabled");
> > >
> > > Please use string_choices.h.
> > Will use str_enabled_disabled() which is already used in
> > thinkpad_acpi.c for similar pattern.
> > >
> > > > +}
> > > > +
> > > > +static DEVICE_ATTR_RO(usb_c_security);
> > > > +
> > > > +static struct attribute *usbc_security_attributes[] = {
> > > > +     &dev_attr_usb_c_security.attr,
> > > > +     NULL,
> > > > +};
> > > > +
> > > > +static umode_t usbc_security_attr_is_visible(struct kobject *kobj,
> > > > +                                          struct attribute *attr, int n)
> > > > +{
> > > > +     return tp_features.usbc_security_supported ? attr->mode : 0;
> > > > +}
> > > > +
> > > > +static const struct attribute_group usbc_security_attr_group = {
> > > > +     .is_visible = usbc_security_attr_is_visible,
> > > > +     .attrs = usbc_security_attributes,
> > > > +};
> > > > +
> > > > +static int tpacpi_usbc_security_init(struct ibm_init_struct *iibm)
> > > > +{
> > > > +     bool enabled;
> > > > +
> > > > +     tp_features.usbc_security_supported =
> > > > +             usbc_security_query(&enabled);
> > >
> > > Fits to one line.
> > Acked.
> > >
> > > > +     tp_features.usbc_security_enabled = enabled;
> > >
> > > I'm not sure I follow the logic here as you always seem set it enabled
> > > disregarding even support or not?
> > >
> > > And that hotkey enabling seems deadcode with it already enabled here.
> > >
> > > I feel like I must blind to something obvious but cannot just find
> > > what that would be.
> > I'm sorry, this is a bug.
> > I will change to the following:
> >     tp_features.usbc_security_supported = usbc_security_query(&enabled);
> >     if (tp_features.usbc_security_supported)
> >         tp_features.usbc_security_enabled = enabled;
> >
> > The hotkey path is not dead code — it updates usbc_security_enabled
> > when the user presses Fn+U Fn+S, since the EC firmware toggles the
> > state and fires the event. The init only captures the state at boot;
> > subsequent toggles come through the hotkey. Sorry for the confusion in
> > the init code.
>
> Oh, I see now what I was missing and it's just as obvious I was I
> thinking... I, for some reason, read these as:
>
>         tp_features.usbc_security_enabled = true;
>
> ...but you had = enabled there. I'm sorry about my confusion.
>
> I find the the way usbc_security_query() is architected a bit confusing
> though, it returns supported/not supported in return value and another
> boolean through the bool pointer. And that looks broken, if
> usbc_security_query() returns early, it won't set *enabled which then
> remains uninitialized (contains pseudogarbage from stack so your testing
> might have been luck and had it always working the way you wanted).
Yes, the usbc_security_query() is confusing and does have the
uninitialized *enabled bug.
I only tested on systems that have the USB-C Security feature, so the
unsupported path
was never exercised and the bug was not caught.

I will do something like this:

    static int usbc_security_query(bool *enabled)
    {
        int status;

        guard(mutex)(&usbc_security_mutex);
        if (!acpi_evalf(hkey_handle, &status, "USCS", "dd", 0))
            return -EIO;

        if (!(status & USCS_CAP_BIT))
            return -ENODEV;

        *enabled = status & USCS_STATUS_BIT;
        return 0;
    }

With this, *enabled is only ever read after a successful return 0, so
the uninitialized value issue may not happen. Init and hotkey paths
become straightforward as well.
Hope this is fine.
>
> > > > +     return 0;
> > > > +}
> > > > +
> > > > +/* tpacpi_usbc_security_hotkey - handle Fn+U Fn+S hotkey (0x131e) */
> > > > +static bool tpacpi_usbc_security_hotkey(void)
> > > > +{
> > > > +     bool enabled;
> > > > +
> > > > +     if (!tp_features.usbc_security_supported)
> > > > +             return false;
> > > > +
> > > > +     if (!usbc_security_query(&enabled))
> > > > +             return false;
> > > > +
> > > > +     tp_features.usbc_security_enabled = enabled;
> > > > +     sysfs_notify(&tpacpi_pdev->dev.kobj, NULL, "usb_c_security");
> > > > +     return true;
> > > > +}
> > > > +
> > > > +static struct ibm_struct usbc_security_driver_data = {
> > > > +     .name = "usbc_security",
> > > > +};
> > > > +
> > > >  /* --------------------------------------------------------------------- */
> > > >
> > > >  static struct attribute *tpacpi_driver_attributes[] = {
> > > > @@ -11325,6 +11433,7 @@ static const struct attribute_group *tpacpi_groups[] = {
> > > >       &dprc_attr_group,
> > > >       &auxmac_attr_group,
> > > >       &hwdd_attr_group,
> > > > +     &usbc_security_attr_group,
> > > >       NULL,
> > > >  };
> > > >
> > > > @@ -11479,6 +11588,8 @@ static bool tpacpi_driver_event(const unsigned int hkey_event)
> > > >       case TP_HKEY_EV_PROFILE_TOGGLE2:
> > > >               platform_profile_cycle();
> > > >               return true;
> > > > +     case TP_HKEY_EV_USB_C_SECURITY:
> > > > +             return tpacpi_usbc_security_hotkey();
> > > >       }
> > > >
> > > >       return false;
> > > > @@ -11930,6 +12041,10 @@ static struct ibm_init_struct ibms_init[] __initdata = {
> > > >               .init = tpacpi_hwdd_init,
> > > >               .data = &hwdd_driver_data,
> > > >       },
> > > > +     {
> > > > +             .init = tpacpi_usbc_security_init,
> > > > +             .data = &usbc_security_driver_data,
> > > > +     },
> > > >  };
> > > >
> > > >  static int __init set_ibm_param(const char *val, const struct kernel_param *kp)
> > > >
> > >
> > > --
> > >  i.
> > >
> >
> > I will update the v2 with all these changes soon, if no further comments.
> >
>
> --
>  i.



-- 

Regards,

      Vishnu Sankar