[PATCH 0/8] Thunderbolt and DP altmode support for cros-ec-typec

[PATCH 0/8] Thunderbolt and DP altmode support for cros-ec-typec

[Abhishek Pandit-Subedi]

Hi Heikki, Tzung-Bi et al,

This patch series adds support for alternate mode entry for the
cros-ec-typec driver for Displayport and Thunderbolt.

Thunderbolt support is added by adapting an RFC Heikki had posted
previously:

https://lore.kernel.org/linux-usb/20191230152857.43917-1-heikki.krogerus@linux.intel.com/

A few comments on the series:

* The cros-ec interface will not accept any VDOs/VDMs so we simply
  ignore any configurations we are passed (i.e. DPConfigure). This means
  the sysfs control of DP lanes won't work.
* ChromeOS has two modes of operation for alt-modes: entirely EC driven
  or AP-driven from userspace (via the typec daemon). Thus, we never
  expect the kernel alt-mode drivers to auto-enter modes.

This was tested on kernel 6.6 with a ChromeOS Brya device and compile
tested against linux-usb (with allmodconfig).

Thanks,
Abhishek


Abhishek Pandit-Subedi (7):
  usb: typec: altmode_match should handle TYPEC_ANY_MODE
  usb: typec: intel_pmc_mux: Null check before use
  usb: typec: Auto enter control for alternate modes
  platform/chrome: cros_ec_typec: Update partner altmode active
  platform/chrome: cros_ec_typec: Displayport support
  platform/chrome: cros_ec_typec: Thunderbolt support
  platform/chrome: cros_ec_typec: Disable auto_enter

Heikki Krogerus (1):
  usb: typec: Add driver for Thunderbolt 3 Alternate Mode

 Documentation/ABI/testing/sysfs-bus-typec     |   9 +
 MAINTAINERS                                   |   5 +-
 drivers/platform/chrome/Makefile              |   3 +
 drivers/platform/chrome/cros_ec_typec.c       |  55 ++-
 drivers/platform/chrome/cros_ec_typec.h       |   1 +
 drivers/platform/chrome/cros_typec_altmode.h  |  48 +++
 .../platform/chrome/cros_typec_displayport.c  | 247 ++++++++++++++
 .../platform/chrome/cros_typec_thunderbolt.c  | 184 ++++++++++
 drivers/usb/typec/altmodes/Kconfig            |   9 +
 drivers/usb/typec/altmodes/Makefile           |   2 +
 drivers/usb/typec/altmodes/displayport.c      |   6 +-
 drivers/usb/typec/altmodes/thunderbolt.c      | 322 ++++++++++++++++++
 drivers/usb/typec/class.c                     |  34 +-
 drivers/usb/typec/mux/intel_pmc_mux.c         |   9 +-
 include/linux/usb/typec.h                     |   2 +
 include/linux/usb/typec_altmode.h             |   2 +
 16 files changed, 918 insertions(+), 20 deletions(-)
 create mode 100644 drivers/platform/chrome/cros_typec_altmode.h
 create mode 100644 drivers/platform/chrome/cros_typec_displayport.c
 create mode 100644 drivers/platform/chrome/cros_typec_thunderbolt.c
 create mode 100644 drivers/usb/typec/altmodes/thunderbolt.c

-- 
2.46.0.792.g87dc391469-goog

[PATCH 1/8] usb: typec: Add driver for Thunderbolt 3 Alternate Mode

[Abhishek Pandit-Subedi]

From: Heikki Krogerus <heikki.krogerus@linux.intel.com>

Thunderbolt 3 Alternate Mode entry flow is described in
USB Type-C Specification Release 2.0.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
---

Changes:
* Delay cable + plug checks so that the module doesn't fail to probe
  if cable + plug information isn't available by the time the partner
  altmode is registered.
* Remove unncessary brace after if (IS_ERR(plug))

The rest of this patch should be the same as Heikki's original RFC.


 drivers/usb/typec/altmodes/Kconfig       |   9 +
 drivers/usb/typec/altmodes/Makefile      |   2 +
 drivers/usb/typec/altmodes/thunderbolt.c | 321 +++++++++++++++++++++++
 3 files changed, 332 insertions(+)
 create mode 100644 drivers/usb/typec/altmodes/thunderbolt.c

diff --git a/drivers/usb/typec/altmodes/Kconfig b/drivers/usb/typec/altmodes/Kconfig
index 1a6b5e872b0d..7867fa7c405d 100644
--- a/drivers/usb/typec/altmodes/Kconfig
+++ b/drivers/usb/typec/altmodes/Kconfig
@@ -23,4 +23,13 @@ config TYPEC_NVIDIA_ALTMODE
 	  To compile this driver as a module, choose M here: the
 	  module will be called typec_nvidia.
 
+config TYPEC_TBT_ALTMODE
+	tristate "Thunderbolt3 Alternate Mode driver"
+	help
+	  Select this option if you have Thunderbolt3 hardware on your
+	  system.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called typec_thunderbolt.
+
 endmenu
diff --git a/drivers/usb/typec/altmodes/Makefile b/drivers/usb/typec/altmodes/Makefile
index 45717548b396..508a68351bd2 100644
--- a/drivers/usb/typec/altmodes/Makefile
+++ b/drivers/usb/typec/altmodes/Makefile
@@ -4,3 +4,5 @@ obj-$(CONFIG_TYPEC_DP_ALTMODE)		+= typec_displayport.o
 typec_displayport-y			:= displayport.o
 obj-$(CONFIG_TYPEC_NVIDIA_ALTMODE)	+= typec_nvidia.o
 typec_nvidia-y				:= nvidia.o
+obj-$(CONFIG_TYPEC_TBT_ALTMODE)		+= typec_thunderbolt.o
+typec_thunderbolt-y			:= thunderbolt.o
diff --git a/drivers/usb/typec/altmodes/thunderbolt.c b/drivers/usb/typec/altmodes/thunderbolt.c
new file mode 100644
index 000000000000..515e775ee41a
--- /dev/null
+++ b/drivers/usb/typec/altmodes/thunderbolt.c
@@ -0,0 +1,321 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * USB Typec-C Thuderbolt3 Alternate Mode driver
+ *
+ * Copyright (C) 2019 Intel Corporation
+ * Author: Heikki Krogerus <heikki.krogerus@linux.intel.com>
+ */
+
+#include <linux/delay.h>
+#include <linux/mutex.h>
+#include <linux/module.h>
+#include <linux/usb/pd_vdo.h>
+#include <linux/usb/typec_altmode.h>
+
+#define USB_TYPEC_VENDOR_INTEL		0x8087
+
+/* TBT3 Device Discover Mode VDO bits */
+#define TBT_MODE			BIT(0)
+#define TBT_ADAPTER(_vdo_)		(((_vdo_) & BIT(16)) >> 16)
+#define   TBT_ADAPTER_LEGACY		0
+#define   TBT_ADAPTER_TBT3		1
+#define TBT_INTEL_SPECIFIC_B0		BIT(26)
+#define TBT_VENDOR_SPECIFIC_B0		BIT(30)
+#define TBT_VENDOR_SPECIFIC_B1		BIT(31)
+
+/* TBT3 Cable Discover Mode VDO bits */
+#define TBT_CABLE_SPEED(_vdo_)		(((_vdo_) & GENMASK(18, 16)) >> 16)
+#define   TBT_CABLE_USB3_GEN1		1
+#define   TBT_CABLE_USB3_PASSIVE	2
+#define   TBT_CABLE_10_AND_20GBPS	3
+#define TBT_CABLE_ROUNDED		BIT(19)
+#define TBT_CABLE_OPTICAL		BIT(21)
+#define TBT_CABLE_RETIMER		BIT(22)
+#define TBT_CABLE_LINK_TRAINING		BIT(23)
+
+/* TBT3 Device Enter Mode VDO bits */
+#define TBT_ENTER_MODE_CABLE_SPEED(_s_)	(((_s_) & GENMASK(2, 0)) << 16)
+#define TBT_ENTER_MODE_ACTIVE_CABLE	BIT(24)
+
+enum tbt_state {
+	TBT_STATE_IDLE,
+	TBT_STATE_SOP_P_ENTER,
+	TBT_STATE_SOP_PP_ENTER,
+	TBT_STATE_ENTER,
+	TBT_STATE_EXIT,
+	TBT_STATE_SOP_PP_EXIT,
+	TBT_STATE_SOP_P_EXIT
+};
+
+struct tbt_altmode {
+	enum tbt_state state;
+	struct typec_cable *cable;
+	struct typec_altmode *alt;
+	struct typec_altmode *plug[2];
+
+	struct work_struct work;
+	struct mutex lock; /* device lock */
+};
+
+static bool tbt_ready(struct typec_altmode *alt);
+
+static int tbt_enter_mode(struct tbt_altmode *tbt)
+{
+	struct typec_altmode *plug = tbt->plug[TYPEC_PLUG_SOP_P];
+	u32 vdo;
+
+	vdo = tbt->alt->vdo & (TBT_VENDOR_SPECIFIC_B0 | TBT_VENDOR_SPECIFIC_B1);
+	vdo |= tbt->alt->vdo & TBT_INTEL_SPECIFIC_B0;
+	vdo |= TBT_MODE;
+
+	if (plug) {
+		if (typec_cable_is_active(tbt->cable))
+			vdo |= TBT_ENTER_MODE_ACTIVE_CABLE;
+
+		vdo |= TBT_ENTER_MODE_CABLE_SPEED(TBT_CABLE_SPEED(plug->vdo));
+		vdo |= plug->vdo & TBT_CABLE_ROUNDED;
+		vdo |= plug->vdo & TBT_CABLE_OPTICAL;
+		vdo |= plug->vdo & TBT_CABLE_RETIMER;
+		vdo |= plug->vdo & TBT_CABLE_LINK_TRAINING;
+	} else {
+		vdo |= TBT_ENTER_MODE_CABLE_SPEED(TBT_CABLE_USB3_PASSIVE);
+	}
+
+	return typec_altmode_enter(tbt->alt, &vdo);
+}
+
+static void tbt_altmode_work(struct work_struct *work)
+{
+	struct tbt_altmode *tbt = container_of(work, struct tbt_altmode, work);
+	int ret;
+
+	mutex_lock(&tbt->lock);
+
+	switch (tbt->state) {
+	case TBT_STATE_SOP_P_ENTER:
+		ret = typec_altmode_enter(tbt->plug[TYPEC_PLUG_SOP_P], NULL);
+		if (ret)
+			dev_dbg(&tbt->plug[TYPEC_PLUG_SOP_P]->dev,
+				"failed to enter mode (%d)\n", ret);
+		break;
+	case TBT_STATE_SOP_PP_ENTER:
+		ret = typec_altmode_enter(tbt->plug[TYPEC_PLUG_SOP_PP], NULL);
+		if (ret)
+			dev_dbg(&tbt->plug[TYPEC_PLUG_SOP_PP]->dev,
+				"failed to enter mode (%d)\n", ret);
+		break;
+	case TBT_STATE_ENTER:
+		ret = tbt_enter_mode(tbt);
+		if (ret)
+			dev_dbg(&tbt->alt->dev, "failed to enter mode (%d)\n",
+				ret);
+		break;
+	case TBT_STATE_EXIT:
+		typec_altmode_exit(tbt->alt);
+		break;
+	case TBT_STATE_SOP_PP_EXIT:
+		typec_altmode_exit(tbt->plug[TYPEC_PLUG_SOP_PP]);
+		break;
+	case TBT_STATE_SOP_P_EXIT:
+		typec_altmode_exit(tbt->plug[TYPEC_PLUG_SOP_P]);
+		break;
+	default:
+		break;
+	}
+
+	tbt->state = TBT_STATE_IDLE;
+
+	mutex_unlock(&tbt->lock);
+}
+
+static int tbt_altmode_vdm(struct typec_altmode *alt,
+			   const u32 hdr, const u32 *vdo, int count)
+{
+	struct tbt_altmode *tbt = typec_altmode_get_drvdata(alt);
+	int cmd_type = PD_VDO_CMDT(hdr);
+	int cmd = PD_VDO_CMD(hdr);
+
+	mutex_lock(&tbt->lock);
+
+	if (tbt->state != TBT_STATE_IDLE) {
+		mutex_unlock(&tbt->lock);
+		return -EBUSY;
+	}
+
+	switch (cmd_type) {
+	case CMDT_RSP_ACK:
+		switch (cmd) {
+		case CMD_ENTER_MODE:
+			/*
+			 * Following the order describeded in USB Type-C Spec
+			 * R2.0 Section 6.7.3.
+			 */
+			if (alt == tbt->plug[TYPEC_PLUG_SOP_P]) {
+				if (tbt->plug[TYPEC_PLUG_SOP_PP])
+					tbt->state = TBT_STATE_SOP_PP_ENTER;
+				else
+					tbt->state = TBT_STATE_ENTER;
+			} else if (alt == tbt->plug[TYPEC_PLUG_SOP_PP]) {
+				tbt->state = TBT_STATE_ENTER;
+			} else {
+				typec_altmode_notify(alt, TYPEC_STATE_MODAL,
+						     NULL);
+			}
+			break;
+		case CMD_EXIT_MODE:
+			if (alt == tbt->alt) {
+				if (tbt->plug[TYPEC_PLUG_SOP_PP])
+					tbt->state = TBT_STATE_SOP_PP_EXIT;
+				else if (tbt->plug[TYPEC_PLUG_SOP_P])
+					tbt->state = TBT_STATE_SOP_P_EXIT;
+			} else if (alt == tbt->plug[TYPEC_PLUG_SOP_PP]) {
+				tbt->state = TBT_STATE_SOP_P_EXIT;
+			}
+			break;
+		}
+		break;
+	case CMDT_RSP_NAK:
+		switch (cmd) {
+		case CMD_ENTER_MODE:
+			dev_warn(&alt->dev, "Enter Mode refused\n");
+			break;
+		default:
+			break;
+		}
+		break;
+	default:
+		break;
+	}
+
+	if (tbt->state != TBT_STATE_IDLE)
+		schedule_work(&tbt->work);
+
+	mutex_unlock(&tbt->lock);
+
+	return 0;
+}
+
+static int tbt_altmode_activate(struct typec_altmode *alt, int activate)
+{
+	struct tbt_altmode *tbt = typec_altmode_get_drvdata(alt);
+	int ret;
+
+	mutex_lock(&tbt->lock);
+
+	if (!tbt_ready(alt))
+		return -ENODEV;
+
+	/* Preventing the user space from entering/exiting the cable alt mode */
+	if (alt != tbt->alt)
+		ret = -EPERM;
+	else if (activate)
+		ret = tbt_enter_mode(tbt);
+	else
+		ret = typec_altmode_exit(alt);
+
+	mutex_unlock(&tbt->lock);
+
+	return ret;
+}
+
+static const struct typec_altmode_ops tbt_altmode_ops = {
+	.vdm		= tbt_altmode_vdm,
+	.activate	= tbt_altmode_activate
+};
+
+static int tbt_altmode_probe(struct typec_altmode *alt)
+{
+	struct tbt_altmode *tbt;
+
+	tbt = devm_kzalloc(&alt->dev, sizeof(*tbt), GFP_KERNEL);
+	if (!tbt)
+		return -ENOMEM;
+
+	INIT_WORK(&tbt->work, tbt_altmode_work);
+	mutex_init(&tbt->lock);
+	tbt->alt = alt;
+
+	alt->desc = "Thunderbolt3";
+	typec_altmode_set_drvdata(alt, tbt);
+	typec_altmode_set_ops(alt, &tbt_altmode_ops);
+
+	if (tbt_ready(alt)) {
+		if (tbt->plug[TYPEC_PLUG_SOP_PP])
+			tbt->state = TBT_STATE_SOP_PP_ENTER;
+		else if (tbt->plug[TYPEC_PLUG_SOP_P])
+			tbt->state = TBT_STATE_SOP_P_ENTER;
+		else
+			tbt->state = TBT_STATE_ENTER;
+		schedule_work(&tbt->work);
+	}
+
+	return 0;
+}
+
+static void tbt_altmode_remove(struct typec_altmode *alt)
+{
+	struct tbt_altmode *tbt = typec_altmode_get_drvdata(alt);
+
+	for (int i = TYPEC_PLUG_SOP_PP; i > 0; --i) {
+		if (tbt->plug[i])
+			typec_altmode_put_plug(tbt->plug[i]);
+	}
+
+	if (tbt->cable)
+		typec_cable_put(tbt->cable);
+}
+
+static bool tbt_ready(struct typec_altmode *alt)
+{
+	struct tbt_altmode *tbt = typec_altmode_get_drvdata(alt);
+	struct typec_altmode *plug;
+
+	if (tbt->cable)
+		return true;
+
+	/* Thundebolt 3 requires a cable with eMarker */
+	tbt->cable = typec_cable_get(typec_altmode2port(tbt->alt));
+	if (!tbt->cable)
+		return false;
+
+	/* We accept systems without SOP' or SOP''. This means the port altmode
+	 * driver will be responsible for properly ordering entry/exit.
+	 */
+	for (int i = 0; i < TYPEC_PLUG_SOP_PP + 1; i++) {
+		plug = typec_altmode_get_plug(tbt->alt, i);
+		if (IS_ERR(plug))
+			continue;
+
+		if (!plug || plug->svid != USB_TYPEC_VENDOR_INTEL)
+			break;
+
+		plug->desc = "Thunderbolt3";
+		plug->ops = &tbt_altmode_ops;
+		typec_altmode_set_drvdata(plug, tbt);
+
+		tbt->plug[i] = plug;
+	}
+
+	return true;
+}
+
+static const struct typec_device_id tbt_typec_id[] = {
+	{ USB_TYPEC_VENDOR_INTEL, TYPEC_ANY_MODE },
+	{ }
+};
+MODULE_DEVICE_TABLE(typec, tbt_typec_id);
+
+static struct typec_altmode_driver tbt_altmode_driver = {
+	.id_table = tbt_typec_id,
+	.probe = tbt_altmode_probe,
+	.remove = tbt_altmode_remove,
+	.driver = {
+		.name = "typec-thunderbolt",
+		.owner = THIS_MODULE,
+	}
+};
+module_typec_altmode_driver(tbt_altmode_driver);
+
+MODULE_AUTHOR("Heikki Krogerus <heikki.krogerus@linux.intel.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Thunderbolt3 USB Type-C Alternate Mode");
-- 
2.46.0.792.g87dc391469-goog

Re: [PATCH 1/8] usb: typec: Add driver for Thunderbolt 3 Alternate Mode

[Heikki Krogerus]

Hi Abhishek,

On Wed, Sep 25, 2024 at 09:25:02AM -0700, Abhishek Pandit-Subedi wrote:
> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> 
> Thunderbolt 3 Alternate Mode entry flow is described in
> USB Type-C Specification Release 2.0.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> ---
> 
> Changes:
> * Delay cable + plug checks so that the module doesn't fail to probe
>   if cable + plug information isn't available by the time the partner
>   altmode is registered.
> * Remove unncessary brace after if (IS_ERR(plug))
> 
> The rest of this patch should be the same as Heikki's original RFC.
> 
> 
>  drivers/usb/typec/altmodes/Kconfig       |   9 +
>  drivers/usb/typec/altmodes/Makefile      |   2 +
>  drivers/usb/typec/altmodes/thunderbolt.c | 321 +++++++++++++++++++++++
>  3 files changed, 332 insertions(+)
>  create mode 100644 drivers/usb/typec/altmodes/thunderbolt.c
> 
> diff --git a/drivers/usb/typec/altmodes/Kconfig b/drivers/usb/typec/altmodes/Kconfig
> index 1a6b5e872b0d..7867fa7c405d 100644
> --- a/drivers/usb/typec/altmodes/Kconfig
> +++ b/drivers/usb/typec/altmodes/Kconfig
> @@ -23,4 +23,13 @@ config TYPEC_NVIDIA_ALTMODE
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called typec_nvidia.
>  
> +config TYPEC_TBT_ALTMODE
> +	tristate "Thunderbolt3 Alternate Mode driver"
> +	help
> +	  Select this option if you have Thunderbolt3 hardware on your
> +	  system.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called typec_thunderbolt.
> +
>  endmenu
> diff --git a/drivers/usb/typec/altmodes/Makefile b/drivers/usb/typec/altmodes/Makefile
> index 45717548b396..508a68351bd2 100644
> --- a/drivers/usb/typec/altmodes/Makefile
> +++ b/drivers/usb/typec/altmodes/Makefile
> @@ -4,3 +4,5 @@ obj-$(CONFIG_TYPEC_DP_ALTMODE)		+= typec_displayport.o
>  typec_displayport-y			:= displayport.o
>  obj-$(CONFIG_TYPEC_NVIDIA_ALTMODE)	+= typec_nvidia.o
>  typec_nvidia-y				:= nvidia.o
> +obj-$(CONFIG_TYPEC_TBT_ALTMODE)		+= typec_thunderbolt.o
> +typec_thunderbolt-y			:= thunderbolt.o
> diff --git a/drivers/usb/typec/altmodes/thunderbolt.c b/drivers/usb/typec/altmodes/thunderbolt.c
> new file mode 100644
> index 000000000000..515e775ee41a
> --- /dev/null
> +++ b/drivers/usb/typec/altmodes/thunderbolt.c
> @@ -0,0 +1,321 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/**
> + * USB Typec-C Thuderbolt3 Alternate Mode driver
> + *
> + * Copyright (C) 2019 Intel Corporation
> + * Author: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/mutex.h>
> +#include <linux/module.h>
> +#include <linux/usb/pd_vdo.h>
> +#include <linux/usb/typec_altmode.h>
> +
> +#define USB_TYPEC_VENDOR_INTEL		0x8087
> +
> +/* TBT3 Device Discover Mode VDO bits */
> +#define TBT_MODE			BIT(0)
> +#define TBT_ADAPTER(_vdo_)		(((_vdo_) & BIT(16)) >> 16)
> +#define   TBT_ADAPTER_LEGACY		0
> +#define   TBT_ADAPTER_TBT3		1
> +#define TBT_INTEL_SPECIFIC_B0		BIT(26)
> +#define TBT_VENDOR_SPECIFIC_B0		BIT(30)
> +#define TBT_VENDOR_SPECIFIC_B1		BIT(31)
> +
> +/* TBT3 Cable Discover Mode VDO bits */
> +#define TBT_CABLE_SPEED(_vdo_)		(((_vdo_) & GENMASK(18, 16)) >> 16)
> +#define   TBT_CABLE_USB3_GEN1		1
> +#define   TBT_CABLE_USB3_PASSIVE	2
> +#define   TBT_CABLE_10_AND_20GBPS	3
> +#define TBT_CABLE_ROUNDED		BIT(19)
> +#define TBT_CABLE_OPTICAL		BIT(21)
> +#define TBT_CABLE_RETIMER		BIT(22)
> +#define TBT_CABLE_LINK_TRAINING		BIT(23)
> +
> +/* TBT3 Device Enter Mode VDO bits */
> +#define TBT_ENTER_MODE_CABLE_SPEED(_s_)	(((_s_) & GENMASK(2, 0)) << 16)
> +#define TBT_ENTER_MODE_ACTIVE_CABLE	BIT(24)

I guess my original RFC was before the introduction of
linux/usb/typec_tbt.h. You can replace all the above definitions with:

#include <linux/usb/typec_tbt.h>

> +enum tbt_state {
> +	TBT_STATE_IDLE,
> +	TBT_STATE_SOP_P_ENTER,
> +	TBT_STATE_SOP_PP_ENTER,
> +	TBT_STATE_ENTER,
> +	TBT_STATE_EXIT,
> +	TBT_STATE_SOP_PP_EXIT,
> +	TBT_STATE_SOP_P_EXIT
> +};
> +
> +struct tbt_altmode {
> +	enum tbt_state state;
> +	struct typec_cable *cable;
> +	struct typec_altmode *alt;
> +	struct typec_altmode *plug[2];

You really have to supply struct typec_thunderbolt_data from this
driver.

I understand that by leaving the mux handling to the alt mode drivers,
you have to refactor at least cros_ec_typec.c, but I really think you
have to do it. So please either add here:

        struct typec_thunderbolt_data data;

or if we don't need to store it, store only the Enter Mode VDO:

        u32 enter_vdo;

> +	struct work_struct work;
> +	struct mutex lock; /* device lock */
> +};
> +
> +static bool tbt_ready(struct typec_altmode *alt);
> +
> +static int tbt_enter_mode(struct tbt_altmode *tbt)
> +{
> +	struct typec_altmode *plug = tbt->plug[TYPEC_PLUG_SOP_P];
> +	u32 vdo;
> +
> +	vdo = tbt->alt->vdo & (TBT_VENDOR_SPECIFIC_B0 | TBT_VENDOR_SPECIFIC_B1);
> +	vdo |= tbt->alt->vdo & TBT_INTEL_SPECIFIC_B0;
> +	vdo |= TBT_MODE;
> +
> +	if (plug) {
> +		if (typec_cable_is_active(tbt->cable))
> +			vdo |= TBT_ENTER_MODE_ACTIVE_CABLE;
> +
> +		vdo |= TBT_ENTER_MODE_CABLE_SPEED(TBT_CABLE_SPEED(plug->vdo));
> +		vdo |= plug->vdo & TBT_CABLE_ROUNDED;

TBT_CABLE_ROUNDED needs to be defined in typec_tbt.h. It's missing for
some reason.

> +		vdo |= plug->vdo & TBT_CABLE_OPTICAL;
> +		vdo |= plug->vdo & TBT_CABLE_RETIMER;
> +		vdo |= plug->vdo & TBT_CABLE_LINK_TRAINING;
> +	} else {
> +		vdo |= TBT_ENTER_MODE_CABLE_SPEED(TBT_CABLE_USB3_PASSIVE);
> +	}

So this vdo I believe will need to be stored:

        tbt->data.enter_vdo;

or

        tbt->enter_vdo = vdo;

> +	return typec_altmode_enter(tbt->alt, &vdo);
> +}
> +
> +static void tbt_altmode_work(struct work_struct *work)
> +{
> +	struct tbt_altmode *tbt = container_of(work, struct tbt_altmode, work);
> +	int ret;
> +
> +	mutex_lock(&tbt->lock);
> +
> +	switch (tbt->state) {
> +	case TBT_STATE_SOP_P_ENTER:
> +		ret = typec_altmode_enter(tbt->plug[TYPEC_PLUG_SOP_P], NULL);
> +		if (ret)
> +			dev_dbg(&tbt->plug[TYPEC_PLUG_SOP_P]->dev,
> +				"failed to enter mode (%d)\n", ret);
> +		break;
> +	case TBT_STATE_SOP_PP_ENTER:
> +		ret = typec_altmode_enter(tbt->plug[TYPEC_PLUG_SOP_PP], NULL);
> +		if (ret)
> +			dev_dbg(&tbt->plug[TYPEC_PLUG_SOP_PP]->dev,
> +				"failed to enter mode (%d)\n", ret);
> +		break;
> +	case TBT_STATE_ENTER:
> +		ret = tbt_enter_mode(tbt);
> +		if (ret)
> +			dev_dbg(&tbt->alt->dev, "failed to enter mode (%d)\n",
> +				ret);
> +		break;
> +	case TBT_STATE_EXIT:
> +		typec_altmode_exit(tbt->alt);
> +		break;
> +	case TBT_STATE_SOP_PP_EXIT:
> +		typec_altmode_exit(tbt->plug[TYPEC_PLUG_SOP_PP]);
> +		break;
> +	case TBT_STATE_SOP_P_EXIT:
> +		typec_altmode_exit(tbt->plug[TYPEC_PLUG_SOP_P]);
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	tbt->state = TBT_STATE_IDLE;
> +
> +	mutex_unlock(&tbt->lock);
> +}
> +
> +static int tbt_altmode_vdm(struct typec_altmode *alt,
> +			   const u32 hdr, const u32 *vdo, int count)
> +{
> +	struct tbt_altmode *tbt = typec_altmode_get_drvdata(alt);
> +	int cmd_type = PD_VDO_CMDT(hdr);
> +	int cmd = PD_VDO_CMD(hdr);
> +
> +	mutex_lock(&tbt->lock);
> +
> +	if (tbt->state != TBT_STATE_IDLE) {
> +		mutex_unlock(&tbt->lock);
> +		return -EBUSY;
> +	}
> +
> +	switch (cmd_type) {
> +	case CMDT_RSP_ACK:
> +		switch (cmd) {
> +		case CMD_ENTER_MODE:
> +			/*
> +			 * Following the order describeded in USB Type-C Spec
> +			 * R2.0 Section 6.7.3.
> +			 */
> +			if (alt == tbt->plug[TYPEC_PLUG_SOP_P]) {
> +				if (tbt->plug[TYPEC_PLUG_SOP_PP])
> +					tbt->state = TBT_STATE_SOP_PP_ENTER;
> +				else
> +					tbt->state = TBT_STATE_ENTER;
> +			} else if (alt == tbt->plug[TYPEC_PLUG_SOP_PP]) {
> +				tbt->state = TBT_STATE_ENTER;
> +			} else {
> +				typec_altmode_notify(alt, TYPEC_STATE_MODAL,
> +						     NULL);

                                typec_altmode_notify(alt, TYPEC_STATE_MODAL, &tbt->data);

or, if this is possible:

                                struct typec_thunderbolt_data;

                                data.device_vdo = tbt->alt->vdo;
                                data.cable_vdo = tbt->plug[TYPEC_PLUG_SOP_P].vdo;
                                data.enter_vdo = tbt->enter_vdo;

                                typec_altmode_notify(alt, TYPEC_STATE_MODAL, &data);

> +			}
> +			break;
> +		case CMD_EXIT_MODE:
> +			if (alt == tbt->alt) {
> +				if (tbt->plug[TYPEC_PLUG_SOP_PP])
> +					tbt->state = TBT_STATE_SOP_PP_EXIT;
> +				else if (tbt->plug[TYPEC_PLUG_SOP_P])
> +					tbt->state = TBT_STATE_SOP_P_EXIT;
> +			} else if (alt == tbt->plug[TYPEC_PLUG_SOP_PP]) {
> +				tbt->state = TBT_STATE_SOP_P_EXIT;
> +			}
> +			break;
> +		}
> +		break;
> +	case CMDT_RSP_NAK:
> +		switch (cmd) {
> +		case CMD_ENTER_MODE:
> +			dev_warn(&alt->dev, "Enter Mode refused\n");
> +			break;
> +		default:
> +			break;
> +		}
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	if (tbt->state != TBT_STATE_IDLE)
> +		schedule_work(&tbt->work);
> +
> +	mutex_unlock(&tbt->lock);
> +
> +	return 0;
> +}
> +
> +static int tbt_altmode_activate(struct typec_altmode *alt, int activate)
> +{
> +	struct tbt_altmode *tbt = typec_altmode_get_drvdata(alt);
> +	int ret;
> +
> +	mutex_lock(&tbt->lock);
> +
> +	if (!tbt_ready(alt))
> +		return -ENODEV;
> +
> +	/* Preventing the user space from entering/exiting the cable alt mode */
> +	if (alt != tbt->alt)
> +		ret = -EPERM;
> +	else if (activate)
> +		ret = tbt_enter_mode(tbt);
> +	else
> +		ret = typec_altmode_exit(alt);
> +
> +	mutex_unlock(&tbt->lock);
> +
> +	return ret;
> +}
> +
> +static const struct typec_altmode_ops tbt_altmode_ops = {
> +	.vdm		= tbt_altmode_vdm,
> +	.activate	= tbt_altmode_activate
> +};
> +
> +static int tbt_altmode_probe(struct typec_altmode *alt)
> +{
> +	struct tbt_altmode *tbt;
> +
> +	tbt = devm_kzalloc(&alt->dev, sizeof(*tbt), GFP_KERNEL);
> +	if (!tbt)
> +		return -ENOMEM;
> +
> +	INIT_WORK(&tbt->work, tbt_altmode_work);
> +	mutex_init(&tbt->lock);
> +	tbt->alt = alt;

        tbt->data.device_mode = alt->vdo;

> +	alt->desc = "Thunderbolt3";
> +	typec_altmode_set_drvdata(alt, tbt);
> +	typec_altmode_set_ops(alt, &tbt_altmode_ops);
> +
> +	if (tbt_ready(alt)) {
> +		if (tbt->plug[TYPEC_PLUG_SOP_PP])
> +			tbt->state = TBT_STATE_SOP_PP_ENTER;
> +		else if (tbt->plug[TYPEC_PLUG_SOP_P])
> +			tbt->state = TBT_STATE_SOP_P_ENTER;
> +		else
> +			tbt->state = TBT_STATE_ENTER;
> +		schedule_work(&tbt->work);
> +	}
> +
> +	return 0;
> +}
> +
> +static void tbt_altmode_remove(struct typec_altmode *alt)
> +{
> +	struct tbt_altmode *tbt = typec_altmode_get_drvdata(alt);
> +
> +	for (int i = TYPEC_PLUG_SOP_PP; i > 0; --i) {
> +		if (tbt->plug[i])
> +			typec_altmode_put_plug(tbt->plug[i]);
> +	}
> +
> +	if (tbt->cable)
> +		typec_cable_put(tbt->cable);
> +}
> +
> +static bool tbt_ready(struct typec_altmode *alt)
> +{
> +	struct tbt_altmode *tbt = typec_altmode_get_drvdata(alt);
> +	struct typec_altmode *plug;
> +
> +	if (tbt->cable)
> +		return true;
> +
> +	/* Thundebolt 3 requires a cable with eMarker */
> +	tbt->cable = typec_cable_get(typec_altmode2port(tbt->alt));
> +	if (!tbt->cable)
> +		return false;
> +
> +	/* We accept systems without SOP' or SOP''. This means the port altmode
> +	 * driver will be responsible for properly ordering entry/exit.
> +	 */
> +	for (int i = 0; i < TYPEC_PLUG_SOP_PP + 1; i++) {
> +		plug = typec_altmode_get_plug(tbt->alt, i);
> +		if (IS_ERR(plug))
> +			continue;
> +
> +		if (!plug || plug->svid != USB_TYPEC_VENDOR_INTEL)
> +			break;
> +
> +		plug->desc = "Thunderbolt3";
> +		plug->ops = &tbt_altmode_ops;
> +		typec_altmode_set_drvdata(plug, tbt);
> +
> +		tbt->plug[i] = plug;
> +	}

        tbt->data.cable_mode = tbt->plug[TYPEC_PLUG_SOP_P].vdo;

> +	return true;
> +}
> +
> +static const struct typec_device_id tbt_typec_id[] = {
> +	{ USB_TYPEC_VENDOR_INTEL, TYPEC_ANY_MODE },

There is also the alias: USB_TYPEC_TBT_SID

> +	{ }
> +};
> +MODULE_DEVICE_TABLE(typec, tbt_typec_id);
> +
> +static struct typec_altmode_driver tbt_altmode_driver = {
> +	.id_table = tbt_typec_id,
> +	.probe = tbt_altmode_probe,
> +	.remove = tbt_altmode_remove,
> +	.driver = {
> +		.name = "typec-thunderbolt",
> +		.owner = THIS_MODULE,
> +	}
> +};
> +module_typec_altmode_driver(tbt_altmode_driver);
> +
> +MODULE_AUTHOR("Heikki Krogerus <heikki.krogerus@linux.intel.com>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Thunderbolt3 USB Type-C Alternate Mode");

thanks,

-- 
heikki

Re: [PATCH 1/8] usb: typec: Add driver for Thunderbolt 3 Alternate Mode

[kernel test robot]

Hi Abhishek,

kernel test robot noticed the following build warnings:

[auto build test WARNING on chrome-platform/for-next]
[also build test WARNING on chrome-platform/for-firmware-next usb/usb-testing usb/usb-next usb/usb-linus linus/master v6.11 next-20240926]
[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/Abhishek-Pandit-Subedi/usb-typec-Add-driver-for-Thunderbolt-3-Alternate-Mode/20240926-002931
base:   https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git for-next
patch link:    https://lore.kernel.org/r/20240925092505.1.I3080b036e8de0b9957c57c1c3059db7149c5e549%40changeid
patch subject: [PATCH 1/8] usb: typec: Add driver for Thunderbolt 3 Alternate Mode
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20240926/202409262224.Bnlmjakc-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240926/202409262224.Bnlmjakc-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/202409262224.Bnlmjakc-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/usb/typec/altmodes/thunderbolt.c:15: warning: expecting prototype for USB Typec(). Prototype was for USB_TYPEC_VENDOR_INTEL() instead


vim +15 drivers/usb/typec/altmodes/thunderbolt.c

    14	
  > 15	#define USB_TYPEC_VENDOR_INTEL		0x8087
    16	

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

[PATCH 2/8] usb: typec: altmode_match should handle TYPEC_ANY_MODE

[Abhishek Pandit-Subedi]

altmode_match is used when searching for the first port altmode that
matches the partner or plug altmode. If the port registered with mode
set to TYPEC_ANY_MODE, it should always match if the SVID matches.

Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
---

 drivers/usb/typec/class.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 9262fcd4144f..179856503d5d 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -230,7 +230,8 @@ static int altmode_match(struct device *dev, void *data)
 	if (!is_typec_altmode(dev))
 		return 0;
 
-	return ((adev->svid == id->svid) && (adev->mode == id->mode));
+	return ((adev->svid == id->svid) &&
+		(adev->mode == id->mode || adev->mode == TYPEC_ANY_MODE));
 }
 
 static void typec_altmode_set_partner(struct altmode *altmode)
-- 
2.46.0.792.g87dc391469-goog

Re: [PATCH 2/8] usb: typec: altmode_match should handle TYPEC_ANY_MODE

[Dmitry Baryshkov]

On Wed, Sep 25, 2024 at 09:25:03AM GMT, Abhishek Pandit-Subedi wrote:
> altmode_match is used when searching for the first port altmode that
> matches the partner or plug altmode. If the port registered with mode
> set to TYPEC_ANY_MODE, it should always match if the SVID matches.
> 
> Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>

Fixes?

> ---
> 
>  drivers/usb/typec/class.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 9262fcd4144f..179856503d5d 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -230,7 +230,8 @@ static int altmode_match(struct device *dev, void *data)
>  	if (!is_typec_altmode(dev))
>  		return 0;
>  
> -	return ((adev->svid == id->svid) && (adev->mode == id->mode));
> +	return ((adev->svid == id->svid) &&
> +		(adev->mode == id->mode || adev->mode == TYPEC_ANY_MODE));
>  }
>  
>  static void typec_altmode_set_partner(struct altmode *altmode)
> -- 
> 2.46.0.792.g87dc391469-goog
> 

-- 
With best wishes
Dmitry

Re: [PATCH 2/8] usb: typec: altmode_match should handle TYPEC_ANY_MODE

[Abhishek Pandit-Subedi]

On Wed, Sep 25, 2024 at 9:54 AM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Wed, Sep 25, 2024 at 09:25:03AM GMT, Abhishek Pandit-Subedi wrote:
> > altmode_match is used when searching for the first port altmode that
> > matches the partner or plug altmode. If the port registered with mode
> > set to TYPEC_ANY_MODE, it should always match if the SVID matches.
> >
> > Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
>
> Fixes?

This is new for Thunderbolt which registers as TYPEC_ANY_MODE so
there's no FIXES. I think Heikki may need to chime in on how the
`mode` is supposed to be used.

IMO, it may be appropriate to get rid of the mode check entirely.

>
> > ---
> >
> >  drivers/usb/typec/class.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> > index 9262fcd4144f..179856503d5d 100644
> > --- a/drivers/usb/typec/class.c
> > +++ b/drivers/usb/typec/class.c
> > @@ -230,7 +230,8 @@ static int altmode_match(struct device *dev, void *data)
> >       if (!is_typec_altmode(dev))
> >               return 0;
> >
> > -     return ((adev->svid == id->svid) && (adev->mode == id->mode));
> > +     return ((adev->svid == id->svid) &&
> > +             (adev->mode == id->mode || adev->mode == TYPEC_ANY_MODE));
> >  }
> >
> >  static void typec_altmode_set_partner(struct altmode *altmode)
> > --
> > 2.46.0.792.g87dc391469-goog
> >
>
> --
> With best wishes
> Dmitry

Re: [PATCH 2/8] usb: typec: altmode_match should handle TYPEC_ANY_MODE

[Heikki Krogerus]

On Wed, Sep 25, 2024 at 10:31:33AM -0700, Abhishek Pandit-Subedi wrote:
> On Wed, Sep 25, 2024 at 9:54 AM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
> >
> > On Wed, Sep 25, 2024 at 09:25:03AM GMT, Abhishek Pandit-Subedi wrote:
> > > altmode_match is used when searching for the first port altmode that
> > > matches the partner or plug altmode. If the port registered with mode
> > > set to TYPEC_ANY_MODE, it should always match if the SVID matches.
> > >
> > > Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> >
> > Fixes?
> 
> This is new for Thunderbolt which registers as TYPEC_ANY_MODE so
> there's no FIXES. I think Heikki may need to chime in on how the
> `mode` is supposed to be used.
> 
> IMO, it may be appropriate to get rid of the mode check entirely.

It's probable okay to just drop it.

> >
> > > ---
> > >
> > >  drivers/usb/typec/class.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> > > index 9262fcd4144f..179856503d5d 100644
> > > --- a/drivers/usb/typec/class.c
> > > +++ b/drivers/usb/typec/class.c
> > > @@ -230,7 +230,8 @@ static int altmode_match(struct device *dev, void *data)
> > >       if (!is_typec_altmode(dev))
> > >               return 0;
> > >
> > > -     return ((adev->svid == id->svid) && (adev->mode == id->mode));
> > > +     return ((adev->svid == id->svid) &&
> > > +             (adev->mode == id->mode || adev->mode == TYPEC_ANY_MODE));
> > >  }
> > >
> > >  static void typec_altmode_set_partner(struct altmode *altmode)
> > > --
> > > 2.46.0.792.g87dc391469-goog
> > >
> >
> > --
> > With best wishes
> > Dmitry

-- 
heikki

[PATCH 3/8] usb: typec: intel_pmc_mux: Null check before use

[Abhishek Pandit-Subedi]

Make sure the data pointer in typec_mux_state is not null before
accessing it.

Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
---

 drivers/usb/typec/mux/intel_pmc_mux.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/typec/mux/intel_pmc_mux.c b/drivers/usb/typec/mux/intel_pmc_mux.c
index 56989a0d0f43..4283fead9a69 100644
--- a/drivers/usb/typec/mux/intel_pmc_mux.c
+++ b/drivers/usb/typec/mux/intel_pmc_mux.c
@@ -331,14 +331,19 @@ static int
 pmc_usb_mux_tbt(struct pmc_usb_port *port, struct typec_mux_state *state)
 {
 	struct typec_thunderbolt_data *data = state->data;
-	u8 cable_rounded = TBT_CABLE_ROUNDED_SUPPORT(data->cable_mode);
-	u8 cable_speed = TBT_CABLE_SPEED(data->cable_mode);
+	u8 cable_rounded, cable_speed;
 	struct altmode_req req = { };
 
+	if (!data)
+		return 0;
+
 	if (IOM_PORT_ACTIVITY_IS(port->iom_status, TBT) ||
 	    IOM_PORT_ACTIVITY_IS(port->iom_status, ALT_MODE_TBT_USB))
 		return 0;
 
+	cable_rounded = TBT_CABLE_ROUNDED_SUPPORT(data->cable_mode);
+	cable_speed = TBT_CABLE_SPEED(data->cable_mode);
+
 	req.usage = PMC_USB_ALT_MODE;
 	req.usage |= port->usb3_port << PMC_USB_MSG_USB3_PORT_SHIFT;
 	req.mode_type = PMC_USB_MODE_TYPE_TBT << PMC_USB_MODE_TYPE_SHIFT;
-- 
2.46.0.792.g87dc391469-goog

Re: [PATCH 3/8] usb: typec: intel_pmc_mux: Null check before use

[Dmitry Baryshkov]

On Wed, Sep 25, 2024 at 09:25:04AM GMT, Abhishek Pandit-Subedi wrote:
> Make sure the data pointer in typec_mux_state is not null before
> accessing it.
> 
> Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>

Is the a fix for an actual issue or just good-to-have thing? In the
former case it lacks a description of how the issue can be triggered and
a Fixes tag.

> ---
> 
>  drivers/usb/typec/mux/intel_pmc_mux.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/typec/mux/intel_pmc_mux.c b/drivers/usb/typec/mux/intel_pmc_mux.c
> index 56989a0d0f43..4283fead9a69 100644
> --- a/drivers/usb/typec/mux/intel_pmc_mux.c
> +++ b/drivers/usb/typec/mux/intel_pmc_mux.c
> @@ -331,14 +331,19 @@ static int
>  pmc_usb_mux_tbt(struct pmc_usb_port *port, struct typec_mux_state *state)
>  {
>  	struct typec_thunderbolt_data *data = state->data;
> -	u8 cable_rounded = TBT_CABLE_ROUNDED_SUPPORT(data->cable_mode);
> -	u8 cable_speed = TBT_CABLE_SPEED(data->cable_mode);
> +	u8 cable_rounded, cable_speed;
>  	struct altmode_req req = { };
>  
> +	if (!data)
> +		return 0;
> +
>  	if (IOM_PORT_ACTIVITY_IS(port->iom_status, TBT) ||
>  	    IOM_PORT_ACTIVITY_IS(port->iom_status, ALT_MODE_TBT_USB))
>  		return 0;
>  
> +	cable_rounded = TBT_CABLE_ROUNDED_SUPPORT(data->cable_mode);
> +	cable_speed = TBT_CABLE_SPEED(data->cable_mode);
> +
>  	req.usage = PMC_USB_ALT_MODE;
>  	req.usage |= port->usb3_port << PMC_USB_MSG_USB3_PORT_SHIFT;
>  	req.mode_type = PMC_USB_MODE_TYPE_TBT << PMC_USB_MODE_TYPE_SHIFT;
> -- 
> 2.46.0.792.g87dc391469-goog
> 

-- 
With best wishes
Dmitry

Re: [PATCH 3/8] usb: typec: intel_pmc_mux: Null check before use

[Abhishek Pandit-Subedi]

On Wed, Sep 25, 2024 at 9:54 AM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Wed, Sep 25, 2024 at 09:25:04AM GMT, Abhishek Pandit-Subedi wrote:
> > Make sure the data pointer in typec_mux_state is not null before
> > accessing it.
> >
> > Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
>
> Is the a fix for an actual issue or just good-to-have thing? In the
> former case it lacks a description of how the issue can be triggered and
> a Fixes tag.

This fixes a segfault that occurs when the new Thunderbolt driver is
used because it calls `typec_altmode_notify` with null data. I'm not
sure if that needs a `Fixes` since what's currently running upstream
doesn't actually trigger this error.

I'll update the description with why this is needed. i.e.
---
Make sure the data pointer in typec_mux_state is not null before
accessing it. The new Thunderbolt driver calls typec_altmode_notify
with a NULL pointer for data which can cause this mux configuration
to crash.

>
> > ---
> >
> >  drivers/usb/typec/mux/intel_pmc_mux.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/typec/mux/intel_pmc_mux.c b/drivers/usb/typec/mux/intel_pmc_mux.c
> > index 56989a0d0f43..4283fead9a69 100644
> > --- a/drivers/usb/typec/mux/intel_pmc_mux.c
> > +++ b/drivers/usb/typec/mux/intel_pmc_mux.c
> > @@ -331,14 +331,19 @@ static int
> >  pmc_usb_mux_tbt(struct pmc_usb_port *port, struct typec_mux_state *state)
> >  {
> >       struct typec_thunderbolt_data *data = state->data;
> > -     u8 cable_rounded = TBT_CABLE_ROUNDED_SUPPORT(data->cable_mode);
> > -     u8 cable_speed = TBT_CABLE_SPEED(data->cable_mode);
> > +     u8 cable_rounded, cable_speed;
> >       struct altmode_req req = { };
> >
> > +     if (!data)
> > +             return 0;
> > +
> >       if (IOM_PORT_ACTIVITY_IS(port->iom_status, TBT) ||
> >           IOM_PORT_ACTIVITY_IS(port->iom_status, ALT_MODE_TBT_USB))
> >               return 0;
> >
> > +     cable_rounded = TBT_CABLE_ROUNDED_SUPPORT(data->cable_mode);
> > +     cable_speed = TBT_CABLE_SPEED(data->cable_mode);
> > +
> >       req.usage = PMC_USB_ALT_MODE;
> >       req.usage |= port->usb3_port << PMC_USB_MSG_USB3_PORT_SHIFT;
> >       req.mode_type = PMC_USB_MODE_TYPE_TBT << PMC_USB_MODE_TYPE_SHIFT;
> > --
> > 2.46.0.792.g87dc391469-goog
> >
>
> --
> With best wishes
> Dmitry

Re: [PATCH 3/8] usb: typec: intel_pmc_mux: Null check before use

[Heikki Krogerus]

On Wed, Sep 25, 2024 at 09:25:04AM -0700, Abhishek Pandit-Subedi wrote:
> Make sure the data pointer in typec_mux_state is not null before
> accessing it.

This really should not be necessary.

> Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> ---
> 
>  drivers/usb/typec/mux/intel_pmc_mux.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/typec/mux/intel_pmc_mux.c b/drivers/usb/typec/mux/intel_pmc_mux.c
> index 56989a0d0f43..4283fead9a69 100644
> --- a/drivers/usb/typec/mux/intel_pmc_mux.c
> +++ b/drivers/usb/typec/mux/intel_pmc_mux.c
> @@ -331,14 +331,19 @@ static int
>  pmc_usb_mux_tbt(struct pmc_usb_port *port, struct typec_mux_state *state)
>  {
>  	struct typec_thunderbolt_data *data = state->data;
> -	u8 cable_rounded = TBT_CABLE_ROUNDED_SUPPORT(data->cable_mode);
> -	u8 cable_speed = TBT_CABLE_SPEED(data->cable_mode);
> +	u8 cable_rounded, cable_speed;
>  	struct altmode_req req = { };
>  
> +	if (!data)
> +		return 0;
> +
>  	if (IOM_PORT_ACTIVITY_IS(port->iom_status, TBT) ||
>  	    IOM_PORT_ACTIVITY_IS(port->iom_status, ALT_MODE_TBT_USB))
>  		return 0;
>  
> +	cable_rounded = TBT_CABLE_ROUNDED_SUPPORT(data->cable_mode);
> +	cable_speed = TBT_CABLE_SPEED(data->cable_mode);
> +
>  	req.usage = PMC_USB_ALT_MODE;
>  	req.usage |= port->usb3_port << PMC_USB_MSG_USB3_PORT_SHIFT;
>  	req.mode_type = PMC_USB_MODE_TYPE_TBT << PMC_USB_MODE_TYPE_SHIFT;
> -- 
> 2.46.0.792.g87dc391469-goog

-- 
heikki

[PATCH 4/8] usb: typec: Auto enter control for alternate modes

[Abhishek Pandit-Subedi]

Add controls for whether an alternate mode is automatically entered when
a partner connects. The auto_enter control is only available on ports
and applies immediately after a partner connects. The default behavior
is to enable auto enter and drivers must explicitly disable it.

Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
---

 Documentation/ABI/testing/sysfs-bus-typec |  9 +++++++
 drivers/usb/typec/altmodes/displayport.c  |  6 +++--
 drivers/usb/typec/altmodes/thunderbolt.c  |  3 ++-
 drivers/usb/typec/class.c                 | 31 +++++++++++++++++++++++
 include/linux/usb/typec.h                 |  2 ++
 include/linux/usb/typec_altmode.h         |  2 ++
 6 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-typec b/Documentation/ABI/testing/sysfs-bus-typec
index 205d9c91e2e1..f09d05727b82 100644
--- a/Documentation/ABI/testing/sysfs-bus-typec
+++ b/Documentation/ABI/testing/sysfs-bus-typec
@@ -12,6 +12,15 @@ Description:
 
 		Valid values are boolean.
 
+What:		/sys/bus/typec/devices/.../auto_enter
+Date:		September 2024
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		Controls whether a mode will be automatically entered when a partner is
+		connected.
+
+		This field is only valid and displayed on a port. Valid values are boolean.
+
 What:		/sys/bus/typec/devices/.../description
 Date:		July 2018
 Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
index 92cc1b136120..7b164086bbbb 100644
--- a/drivers/usb/typec/altmodes/displayport.c
+++ b/drivers/usb/typec/altmodes/displayport.c
@@ -767,8 +767,10 @@ int dp_altmode_probe(struct typec_altmode *alt)
 	if (plug)
 		typec_altmode_set_drvdata(plug, dp);
 
-	dp->state = plug ? DP_STATE_ENTER_PRIME : DP_STATE_ENTER;
-	schedule_work(&dp->work);
+	if (port->auto_enter) {
+		dp->state = plug ? DP_STATE_ENTER_PRIME : DP_STATE_ENTER;
+		schedule_work(&dp->work);
+	}
 
 	return 0;
 }
diff --git a/drivers/usb/typec/altmodes/thunderbolt.c b/drivers/usb/typec/altmodes/thunderbolt.c
index 515e775ee41a..ba2ddaf3245e 100644
--- a/drivers/usb/typec/altmodes/thunderbolt.c
+++ b/drivers/usb/typec/altmodes/thunderbolt.c
@@ -225,6 +225,7 @@ static const struct typec_altmode_ops tbt_altmode_ops = {
 
 static int tbt_altmode_probe(struct typec_altmode *alt)
 {
+	const struct typec_altmode *port = typec_altmode_get_partner(alt);
 	struct tbt_altmode *tbt;
 
 	tbt = devm_kzalloc(&alt->dev, sizeof(*tbt), GFP_KERNEL);
@@ -239,7 +240,7 @@ static int tbt_altmode_probe(struct typec_altmode *alt)
 	typec_altmode_set_drvdata(alt, tbt);
 	typec_altmode_set_ops(alt, &tbt_altmode_ops);
 
-	if (tbt_ready(alt)) {
+	if (port->auto_enter && tbt_ready(alt)) {
 		if (tbt->plug[TYPEC_PLUG_SOP_PP])
 			tbt->state = TBT_STATE_SOP_PP_ENTER;
 		else if (tbt->plug[TYPEC_PLUG_SOP_P])
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 179856503d5d..a7ae0cdecca0 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -397,6 +397,31 @@ static ssize_t active_store(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RW(active);
 
+static ssize_t
+auto_enter_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct typec_altmode *alt = to_typec_altmode(dev);
+
+	return sprintf(buf, "%s\n", alt->auto_enter ? "yes" : "no");
+}
+
+static ssize_t auto_enter_store(struct device *dev, struct device_attribute *attr,
+			    const char *buf, size_t size)
+{
+	struct typec_altmode *adev = to_typec_altmode(dev);
+	bool auto_enter;
+	int ret;
+
+	ret = kstrtobool(buf, &auto_enter);
+	if (ret)
+		return ret;
+
+	adev->auto_enter = auto_enter;
+
+	return size;
+}
+static DEVICE_ATTR_RW(auto_enter);
+
 static ssize_t
 supported_roles_show(struct device *dev, struct device_attribute *attr,
 		     char *buf)
@@ -440,6 +465,7 @@ static DEVICE_ATTR_RO(svid);
 
 static struct attribute *typec_altmode_attrs[] = {
 	&dev_attr_active.attr,
+	&dev_attr_auto_enter.attr,
 	&dev_attr_mode.attr,
 	&dev_attr_svid.attr,
 	&dev_attr_vdo.attr,
@@ -455,6 +481,10 @@ static umode_t typec_altmode_attr_is_visible(struct kobject *kobj,
 		if (!adev->ops || !adev->ops->activate)
 			return 0444;
 
+	if (attr == &dev_attr_auto_enter.attr)
+		if (!is_typec_port(adev->dev.parent))
+			return 0;
+
 	return attr->mode;
 }
 
@@ -557,6 +587,7 @@ typec_register_altmode(struct device *parent,
 	if (is_port) {
 		alt->attrs[3] = &dev_attr_supported_roles.attr;
 		alt->adev.active = true; /* Enabled by default */
+		alt->adev.auto_enter = !desc->no_auto_enter;
 	}
 
 	sprintf(alt->group_name, "mode%d", desc->mode);
diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index 549275f8ac1b..67242355f78e 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -128,6 +128,7 @@ int typec_cable_set_identity(struct typec_cable *cable);
  * @svid: Standard or Vendor ID
  * @mode: Index of the Mode
  * @vdo: VDO returned by Discover Modes USB PD command
+ * @no_auto_enter: Only for ports. Disables auto enter which is default behavior.
  * @roles: Only for ports. DRP if the mode is available in both roles
  *
  * Description of an Alternate Mode which a connector, cable plug or partner
@@ -137,6 +138,7 @@ struct typec_altmode_desc {
 	u16			svid;
 	u8			mode;
 	u32			vdo;
+	bool			no_auto_enter;
 	/* Only used with ports */
 	enum typec_port_data	roles;
 };
diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
index b3c0866ea70f..ab7c3ebe4926 100644
--- a/include/linux/usb/typec_altmode.h
+++ b/include/linux/usb/typec_altmode.h
@@ -18,6 +18,7 @@ struct typec_altmode_ops;
  * @mode: Index of the Mode
  * @vdo: VDO returned by Discover Modes USB PD command
  * @active: Tells has the mode been entered or not
+ * @auto_enter: Tells whether to auto-enter mode (only valid for port mode).
  * @desc: Optional human readable description of the mode
  * @ops: Operations vector from the driver
  * @cable_ops: Cable operations vector from the driver.
@@ -28,6 +29,7 @@ struct typec_altmode {
 	int				mode;
 	u32				vdo;
 	unsigned int			active:1;
+	unsigned int			auto_enter:1;
 
 	char				*desc;
 	const struct typec_altmode_ops	*ops;
-- 
2.46.0.792.g87dc391469-goog

[PATCH 5/8] platform/chrome: cros_ec_typec: Update partner altmode active

[Abhishek Pandit-Subedi]

Mux configuration is often the final piece of mode entry and can be used
to determine whether a partner altmode is active. When mux configuration
is done, use the active port altmode's SVID to set the partner active
field for all partner alt modes.

Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
---

 drivers/platform/chrome/cros_ec_typec.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index 4d305876ec08..6c0228981627 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -618,6 +618,7 @@ static int cros_typec_configure_mux(struct cros_typec_data *typec, int port_num,
 	};
 	struct ec_params_usb_pd_mux_ack mux_ack;
 	enum typec_orientation orientation;
+	struct cros_typec_altmode_node *node, *n;
 	int ret;
 
 	ret = cros_ec_cmd(typec->ec, 0, EC_CMD_USB_PD_MUX_INFO,
@@ -676,6 +677,16 @@ static int cros_typec_configure_mux(struct cros_typec_data *typec, int port_num,
 			port->mux_flags);
 	}
 
+	/* Iterate all partner alt-modes and set the active alternate mode. */
+	list_for_each_entry_safe(node, n, &port->partner_mode_list, list) {
+		if (port->state.alt != NULL &&
+		    node->amode->svid == port->state.alt->svid) {
+			typec_altmode_update_active(node->amode, true);
+		} else {
+			typec_altmode_update_active(node->amode, false);
+		}
+	}
+
 mux_ack:
 	if (!typec->needs_mux_ack)
 		return ret;
-- 
2.46.0.792.g87dc391469-goog

Re: [PATCH 5/8] platform/chrome: cros_ec_typec: Update partner altmode active

[Heikki Krogerus]

On Wed, Sep 25, 2024 at 09:25:06AM -0700, Abhishek Pandit-Subedi wrote:
> Mux configuration is often the final piece of mode entry and can be used
> to determine whether a partner altmode is active. When mux configuration
> is done, use the active port altmode's SVID to set the partner active
> field for all partner alt modes.
> 
> Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> ---
> 
>  drivers/platform/chrome/cros_ec_typec.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> index 4d305876ec08..6c0228981627 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -618,6 +618,7 @@ static int cros_typec_configure_mux(struct cros_typec_data *typec, int port_num,
>  	};
>  	struct ec_params_usb_pd_mux_ack mux_ack;
>  	enum typec_orientation orientation;
> +	struct cros_typec_altmode_node *node, *n;
>  	int ret;
>  
>  	ret = cros_ec_cmd(typec->ec, 0, EC_CMD_USB_PD_MUX_INFO,
> @@ -676,6 +677,16 @@ static int cros_typec_configure_mux(struct cros_typec_data *typec, int port_num,
>  			port->mux_flags);
>  	}
>  
> +	/* Iterate all partner alt-modes and set the active alternate mode. */
> +	list_for_each_entry_safe(node, n, &port->partner_mode_list, list) {
> +		if (port->state.alt != NULL &&
> +		    node->amode->svid == port->state.alt->svid) {
> +			typec_altmode_update_active(node->amode, true);
> +		} else {
> +			typec_altmode_update_active(node->amode, false);
> +		}
> +	}

This can be done in the alt mode drivers.

thanks,

-- 
heikki

[PATCH 6/8] platform/chrome: cros_ec_typec: Displayport support

[Abhishek Pandit-Subedi]

Add support for entering and exiting displayport alt-mode on systems
using AP driven alt-mode.

Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
---

 MAINTAINERS                                   |   5 +-
 drivers/platform/chrome/Makefile              |   2 +
 drivers/platform/chrome/cros_ec_typec.c       |  13 +-
 drivers/platform/chrome/cros_ec_typec.h       |   1 +
 drivers/platform/chrome/cros_typec_altmode.h  |  34 +++
 .../platform/chrome/cros_typec_displayport.c  | 247 ++++++++++++++++++
 6 files changed, 299 insertions(+), 3 deletions(-)
 create mode 100644 drivers/platform/chrome/cros_typec_altmode.h
 create mode 100644 drivers/platform/chrome/cros_typec_displayport.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 10430778c998..d8baf38cacc6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5282,11 +5282,12 @@ F:	include/linux/platform_data/cros_usbpd_notify.h
 
 CHROMEOS EC USB TYPE-C DRIVER
 M:	Prashant Malani <pmalani@chromium.org>
+M:	Benson Leung <bleung@chromium.org>
+M:	Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
 L:	chrome-platform@lists.linux.dev
 S:	Maintained
 F:	drivers/platform/chrome/cros_ec_typec.*
-F:	drivers/platform/chrome/cros_typec_switch.c
-F:	drivers/platform/chrome/cros_typec_vdm.*
+F:	drivers/platform/chrome/cros_typec_*
 
 CHROMEOS HPS DRIVER
 M:	Dan Callaghan <dcallagh@chromium.org>
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index 2dcc6ccc2302..fe6c5234ac27 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -18,7 +18,9 @@ obj-$(CONFIG_CROS_EC_SPI)		+= cros_ec_spi.o
 obj-$(CONFIG_CROS_EC_UART)		+= cros_ec_uart.o
 cros_ec_lpcs-objs			:= cros_ec_lpc.o cros_ec_lpc_mec.o
 cros-ec-typec-objs			:= cros_ec_typec.o cros_typec_vdm.o
+cros-ec-typec-$(CONFIG_TYPEC_DP_ALTMODE) += cros_typec_displayport.o
 obj-$(CONFIG_CROS_EC_TYPEC)		+= cros-ec-typec.o
+
 obj-$(CONFIG_CROS_EC_LPC)		+= cros_ec_lpcs.o
 obj-$(CONFIG_CROS_EC_PROTO)		+= cros_ec_proto.o cros_ec_trace.o
 obj-$(CONFIG_CROS_KBD_LED_BACKLIGHT)	+= cros_kbd_led_backlight.o
diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index 6c0228981627..f9221d0d95f5 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -18,6 +18,7 @@
 
 #include "cros_ec_typec.h"
 #include "cros_typec_vdm.h"
+#include "cros_typec_altmode.h"
 
 #define DRV_NAME "cros-ec-typec"
 
@@ -293,12 +294,16 @@ static int cros_typec_register_port_altmodes(struct cros_typec_data *typec,
 	desc.svid = USB_TYPEC_DP_SID;
 	desc.mode = USB_TYPEC_DP_MODE;
 	desc.vdo = DP_PORT_VDO;
-	amode = typec_port_register_altmode(port->port, &desc);
+	amode = cros_typec_register_displayport(port, &desc,
+						typec->ap_driven_altmode);
 	if (IS_ERR(amode))
 		return PTR_ERR(amode);
 	port->port_altmode[CROS_EC_ALTMODE_DP] = amode;
+
+#if !IS_ENABLED(CONFIG_TYPEC_DP_ALTMODE)
 	typec_altmode_set_drvdata(amode, port);
 	amode->ops = &port_amode_ops;
+#endif
 
 	/*
 	 * Register TBT compatibility alt mode. The EC will not enter the mode
@@ -575,6 +580,10 @@ static int cros_typec_enable_dp(struct cros_typec_data *typec,
 	if (!ret)
 		ret = typec_mux_set(port->mux, &port->state);
 
+	if (!ret)
+		cros_typec_displayport_status_update(port->state.alt,
+						     port->state.data);
+
 	return ret;
 }
 
@@ -1254,6 +1263,8 @@ static int cros_typec_probe(struct platform_device *pdev)
 
 	typec->typec_cmd_supported = cros_ec_check_features(ec_dev, EC_FEATURE_TYPEC_CMD);
 	typec->needs_mux_ack = cros_ec_check_features(ec_dev, EC_FEATURE_TYPEC_MUX_REQUIRE_AP_ACK);
+	typec->ap_driven_altmode = cros_ec_check_features(
+		ec_dev, EC_FEATURE_TYPEC_REQUIRE_AP_MODE_ENTRY);
 
 	ret = cros_ec_cmd(typec->ec, 0, EC_CMD_USB_PD_PORTS, NULL, 0,
 			  &resp, sizeof(resp));
diff --git a/drivers/platform/chrome/cros_ec_typec.h b/drivers/platform/chrome/cros_ec_typec.h
index deda180a646f..9fd5342bb0ad 100644
--- a/drivers/platform/chrome/cros_ec_typec.h
+++ b/drivers/platform/chrome/cros_ec_typec.h
@@ -39,6 +39,7 @@ struct cros_typec_data {
 	struct work_struct port_work;
 	bool typec_cmd_supported;
 	bool needs_mux_ack;
+	bool ap_driven_altmode;
 };
 
 /* Per port data. */
diff --git a/drivers/platform/chrome/cros_typec_altmode.h b/drivers/platform/chrome/cros_typec_altmode.h
new file mode 100644
index 000000000000..a8b37a18c83a
--- /dev/null
+++ b/drivers/platform/chrome/cros_typec_altmode.h
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __CROS_TYPEC_ALTMODE_H__
+#define __CROS_TYPEC_ALTMODE_H__
+
+struct cros_typec_port;
+struct typec_altmode;
+struct typec_altmode_desc;
+struct typec_displayport_data;
+
+#if IS_ENABLED(CONFIG_TYPEC_DP_ALTMODE)
+struct typec_altmode *
+cros_typec_register_displayport(struct cros_typec_port *port,
+				struct typec_altmode_desc *desc,
+				bool ap_mode_entry);
+
+int cros_typec_displayport_status_update(struct typec_altmode *altmode,
+					 struct typec_displayport_data *data);
+#else
+struct typec_altmode *
+cros_typec_register_displayport(struct cros_typec_port *port,
+				struct typec_altmode_desc *desc,
+				bool ap_mode_entry)
+{
+	return typec_port_register_altmode(port->port, desc);
+}
+
+int cros_typec_displayport_status_update(struct typec_altmode *altmode,
+					 struct typec_displayport_data *data)
+{
+	return 0;
+}
+#endif
+#endif /* __CROS_TYPEC_ALTMODE_H__ */
diff --git a/drivers/platform/chrome/cros_typec_displayport.c b/drivers/platform/chrome/cros_typec_displayport.c
new file mode 100644
index 000000000000..bccd57290601
--- /dev/null
+++ b/drivers/platform/chrome/cros_typec_displayport.c
@@ -0,0 +1,247 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Alt-mode implementation for Displayport on ChromeOS EC.
+ *
+ * Copyright 2024 Google LLC
+ * Author: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
+ */
+#include "cros_ec_typec.h"
+
+#include <linux/usb/typec_dp.h>
+#include <linux/usb/pd_vdo.h>
+
+#include "cros_typec_altmode.h"
+
+struct typec_dp_data {
+	struct typec_displayport_data data;
+	struct work_struct work;
+
+	struct cros_typec_port *port;
+	struct typec_altmode *alt;
+	bool ap_mode_entry;
+	bool configured;
+	bool pending_status_update;
+
+	u32 header;
+	u32 *vdo_data;
+	u8 vdo_size;
+};
+
+static void cros_typec_displayport_work(struct work_struct *work)
+{
+	struct typec_dp_data *data =
+		container_of(work, struct typec_dp_data, work);
+
+	if (typec_altmode_vdm(data->alt, data->header, data->vdo_data,
+			      data->vdo_size))
+		dev_err(&data->alt->dev, "VDM 0x%x failed", data->header);
+
+	data->header = 0;
+	data->vdo_data = NULL;
+	data->vdo_size = 0;
+}
+
+static int cros_typec_displayport_enter(struct typec_altmode *alt, u32 *vdo)
+{
+	struct typec_dp_data *data = typec_altmode_get_drvdata(alt);
+	struct ec_params_typec_control req = {
+		.port = data->port->port_num,
+		.command = TYPEC_CONTROL_COMMAND_ENTER_MODE,
+		.mode_to_enter = CROS_EC_ALTMODE_DP,
+	};
+	int svdm_version;
+	int ret;
+
+	if (!data->ap_mode_entry) {
+		const struct typec_altmode *partner =
+			typec_altmode_get_partner(alt);
+		dev_warn(&partner->dev,
+			 "EC does not support ap driven mode entry\n");
+		return -EOPNOTSUPP;
+	}
+
+	ret = cros_ec_cmd(data->port->typec_data->ec, 0, EC_CMD_TYPEC_CONTROL,
+			  &req, sizeof(req), NULL, 0);
+	if (ret < 0)
+		return ret;
+
+	svdm_version = typec_altmode_get_svdm_version(alt);
+	if (svdm_version < 0)
+		return svdm_version;
+
+	data->header = VDO(USB_TYPEC_DP_SID, 1, svdm_version, CMD_ENTER_MODE);
+	data->header |= VDO_OPOS(USB_TYPEC_DP_MODE);
+	data->header |= VDO_CMDT(CMDT_RSP_ACK);
+
+	data->vdo_data = NULL;
+	data->vdo_size = 1;
+
+	schedule_work(&data->work);
+
+	return ret;
+}
+
+static int cros_typec_displayport_exit(struct typec_altmode *alt)
+{
+	struct typec_dp_data *data = typec_altmode_get_drvdata(alt);
+	struct ec_params_typec_control req = {
+		.port = data->port->port_num,
+		.command = TYPEC_CONTROL_COMMAND_EXIT_MODES,
+	};
+	int svdm_version;
+	int ret;
+
+	if (!data->ap_mode_entry) {
+		const struct typec_altmode *partner =
+			typec_altmode_get_partner(alt);
+		dev_warn(&partner->dev,
+			 "EC does not support ap driven mode entry\n");
+		return -EOPNOTSUPP;
+	}
+
+	ret = cros_ec_cmd(data->port->typec_data->ec, 0, EC_CMD_TYPEC_CONTROL,
+			  &req, sizeof(req), NULL, 0);
+
+	if (ret < 0)
+		return ret;
+
+	svdm_version = typec_altmode_get_svdm_version(alt);
+	if (svdm_version < 0)
+		return svdm_version;
+
+	data->header = VDO(USB_TYPEC_DP_SID, 1, svdm_version, CMD_EXIT_MODE);
+	data->header |= VDO_OPOS(USB_TYPEC_DP_MODE);
+	data->header |= VDO_CMDT(CMDT_RSP_ACK);
+
+	data->vdo_data = NULL;
+	data->vdo_size = 1;
+
+	schedule_work(&data->work);
+
+	return ret;
+}
+
+int cros_typec_displayport_status_update(struct typec_altmode *altmode,
+					 struct typec_displayport_data *data)
+{
+	struct typec_dp_data *dp_data = typec_altmode_get_drvdata(altmode);
+
+	if (!dp_data->pending_status_update) {
+		dev_dbg(&altmode->dev,
+			"Got DPStatus without a pending request");
+		return 0;
+	}
+
+	if (dp_data->configured && dp_data->data.conf != data->conf)
+		dev_dbg(&altmode->dev,
+			"DP Conf doesn't match. Requested 0x%04x, Actual 0x%04x",
+			dp_data->data.conf, data->conf);
+
+	dp_data->header |= VDO_CMDT(CMDT_RSP_ACK);
+	dp_data->data = *data;
+	dp_data->vdo_data = &dp_data->data.status;
+	dp_data->vdo_size = 2;
+	dp_data->pending_status_update = false;
+
+	schedule_work(&dp_data->work);
+	return 0;
+}
+
+static int cros_typec_displayport_vdm(struct typec_altmode *alt, u32 header,
+				      const u32 *data, int count)
+{
+	struct typec_dp_data *dp_data = typec_altmode_get_drvdata(alt);
+
+	int cmd_type = PD_VDO_CMDT(header);
+	int cmd = PD_VDO_CMD(header);
+	int svdm_version;
+
+	if (!dp_data->ap_mode_entry) {
+		const struct typec_altmode *partner =
+			typec_altmode_get_partner(alt);
+		dev_warn(&partner->dev,
+			 "EC does not support ap driven mode entry\n");
+		return -EOPNOTSUPP;
+	}
+
+	svdm_version = typec_altmode_get_svdm_version(alt);
+	if (svdm_version < 0)
+		return svdm_version;
+
+	switch (cmd_type) {
+	case CMDT_INIT:
+		if (PD_VDO_SVDM_VER(header) < svdm_version) {
+			typec_partner_set_svdm_version(dp_data->port->partner,
+						       PD_VDO_SVDM_VER(header));
+			svdm_version = PD_VDO_SVDM_VER(header);
+		}
+
+		dp_data->header = VDO(USB_TYPEC_DP_SID, 1, svdm_version, cmd);
+		dp_data->header |= VDO_OPOS(USB_TYPEC_DP_MODE);
+
+		/*
+		 * DP_CMD_CONFIGURE: We can't actually do anything with the
+		 * provided VDO yet so just send back an ACK.
+		 *
+		 * DP_CMD_STATUS_UPDATE: We wait for Mux changes to send
+		 * DPStatus Acks.
+		 */
+		switch (cmd) {
+		case DP_CMD_CONFIGURE:
+			dp_data->data.conf = *data;
+			dp_data->header |= VDO_CMDT(CMDT_RSP_ACK);
+			dp_data->configured = true;
+			schedule_work(&dp_data->work);
+			break;
+		case DP_CMD_STATUS_UPDATE:
+			dp_data->pending_status_update = true;
+			break;
+		default:
+			dp_data->header |= VDO_CMDT(CMDT_RSP_ACK);
+			schedule_work(&dp_data->work);
+			break;
+		}
+
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static const struct typec_altmode_ops cros_typec_displayport_ops = {
+	.enter = cros_typec_displayport_enter,
+	.exit = cros_typec_displayport_exit,
+	.vdm = cros_typec_displayport_vdm,
+};
+
+struct typec_altmode *
+cros_typec_register_displayport(struct cros_typec_port *port,
+				struct typec_altmode_desc *desc,
+				bool ap_mode_entry)
+{
+	struct typec_altmode *alt;
+	struct typec_dp_data *data;
+
+	alt = typec_port_register_altmode(port->port, desc);
+	if (IS_ERR(alt))
+		return alt;
+
+	data = devm_kzalloc(&alt->dev, sizeof(*data), GFP_KERNEL);
+	if (!data) {
+		typec_unregister_altmode(alt);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	INIT_WORK(&data->work, cros_typec_displayport_work);
+	data->alt = alt;
+	data->port = port;
+	data->ap_mode_entry = ap_mode_entry;
+	data->configured = false;
+
+	typec_altmode_set_ops(alt, &cros_typec_displayport_ops);
+	typec_altmode_set_drvdata(alt, data);
+
+	return alt;
+}
-- 
2.46.0.792.g87dc391469-goog

Re: [PATCH 6/8] platform/chrome: cros_ec_typec: Displayport support

[Heikki Krogerus]

> diff --git a/drivers/platform/chrome/cros_typec_altmode.h b/drivers/platform/chrome/cros_typec_altmode.h
> new file mode 100644
> index 000000000000..a8b37a18c83a
> --- /dev/null
> +++ b/drivers/platform/chrome/cros_typec_altmode.h
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef __CROS_TYPEC_ALTMODE_H__
> +#define __CROS_TYPEC_ALTMODE_H__
> +
> +struct cros_typec_port;
> +struct typec_altmode;
> +struct typec_altmode_desc;
> +struct typec_displayport_data;
> +
> +#if IS_ENABLED(CONFIG_TYPEC_DP_ALTMODE)
> +struct typec_altmode *
> +cros_typec_register_displayport(struct cros_typec_port *port,
> +				struct typec_altmode_desc *desc,
> +				bool ap_mode_entry);
> +
> +int cros_typec_displayport_status_update(struct typec_altmode *altmode,
> +					 struct typec_displayport_data *data);
> +#else
> +struct typec_altmode *

static inline struct typec_altmode *

> +cros_typec_register_displayport(struct cros_typec_port *port,
> +				struct typec_altmode_desc *desc,
> +				bool ap_mode_entry)
> +{
> +	return typec_port_register_altmode(port->port, desc);
> +}
> +
> +int cros_typec_displayport_status_update(struct typec_altmode *altmode,
> +					 struct typec_displayport_data *data)

static inline int

> +{
> +	return 0;
> +}
> +#endif
> +#endif /* __CROS_TYPEC_ALTMODE_H__ */

thanks,

-- 
heikki

Re: [PATCH 6/8] platform/chrome: cros_ec_typec: Displayport support

[Dmitry Baryshkov]

On Wed, Sep 25, 2024 at 09:25:07AM GMT, Abhishek Pandit-Subedi wrote:
> Add support for entering and exiting displayport alt-mode on systems
> using AP driven alt-mode.
> 
> Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> ---
> 
>  MAINTAINERS                                   |   5 +-
>  drivers/platform/chrome/Makefile              |   2 +
>  drivers/platform/chrome/cros_ec_typec.c       |  13 +-
>  drivers/platform/chrome/cros_ec_typec.h       |   1 +
>  drivers/platform/chrome/cros_typec_altmode.h  |  34 +++
>  .../platform/chrome/cros_typec_displayport.c  | 247 ++++++++++++++++++
>  6 files changed, 299 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/platform/chrome/cros_typec_altmode.h
>  create mode 100644 drivers/platform/chrome/cros_typec_displayport.c
> 
> diff --git a/drivers/platform/chrome/cros_typec_altmode.h b/drivers/platform/chrome/cros_typec_altmode.h
> new file mode 100644
> index 000000000000..a8b37a18c83a
> --- /dev/null
> +++ b/drivers/platform/chrome/cros_typec_altmode.h
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef __CROS_TYPEC_ALTMODE_H__
> +#define __CROS_TYPEC_ALTMODE_H__
> +
> +struct cros_typec_port;
> +struct typec_altmode;
> +struct typec_altmode_desc;
> +struct typec_displayport_data;
> +
> +#if IS_ENABLED(CONFIG_TYPEC_DP_ALTMODE)
> +struct typec_altmode *
> +cros_typec_register_displayport(struct cros_typec_port *port,
> +				struct typec_altmode_desc *desc,
> +				bool ap_mode_entry);
> +
> +int cros_typec_displayport_status_update(struct typec_altmode *altmode,
> +					 struct typec_displayport_data *data);
> +#else

As pointed out by the robot, the stubs should be 'static inline'

> +struct typec_altmode *
> +cros_typec_register_displayport(struct cros_typec_port *port,
> +				struct typec_altmode_desc *desc,
> +				bool ap_mode_entry)
> +{
> +	return typec_port_register_altmode(port->port, desc);
> +}
> +
> +int cros_typec_displayport_status_update(struct typec_altmode *altmode,
> +					 struct typec_displayport_data *data)
> +{
> +	return 0;
> +}
> +#endif
> +#endif /* __CROS_TYPEC_ALTMODE_H__ */

-- 
With best wishes
Dmitry

Re: [PATCH 6/8] platform/chrome: cros_ec_typec: Displayport support

[kernel test robot]

Hi Abhishek,

kernel test robot noticed the following build warnings:

[auto build test WARNING on chrome-platform/for-next]
[also build test WARNING on chrome-platform/for-firmware-next usb/usb-testing usb/usb-next usb/usb-linus linus/master v6.11 next-20240927]
[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/Abhishek-Pandit-Subedi/usb-typec-Add-driver-for-Thunderbolt-3-Alternate-Mode/20240926-002931
base:   https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git for-next
patch link:    https://lore.kernel.org/r/20240925092505.6.I142fc0c09df58689b98f0cebf1c5e48b9d4fa800%40changeid
patch subject: [PATCH 6/8] platform/chrome: cros_ec_typec: Displayport support
config: i386-randconfig-001-20240928 (https://download.01.org/0day-ci/archive/20240928/202409281250.dtPQ2TrT-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240928/202409281250.dtPQ2TrT-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/202409281250.dtPQ2TrT-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/platform/chrome/cros_ec_typec.c:21:
>> drivers/platform/chrome/cros_typec_altmode.h:21:1: warning: no previous prototype for function 'cros_typec_register_displayport' [-Wmissing-prototypes]
      21 | cros_typec_register_displayport(struct cros_typec_port *port,
         | ^
   drivers/platform/chrome/cros_typec_altmode.h:20:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
      20 | struct typec_altmode *
         | ^
         | static 
>> drivers/platform/chrome/cros_typec_altmode.h:28:5: warning: no previous prototype for function 'cros_typec_displayport_status_update' [-Wmissing-prototypes]
      28 | int cros_typec_displayport_status_update(struct typec_altmode *altmode,
         |     ^
   drivers/platform/chrome/cros_typec_altmode.h:28:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
      28 | int cros_typec_displayport_status_update(struct typec_altmode *altmode,
         | ^
         | static 
   2 warnings generated.


vim +/cros_typec_register_displayport +21 drivers/platform/chrome/cros_typec_altmode.h

    10	
    11	#if IS_ENABLED(CONFIG_TYPEC_DP_ALTMODE)
    12	struct typec_altmode *
    13	cros_typec_register_displayport(struct cros_typec_port *port,
    14					struct typec_altmode_desc *desc,
    15					bool ap_mode_entry);
    16	
    17	int cros_typec_displayport_status_update(struct typec_altmode *altmode,
    18						 struct typec_displayport_data *data);
    19	#else
    20	struct typec_altmode *
  > 21	cros_typec_register_displayport(struct cros_typec_port *port,
    22					struct typec_altmode_desc *desc,
    23					bool ap_mode_entry)
    24	{
    25		return typec_port_register_altmode(port->port, desc);
    26	}
    27	
  > 28	int cros_typec_displayport_status_update(struct typec_altmode *altmode,

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

[PATCH 7/8] platform/chrome: cros_ec_typec: Thunderbolt support

[Abhishek Pandit-Subedi]

Add support for entering and exiting Thunderbolt alt-mode using AP
driven alt-mode.

Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
---

 drivers/platform/chrome/Makefile              |   1 +
 drivers/platform/chrome/cros_ec_typec.c       |  29 +--
 drivers/platform/chrome/cros_typec_altmode.h  |  14 ++
 .../platform/chrome/cros_typec_thunderbolt.c  | 184 ++++++++++++++++++
 4 files changed, 216 insertions(+), 12 deletions(-)
 create mode 100644 drivers/platform/chrome/cros_typec_thunderbolt.c

diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index fe6c5234ac27..da7a44738171 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -19,6 +19,7 @@ obj-$(CONFIG_CROS_EC_UART)		+= cros_ec_uart.o
 cros_ec_lpcs-objs			:= cros_ec_lpc.o cros_ec_lpc_mec.o
 cros-ec-typec-objs			:= cros_ec_typec.o cros_typec_vdm.o
 cros-ec-typec-$(CONFIG_TYPEC_DP_ALTMODE) += cros_typec_displayport.o
+cros-ec-typec-$(CONFIG_TYPEC_TBT_ALTMODE) += cros_typec_thunderbolt.o
 obj-$(CONFIG_CROS_EC_TYPEC)		+= cros-ec-typec.o
 
 obj-$(CONFIG_CROS_EC_LPC)		+= cros_ec_lpcs.o
diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index f9221d0d95f5..ec13d84d11b8 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -304,21 +304,26 @@ static int cros_typec_register_port_altmodes(struct cros_typec_data *typec,
 	typec_altmode_set_drvdata(amode, port);
 	amode->ops = &port_amode_ops;
 #endif
-
 	/*
 	 * Register TBT compatibility alt mode. The EC will not enter the mode
-	 * if it doesn't support it, so it's safe to register it unconditionally
-	 * here for now.
+	 * if it doesn't support it and it will not enter automatically by
+	 * design so we can use the |ap_driven_altmode| feature to check if we
+	 * should register it.
 	 */
-	memset(&desc, 0, sizeof(desc));
-	desc.svid = USB_TYPEC_TBT_SID;
-	desc.mode = TYPEC_ANY_MODE;
-	amode = typec_port_register_altmode(port->port, &desc);
-	if (IS_ERR(amode))
-		return PTR_ERR(amode);
-	port->port_altmode[CROS_EC_ALTMODE_TBT] = amode;
-	typec_altmode_set_drvdata(amode, port);
-	amode->ops = &port_amode_ops;
+	if (typec->ap_driven_altmode) {
+		memset(&desc, 0, sizeof(desc));
+		desc.svid = USB_TYPEC_TBT_SID;
+		desc.mode = TYPEC_ANY_MODE;
+		amode = cros_typec_register_thunderbolt(port, &desc);
+		if (IS_ERR(amode))
+			return PTR_ERR(amode);
+		port->port_altmode[CROS_EC_ALTMODE_TBT] = amode;
+
+#if !IS_ENABLED(CONFIG_TYPEC_TBT_ALTMODE)
+		typec_altmode_set_drvdata(amode, port);
+		amode->ops = &port_amode_ops;
+#endif
+	}
 
 	port->state.alt = NULL;
 	port->state.mode = TYPEC_STATE_USB;
diff --git a/drivers/platform/chrome/cros_typec_altmode.h b/drivers/platform/chrome/cros_typec_altmode.h
index a8b37a18c83a..24e766189211 100644
--- a/drivers/platform/chrome/cros_typec_altmode.h
+++ b/drivers/platform/chrome/cros_typec_altmode.h
@@ -31,4 +31,18 @@ int cros_typec_displayport_status_update(struct typec_altmode *altmode,
 	return 0;
 }
 #endif
+
+#if IS_ENABLED(CONFIG_TYPEC_TBT_ALTMODE)
+struct typec_altmode *
+cros_typec_register_thunderbolt(struct cros_typec_port *port,
+				struct typec_altmode_desc *desc);
+#else
+struct typec_altmode *
+cros_typec_register_thunderbolt(struct cros_typec_port *port,
+				struct typec_altmode_desc *desc)
+{
+	return typec_port_register_altmode(port->port, desc);
+}
+#endif
+
 #endif /* __CROS_TYPEC_ALTMODE_H__ */
diff --git a/drivers/platform/chrome/cros_typec_thunderbolt.c b/drivers/platform/chrome/cros_typec_thunderbolt.c
new file mode 100644
index 000000000000..b399237b773f
--- /dev/null
+++ b/drivers/platform/chrome/cros_typec_thunderbolt.c
@@ -0,0 +1,184 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Alt-mode implementation for Thunderbolt on ChromeOS EC.
+ *
+ * Copyright 2024 Google LLC
+ * Author: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
+ */
+#include "cros_ec_typec.h"
+
+#include <linux/usb/typec_tbt.h>
+#include <linux/usb/pd_vdo.h>
+
+#include "cros_typec_altmode.h"
+
+struct typec_tbt_data {
+	struct work_struct work;
+
+	struct cros_typec_port *port;
+	struct typec_altmode *alt;
+
+	u32 header;
+	u32 *vdo_data;
+	u8 vdo_size;
+};
+
+static void cros_typec_thunderbolt_work(struct work_struct *work)
+{
+	struct typec_tbt_data *data =
+		container_of(work, struct typec_tbt_data, work);
+
+	if (typec_altmode_vdm(data->alt, data->header, data->vdo_data,
+			      data->vdo_size))
+		dev_err(&data->alt->dev, "VDM 0x%x failed", data->header);
+
+	data->header = 0;
+	data->vdo_data = NULL;
+	data->vdo_size = 0;
+}
+
+static int cros_typec_thunderbolt_enter(struct typec_altmode *alt, u32 *vdo)
+{
+	struct typec_tbt_data *data = typec_altmode_get_drvdata(alt);
+	struct ec_params_typec_control req = {
+		.port = data->port->port_num,
+		.command = TYPEC_CONTROL_COMMAND_ENTER_MODE,
+		.mode_to_enter = CROS_EC_ALTMODE_TBT,
+	};
+	int svdm_version;
+	int ret;
+
+	ret = cros_ec_cmd(data->port->typec_data->ec, 0, EC_CMD_TYPEC_CONTROL,
+			  &req, sizeof(req), NULL, 0);
+	if (ret < 0)
+		return ret;
+
+	svdm_version = typec_altmode_get_svdm_version(alt);
+	if (svdm_version < 0)
+		return svdm_version;
+
+	data->header = VDO(USB_TYPEC_TBT_SID, 1, svdm_version, CMD_ENTER_MODE);
+	data->header |= VDO_OPOS(TYPEC_TBT_MODE);
+	data->header |= VDO_CMDT(CMDT_RSP_ACK);
+
+	data->vdo_data = NULL;
+	data->vdo_size = 1;
+
+	schedule_work(&data->work);
+
+	return ret;
+}
+
+static int cros_typec_thunderbolt_exit(struct typec_altmode *alt)
+{
+	struct typec_tbt_data *data = typec_altmode_get_drvdata(alt);
+	struct ec_params_typec_control req = {
+		.port = data->port->port_num,
+		.command = TYPEC_CONTROL_COMMAND_EXIT_MODES,
+	};
+	int svdm_version;
+	int ret;
+
+	ret = cros_ec_cmd(data->port->typec_data->ec, 0, EC_CMD_TYPEC_CONTROL,
+			  &req, sizeof(req), NULL, 0);
+
+	if (ret < 0)
+		return ret;
+
+	svdm_version = typec_altmode_get_svdm_version(alt);
+	if (svdm_version < 0)
+		return svdm_version;
+
+	data->header = VDO(USB_TYPEC_TBT_SID, 1, svdm_version, CMD_EXIT_MODE);
+	data->header |= VDO_OPOS(TYPEC_TBT_MODE);
+	data->header |= VDO_CMDT(CMDT_RSP_ACK);
+
+	data->vdo_data = NULL;
+	data->vdo_size = 1;
+
+	schedule_work(&data->work);
+
+	return ret;
+}
+
+static int cros_typec_thunderbolt_vdm(struct typec_altmode *alt, u32 header,
+				      const u32 *data, int count)
+{
+	struct typec_tbt_data *tbt_data = typec_altmode_get_drvdata(alt);
+
+	int cmd_type = PD_VDO_CMDT(header);
+	int cmd = PD_VDO_CMD(header);
+	int svdm_version;
+
+	svdm_version = typec_altmode_get_svdm_version(alt);
+	if (svdm_version < 0)
+		return svdm_version;
+
+	switch (cmd_type) {
+	case CMDT_INIT:
+		if (PD_VDO_SVDM_VER(header) < svdm_version) {
+			typec_partner_set_svdm_version(tbt_data->port->partner,
+						       PD_VDO_SVDM_VER(header));
+			svdm_version = PD_VDO_SVDM_VER(header);
+		}
+
+		tbt_data->header = VDO(USB_TYPEC_TBT_SID, 1, svdm_version, cmd);
+		tbt_data->header |= VDO_OPOS(TYPEC_TBT_MODE);
+
+		/*
+		 * TODO - Just always reply to the VDMs that we are done.
+		 */
+		switch (cmd) {
+		case CMD_ENTER_MODE:
+			/* Don't respond to the enter mode vdm because it
+			 * triggers mux configuration. This is handled directly
+			 * by the cros_ec_typec driver so the Thunderbolt driver
+			 * doesn't need to be involved.
+			 */
+			break;
+		default:
+			tbt_data->header |= VDO_CMDT(CMDT_RSP_ACK);
+			schedule_work(&tbt_data->work);
+			break;
+		}
+
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static const struct typec_altmode_ops cros_typec_thunderbolt_ops = {
+	.enter = cros_typec_thunderbolt_enter,
+	.exit = cros_typec_thunderbolt_exit,
+	.vdm = cros_typec_thunderbolt_vdm,
+};
+
+struct typec_altmode *
+cros_typec_register_thunderbolt(struct cros_typec_port *port,
+				struct typec_altmode_desc *desc)
+{
+	struct typec_altmode *alt;
+	struct typec_tbt_data *data;
+
+	alt = typec_port_register_altmode(port->port, desc);
+	if (IS_ERR(alt))
+		return alt;
+
+	data = devm_kzalloc(&alt->dev, sizeof(*data), GFP_KERNEL);
+	if (!data) {
+		typec_unregister_altmode(alt);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	INIT_WORK(&data->work, cros_typec_thunderbolt_work);
+	data->alt = alt;
+	data->port = port;
+
+	typec_altmode_set_ops(alt, &cros_typec_thunderbolt_ops);
+	typec_altmode_set_drvdata(alt, data);
+
+	return alt;
+}
-- 
2.46.0.792.g87dc391469-goog

Re: [PATCH 7/8] platform/chrome: cros_ec_typec: Thunderbolt support

[Dmitry Baryshkov]

On Wed, Sep 25, 2024 at 09:25:08AM GMT, Abhishek Pandit-Subedi wrote:
> Add support for entering and exiting Thunderbolt alt-mode using AP
> driven alt-mode.
> 
> Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> ---
> 
>  drivers/platform/chrome/Makefile              |   1 +
>  drivers/platform/chrome/cros_ec_typec.c       |  29 +--
>  drivers/platform/chrome/cros_typec_altmode.h  |  14 ++
>  .../platform/chrome/cros_typec_thunderbolt.c  | 184 ++++++++++++++++++
>  4 files changed, 216 insertions(+), 12 deletions(-)
>  create mode 100644 drivers/platform/chrome/cros_typec_thunderbolt.c

This patch looks like nearly exact 1:1 copy of the previous one. Please
merge both altmode implementations in the same way as tcpm.c does.

-- 
With best wishes
Dmitry

Re: [PATCH 7/8] platform/chrome: cros_ec_typec: Thunderbolt support

[Abhishek Pandit-Subedi]

On Wed, Sep 25, 2024 at 10:13 AM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Wed, Sep 25, 2024 at 09:25:08AM GMT, Abhishek Pandit-Subedi wrote:
> > Add support for entering and exiting Thunderbolt alt-mode using AP
> > driven alt-mode.
> >
> > Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> > ---
> >
> >  drivers/platform/chrome/Makefile              |   1 +
> >  drivers/platform/chrome/cros_ec_typec.c       |  29 +--
> >  drivers/platform/chrome/cros_typec_altmode.h  |  14 ++
> >  .../platform/chrome/cros_typec_thunderbolt.c  | 184 ++++++++++++++++++
> >  4 files changed, 216 insertions(+), 12 deletions(-)
> >  create mode 100644 drivers/platform/chrome/cros_typec_thunderbolt.c
>
> This patch looks like nearly exact 1:1 copy of the previous one. Please
> merge both altmode implementations in the same way as tcpm.c does.

It's easier for tcpm.c to have a merged implementation because it
simply forwards VDMs to the internal state machine to handle without
doing anything with them. Our implementation is closer to
ucsi/displayport.c which needs to maintain an internal state machine
for DP and TBT VDMs and respond differently.

I can merge the two but I'd like to understand intent (reduce code
duplication? reduce the number of files?). As it is, keeping the files
separate makes it easier to understand how each alt-mode operates in
my opinion.

>
> --
> With best wishes
> Dmitry

Re: [PATCH 7/8] platform/chrome: cros_ec_typec: Thunderbolt support

[Dmitry Baryshkov]

On Wed, Sep 25, 2024 at 11:42:46AM GMT, Abhishek Pandit-Subedi wrote:
> On Wed, Sep 25, 2024 at 10:13 AM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
> >
> > On Wed, Sep 25, 2024 at 09:25:08AM GMT, Abhishek Pandit-Subedi wrote:
> > > Add support for entering and exiting Thunderbolt alt-mode using AP
> > > driven alt-mode.
> > >
> > > Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> > > ---
> > >
> > >  drivers/platform/chrome/Makefile              |   1 +
> > >  drivers/platform/chrome/cros_ec_typec.c       |  29 +--
> > >  drivers/platform/chrome/cros_typec_altmode.h  |  14 ++
> > >  .../platform/chrome/cros_typec_thunderbolt.c  | 184 ++++++++++++++++++
> > >  4 files changed, 216 insertions(+), 12 deletions(-)
> > >  create mode 100644 drivers/platform/chrome/cros_typec_thunderbolt.c
> >
> > This patch looks like nearly exact 1:1 copy of the previous one. Please
> > merge both altmode implementations in the same way as tcpm.c does.
> 
> It's easier for tcpm.c to have a merged implementation because it
> simply forwards VDMs to the internal state machine to handle without
> doing anything with them. Our implementation is closer to
> ucsi/displayport.c which needs to maintain an internal state machine
> for DP and TBT VDMs and respond differently.
> 
> I can merge the two but I'd like to understand intent (reduce code
> duplication? reduce the number of files?). As it is, keeping the files
> separate makes it easier to understand how each alt-mode operates in
> my opinion.

Separate common code and AltMode-specific code. This way we reduce a
risk of errors fixed in only one of two drivers and at the same time the
driver clearly separates common vs specific code paths (e.g. VDM
handling is mode-specific, while the rest of the code is common).

> 
> >
> > --
> > With best wishes
> > Dmitry

-- 
With best wishes
Dmitry

Re: [PATCH 7/8] platform/chrome: cros_ec_typec: Thunderbolt support

[Abhishek Pandit-Subedi]

On Wed, Sep 25, 2024 at 2:20 PM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Wed, Sep 25, 2024 at 11:42:46AM GMT, Abhishek Pandit-Subedi wrote:
> > On Wed, Sep 25, 2024 at 10:13 AM Dmitry Baryshkov
> > <dmitry.baryshkov@linaro.org> wrote:
> > >
> > > On Wed, Sep 25, 2024 at 09:25:08AM GMT, Abhishek Pandit-Subedi wrote:
> > > > Add support for entering and exiting Thunderbolt alt-mode using AP
> > > > driven alt-mode.
> > > >
> > > > Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> > > > ---
> > > >
> > > >  drivers/platform/chrome/Makefile              |   1 +
> > > >  drivers/platform/chrome/cros_ec_typec.c       |  29 +--
> > > >  drivers/platform/chrome/cros_typec_altmode.h  |  14 ++
> > > >  .../platform/chrome/cros_typec_thunderbolt.c  | 184 ++++++++++++++++++
> > > >  4 files changed, 216 insertions(+), 12 deletions(-)
> > > >  create mode 100644 drivers/platform/chrome/cros_typec_thunderbolt.c
> > >
> > > This patch looks like nearly exact 1:1 copy of the previous one. Please
> > > merge both altmode implementations in the same way as tcpm.c does.
> >
> > It's easier for tcpm.c to have a merged implementation because it
> > simply forwards VDMs to the internal state machine to handle without
> > doing anything with them. Our implementation is closer to
> > ucsi/displayport.c which needs to maintain an internal state machine
> > for DP and TBT VDMs and respond differently.
> >
> > I can merge the two but I'd like to understand intent (reduce code
> > duplication? reduce the number of files?). As it is, keeping the files
> > separate makes it easier to understand how each alt-mode operates in
> > my opinion.
>
> Separate common code and AltMode-specific code. This way we reduce a
> risk of errors fixed in only one of two drivers and at the same time the
> driver clearly separates common vs specific code paths (e.g. VDM
> handling is mode-specific, while the rest of the code is common).

Ack -- I'll provide a merged solution in the next version of this patch series.

>
> >
> > >
> > > --
> > > With best wishes
> > > Dmitry
>
> --
> With best wishes
> Dmitry

Re: [PATCH 7/8] platform/chrome: cros_ec_typec: Thunderbolt support

[kernel test robot]

Hi Abhishek,

kernel test robot noticed the following build errors:

[auto build test ERROR on chrome-platform/for-next]
[also build test ERROR on chrome-platform/for-firmware-next usb/usb-testing usb/usb-next usb/usb-linus linus/master v6.11 next-20240926]
[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/Abhishek-Pandit-Subedi/usb-typec-Add-driver-for-Thunderbolt-3-Alternate-Mode/20240926-002931
base:   https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git for-next
patch link:    https://lore.kernel.org/r/20240925092505.7.Ic61ced3cdfb5d6776435356061f12307da719829%40changeid
patch subject: [PATCH 7/8] platform/chrome: cros_ec_typec: Thunderbolt support
config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20240926/202409262259.lXP7G1FE-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240926/202409262259.lXP7G1FE-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/202409262259.lXP7G1FE-lkp@intel.com/

All errors (new ones prefixed by >>):

   sh4-linux-ld: drivers/platform/chrome/cros_typec_thunderbolt.o: in function `cros_typec_register_displayport':
>> (.text+0x34c): multiple definition of `cros_typec_register_displayport'; drivers/platform/chrome/cros_ec_typec.o:(.text+0x2118): first defined here
   sh4-linux-ld: drivers/platform/chrome/cros_typec_thunderbolt.o: in function `cros_typec_displayport_status_update':
>> (.text+0x37c): multiple definition of `cros_typec_displayport_status_update'; drivers/platform/chrome/cros_ec_typec.o:(.text+0x2148): first defined here

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

Re: [PATCH 7/8] platform/chrome: cros_ec_typec: Thunderbolt support

[kernel test robot]

Hi Abhishek,

kernel test robot noticed the following build errors:

[auto build test ERROR on chrome-platform/for-next]
[also build test ERROR on chrome-platform/for-firmware-next usb/usb-testing usb/usb-next usb/usb-linus linus/master v6.11 next-20240927]
[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/Abhishek-Pandit-Subedi/usb-typec-Add-driver-for-Thunderbolt-3-Alternate-Mode/20240926-002931
base:   https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git for-next
patch link:    https://lore.kernel.org/r/20240925092505.7.Ic61ced3cdfb5d6776435356061f12307da719829%40changeid
patch subject: [PATCH 7/8] platform/chrome: cros_ec_typec: Thunderbolt support
config: i386-randconfig-r133-20240927 (https://download.01.org/0day-ci/archive/20240927/202409271548.taoANDmm-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240927/202409271548.taoANDmm-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/202409271548.taoANDmm-lkp@intel.com/

All errors (new ones prefixed by >>):

   ld: drivers/platform/chrome/cros_typec_thunderbolt.o: in function `cros_typec_register_displayport':
>> drivers/platform/chrome/cros_typec_altmode.h:24: multiple definition of `cros_typec_register_displayport'; drivers/platform/chrome/cros_ec_typec.o:drivers/platform/chrome/cros_typec_altmode.h:24: first defined here
   ld: drivers/platform/chrome/cros_typec_thunderbolt.o: in function `cros_typec_displayport_status_update':
>> drivers/platform/chrome/cros_typec_altmode.h:30: multiple definition of `cros_typec_displayport_status_update'; drivers/platform/chrome/cros_ec_typec.o:drivers/platform/chrome/cros_typec_altmode.h:30: first defined here


vim +24 drivers/platform/chrome/cros_typec_altmode.h

a57f16462c7621 Abhishek Pandit-Subedi 2024-09-25  10  
a57f16462c7621 Abhishek Pandit-Subedi 2024-09-25  11  #if IS_ENABLED(CONFIG_TYPEC_DP_ALTMODE)
a57f16462c7621 Abhishek Pandit-Subedi 2024-09-25  12  struct typec_altmode *
a57f16462c7621 Abhishek Pandit-Subedi 2024-09-25  13  cros_typec_register_displayport(struct cros_typec_port *port,
a57f16462c7621 Abhishek Pandit-Subedi 2024-09-25  14  				struct typec_altmode_desc *desc,
a57f16462c7621 Abhishek Pandit-Subedi 2024-09-25  15  				bool ap_mode_entry);
a57f16462c7621 Abhishek Pandit-Subedi 2024-09-25  16  
a57f16462c7621 Abhishek Pandit-Subedi 2024-09-25  17  int cros_typec_displayport_status_update(struct typec_altmode *altmode,
a57f16462c7621 Abhishek Pandit-Subedi 2024-09-25  18  					 struct typec_displayport_data *data);
a57f16462c7621 Abhishek Pandit-Subedi 2024-09-25  19  #else
a57f16462c7621 Abhishek Pandit-Subedi 2024-09-25  20  struct typec_altmode *
a57f16462c7621 Abhishek Pandit-Subedi 2024-09-25  21  cros_typec_register_displayport(struct cros_typec_port *port,
a57f16462c7621 Abhishek Pandit-Subedi 2024-09-25  22  				struct typec_altmode_desc *desc,
a57f16462c7621 Abhishek Pandit-Subedi 2024-09-25  23  				bool ap_mode_entry)
a57f16462c7621 Abhishek Pandit-Subedi 2024-09-25 @24  {
a57f16462c7621 Abhishek Pandit-Subedi 2024-09-25  25  	return typec_port_register_altmode(port->port, desc);
a57f16462c7621 Abhishek Pandit-Subedi 2024-09-25  26  }
a57f16462c7621 Abhishek Pandit-Subedi 2024-09-25  27  
a57f16462c7621 Abhishek Pandit-Subedi 2024-09-25  28  int cros_typec_displayport_status_update(struct typec_altmode *altmode,
a57f16462c7621 Abhishek Pandit-Subedi 2024-09-25  29  					 struct typec_displayport_data *data)
a57f16462c7621 Abhishek Pandit-Subedi 2024-09-25 @30  {
a57f16462c7621 Abhishek Pandit-Subedi 2024-09-25  31  	return 0;
a57f16462c7621 Abhishek Pandit-Subedi 2024-09-25  32  }
a57f16462c7621 Abhishek Pandit-Subedi 2024-09-25  33  #endif
4db96705bb611f Abhishek Pandit-Subedi 2024-09-25  34  

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

[PATCH 8/8] platform/chrome: cros_ec_typec: Disable auto_enter

[Abhishek Pandit-Subedi]

Altmodes with cros_ec are either automatically entered by the EC or
entered by typecd in userspace so we should not auto enter from the
kernel.

Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
---

 drivers/platform/chrome/cros_ec_typec.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index ec13d84d11b8..e06a0f2712ce 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -294,6 +294,7 @@ static int cros_typec_register_port_altmodes(struct cros_typec_data *typec,
 	desc.svid = USB_TYPEC_DP_SID;
 	desc.mode = USB_TYPEC_DP_MODE;
 	desc.vdo = DP_PORT_VDO;
+	desc.no_auto_enter = true;
 	amode = cros_typec_register_displayport(port, &desc,
 						typec->ap_driven_altmode);
 	if (IS_ERR(amode))
@@ -314,6 +315,7 @@ static int cros_typec_register_port_altmodes(struct cros_typec_data *typec,
 		memset(&desc, 0, sizeof(desc));
 		desc.svid = USB_TYPEC_TBT_SID;
 		desc.mode = TYPEC_ANY_MODE;
+		desc.no_auto_enter = true;
 		amode = cros_typec_register_thunderbolt(port, &desc);
 		if (IS_ERR(amode))
 			return PTR_ERR(amode);
-- 
2.46.0.792.g87dc391469-goog

Re: [PATCH 8/8] platform/chrome: cros_ec_typec: Disable auto_enter

[Dmitry Baryshkov]

On Wed, Sep 25, 2024 at 09:25:09AM GMT, Abhishek Pandit-Subedi wrote:
> Altmodes with cros_ec are either automatically entered by the EC or
> entered by typecd in userspace so we should not auto enter from the
> kernel.

This makes policy decision for the whole platform. Consider somebody
running normal Linux distro on chromebooks. Can this be configured by
the userspace itself?

> 
> Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> ---
> 
>  drivers/platform/chrome/cros_ec_typec.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> index ec13d84d11b8..e06a0f2712ce 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -294,6 +294,7 @@ static int cros_typec_register_port_altmodes(struct cros_typec_data *typec,
>  	desc.svid = USB_TYPEC_DP_SID;
>  	desc.mode = USB_TYPEC_DP_MODE;
>  	desc.vdo = DP_PORT_VDO;
> +	desc.no_auto_enter = true;
>  	amode = cros_typec_register_displayport(port, &desc,
>  						typec->ap_driven_altmode);
>  	if (IS_ERR(amode))
> @@ -314,6 +315,7 @@ static int cros_typec_register_port_altmodes(struct cros_typec_data *typec,
>  		memset(&desc, 0, sizeof(desc));
>  		desc.svid = USB_TYPEC_TBT_SID;
>  		desc.mode = TYPEC_ANY_MODE;
> +		desc.no_auto_enter = true;
>  		amode = cros_typec_register_thunderbolt(port, &desc);
>  		if (IS_ERR(amode))
>  			return PTR_ERR(amode);
> -- 
> 2.46.0.792.g87dc391469-goog
> 

-- 
With best wishes
Dmitry

Re: [PATCH 8/8] platform/chrome: cros_ec_typec: Disable auto_enter

[Abhishek Pandit-Subedi]

On Wed, Sep 25, 2024 at 10:03 AM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Wed, Sep 25, 2024 at 09:25:09AM GMT, Abhishek Pandit-Subedi wrote:
> > Altmodes with cros_ec are either automatically entered by the EC or
> > entered by typecd in userspace so we should not auto enter from the
> > kernel.
>
> This makes policy decision for the whole platform. Consider somebody
> running normal Linux distro on chromebooks. Can this be configured by
> the userspace itself?

This is just the default for when the driver registers. You will then
find that there is now a sysfs entry for auto_enter that you can
control from userspace to allow you to auto-enter the mode on partner
attach (which you could probably write a udev rule for).

I still need to send a follow-up for how to handle auto-enter when
there are multiple modes available on the partner that are all
enabled. This first patch series is mostly about supporting existing
use-case for ChromeOS.


>
> >
> > Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> > ---
> >
> >  drivers/platform/chrome/cros_ec_typec.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> > index ec13d84d11b8..e06a0f2712ce 100644
> > --- a/drivers/platform/chrome/cros_ec_typec.c
> > +++ b/drivers/platform/chrome/cros_ec_typec.c
> > @@ -294,6 +294,7 @@ static int cros_typec_register_port_altmodes(struct cros_typec_data *typec,
> >       desc.svid = USB_TYPEC_DP_SID;
> >       desc.mode = USB_TYPEC_DP_MODE;
> >       desc.vdo = DP_PORT_VDO;
> > +     desc.no_auto_enter = true;
> >       amode = cros_typec_register_displayport(port, &desc,
> >                                               typec->ap_driven_altmode);
> >       if (IS_ERR(amode))
> > @@ -314,6 +315,7 @@ static int cros_typec_register_port_altmodes(struct cros_typec_data *typec,
> >               memset(&desc, 0, sizeof(desc));
> >               desc.svid = USB_TYPEC_TBT_SID;
> >               desc.mode = TYPEC_ANY_MODE;
> > +             desc.no_auto_enter = true;
> >               amode = cros_typec_register_thunderbolt(port, &desc);
> >               if (IS_ERR(amode))
> >                       return PTR_ERR(amode);
> > --
> > 2.46.0.792.g87dc391469-goog
> >
>
> --
> With best wishes
> Dmitry

Re: [PATCH 8/8] platform/chrome: cros_ec_typec: Disable auto_enter

[Dmitry Baryshkov]

On Wed, Sep 25, 2024 at 10:39:00AM GMT, Abhishek Pandit-Subedi wrote:
> On Wed, Sep 25, 2024 at 10:03 AM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
> >
> > On Wed, Sep 25, 2024 at 09:25:09AM GMT, Abhishek Pandit-Subedi wrote:
> > > Altmodes with cros_ec are either automatically entered by the EC or
> > > entered by typecd in userspace so we should not auto enter from the
> > > kernel.
> >
> > This makes policy decision for the whole platform. Consider somebody
> > running normal Linux distro on chromebooks. Can this be configured by
> > the userspace itself?
> 
> This is just the default for when the driver registers. You will then
> find that there is now a sysfs entry for auto_enter that you can
> control from userspace to allow you to auto-enter the mode on partner
> attach (which you could probably write a udev rule for).

I think, a usual policy is to handle everything automatically, unless
userspace configures it in a different way. Otherwise it might be really
surprising to users, if the kernel expects an action from the
non-existing userspace agent.

> I still need to send a follow-up for how to handle auto-enter when
> there are multiple modes available on the partner that are all
> enabled. This first patch series is mostly about supporting existing
> use-case for ChromeOS.

Doesn't EC decide that, which AltMode to select?

> > > Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> > > ---
> > >
> > >  drivers/platform/chrome/cros_ec_typec.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> > > index ec13d84d11b8..e06a0f2712ce 100644
> > > --- a/drivers/platform/chrome/cros_ec_typec.c
> > > +++ b/drivers/platform/chrome/cros_ec_typec.c
> > > @@ -294,6 +294,7 @@ static int cros_typec_register_port_altmodes(struct cros_typec_data *typec,
> > >       desc.svid = USB_TYPEC_DP_SID;
> > >       desc.mode = USB_TYPEC_DP_MODE;
> > >       desc.vdo = DP_PORT_VDO;
> > > +     desc.no_auto_enter = true;
> > >       amode = cros_typec_register_displayport(port, &desc,
> > >                                               typec->ap_driven_altmode);
> > >       if (IS_ERR(amode))
> > > @@ -314,6 +315,7 @@ static int cros_typec_register_port_altmodes(struct cros_typec_data *typec,
> > >               memset(&desc, 0, sizeof(desc));
> > >               desc.svid = USB_TYPEC_TBT_SID;
> > >               desc.mode = TYPEC_ANY_MODE;
> > > +             desc.no_auto_enter = true;
> > >               amode = cros_typec_register_thunderbolt(port, &desc);
> > >               if (IS_ERR(amode))
> > >                       return PTR_ERR(amode);
> > > --
> > > 2.46.0.792.g87dc391469-goog
> > >
> >
> > --
> > With best wishes
> > Dmitry

-- 
With best wishes
Dmitry

Re: [PATCH 8/8] platform/chrome: cros_ec_typec: Disable auto_enter

[Abhishek Pandit-Subedi]

On Wed, Sep 25, 2024 at 2:24 PM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Wed, Sep 25, 2024 at 10:39:00AM GMT, Abhishek Pandit-Subedi wrote:
> > On Wed, Sep 25, 2024 at 10:03 AM Dmitry Baryshkov
> > <dmitry.baryshkov@linaro.org> wrote:
> > >
> > > On Wed, Sep 25, 2024 at 09:25:09AM GMT, Abhishek Pandit-Subedi wrote:
> > > > Altmodes with cros_ec are either automatically entered by the EC or
> > > > entered by typecd in userspace so we should not auto enter from the
> > > > kernel.
> > >
> > > This makes policy decision for the whole platform. Consider somebody
> > > running normal Linux distro on chromebooks. Can this be configured by
> > > the userspace itself?
> >
> > This is just the default for when the driver registers. You will then
> > find that there is now a sysfs entry for auto_enter that you can
> > control from userspace to allow you to auto-enter the mode on partner
> > attach (which you could probably write a udev rule for).
>
> I think, a usual policy is to handle everything automatically, unless
> userspace configures it in a different way. Otherwise it might be really
> surprising to users, if the kernel expects an action from the
> non-existing userspace agent.



>
> > I still need to send a follow-up for how to handle auto-enter when
> > there are multiple modes available on the partner that are all
> > enabled. This first patch series is mostly about supporting existing
> > use-case for ChromeOS.
>
> Doesn't EC decide that, which AltMode to select?

On ChromeOS systems with only DisplayPort supported, ChromeOS EC
automatically enters DP altmode.

However, systems with Thunderbolt + USB4 have the AP_DRIVEN_MODE
feature enabled which expects the AP to enter/exit modes (due to the
security implication of TBT PCIE tunnels). ChromeOS has a userspace
daemon, typecd (https://chromium.googlesource.com/chromiumos/platform2/+/HEAD/typecd),
which handles the mode entry policy for ChromeOS. Currently, it talks
to the EC directly (via ectool) and this patch series is moving this
support to the kernel so we can use sysfs instead.

Currently, if you put a normal Linux distro on a ChromeOS device with
AP_DRIVEN_MODE enabled from the EC, it will not automatically enter
any alternate modes and there's no mechanism to do so via sysfs
either. You will need to either port typecd or compile + deploy ectool
to make the mode requests directly.

>
> > > > Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> > > > ---
> > > >
> > > >  drivers/platform/chrome/cros_ec_typec.c | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> > > > index ec13d84d11b8..e06a0f2712ce 100644
> > > > --- a/drivers/platform/chrome/cros_ec_typec.c
> > > > +++ b/drivers/platform/chrome/cros_ec_typec.c
> > > > @@ -294,6 +294,7 @@ static int cros_typec_register_port_altmodes(struct cros_typec_data *typec,
> > > >       desc.svid = USB_TYPEC_DP_SID;
> > > >       desc.mode = USB_TYPEC_DP_MODE;
> > > >       desc.vdo = DP_PORT_VDO;
> > > > +     desc.no_auto_enter = true;
> > > >       amode = cros_typec_register_displayport(port, &desc,
> > > >                                               typec->ap_driven_altmode);
> > > >       if (IS_ERR(amode))
> > > > @@ -314,6 +315,7 @@ static int cros_typec_register_port_altmodes(struct cros_typec_data *typec,
> > > >               memset(&desc, 0, sizeof(desc));
> > > >               desc.svid = USB_TYPEC_TBT_SID;
> > > >               desc.mode = TYPEC_ANY_MODE;
> > > > +             desc.no_auto_enter = true;
> > > >               amode = cros_typec_register_thunderbolt(port, &desc);
> > > >               if (IS_ERR(amode))
> > > >                       return PTR_ERR(amode);
> > > > --
> > > > 2.46.0.792.g87dc391469-goog
> > > >
> > >
> > > --
> > > With best wishes
> > > Dmitry
>
> --
> With best wishes
> Dmitry

Re: [PATCH 8/8] platform/chrome: cros_ec_typec: Disable auto_enter

[Dmitry Baryshkov]

On Wed, 25 Sept 2024 at 23:35, Abhishek Pandit-Subedi
<abhishekpandit@chromium.org> wrote:
>
> On Wed, Sep 25, 2024 at 2:24 PM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
> >
> > On Wed, Sep 25, 2024 at 10:39:00AM GMT, Abhishek Pandit-Subedi wrote:
> > > On Wed, Sep 25, 2024 at 10:03 AM Dmitry Baryshkov
> > > <dmitry.baryshkov@linaro.org> wrote:
> > > >
> > > > On Wed, Sep 25, 2024 at 09:25:09AM GMT, Abhishek Pandit-Subedi wrote:
> > > > > Altmodes with cros_ec are either automatically entered by the EC or
> > > > > entered by typecd in userspace so we should not auto enter from the
> > > > > kernel.
> > > >
> > > > This makes policy decision for the whole platform. Consider somebody
> > > > running normal Linux distro on chromebooks. Can this be configured by
> > > > the userspace itself?
> > >
> > > This is just the default for when the driver registers. You will then
> > > find that there is now a sysfs entry for auto_enter that you can
> > > control from userspace to allow you to auto-enter the mode on partner
> > > attach (which you could probably write a udev rule for).
> >
> > I think, a usual policy is to handle everything automatically, unless
> > userspace configures it in a different way. Otherwise it might be really
> > surprising to users, if the kernel expects an action from the
> > non-existing userspace agent.
>
>
>
> >
> > > I still need to send a follow-up for how to handle auto-enter when
> > > there are multiple modes available on the partner that are all
> > > enabled. This first patch series is mostly about supporting existing
> > > use-case for ChromeOS.
> >
> > Doesn't EC decide that, which AltMode to select?
>
> On ChromeOS systems with only DisplayPort supported, ChromeOS EC
> automatically enters DP altmode.
>
> However, systems with Thunderbolt + USB4 have the AP_DRIVEN_MODE
> feature enabled which expects the AP to enter/exit modes (due to the
> security implication of TBT PCIE tunnels). ChromeOS has a userspace
> daemon, typecd (https://chromium.googlesource.com/chromiumos/platform2/+/HEAD/typecd),
> which handles the mode entry policy for ChromeOS. Currently, it talks
> to the EC directly (via ectool) and this patch series is moving this
> support to the kernel so we can use sysfs instead.
>
> Currently, if you put a normal Linux distro on a ChromeOS device with
> AP_DRIVEN_MODE enabled from the EC, it will not automatically enter
> any alternate modes and there's no mechanism to do so via sysfs
> either. You will need to either port typecd or compile + deploy ectool
> to make the mode requests directly.

Well, I'd say that without a proper sysfs interface the feature is
incomplete. At the very least it must be documented in the commit
messages, so that anybody who tries to get this up and running can
find necessary userspace bits (and then can implement necessary driver
support).

Also consider extending bolt, a usual ThunderBolt daemon used on
Linux, to support new sysfs attributes.

>
> >
> > > > > Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> > > > > ---
> > > > >
> > > > >  drivers/platform/chrome/cros_ec_typec.c | 2 ++
> > > > >  1 file changed, 2 insertions(+)
> > > > >
> > > > > diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> > > > > index ec13d84d11b8..e06a0f2712ce 100644
> > > > > --- a/drivers/platform/chrome/cros_ec_typec.c
> > > > > +++ b/drivers/platform/chrome/cros_ec_typec.c
> > > > > @@ -294,6 +294,7 @@ static int cros_typec_register_port_altmodes(struct cros_typec_data *typec,
> > > > >       desc.svid = USB_TYPEC_DP_SID;
> > > > >       desc.mode = USB_TYPEC_DP_MODE;
> > > > >       desc.vdo = DP_PORT_VDO;
> > > > > +     desc.no_auto_enter = true;
> > > > >       amode = cros_typec_register_displayport(port, &desc,
> > > > >                                               typec->ap_driven_altmode);
> > > > >       if (IS_ERR(amode))
> > > > > @@ -314,6 +315,7 @@ static int cros_typec_register_port_altmodes(struct cros_typec_data *typec,
> > > > >               memset(&desc, 0, sizeof(desc));
> > > > >               desc.svid = USB_TYPEC_TBT_SID;
> > > > >               desc.mode = TYPEC_ANY_MODE;
> > > > > +             desc.no_auto_enter = true;
> > > > >               amode = cros_typec_register_thunderbolt(port, &desc);
> > > > >               if (IS_ERR(amode))
> > > > >                       return PTR_ERR(amode);
> > > > > --
> > > > > 2.46.0.792.g87dc391469-goog
> > > > >
> > > >
> > > > --
> > > > With best wishes
> > > > Dmitry
> >
> > --
> > With best wishes
> > Dmitry



-- 
With best wishes
Dmitry

Re: [PATCH 0/8] Thunderbolt and DP altmode support for cros-ec-typec

[Dmitry Baryshkov]

On Wed, Sep 25, 2024 at 09:25:01AM GMT, Abhishek Pandit-Subedi wrote:
> 
> Hi Heikki, Tzung-Bi et al,
> 
> This patch series adds support for alternate mode entry for the
> cros-ec-typec driver for Displayport and Thunderbolt.
> 
> Thunderbolt support is added by adapting an RFC Heikki had posted
> previously:
> 
> https://lore.kernel.org/linux-usb/20191230152857.43917-1-heikki.krogerus@linux.intel.com/
> 
> A few comments on the series:
> 
> * The cros-ec interface will not accept any VDOs/VDMs so we simply
>   ignore any configurations we are passed (i.e. DPConfigure). This means
>   the sysfs control of DP lanes won't work.
> * ChromeOS has two modes of operation for alt-modes: entirely EC driven
>   or AP-driven from userspace (via the typec daemon). Thus, we never
>   expect the kernel alt-mode drivers to auto-enter modes.
> 
> This was tested on kernel 6.6 with a ChromeOS Brya device and compile
> tested against linux-usb (with allmodconfig).

Please test on top of the linux-usb or linux-next. 6.6 is nine months
old kernel.

Also for v2 please consider CC'ing both lists for all patches. Otherwise
it's hard to follow the changes.

> 
> Thanks,
> Abhishek
> 
> 
> 

-- 
With best wishes
Dmitry

Re: [PATCH 0/8] Thunderbolt and DP altmode support for cros-ec-typec

[Abhishek Pandit-Subedi]

On Wed, Sep 25, 2024 at 10:12 AM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Wed, Sep 25, 2024 at 09:25:01AM GMT, Abhishek Pandit-Subedi wrote:
> >
> > Hi Heikki, Tzung-Bi et al,
> >
> > This patch series adds support for alternate mode entry for the
> > cros-ec-typec driver for Displayport and Thunderbolt.
> >
> > Thunderbolt support is added by adapting an RFC Heikki had posted
> > previously:
> >
> > https://lore.kernel.org/linux-usb/20191230152857.43917-1-heikki.krogerus@linux.intel.com/
> >
> > A few comments on the series:
> >
> > * The cros-ec interface will not accept any VDOs/VDMs so we simply
> >   ignore any configurations we are passed (i.e. DPConfigure). This means
> >   the sysfs control of DP lanes won't work.
> > * ChromeOS has two modes of operation for alt-modes: entirely EC driven
> >   or AP-driven from userspace (via the typec daemon). Thus, we never
> >   expect the kernel alt-mode drivers to auto-enter modes.
> >
> > This was tested on kernel 6.6 with a ChromeOS Brya device and compile
> > tested against linux-usb (with allmodconfig).
>
> Please test on top of the linux-usb or linux-next. 6.6 is nine months
> old kernel.

Ack -- I'm going to try to load the upstream kernel on my ChromeOS
device + test.

>
> Also for v2 please consider CC'ing both lists for all patches. Otherwise
> it's hard to follow the changes.

Really sorry about that. I'm using patman to send patches and didn't
realize it would split the patch series between the two. v2 will send
all 8 patches to both lists.

>
> >
> > Thanks,
> > Abhishek
> >
> >
> >
>
> --
> With best wishes
> Dmitry