[PATCH] drm/bridge: anx7625: Add anx7625 port switching.

[PATCH] drm/bridge: anx7625: Add anx7625 port switching.

[Pi-Hsun Shih]

When output 2 lanes DP data, anx7625 can output to either TX1/RX1 or
TX2/RX2. In typical usage, these two TX/RX pairs corresponds to two
orientations of typec.

On some board one anx7625 is used as DPI to DP converter for two typec
ports. In this case, the TX1/RX1 and TX2/RX2 are connected to two usb
muxes, which mux the DP data with the rest of the USB3 data, and
connects to the two typec ports.

This patch adds option for anx7625 to acts as a usb typec switch and
switch output lanes based on the typec orientation, or acts as two usb
typec mux and switch output lanes depending on whether the two ports
currently has DP enabled.

Signed-off-by: Pi-Hsun Shih <pihsun@chromium.org>

====================================================================

This is an attempt to use typec framework with how we're using anx7625
on Chrome OS asurada board.

An example of the dts for the two ports case can be found at
https://crrev.com/c/2507199/6

Sending this as a RFC patch since I'm not sure about the best approach
here. Should the logic of switching output lanes depends on ports be
coupled inside anx7625 driver, or in another driver, or is there
any existing way to accomplish this?

---
 drivers/gpu/drm/bridge/analogix/anx7625.c | 135 ++++++++++++++++++++++
 drivers/gpu/drm/bridge/analogix/anx7625.h |  24 ++++
 2 files changed, 159 insertions(+)

diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 65cc05982f82..75f35a197196 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -13,6 +13,9 @@
 #include <linux/mutex.h>
 #include <linux/slab.h>
 #include <linux/types.h>
+#include <linux/usb/typec.h>
+#include <linux/usb/typec_dp.h>
+#include <linux/usb/typec_mux.h>
 #include <linux/workqueue.h>
 
 #include <linux/of_gpio.h>
@@ -1224,6 +1227,122 @@ static irqreturn_t anx7625_intr_hpd_isr(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+static void anx7625_set_crosspoint_switch(struct anx7625_data *ctx,
+					  enum typec_orientation orientation)
+{
+	if (orientation == TYPEC_ORIENTATION_NORMAL) {
+		anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_0,
+				  SW_SEL1_SSRX_B10_B11 | SW_SEL1_ML0_A10_A11);
+		anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_1,
+				  SW_SEL2_SSTX_A2_A3 | SW_SEL2_ML1_B2_B3);
+	} else if (orientation == TYPEC_ORIENTATION_REVERSE) {
+		anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_0,
+				  SW_SEL1_SSRX_A10_A11 | SW_SEL1_ML0_B10_B11);
+		anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_1,
+				  SW_SEL2_SSTX_B2_B3 | SW_SEL2_ML1_A2_A3);
+	}
+}
+
+static int anx7625_usb_set_orientation(struct typec_switch *sw,
+				       enum typec_orientation orientation)
+{
+	struct anx7625_data *ctx = typec_switch_get_drvdata(sw);
+
+	anx7625_set_crosspoint_switch(ctx, orientation);
+	return 0;
+}
+
+static int anx7625_register_usb(struct device *device,
+				struct anx7625_data *ctx)
+{
+	struct typec_switch_desc sw_desc = { };
+	struct fwnode_handle *fwnode = of_fwnode_handle(device->of_node);
+
+	sw_desc.fwnode = fwnode;
+	sw_desc.drvdata = ctx;
+	sw_desc.name = fwnode_get_name(fwnode);
+	sw_desc.set = anx7625_usb_set_orientation;
+
+	ctx->typec_sw = typec_switch_register(device, &sw_desc);
+	if (IS_ERR(ctx->typec_sw))
+		return PTR_ERR(ctx->typec_sw);
+
+	return 0;
+}
+
+static void anx7625_usb_two_ports_update(struct anx7625_data *ctx)
+{
+	if (ctx->typec_ports[0].has_dp && ctx->typec_ports[1].has_dp)
+		// Both ports available, do nothing to retain the current one.
+		return;
+	else if (ctx->typec_ports[0].has_dp)
+		anx7625_set_crosspoint_switch(ctx, TYPEC_ORIENTATION_NORMAL);
+	else if (ctx->typec_ports[1].has_dp)
+		anx7625_set_crosspoint_switch(ctx, TYPEC_ORIENTATION_REVERSE);
+}
+
+static int anx7625_usb_mux_set(struct typec_mux *mux,
+			       struct typec_mux_state *state)
+{
+	struct anx7625_port_data *data = typec_mux_get_drvdata(mux);
+
+	if (state->alt && state->alt->svid == USB_TYPEC_DP_SID &&
+	    state->alt->mode == USB_TYPEC_DP_MODE)
+		data->has_dp = true;
+	else
+		data->has_dp = false;
+
+	anx7625_usb_two_ports_update(data->ctx);
+	return 0;
+}
+
+static int anx7625_register_usb_two_ports(struct device *device,
+					  struct anx7625_data *ctx)
+{
+	struct typec_mux_desc mux_desc = { };
+	struct fwnode_handle *fwnode;
+	struct anx7625_port_data *port_data;
+	u32 port_num;
+	int ret;
+
+	device_for_each_child_node(device, fwnode) {
+		if (fwnode_property_read_u32(fwnode, "reg", &port_num))
+			continue;
+
+		if (port_num >= 2) {
+			DRM_DEV_ERROR(device, "reg too large for ports.");
+			continue;
+		}
+
+		port_data = &ctx->typec_ports[port_num];
+
+		port_data->ctx = ctx;
+		mux_desc.fwnode = fwnode;
+		mux_desc.drvdata = port_data;
+		mux_desc.name = fwnode_get_name(fwnode);
+		mux_desc.set = anx7625_usb_mux_set;
+
+		port_data->typec_mux =
+			typec_mux_register(device, &mux_desc);
+		if (IS_ERR(port_data->typec_mux)) {
+			ret = PTR_ERR(port_data->typec_mux);
+			DRM_DEV_ERROR(device,
+				      "mux register for port %d failed: %d",
+				      port_num, ret);
+			goto err;
+		}
+	}
+
+	return 0;
+
+err:
+	for (port_num = 0; port_num < 2; port_num++) {
+		typec_mux_unregister(ctx->typec_ports[port_num].typec_mux);
+		ctx->typec_ports[port_num].typec_mux = NULL;
+	}
+	return ret;
+}
+
 static int anx7625_parse_dt(struct device *dev,
 			    struct anx7625_platform_data *pdata)
 {
@@ -1239,6 +1358,9 @@ static int anx7625_parse_dt(struct device *dev,
 
 	DRM_DEV_DEBUG_DRIVER(dev, "found dsi host node.\n");
 
+	pdata->tx_rx_to_two_ports =
+		of_property_read_bool(dev->of_node, "anx,tx-rx-to-two-ports");
+
 	ret = drm_of_find_panel_or_bridge(np, 1, 0, &panel, NULL);
 	if (ret < 0) {
 		if (ret == -ENODEV)
@@ -1784,6 +1906,11 @@ static int anx7625_i2c_probe(struct i2c_client *client,
 	if (platform->pdata.intp_irq)
 		queue_work(platform->workqueue, &platform->work);
 
+	if (platform->pdata.tx_rx_to_two_ports)
+		anx7625_register_usb_two_ports(dev, platform);
+	else
+		anx7625_register_usb(dev, platform);
+
 	platform->bridge.funcs = &anx7625_bridge_funcs;
 	platform->bridge.of_node = client->dev.of_node;
 	platform->bridge.ops = DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_HPD;
@@ -1807,9 +1934,17 @@ static int anx7625_i2c_probe(struct i2c_client *client,
 static int anx7625_i2c_remove(struct i2c_client *client)
 {
 	struct anx7625_data *platform = i2c_get_clientdata(client);
+	int i;
 
 	drm_bridge_remove(&platform->bridge);
 
+	if (platform->pdata.tx_rx_to_two_ports)
+		for (i = 0; i < 2; i++)
+			typec_mux_unregister(
+				platform->typec_ports[i].typec_mux);
+	else
+		typec_switch_unregister(platform->typec_sw);
+
 	if (platform->pdata.intp_irq)
 		destroy_workqueue(platform->workqueue);
 
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.h b/drivers/gpu/drm/bridge/analogix/anx7625.h
index 193ad86c5450..bf546f3a4c06 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.h
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.h
@@ -55,6 +55,18 @@
 #define HPD_STATUS_CHANGE 0x80
 #define HPD_STATUS 0x80
 
+#define TCPC_SWITCH_0 0xB4
+#define SW_SEL1_ML0_A10_A11 BIT(0)
+#define SW_SEL1_ML0_B10_B11 BIT(1)
+#define SW_SEL1_SSRX_A10_A11 BIT(4)
+#define SW_SEL1_SSRX_B10_B11 BIT(5)
+
+#define TCPC_SWITCH_1 0xB5
+#define SW_SEL2_ML1_B2_B3 BIT(0)
+#define SW_SEL2_ML1_A2_A3 BIT(1)
+#define SW_SEL2_SSTX_B2_B3 BIT(4)
+#define SW_SEL2_SSTX_A2_A3 BIT(5)
+
 /******** END of I2C Address 0x58 ********/
 
 /***************************************************************/
@@ -354,6 +366,7 @@ struct anx7625_platform_data {
 	int intp_irq;
 	u32 low_power_mode;
 	struct device_node *mipi_host_node;
+	bool tx_rx_to_two_ports;
 };
 
 struct anx7625_i2c_client {
@@ -366,6 +379,14 @@ struct anx7625_i2c_client {
 	struct i2c_client *tcpc_client;
 };
 
+struct anx7625_data;
+
+struct anx7625_port_data {
+	bool has_dp;
+	struct typec_mux *typec_mux;
+	struct anx7625_data *ctx;
+};
+
 struct anx7625_data {
 	struct anx7625_platform_data pdata;
 	atomic_t power_status;
@@ -385,6 +406,9 @@ struct anx7625_data {
 	struct drm_bridge bridge;
 	u8 bridge_attached;
 	struct mipi_dsi_device *dsi;
+
+	struct typec_switch *typec_sw;
+	struct anx7625_port_data typec_ports[2];
 };
 
 #endif  /* __ANX7625_H__ */

base-commit: 3e14f70c05cda4794901ed8f976de3a88deebcc0
-- 
2.29.2.299.gdc1121823c-goog

[PATCH] drm/bridge: anx7625: Add anx7625 port switching.

[Pi-Hsun Shih]

When output 2 lanes DP data, anx7625 can output to either TX1/RX1 or
TX2/RX2. In typical usage, these two TX/RX pairs corresponds to two
orientations of typec.

On some board one anx7625 is used as DPI to DP converter for two typec
ports. In this case, the TX1/RX1 and TX2/RX2 are connected to two usb
muxes, which mux the DP data with the rest of the USB3 data, and
connects to the two typec ports.

This patch adds option for anx7625 to acts as a usb typec switch and
switch output lanes based on the typec orientation, or acts as two usb
typec mux and switch output lanes depending on whether the two ports
currently has DP enabled.

Signed-off-by: Pi-Hsun Shih <pihsun@chromium.org>

====================================================================

This is an attempt to use typec framework with how we're using anx7625
on Chrome OS asurada board.

An example of the dts for the two ports case can be found at
https://crrev.com/c/2507199/6

Sending this as a RFC patch since I'm not sure about the best approach
here. Should the logic of switching output lanes depends on ports be
coupled inside anx7625 driver, or in another driver, or is there
any existing way to accomplish this?

---
 drivers/gpu/drm/bridge/analogix/anx7625.c | 135 ++++++++++++++++++++++
 drivers/gpu/drm/bridge/analogix/anx7625.h |  24 ++++
 2 files changed, 159 insertions(+)

diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 65cc05982f82..75f35a197196 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -13,6 +13,9 @@
 #include <linux/mutex.h>
 #include <linux/slab.h>
 #include <linux/types.h>
+#include <linux/usb/typec.h>
+#include <linux/usb/typec_dp.h>
+#include <linux/usb/typec_mux.h>
 #include <linux/workqueue.h>
 
 #include <linux/of_gpio.h>
@@ -1224,6 +1227,122 @@ static irqreturn_t anx7625_intr_hpd_isr(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+static void anx7625_set_crosspoint_switch(struct anx7625_data *ctx,
+					  enum typec_orientation orientation)
+{
+	if (orientation == TYPEC_ORIENTATION_NORMAL) {
+		anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_0,
+				  SW_SEL1_SSRX_B10_B11 | SW_SEL1_ML0_A10_A11);
+		anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_1,
+				  SW_SEL2_SSTX_A2_A3 | SW_SEL2_ML1_B2_B3);
+	} else if (orientation == TYPEC_ORIENTATION_REVERSE) {
+		anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_0,
+				  SW_SEL1_SSRX_A10_A11 | SW_SEL1_ML0_B10_B11);
+		anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_1,
+				  SW_SEL2_SSTX_B2_B3 | SW_SEL2_ML1_A2_A3);
+	}
+}
+
+static int anx7625_usb_set_orientation(struct typec_switch *sw,
+				       enum typec_orientation orientation)
+{
+	struct anx7625_data *ctx = typec_switch_get_drvdata(sw);
+
+	anx7625_set_crosspoint_switch(ctx, orientation);
+	return 0;
+}
+
+static int anx7625_register_usb(struct device *device,
+				struct anx7625_data *ctx)
+{
+	struct typec_switch_desc sw_desc = { };
+	struct fwnode_handle *fwnode = of_fwnode_handle(device->of_node);
+
+	sw_desc.fwnode = fwnode;
+	sw_desc.drvdata = ctx;
+	sw_desc.name = fwnode_get_name(fwnode);
+	sw_desc.set = anx7625_usb_set_orientation;
+
+	ctx->typec_sw = typec_switch_register(device, &sw_desc);
+	if (IS_ERR(ctx->typec_sw))
+		return PTR_ERR(ctx->typec_sw);
+
+	return 0;
+}
+
+static void anx7625_usb_two_ports_update(struct anx7625_data *ctx)
+{
+	if (ctx->typec_ports[0].has_dp && ctx->typec_ports[1].has_dp)
+		// Both ports available, do nothing to retain the current one.
+		return;
+	else if (ctx->typec_ports[0].has_dp)
+		anx7625_set_crosspoint_switch(ctx, TYPEC_ORIENTATION_NORMAL);
+	else if (ctx->typec_ports[1].has_dp)
+		anx7625_set_crosspoint_switch(ctx, TYPEC_ORIENTATION_REVERSE);
+}
+
+static int anx7625_usb_mux_set(struct typec_mux *mux,
+			       struct typec_mux_state *state)
+{
+	struct anx7625_port_data *data = typec_mux_get_drvdata(mux);
+
+	if (state->alt && state->alt->svid == USB_TYPEC_DP_SID &&
+	    state->alt->mode == USB_TYPEC_DP_MODE)
+		data->has_dp = true;
+	else
+		data->has_dp = false;
+
+	anx7625_usb_two_ports_update(data->ctx);
+	return 0;
+}
+
+static int anx7625_register_usb_two_ports(struct device *device,
+					  struct anx7625_data *ctx)
+{
+	struct typec_mux_desc mux_desc = { };
+	struct fwnode_handle *fwnode;
+	struct anx7625_port_data *port_data;
+	u32 port_num;
+	int ret;
+
+	device_for_each_child_node(device, fwnode) {
+		if (fwnode_property_read_u32(fwnode, "reg", &port_num))
+			continue;
+
+		if (port_num >= 2) {
+			DRM_DEV_ERROR(device, "reg too large for ports.");
+			continue;
+		}
+
+		port_data = &ctx->typec_ports[port_num];
+
+		port_data->ctx = ctx;
+		mux_desc.fwnode = fwnode;
+		mux_desc.drvdata = port_data;
+		mux_desc.name = fwnode_get_name(fwnode);
+		mux_desc.set = anx7625_usb_mux_set;
+
+		port_data->typec_mux =
+			typec_mux_register(device, &mux_desc);
+		if (IS_ERR(port_data->typec_mux)) {
+			ret = PTR_ERR(port_data->typec_mux);
+			DRM_DEV_ERROR(device,
+				      "mux register for port %d failed: %d",
+				      port_num, ret);
+			goto err;
+		}
+	}
+
+	return 0;
+
+err:
+	for (port_num = 0; port_num < 2; port_num++) {
+		typec_mux_unregister(ctx->typec_ports[port_num].typec_mux);
+		ctx->typec_ports[port_num].typec_mux = NULL;
+	}
+	return ret;
+}
+
 static int anx7625_parse_dt(struct device *dev,
 			    struct anx7625_platform_data *pdata)
 {
@@ -1239,6 +1358,9 @@ static int anx7625_parse_dt(struct device *dev,
 
 	DRM_DEV_DEBUG_DRIVER(dev, "found dsi host node.\n");
 
+	pdata->tx_rx_to_two_ports =
+		of_property_read_bool(dev->of_node, "anx,tx-rx-to-two-ports");
+
 	ret = drm_of_find_panel_or_bridge(np, 1, 0, &panel, NULL);
 	if (ret < 0) {
 		if (ret == -ENODEV)
@@ -1784,6 +1906,11 @@ static int anx7625_i2c_probe(struct i2c_client *client,
 	if (platform->pdata.intp_irq)
 		queue_work(platform->workqueue, &platform->work);
 
+	if (platform->pdata.tx_rx_to_two_ports)
+		anx7625_register_usb_two_ports(dev, platform);
+	else
+		anx7625_register_usb(dev, platform);
+
 	platform->bridge.funcs = &anx7625_bridge_funcs;
 	platform->bridge.of_node = client->dev.of_node;
 	platform->bridge.ops = DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_HPD;
@@ -1807,9 +1934,17 @@ static int anx7625_i2c_probe(struct i2c_client *client,
 static int anx7625_i2c_remove(struct i2c_client *client)
 {
 	struct anx7625_data *platform = i2c_get_clientdata(client);
+	int i;
 
 	drm_bridge_remove(&platform->bridge);
 
+	if (platform->pdata.tx_rx_to_two_ports)
+		for (i = 0; i < 2; i++)
+			typec_mux_unregister(
+				platform->typec_ports[i].typec_mux);
+	else
+		typec_switch_unregister(platform->typec_sw);
+
 	if (platform->pdata.intp_irq)
 		destroy_workqueue(platform->workqueue);
 
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.h b/drivers/gpu/drm/bridge/analogix/anx7625.h
index 193ad86c5450..bf546f3a4c06 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.h
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.h
@@ -55,6 +55,18 @@
 #define HPD_STATUS_CHANGE 0x80
 #define HPD_STATUS 0x80
 
+#define TCPC_SWITCH_0 0xB4
+#define SW_SEL1_ML0_A10_A11 BIT(0)
+#define SW_SEL1_ML0_B10_B11 BIT(1)
+#define SW_SEL1_SSRX_A10_A11 BIT(4)
+#define SW_SEL1_SSRX_B10_B11 BIT(5)
+
+#define TCPC_SWITCH_1 0xB5
+#define SW_SEL2_ML1_B2_B3 BIT(0)
+#define SW_SEL2_ML1_A2_A3 BIT(1)
+#define SW_SEL2_SSTX_B2_B3 BIT(4)
+#define SW_SEL2_SSTX_A2_A3 BIT(5)
+
 /******** END of I2C Address 0x58 ********/
 
 /***************************************************************/
@@ -354,6 +366,7 @@ struct anx7625_platform_data {
 	int intp_irq;
 	u32 low_power_mode;
 	struct device_node *mipi_host_node;
+	bool tx_rx_to_two_ports;
 };
 
 struct anx7625_i2c_client {
@@ -366,6 +379,14 @@ struct anx7625_i2c_client {
 	struct i2c_client *tcpc_client;
 };
 
+struct anx7625_data;
+
+struct anx7625_port_data {
+	bool has_dp;
+	struct typec_mux *typec_mux;
+	struct anx7625_data *ctx;
+};
+
 struct anx7625_data {
 	struct anx7625_platform_data pdata;
 	atomic_t power_status;
@@ -385,6 +406,9 @@ struct anx7625_data {
 	struct drm_bridge bridge;
 	u8 bridge_attached;
 	struct mipi_dsi_device *dsi;
+
+	struct typec_switch *typec_sw;
+	struct anx7625_port_data typec_ports[2];
 };
 
 #endif  /* __ANX7625_H__ */

base-commit: 3e14f70c05cda4794901ed8f976de3a88deebcc0
-- 
2.29.2.299.gdc1121823c-goog

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/bridge: anx7625: Add anx7625 port switching.

[Prashant Malani]

Hi Pi-Hsun,

I haven't gone through the code, but did have a high-level comment
(kindly see inline)

On Thu, Nov 12, 2020 at 02:40:40PM +0800, Pi-Hsun Shih wrote:
> When output 2 lanes DP data, anx7625 can output to either TX1/RX1 or
> TX2/RX2. In typical usage, these two TX/RX pairs corresponds to two
> orientations of typec.
> 
> On some board one anx7625 is used as DPI to DP converter for two typec
> ports. In this case, the TX1/RX1 and TX2/RX2 are connected to two usb
> muxes, which mux the DP data with the rest of the USB3 data, and
> connects to the two typec ports.
> 
> This patch adds option for anx7625 to acts as a usb typec switch and
> switch output lanes based on the typec orientation, or acts as two usb
> typec mux and switch output lanes depending on whether the two ports
> currently has DP enabled.
> 
> Signed-off-by: Pi-Hsun Shih <pihsun@chromium.org>
> 
> ====================================================================
> 
> This is an attempt to use typec framework with how we're using anx7625
> on Chrome OS asurada board.
> 
> An example of the dts for the two ports case can be found at
> https://crrev.com/c/2507199/6

Do you plan on submitting DT schemas & bindings documentation for the switch(es)
that are intended to be used?

I would strongly recommend that for usb-c-connector since AFAIK they don't exist, and
I don't believe there is explicit support for them in the Type C connector class framework
(even .

IMO this would be needed to ensure an implementation here doesn't break
in the event of modifications to the connector class framework (or Type
C port drivers like cros-ec-typec) in the future. I think some patches
were floated for this for orientation switch [1] so those might provide
some hints about how to proceed.

I've CC-ed Heikki (Type C maintainer) in case he has additional comments regarding this.

> 
> Sending this as a RFC patch since I'm not sure about the best approach
> here. Should the logic of switching output lanes depends on ports be
> coupled inside anx7625 driver, or in another driver, or is there
> any existing way to accomplish this?

Might be good to add [RFC] as a tag instead of [PATCH] in case this
iteration is chiefly to solicit comments.

Best regards,

-Prashant

[1]:
https://lore.kernel.org/linux-usb/1604403610-16577-1-git-send-email-jun.li@nxp.com/
> 

Re: [PATCH] drm/bridge: anx7625: Add anx7625 port switching.

[Prashant Malani]

Hi Pi-Hsun,

I haven't gone through the code, but did have a high-level comment
(kindly see inline)

On Thu, Nov 12, 2020 at 02:40:40PM +0800, Pi-Hsun Shih wrote:
> When output 2 lanes DP data, anx7625 can output to either TX1/RX1 or
> TX2/RX2. In typical usage, these two TX/RX pairs corresponds to two
> orientations of typec.
> 
> On some board one anx7625 is used as DPI to DP converter for two typec
> ports. In this case, the TX1/RX1 and TX2/RX2 are connected to two usb
> muxes, which mux the DP data with the rest of the USB3 data, and
> connects to the two typec ports.
> 
> This patch adds option for anx7625 to acts as a usb typec switch and
> switch output lanes based on the typec orientation, or acts as two usb
> typec mux and switch output lanes depending on whether the two ports
> currently has DP enabled.
> 
> Signed-off-by: Pi-Hsun Shih <pihsun@chromium.org>
> 
> ====================================================================
> 
> This is an attempt to use typec framework with how we're using anx7625
> on Chrome OS asurada board.
> 
> An example of the dts for the two ports case can be found at
> https://crrev.com/c/2507199/6

Do you plan on submitting DT schemas & bindings documentation for the switch(es)
that are intended to be used?

I would strongly recommend that for usb-c-connector since AFAIK they don't exist, and
I don't believe there is explicit support for them in the Type C connector class framework
(even .

IMO this would be needed to ensure an implementation here doesn't break
in the event of modifications to the connector class framework (or Type
C port drivers like cros-ec-typec) in the future. I think some patches
were floated for this for orientation switch [1] so those might provide
some hints about how to proceed.

I've CC-ed Heikki (Type C maintainer) in case he has additional comments regarding this.

> 
> Sending this as a RFC patch since I'm not sure about the best approach
> here. Should the logic of switching output lanes depends on ports be
> coupled inside anx7625 driver, or in another driver, or is there
> any existing way to accomplish this?

Might be good to add [RFC] as a tag instead of [PATCH] in case this
iteration is chiefly to solicit comments.

Best regards,

-Prashant

[1]:
https://lore.kernel.org/linux-usb/1604403610-16577-1-git-send-email-jun.li@nxp.com/
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/bridge: anx7625: Add anx7625 port switching.

[Pi-Hsun Shih]

Hi Prashant,

Please see inline reply as below.

On Thu, Nov 12, 2020 at 4:59 PM Prashant Malani <pmalani@chromium.org> wrote:
>
> Hi Pi-Hsun,
>
> I haven't gone through the code, but did have a high-level comment
> (kindly see inline)
>
> On Thu, Nov 12, 2020 at 02:40:40PM +0800, Pi-Hsun Shih wrote:
> > When output 2 lanes DP data, anx7625 can output to either TX1/RX1 or
> > TX2/RX2. In typical usage, these two TX/RX pairs corresponds to two
> > orientations of typec.
> >
> > On some board one anx7625 is used as DPI to DP converter for two typec
> > ports. In this case, the TX1/RX1 and TX2/RX2 are connected to two usb
> > muxes, which mux the DP data with the rest of the USB3 data, and
> > connects to the two typec ports.
> >
> > This patch adds option for anx7625 to acts as a usb typec switch and
> > switch output lanes based on the typec orientation, or acts as two usb
> > typec mux and switch output lanes depending on whether the two ports
> > currently has DP enabled.
> >
> > Signed-off-by: Pi-Hsun Shih <pihsun@chromium.org>
> >
> > ====================================================================
> >
> > This is an attempt to use typec framework with how we're using anx7625
> > on Chrome OS asurada board.
> >
> > An example of the dts for the two ports case can be found at
> > https://crrev.com/c/2507199/6
>
> Do you plan on submitting DT schemas & bindings documentation for the switch(es)
> that are intended to be used?

Yes I plan to submit corresponding DT schemas & bindings documentation
changes if this change looks good.

>
> I would strongly recommend that for usb-c-connector since AFAIK they don't exist, and
> I don't believe there is explicit support for them in the Type C connector class framework
> (even .
>
> IMO this would be needed to ensure an implementation here doesn't break
> in the event of modifications to the connector class framework (or Type
> C port drivers like cros-ec-typec) in the future. I think some patches
> were floated for this for orientation switch [1] so those might provide
> some hints about how to proceed.
>
> I've CC-ed Heikki (Type C maintainer) in case he has additional comments regarding this.
>
> >
> > Sending this as a RFC patch since I'm not sure about the best approach
> > here. Should the logic of switching output lanes depends on ports be
> > coupled inside anx7625 driver, or in another driver, or is there
> > any existing way to accomplish this?
>
> Might be good to add [RFC] as a tag instead of [PATCH] in case this
> iteration is chiefly to solicit comments.

Ah I did have [RFC] tag in some local .patch files before. I guess I
somehow forgot it in later `git format-patch` runs...
I'll add the tag in the next version, thanks for the comments.

>
> Best regards,
>
> -Prashant
>
> [1]:
> https://lore.kernel.org/linux-usb/1604403610-16577-1-git-send-email-jun.li@nxp.com/
> >

Re: [PATCH] drm/bridge: anx7625: Add anx7625 port switching.

[Pi-Hsun Shih]

Hi Prashant,

Please see inline reply as below.

On Thu, Nov 12, 2020 at 4:59 PM Prashant Malani <pmalani@chromium.org> wrote:
>
> Hi Pi-Hsun,
>
> I haven't gone through the code, but did have a high-level comment
> (kindly see inline)
>
> On Thu, Nov 12, 2020 at 02:40:40PM +0800, Pi-Hsun Shih wrote:
> > When output 2 lanes DP data, anx7625 can output to either TX1/RX1 or
> > TX2/RX2. In typical usage, these two TX/RX pairs corresponds to two
> > orientations of typec.
> >
> > On some board one anx7625 is used as DPI to DP converter for two typec
> > ports. In this case, the TX1/RX1 and TX2/RX2 are connected to two usb
> > muxes, which mux the DP data with the rest of the USB3 data, and
> > connects to the two typec ports.
> >
> > This patch adds option for anx7625 to acts as a usb typec switch and
> > switch output lanes based on the typec orientation, or acts as two usb
> > typec mux and switch output lanes depending on whether the two ports
> > currently has DP enabled.
> >
> > Signed-off-by: Pi-Hsun Shih <pihsun@chromium.org>
> >
> > ====================================================================
> >
> > This is an attempt to use typec framework with how we're using anx7625
> > on Chrome OS asurada board.
> >
> > An example of the dts for the two ports case can be found at
> > https://crrev.com/c/2507199/6
>
> Do you plan on submitting DT schemas & bindings documentation for the switch(es)
> that are intended to be used?

Yes I plan to submit corresponding DT schemas & bindings documentation
changes if this change looks good.

>
> I would strongly recommend that for usb-c-connector since AFAIK they don't exist, and
> I don't believe there is explicit support for them in the Type C connector class framework
> (even .
>
> IMO this would be needed to ensure an implementation here doesn't break
> in the event of modifications to the connector class framework (or Type
> C port drivers like cros-ec-typec) in the future. I think some patches
> were floated for this for orientation switch [1] so those might provide
> some hints about how to proceed.
>
> I've CC-ed Heikki (Type C maintainer) in case he has additional comments regarding this.
>
> >
> > Sending this as a RFC patch since I'm not sure about the best approach
> > here. Should the logic of switching output lanes depends on ports be
> > coupled inside anx7625 driver, or in another driver, or is there
> > any existing way to accomplish this?
>
> Might be good to add [RFC] as a tag instead of [PATCH] in case this
> iteration is chiefly to solicit comments.

Ah I did have [RFC] tag in some local .patch files before. I guess I
somehow forgot it in later `git format-patch` runs...
I'll add the tag in the next version, thanks for the comments.

>
> Best regards,
>
> -Prashant
>
> [1]:
> https://lore.kernel.org/linux-usb/1604403610-16577-1-git-send-email-jun.li@nxp.com/
> >
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/bridge: anx7625: Add anx7625 port switching.

[Prashant Malani]

On Thu, Nov 12, 2020 at 05:07:05PM +0800, Pi-Hsun Shih wrote:
> Hi Prashant,
> 
> Please see inline reply as below.
> 
> On Thu, Nov 12, 2020 at 4:59 PM Prashant Malani <pmalani@chromium.org> wrote:
> >
> > Hi Pi-Hsun,
> >
> > I haven't gone through the code, but did have a high-level comment
> > (kindly see inline)
> >
> > On Thu, Nov 12, 2020 at 02:40:40PM +0800, Pi-Hsun Shih wrote:
> > > When output 2 lanes DP data, anx7625 can output to either TX1/RX1 or
> > > TX2/RX2. In typical usage, these two TX/RX pairs corresponds to two
> > > orientations of typec.
> > >
> > > On some board one anx7625 is used as DPI to DP converter for two typec
> > > ports. In this case, the TX1/RX1 and TX2/RX2 are connected to two usb
> > > muxes, which mux the DP data with the rest of the USB3 data, and
> > > connects to the two typec ports.
> > >
> > > This patch adds option for anx7625 to acts as a usb typec switch and
> > > switch output lanes based on the typec orientation, or acts as two usb
> > > typec mux and switch output lanes depending on whether the two ports
> > > currently has DP enabled.
> > >
> > > Signed-off-by: Pi-Hsun Shih <pihsun@chromium.org>
> > >
> > > ====================================================================
> > >
> > > This is an attempt to use typec framework with how we're using anx7625
> > > on Chrome OS asurada board.
> > >
> > > An example of the dts for the two ports case can be found at
> > > https://crrev.com/c/2507199/6
> >
> > Do you plan on submitting DT schemas & bindings documentation for the switch(es)
> > that are intended to be used?
> 
> Yes I plan to submit corresponding DT schemas & bindings documentation
> changes if this change looks good.
> 

That's great. Thanks for confirming :)


BR,

-Prashant

Re: [PATCH] drm/bridge: anx7625: Add anx7625 port switching.

[Prashant Malani]

On Thu, Nov 12, 2020 at 05:07:05PM +0800, Pi-Hsun Shih wrote:
> Hi Prashant,
> 
> Please see inline reply as below.
> 
> On Thu, Nov 12, 2020 at 4:59 PM Prashant Malani <pmalani@chromium.org> wrote:
> >
> > Hi Pi-Hsun,
> >
> > I haven't gone through the code, but did have a high-level comment
> > (kindly see inline)
> >
> > On Thu, Nov 12, 2020 at 02:40:40PM +0800, Pi-Hsun Shih wrote:
> > > When output 2 lanes DP data, anx7625 can output to either TX1/RX1 or
> > > TX2/RX2. In typical usage, these two TX/RX pairs corresponds to two
> > > orientations of typec.
> > >
> > > On some board one anx7625 is used as DPI to DP converter for two typec
> > > ports. In this case, the TX1/RX1 and TX2/RX2 are connected to two usb
> > > muxes, which mux the DP data with the rest of the USB3 data, and
> > > connects to the two typec ports.
> > >
> > > This patch adds option for anx7625 to acts as a usb typec switch and
> > > switch output lanes based on the typec orientation, or acts as two usb
> > > typec mux and switch output lanes depending on whether the two ports
> > > currently has DP enabled.
> > >
> > > Signed-off-by: Pi-Hsun Shih <pihsun@chromium.org>
> > >
> > > ====================================================================
> > >
> > > This is an attempt to use typec framework with how we're using anx7625
> > > on Chrome OS asurada board.
> > >
> > > An example of the dts for the two ports case can be found at
> > > https://crrev.com/c/2507199/6
> >
> > Do you plan on submitting DT schemas & bindings documentation for the switch(es)
> > that are intended to be used?
> 
> Yes I plan to submit corresponding DT schemas & bindings documentation
> changes if this change looks good.
> 

That's great. Thanks for confirming :)


BR,

-Prashant
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/bridge: anx7625: Add anx7625 port switching.

[kernel test robot]

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

Hi Pi-Hsun,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on 3e14f70c05cda4794901ed8f976de3a88deebcc0]

url:    https://github.com/0day-ci/linux/commits/Pi-Hsun-Shih/drm-bridge-anx7625-Add-anx7625-port-switching/20201112-144229
base:    3e14f70c05cda4794901ed8f976de3a88deebcc0
config: i386-randconfig-r016-20201111 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/96b15cfd320004cab18e33f081007ac3c312a30d
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Pi-Hsun-Shih/drm-bridge-anx7625-Add-anx7625-port-switching/20201112-144229
        git checkout 96b15cfd320004cab18e33f081007ac3c312a30d
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   ld: drivers/gpu/drm/bridge/analogix/anx7625.o: in function `anx7625_i2c_remove':
>> drivers/gpu/drm/bridge/analogix/anx7625.c:1943: undefined reference to `typec_mux_unregister'
>> ld: drivers/gpu/drm/bridge/analogix/anx7625.c:1943: undefined reference to `typec_mux_unregister'
>> ld: drivers/gpu/drm/bridge/analogix/anx7625.c:1946: undefined reference to `typec_switch_unregister'
   ld: drivers/gpu/drm/bridge/analogix/anx7625.o: in function `anx7625_usb_mux_set':
>> drivers/gpu/drm/bridge/analogix/anx7625.c:1287: undefined reference to `typec_mux_get_drvdata'
   ld: drivers/gpu/drm/bridge/analogix/anx7625.o: in function `anx7625_register_usb_two_ports':
>> drivers/gpu/drm/bridge/analogix/anx7625.c:1326: undefined reference to `typec_mux_register'
   ld: drivers/gpu/drm/bridge/analogix/anx7625.o: in function `anx7625_register_usb':
>> drivers/gpu/drm/bridge/analogix/anx7625.c:1266: undefined reference to `typec_switch_register'
   ld: drivers/gpu/drm/bridge/analogix/anx7625.o: in function `anx7625_register_usb_two_ports':
   drivers/gpu/drm/bridge/analogix/anx7625.c:1340: undefined reference to `typec_mux_unregister'
   ld: drivers/gpu/drm/bridge/analogix/anx7625.o: in function `anx7625_usb_set_orientation':
>> drivers/gpu/drm/bridge/analogix/anx7625.c:1249: undefined reference to `typec_switch_get_drvdata'

vim +1943 drivers/gpu/drm/bridge/analogix/anx7625.c

  1933	
  1934	static int anx7625_i2c_remove(struct i2c_client *client)
  1935	{
  1936		struct anx7625_data *platform = i2c_get_clientdata(client);
  1937		int i;
  1938	
  1939		drm_bridge_remove(&platform->bridge);
  1940	
  1941		if (platform->pdata.tx_rx_to_two_ports)
  1942			for (i = 0; i < 2; i++)
> 1943				typec_mux_unregister(
  1944					platform->typec_ports[i].typec_mux);
  1945		else
> 1946			typec_switch_unregister(platform->typec_sw);
  1947	
  1948		if (platform->pdata.intp_irq)
  1949			destroy_workqueue(platform->workqueue);
  1950	
  1951		anx7625_unregister_i2c_dummy_clients(platform);
  1952	
  1953		kfree(platform);
  1954		return 0;
  1955	}
  1956	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 33059 bytes --]