[PATCH v2 0/2] Add support for the TUSB1046-DCI Type-C crosspoint switch

[PATCH v2 0/2] Add support for the TUSB1046-DCI Type-C crosspoint switch

[Romain Gantois]

Hello everyone,

This is version two of my series which adds support for a Type-C linear
redriver crosspoint switch which can function as a Type-C switch and
DisplayPort altmode multiplexer.

Best Regards,

Romain Gantois

Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
Changes in v2:
- Fixed indentation in dt binding.
- Added locking and svid checking to TUSB1046 driver.
- Link to v1: https://lore.kernel.org/r/20241018-tusb1046-v1-0-a38312f18691@bootlin.com

---
Romain Gantois (2):
      dt-bindings: usb: Describe TUSB1046 crosspoint switch
      usb: typec: mux: Add support for the TUSB1046 crosspoint switch

 .../devicetree/bindings/usb/ti,tusb1046.yaml       |  49 ++++++
 MAINTAINERS                                        |   7 +
 drivers/usb/typec/mux/Kconfig                      |   9 +
 drivers/usb/typec/mux/Makefile                     |   1 +
 drivers/usb/typec/mux/tusb1046.c                   | 196 +++++++++++++++++++++
 5 files changed, 262 insertions(+)
---
base-commit: 9852d85ec9d492ebef56dc5f229416c925758edc
change-id: 20241014-tusb1046-ebd08264789d

Best regards,
-- 
Romain Gantois <romain.gantois@bootlin.com>

[PATCH v2 1/2] dt-bindings: usb: Describe TUSB1046 crosspoint switch

[Romain Gantois]

Describe the Texas Instruments TUSB1046-DCI USB Type-C linear redriver
crosspoint switch. This component is used to handle orientation switching
and DisplayPort altmode multiplexing for Type-C signals.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
 .../devicetree/bindings/usb/ti,tusb1046.yaml       | 49 ++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/ti,tusb1046.yaml b/Documentation/devicetree/bindings/usb/ti,tusb1046.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..f713cac4a8ac8e89c017999bc11e4b3a38d3ac2e
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ti,tusb1046.yaml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/ti,tusb1046.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments TUSB1046-DCI Type-C crosspoint switch
+
+maintainers:
+  - Romain Gantois <romain.gantois@bootlin.com>
+
+allOf:
+  - $ref: usb-switch.yaml#
+
+properties:
+  compatible:
+    const: ti,tusb1046
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - port
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        typec-mux@44 {
+            compatible = "ti,tusb1046";
+            reg = <0x44>;
+
+            mode-switch;
+            orientation-switch;
+
+            port {
+                endpoint {
+                    remote-endpoint = <&typec_controller>;
+                };
+            };
+        };
+    };
+...

-- 
2.47.0

[PATCH v2 2/2] usb: typec: mux: Add support for the TUSB1046 crosspoint switch

[Romain Gantois]

The TUSB1046-DCI is a USB-C linear redriver crosspoint switch, which can
mux SuperSpeed lanes from a Type-C connector to a USB3.0 data lane or up to
4 display port lanes.

Add support for driving the TUSB1046 as a Type-C orientation switch and
DisplayPort altmode multiplexer.

Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
 MAINTAINERS                      |   7 ++
 drivers/usb/typec/mux/Kconfig    |   9 ++
 drivers/usb/typec/mux/Makefile   |   1 +
 drivers/usb/typec/mux/tusb1046.c | 196 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 213 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index c27f3190737f8b85779bde5489639c8b899f4fd8..c880588b63f27d628edeec09fa7d904eeabbde92 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -24142,6 +24142,13 @@ L:	linux-usb@vger.kernel.org
 S:	Orphan
 F:	drivers/usb/typec/tcpm/
 
+USB TYPEC TUSB1046 MUX DRIVER
+M:	Romain Gantois <romain.gantois@bootlin.com>
+L:	linux-usb@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/usb/ti,tusb1046.yaml
+F:	drivers/usb/typec/mux/tusb1046.c
+
 USB UHCI DRIVER
 M:	Alan Stern <stern@rowland.harvard.edu>
 L:	linux-usb@vger.kernel.org
diff --git a/drivers/usb/typec/mux/Kconfig b/drivers/usb/typec/mux/Kconfig
index ce7db6ad30572a0a74890f5f11944fb3ff07f635..67381b4ef4f68f4a6e73f157365ee24d0ab7109a 100644
--- a/drivers/usb/typec/mux/Kconfig
+++ b/drivers/usb/typec/mux/Kconfig
@@ -66,6 +66,15 @@ config TYPEC_MUX_PTN36502
 	  Say Y or M if your system has a NXP PTN36502 Type-C redriver chip
 	  found on some devices with a Type-C port.
 
+config TYPEC_MUX_TUSB1046
+	tristate "TI TUSB1046 Type-C crosspoint switch driver"
+	depends on I2C
+	help
+	  Driver for the Texas Instruments TUSB1046-DCI crosspoint switch.
+	  Supports flipping USB-C SuperSpeed lanes to adapt to orientation
+	  changes, as well as muxing DisplayPort and sideband signals to a
+	  common Type-C connector.
+
 config TYPEC_MUX_WCD939X_USBSS
 	tristate "Qualcomm WCD939x USBSS Analog Audio Switch driver"
 	depends on I2C
diff --git a/drivers/usb/typec/mux/Makefile b/drivers/usb/typec/mux/Makefile
index bb96f30267af05b33b9277dcf1cc0e1527d2dcdd..60879446da9365183567d3374a2fb7b5171fb3d7 100644
--- a/drivers/usb/typec/mux/Makefile
+++ b/drivers/usb/typec/mux/Makefile
@@ -7,4 +7,5 @@ obj-$(CONFIG_TYPEC_MUX_INTEL_PMC)	+= intel_pmc_mux.o
 obj-$(CONFIG_TYPEC_MUX_IT5205)		+= it5205.o
 obj-$(CONFIG_TYPEC_MUX_NB7VPQ904M)	+= nb7vpq904m.o
 obj-$(CONFIG_TYPEC_MUX_PTN36502)	+= ptn36502.o
+obj-$(CONFIG_TYPEC_MUX_TUSB1046)	+= tusb1046.o
 obj-$(CONFIG_TYPEC_MUX_WCD939X_USBSS)	+= wcd939x-usbss.o
diff --git a/drivers/usb/typec/mux/tusb1046.c b/drivers/usb/typec/mux/tusb1046.c
new file mode 100644
index 0000000000000000000000000000000000000000..b4f45c217b59fe033551e8e6379ad2d33ebedbcb
--- /dev/null
+++ b/drivers/usb/typec/mux/tusb1046.c
@@ -0,0 +1,196 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Driver for the TUSB1046-DCI USB Type-C crosspoint switch
+ *
+ * Copyright (C) 2024 Bootlin
+ */
+
+#include <linux/bits.h>
+#include <linux/i2c.h>
+#include <linux/usb/typec_mux.h>
+#include <linux/usb/typec_dp.h>
+#include <linux/usb/typec_altmode.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/err.h>
+#include <linux/of_device.h>
+#include <linux/device.h>
+#include <linux/mutex.h>
+
+#define TUSB1046_REG_GENERAL 0xa
+
+/* General register bits */
+#define TUSB1046_GENERAL_FLIPSEL BIT(2)
+#define TUSB1046_GENERAL_CTLSEL  GENMASK(1, 0)
+
+/* Mux modes */
+#define TUSB1046_CTLSEL_DISABLED          0x0
+#define TUSB1046_CTLSEL_USB3              0x1
+#define TUSB1046_CTLSEL_4LANE_DP          0x2
+#define TUSB1046_CTLSEL_USB3_AND_2LANE_DP 0x3
+
+struct tusb1046_priv {
+	struct i2c_client *client;
+	struct typec_switch_dev *sw;
+	struct typec_mux_dev *mux;
+
+	/* Lock General register during accesses */
+	struct mutex general_reg_lock;
+};
+
+static int tusb1046_mux_set(struct typec_mux_dev *mux,
+			    struct typec_mux_state *state)
+{
+	struct tusb1046_priv *priv = typec_mux_get_drvdata(mux);
+	struct i2c_client *client = priv->client;
+	struct device *dev = &client->dev;
+	int mode, val, ret = 0;
+
+	if (state->mode >= TYPEC_STATE_MODAL &&
+	    state->alt->svid != USB_TYPEC_DP_SID)
+		return -EINVAL;
+
+	dev_dbg(dev, "mux mode requested: %lu\n", state->mode);
+
+	mutex_lock(&priv->general_reg_lock);
+
+	val = i2c_smbus_read_byte_data(client, TUSB1046_REG_GENERAL);
+	if (val < 0) {
+		dev_err(dev, "failed to read ctlsel status, err %d\n", val);
+		ret = val;
+		goto out_unlock;
+	}
+
+	switch (state->mode) {
+	case TYPEC_STATE_USB:
+		mode = TUSB1046_CTLSEL_USB3;
+		break;
+	case TYPEC_DP_STATE_C:
+	case TYPEC_DP_STATE_E:
+		mode = TUSB1046_CTLSEL_4LANE_DP;
+		break;
+	case TYPEC_DP_STATE_D:
+		mode = TUSB1046_CTLSEL_USB3_AND_2LANE_DP;
+		break;
+	case TYPEC_STATE_SAFE:
+	default:
+		mode = TUSB1046_CTLSEL_DISABLED;
+		break;
+	}
+
+	val &= ~TUSB1046_GENERAL_CTLSEL;
+	val |= mode;
+
+	ret = i2c_smbus_write_byte_data(client, TUSB1046_REG_GENERAL, val);
+
+out_unlock:
+	mutex_unlock(&priv->general_reg_lock);
+	return ret;
+}
+
+static int tusb1046_switch_set(struct typec_switch_dev *sw,
+			       enum typec_orientation orientation)
+{
+	struct tusb1046_priv *priv = typec_switch_get_drvdata(sw);
+	struct i2c_client *client = priv->client;
+	struct device *dev = &client->dev;
+	int val, ret = 0;
+
+	dev_dbg(dev, "setting USB3.0 lane flip for orientation %d\n", orientation);
+
+	mutex_lock(&priv->general_reg_lock);
+
+	val = i2c_smbus_read_byte_data(client, TUSB1046_REG_GENERAL);
+	if (val < 0) {
+		dev_err(dev, "failed to read flipsel status, err %d\n", val);
+		ret = val;
+		goto out_unlock;
+	}
+
+	if (orientation == TYPEC_ORIENTATION_REVERSE)
+		val |= TUSB1046_GENERAL_FLIPSEL;
+	else
+		val &= ~TUSB1046_GENERAL_FLIPSEL;
+
+	ret = i2c_smbus_write_byte_data(client, TUSB1046_REG_GENERAL, val);
+
+out_unlock:
+	mutex_unlock(&priv->general_reg_lock);
+	return ret;
+}
+
+static int tusb1046_i2c_probe(struct i2c_client *client)
+{
+	struct typec_switch_desc sw_desc = { };
+	struct typec_mux_desc mux_desc = { };
+	struct device *dev = &client->dev;
+	struct tusb1046_priv *priv;
+	int ret = 0;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return dev_err_probe(dev, -ENOMEM, "failed to allocate driver data\n");
+
+	priv->client = client;
+
+	mutex_init(&priv->general_reg_lock);
+
+	sw_desc.drvdata = priv;
+	sw_desc.fwnode = dev_fwnode(dev);
+	sw_desc.set = tusb1046_switch_set;
+
+	priv->sw = typec_switch_register(dev, &sw_desc);
+	if (IS_ERR(priv->sw)) {
+		ret = dev_err_probe(dev, PTR_ERR(priv->sw), "failed to register type-c switch\n");
+		goto err_destroy_mutex;
+	}
+
+	mux_desc.drvdata = priv;
+	mux_desc.fwnode = dev_fwnode(dev);
+	mux_desc.set = tusb1046_mux_set;
+
+	priv->mux = typec_mux_register(dev, &mux_desc);
+	if (IS_ERR(priv->mux)) {
+		ret = dev_err_probe(dev, PTR_ERR(priv->mux), "failed to register type-c mux\n");
+		goto err_unregister_switch;
+	}
+
+	i2c_set_clientdata(client, priv);
+
+	return 0;
+
+err_unregister_switch:
+	typec_switch_unregister(priv->sw);
+err_destroy_mutex:
+	mutex_destroy(&priv->general_reg_lock);
+	return ret;
+}
+
+static void tusb1046_i2c_remove(struct i2c_client *client)
+{
+	struct tusb1046_priv *priv = i2c_get_clientdata(client);
+
+	typec_switch_unregister(priv->sw);
+	typec_mux_unregister(priv->mux);
+	mutex_destroy(&priv->general_reg_lock);
+}
+
+static const struct of_device_id tusb1046_match_table[] = {
+	{.compatible = "ti,tusb1046"},
+	{},
+};
+
+static struct i2c_driver tusb1046_driver = {
+	.driver = {
+		.name = "tusb1046",
+		.of_match_table = tusb1046_match_table,
+	},
+	.probe = tusb1046_i2c_probe,
+	.remove = tusb1046_i2c_remove,
+};
+
+module_i2c_driver(tusb1046_driver);
+
+MODULE_DESCRIPTION("TUSB1046 USB Type-C switch driver");
+MODULE_AUTHOR("Romain Gantois <romain.gantois@bootlin.com>");
+MODULE_LICENSE("GPL");

-- 
2.47.0

Re: [PATCH v2 2/2] usb: typec: mux: Add support for the TUSB1046 crosspoint switch

[Dmitry Baryshkov]

On Thu, Oct 24, 2024 at 10:54:17AM +0200, Romain Gantois wrote:
> The TUSB1046-DCI is a USB-C linear redriver crosspoint switch, which can
> mux SuperSpeed lanes from a Type-C connector to a USB3.0 data lane or up to
> 4 display port lanes.
> 
> Add support for driving the TUSB1046 as a Type-C orientation switch and
> DisplayPort altmode multiplexer.
> 
> Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
> ---
>  MAINTAINERS                      |   7 ++
>  drivers/usb/typec/mux/Kconfig    |   9 ++
>  drivers/usb/typec/mux/Makefile   |   1 +
>  drivers/usb/typec/mux/tusb1046.c | 196 +++++++++++++++++++++++++++++++++++++++
>  4 files changed, 213 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c27f3190737f8b85779bde5489639c8b899f4fd8..c880588b63f27d628edeec09fa7d904eeabbde92 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -24142,6 +24142,13 @@ L:	linux-usb@vger.kernel.org
>  S:	Orphan
>  F:	drivers/usb/typec/tcpm/
>  
> +USB TYPEC TUSB1046 MUX DRIVER
> +M:	Romain Gantois <romain.gantois@bootlin.com>
> +L:	linux-usb@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/usb/ti,tusb1046.yaml
> +F:	drivers/usb/typec/mux/tusb1046.c
> +
>  USB UHCI DRIVER
>  M:	Alan Stern <stern@rowland.harvard.edu>
>  L:	linux-usb@vger.kernel.org
> diff --git a/drivers/usb/typec/mux/Kconfig b/drivers/usb/typec/mux/Kconfig
> index ce7db6ad30572a0a74890f5f11944fb3ff07f635..67381b4ef4f68f4a6e73f157365ee24d0ab7109a 100644
> --- a/drivers/usb/typec/mux/Kconfig
> +++ b/drivers/usb/typec/mux/Kconfig
> @@ -66,6 +66,15 @@ config TYPEC_MUX_PTN36502
>  	  Say Y or M if your system has a NXP PTN36502 Type-C redriver chip
>  	  found on some devices with a Type-C port.
>  
> +config TYPEC_MUX_TUSB1046
> +	tristate "TI TUSB1046 Type-C crosspoint switch driver"
> +	depends on I2C
> +	help
> +	  Driver for the Texas Instruments TUSB1046-DCI crosspoint switch.
> +	  Supports flipping USB-C SuperSpeed lanes to adapt to orientation
> +	  changes, as well as muxing DisplayPort and sideband signals to a
> +	  common Type-C connector.
> +
>  config TYPEC_MUX_WCD939X_USBSS
>  	tristate "Qualcomm WCD939x USBSS Analog Audio Switch driver"
>  	depends on I2C
> diff --git a/drivers/usb/typec/mux/Makefile b/drivers/usb/typec/mux/Makefile
> index bb96f30267af05b33b9277dcf1cc0e1527d2dcdd..60879446da9365183567d3374a2fb7b5171fb3d7 100644
> --- a/drivers/usb/typec/mux/Makefile
> +++ b/drivers/usb/typec/mux/Makefile
> @@ -7,4 +7,5 @@ obj-$(CONFIG_TYPEC_MUX_INTEL_PMC)	+= intel_pmc_mux.o
>  obj-$(CONFIG_TYPEC_MUX_IT5205)		+= it5205.o
>  obj-$(CONFIG_TYPEC_MUX_NB7VPQ904M)	+= nb7vpq904m.o
>  obj-$(CONFIG_TYPEC_MUX_PTN36502)	+= ptn36502.o
> +obj-$(CONFIG_TYPEC_MUX_TUSB1046)	+= tusb1046.o
>  obj-$(CONFIG_TYPEC_MUX_WCD939X_USBSS)	+= wcd939x-usbss.o
> diff --git a/drivers/usb/typec/mux/tusb1046.c b/drivers/usb/typec/mux/tusb1046.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..b4f45c217b59fe033551e8e6379ad2d33ebedbcb
> --- /dev/null
> +++ b/drivers/usb/typec/mux/tusb1046.c
> @@ -0,0 +1,196 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Driver for the TUSB1046-DCI USB Type-C crosspoint switch
> + *
> + * Copyright (C) 2024 Bootlin
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/i2c.h>
> +#include <linux/usb/typec_mux.h>
> +#include <linux/usb/typec_dp.h>
> +#include <linux/usb/typec_altmode.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/err.h>
> +#include <linux/of_device.h>
> +#include <linux/device.h>
> +#include <linux/mutex.h>
> +
> +#define TUSB1046_REG_GENERAL 0xa
> +
> +/* General register bits */
> +#define TUSB1046_GENERAL_FLIPSEL BIT(2)
> +#define TUSB1046_GENERAL_CTLSEL  GENMASK(1, 0)
> +
> +/* Mux modes */
> +#define TUSB1046_CTLSEL_DISABLED          0x0
> +#define TUSB1046_CTLSEL_USB3              0x1
> +#define TUSB1046_CTLSEL_4LANE_DP          0x2
> +#define TUSB1046_CTLSEL_USB3_AND_2LANE_DP 0x3
> +
> +struct tusb1046_priv {
> +	struct i2c_client *client;
> +	struct typec_switch_dev *sw;
> +	struct typec_mux_dev *mux;
> +
> +	/* Lock General register during accesses */
> +	struct mutex general_reg_lock;
> +};
> +
> +static int tusb1046_mux_set(struct typec_mux_dev *mux,
> +			    struct typec_mux_state *state)
> +{
> +	struct tusb1046_priv *priv = typec_mux_get_drvdata(mux);
> +	struct i2c_client *client = priv->client;
> +	struct device *dev = &client->dev;
> +	int mode, val, ret = 0;
> +
> +	if (state->mode >= TYPEC_STATE_MODAL &&
> +	    state->alt->svid != USB_TYPEC_DP_SID)
> +		return -EINVAL;
> +
> +	dev_dbg(dev, "mux mode requested: %lu\n", state->mode);
> +
> +	mutex_lock(&priv->general_reg_lock);
> +
> +	val = i2c_smbus_read_byte_data(client, TUSB1046_REG_GENERAL);
> +	if (val < 0) {
> +		dev_err(dev, "failed to read ctlsel status, err %d\n", val);
> +		ret = val;
> +		goto out_unlock;
> +	}
> +
> +	switch (state->mode) {
> +	case TYPEC_STATE_USB:
> +		mode = TUSB1046_CTLSEL_USB3;
> +		break;
> +	case TYPEC_DP_STATE_C:

These are only valid if you have checked that altmode SVID is a
DisplayPort SVID.

> +	case TYPEC_DP_STATE_E:
> +		mode = TUSB1046_CTLSEL_4LANE_DP;
> +		break;
> +	case TYPEC_DP_STATE_D:
> +		mode = TUSB1046_CTLSEL_USB3_AND_2LANE_DP;
> +		break;
> +	case TYPEC_STATE_SAFE:
> +	default:
> +		mode = TUSB1046_CTLSEL_DISABLED;
> +		break;
> +	}
> +
> +	val &= ~TUSB1046_GENERAL_CTLSEL;
> +	val |= mode;
> +
> +	ret = i2c_smbus_write_byte_data(client, TUSB1046_REG_GENERAL, val);
> +
> +out_unlock:
> +	mutex_unlock(&priv->general_reg_lock);
> +	return ret;
> +}
> +

-- 
With best wishes
Dmitry

Re: [PATCH v2 2/2] usb: typec: mux: Add support for the TUSB1046 crosspoint switch

[Romain Gantois]

Hello Dmitry,

On vendredi 25 octobre 2024 08:39:54 UTC+2 Dmitry Baryshkov wrote:
> On Thu, Oct 24, 2024 at 10:54:17AM +0200, Romain Gantois wrote:
...
> > +
> > +static int tusb1046_mux_set(struct typec_mux_dev *mux,
> > +			    struct typec_mux_state *state)
> > +{
> > +	struct tusb1046_priv *priv = typec_mux_get_drvdata(mux);
> > +	struct i2c_client *client = priv->client;
> > +	struct device *dev = &client->dev;
> > +	int mode, val, ret = 0;
> > +
> > +	if (state->mode >= TYPEC_STATE_MODAL &&
> > +	    state->alt->svid != USB_TYPEC_DP_SID)
> > +		return -EINVAL;
> > +
> > +	dev_dbg(dev, "mux mode requested: %lu\n", state->mode);
> > +
> > +	mutex_lock(&priv->general_reg_lock);
> > +
> > +	val = i2c_smbus_read_byte_data(client, TUSB1046_REG_GENERAL);
> > +	if (val < 0) {
> > +		dev_err(dev, "failed to read ctlsel status, err %d\n", val);
> > +		ret = val;
> > +		goto out_unlock;
> > +	}
> > +
> > +	switch (state->mode) {
> > +	case TYPEC_STATE_USB:
> > +		mode = TUSB1046_CTLSEL_USB3;
> > +		break;
> 
> > +	case TYPEC_DP_STATE_C:
> These are only valid if you have checked that altmode SVID is a
> DisplayPort SVID.

I did check it near the beginning of the function didn't I?

> > +	if (state->mode >= TYPEC_STATE_MODAL &&
> > +	    state->alt->svid != USB_TYPEC_DP_SID)
> > +		return -EINVAL;

Or is there something I'm missing?

Thanks for the review,

-- 
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v2 2/2] usb: typec: mux: Add support for the TUSB1046 crosspoint switch

[Dmitry Baryshkov]

On Fri, Oct 25, 2024 at 09:09:11AM +0200, Romain Gantois wrote:
> Hello Dmitry,
> 
> On vendredi 25 octobre 2024 08:39:54 UTC+2 Dmitry Baryshkov wrote:
> > On Thu, Oct 24, 2024 at 10:54:17AM +0200, Romain Gantois wrote:
> ...
> > > +
> > > +static int tusb1046_mux_set(struct typec_mux_dev *mux,
> > > +			    struct typec_mux_state *state)
> > > +{
> > > +	struct tusb1046_priv *priv = typec_mux_get_drvdata(mux);
> > > +	struct i2c_client *client = priv->client;
> > > +	struct device *dev = &client->dev;
> > > +	int mode, val, ret = 0;
> > > +
> > > +	if (state->mode >= TYPEC_STATE_MODAL &&
> > > +	    state->alt->svid != USB_TYPEC_DP_SID)
> > > +		return -EINVAL;
> > > +
> > > +	dev_dbg(dev, "mux mode requested: %lu\n", state->mode);
> > > +
> > > +	mutex_lock(&priv->general_reg_lock);
> > > +
> > > +	val = i2c_smbus_read_byte_data(client, TUSB1046_REG_GENERAL);
> > > +	if (val < 0) {
> > > +		dev_err(dev, "failed to read ctlsel status, err %d\n", val);
> > > +		ret = val;
> > > +		goto out_unlock;
> > > +	}
> > > +
> > > +	switch (state->mode) {
> > > +	case TYPEC_STATE_USB:
> > > +		mode = TUSB1046_CTLSEL_USB3;
> > > +		break;
> > 
> > > +	case TYPEC_DP_STATE_C:
> > These are only valid if you have checked that altmode SVID is a
> > DisplayPort SVID.
> 
> I did check it near the beginning of the function didn't I?
> 
> > > +	if (state->mode >= TYPEC_STATE_MODAL &&
> > > +	    state->alt->svid != USB_TYPEC_DP_SID)
> > > +		return -EINVAL;
> 
> Or is there something I'm missing?

Indeed, excuse me. I missed it.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


> 
> Thanks for the review,
> 
> -- 
> Romain Gantois, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
> 
> 
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2 2/2] usb: typec: mux: Add support for the TUSB1046 crosspoint switch

[Heikki Krogerus]

On Thu, Oct 24, 2024 at 10:54:17AM +0200, Romain Gantois wrote:
> The TUSB1046-DCI is a USB-C linear redriver crosspoint switch, which can
> mux SuperSpeed lanes from a Type-C connector to a USB3.0 data lane or up to
> 4 display port lanes.
> 
> Add support for driving the TUSB1046 as a Type-C orientation switch and
> DisplayPort altmode multiplexer.
> 
> Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  MAINTAINERS                      |   7 ++
>  drivers/usb/typec/mux/Kconfig    |   9 ++
>  drivers/usb/typec/mux/Makefile   |   1 +
>  drivers/usb/typec/mux/tusb1046.c | 196 +++++++++++++++++++++++++++++++++++++++
>  4 files changed, 213 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c27f3190737f8b85779bde5489639c8b899f4fd8..c880588b63f27d628edeec09fa7d904eeabbde92 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -24142,6 +24142,13 @@ L:	linux-usb@vger.kernel.org
>  S:	Orphan
>  F:	drivers/usb/typec/tcpm/
>  
> +USB TYPEC TUSB1046 MUX DRIVER
> +M:	Romain Gantois <romain.gantois@bootlin.com>
> +L:	linux-usb@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/usb/ti,tusb1046.yaml
> +F:	drivers/usb/typec/mux/tusb1046.c
> +
>  USB UHCI DRIVER
>  M:	Alan Stern <stern@rowland.harvard.edu>
>  L:	linux-usb@vger.kernel.org
> diff --git a/drivers/usb/typec/mux/Kconfig b/drivers/usb/typec/mux/Kconfig
> index ce7db6ad30572a0a74890f5f11944fb3ff07f635..67381b4ef4f68f4a6e73f157365ee24d0ab7109a 100644
> --- a/drivers/usb/typec/mux/Kconfig
> +++ b/drivers/usb/typec/mux/Kconfig
> @@ -66,6 +66,15 @@ config TYPEC_MUX_PTN36502
>  	  Say Y or M if your system has a NXP PTN36502 Type-C redriver chip
>  	  found on some devices with a Type-C port.
>  
> +config TYPEC_MUX_TUSB1046
> +	tristate "TI TUSB1046 Type-C crosspoint switch driver"
> +	depends on I2C
> +	help
> +	  Driver for the Texas Instruments TUSB1046-DCI crosspoint switch.
> +	  Supports flipping USB-C SuperSpeed lanes to adapt to orientation
> +	  changes, as well as muxing DisplayPort and sideband signals to a
> +	  common Type-C connector.
> +
>  config TYPEC_MUX_WCD939X_USBSS
>  	tristate "Qualcomm WCD939x USBSS Analog Audio Switch driver"
>  	depends on I2C
> diff --git a/drivers/usb/typec/mux/Makefile b/drivers/usb/typec/mux/Makefile
> index bb96f30267af05b33b9277dcf1cc0e1527d2dcdd..60879446da9365183567d3374a2fb7b5171fb3d7 100644
> --- a/drivers/usb/typec/mux/Makefile
> +++ b/drivers/usb/typec/mux/Makefile
> @@ -7,4 +7,5 @@ obj-$(CONFIG_TYPEC_MUX_INTEL_PMC)	+= intel_pmc_mux.o
>  obj-$(CONFIG_TYPEC_MUX_IT5205)		+= it5205.o
>  obj-$(CONFIG_TYPEC_MUX_NB7VPQ904M)	+= nb7vpq904m.o
>  obj-$(CONFIG_TYPEC_MUX_PTN36502)	+= ptn36502.o
> +obj-$(CONFIG_TYPEC_MUX_TUSB1046)	+= tusb1046.o
>  obj-$(CONFIG_TYPEC_MUX_WCD939X_USBSS)	+= wcd939x-usbss.o
> diff --git a/drivers/usb/typec/mux/tusb1046.c b/drivers/usb/typec/mux/tusb1046.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..b4f45c217b59fe033551e8e6379ad2d33ebedbcb
> --- /dev/null
> +++ b/drivers/usb/typec/mux/tusb1046.c
> @@ -0,0 +1,196 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Driver for the TUSB1046-DCI USB Type-C crosspoint switch
> + *
> + * Copyright (C) 2024 Bootlin
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/i2c.h>
> +#include <linux/usb/typec_mux.h>
> +#include <linux/usb/typec_dp.h>
> +#include <linux/usb/typec_altmode.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/err.h>
> +#include <linux/of_device.h>
> +#include <linux/device.h>
> +#include <linux/mutex.h>
> +
> +#define TUSB1046_REG_GENERAL 0xa
> +
> +/* General register bits */
> +#define TUSB1046_GENERAL_FLIPSEL BIT(2)
> +#define TUSB1046_GENERAL_CTLSEL  GENMASK(1, 0)
> +
> +/* Mux modes */
> +#define TUSB1046_CTLSEL_DISABLED          0x0
> +#define TUSB1046_CTLSEL_USB3              0x1
> +#define TUSB1046_CTLSEL_4LANE_DP          0x2
> +#define TUSB1046_CTLSEL_USB3_AND_2LANE_DP 0x3
> +
> +struct tusb1046_priv {
> +	struct i2c_client *client;
> +	struct typec_switch_dev *sw;
> +	struct typec_mux_dev *mux;
> +
> +	/* Lock General register during accesses */
> +	struct mutex general_reg_lock;
> +};
> +
> +static int tusb1046_mux_set(struct typec_mux_dev *mux,
> +			    struct typec_mux_state *state)
> +{
> +	struct tusb1046_priv *priv = typec_mux_get_drvdata(mux);
> +	struct i2c_client *client = priv->client;
> +	struct device *dev = &client->dev;
> +	int mode, val, ret = 0;
> +
> +	if (state->mode >= TYPEC_STATE_MODAL &&
> +	    state->alt->svid != USB_TYPEC_DP_SID)
> +		return -EINVAL;
> +
> +	dev_dbg(dev, "mux mode requested: %lu\n", state->mode);
> +
> +	mutex_lock(&priv->general_reg_lock);
> +
> +	val = i2c_smbus_read_byte_data(client, TUSB1046_REG_GENERAL);
> +	if (val < 0) {
> +		dev_err(dev, "failed to read ctlsel status, err %d\n", val);
> +		ret = val;
> +		goto out_unlock;
> +	}
> +
> +	switch (state->mode) {
> +	case TYPEC_STATE_USB:
> +		mode = TUSB1046_CTLSEL_USB3;
> +		break;
> +	case TYPEC_DP_STATE_C:
> +	case TYPEC_DP_STATE_E:
> +		mode = TUSB1046_CTLSEL_4LANE_DP;
> +		break;
> +	case TYPEC_DP_STATE_D:
> +		mode = TUSB1046_CTLSEL_USB3_AND_2LANE_DP;
> +		break;
> +	case TYPEC_STATE_SAFE:
> +	default:
> +		mode = TUSB1046_CTLSEL_DISABLED;
> +		break;
> +	}
> +
> +	val &= ~TUSB1046_GENERAL_CTLSEL;
> +	val |= mode;
> +
> +	ret = i2c_smbus_write_byte_data(client, TUSB1046_REG_GENERAL, val);
> +
> +out_unlock:
> +	mutex_unlock(&priv->general_reg_lock);
> +	return ret;
> +}
> +
> +static int tusb1046_switch_set(struct typec_switch_dev *sw,
> +			       enum typec_orientation orientation)
> +{
> +	struct tusb1046_priv *priv = typec_switch_get_drvdata(sw);
> +	struct i2c_client *client = priv->client;
> +	struct device *dev = &client->dev;
> +	int val, ret = 0;
> +
> +	dev_dbg(dev, "setting USB3.0 lane flip for orientation %d\n", orientation);
> +
> +	mutex_lock(&priv->general_reg_lock);
> +
> +	val = i2c_smbus_read_byte_data(client, TUSB1046_REG_GENERAL);
> +	if (val < 0) {
> +		dev_err(dev, "failed to read flipsel status, err %d\n", val);
> +		ret = val;
> +		goto out_unlock;
> +	}
> +
> +	if (orientation == TYPEC_ORIENTATION_REVERSE)
> +		val |= TUSB1046_GENERAL_FLIPSEL;
> +	else
> +		val &= ~TUSB1046_GENERAL_FLIPSEL;
> +
> +	ret = i2c_smbus_write_byte_data(client, TUSB1046_REG_GENERAL, val);
> +
> +out_unlock:
> +	mutex_unlock(&priv->general_reg_lock);
> +	return ret;
> +}
> +
> +static int tusb1046_i2c_probe(struct i2c_client *client)
> +{
> +	struct typec_switch_desc sw_desc = { };
> +	struct typec_mux_desc mux_desc = { };
> +	struct device *dev = &client->dev;
> +	struct tusb1046_priv *priv;
> +	int ret = 0;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return dev_err_probe(dev, -ENOMEM, "failed to allocate driver data\n");
> +
> +	priv->client = client;
> +
> +	mutex_init(&priv->general_reg_lock);
> +
> +	sw_desc.drvdata = priv;
> +	sw_desc.fwnode = dev_fwnode(dev);
> +	sw_desc.set = tusb1046_switch_set;
> +
> +	priv->sw = typec_switch_register(dev, &sw_desc);
> +	if (IS_ERR(priv->sw)) {
> +		ret = dev_err_probe(dev, PTR_ERR(priv->sw), "failed to register type-c switch\n");
> +		goto err_destroy_mutex;
> +	}
> +
> +	mux_desc.drvdata = priv;
> +	mux_desc.fwnode = dev_fwnode(dev);
> +	mux_desc.set = tusb1046_mux_set;
> +
> +	priv->mux = typec_mux_register(dev, &mux_desc);
> +	if (IS_ERR(priv->mux)) {
> +		ret = dev_err_probe(dev, PTR_ERR(priv->mux), "failed to register type-c mux\n");
> +		goto err_unregister_switch;
> +	}
> +
> +	i2c_set_clientdata(client, priv);
> +
> +	return 0;
> +
> +err_unregister_switch:
> +	typec_switch_unregister(priv->sw);
> +err_destroy_mutex:
> +	mutex_destroy(&priv->general_reg_lock);
> +	return ret;
> +}
> +
> +static void tusb1046_i2c_remove(struct i2c_client *client)
> +{
> +	struct tusb1046_priv *priv = i2c_get_clientdata(client);
> +
> +	typec_switch_unregister(priv->sw);
> +	typec_mux_unregister(priv->mux);
> +	mutex_destroy(&priv->general_reg_lock);
> +}
> +
> +static const struct of_device_id tusb1046_match_table[] = {
> +	{.compatible = "ti,tusb1046"},
> +	{},
> +};
> +
> +static struct i2c_driver tusb1046_driver = {
> +	.driver = {
> +		.name = "tusb1046",
> +		.of_match_table = tusb1046_match_table,
> +	},
> +	.probe = tusb1046_i2c_probe,
> +	.remove = tusb1046_i2c_remove,
> +};
> +
> +module_i2c_driver(tusb1046_driver);
> +
> +MODULE_DESCRIPTION("TUSB1046 USB Type-C switch driver");
> +MODULE_AUTHOR("Romain Gantois <romain.gantois@bootlin.com>");
> +MODULE_LICENSE("GPL");
> 
> -- 
> 2.47.0

-- 
heikki