Hi Chen-Yu, Thanks for the review. On Mon, Jan 9, 2023 at 7:26 PM Chen-Yu Tsai <wenst@xxxxxxxxxxxx> wrote: > > On Mon, Jan 9, 2023 at 4:41 PM Pin-yen Lin <treapking@xxxxxxxxxxxx> wrote: > > > > Register USB Type-C mode switches when the "mode-switch" property and > > relevant port are available in Device Tree. Configure the crosspoint > ^ ports > Thanks for catching this. I'll fix in v10. > > switch based on the entered alternate mode for a specific Type-C > > connector. > > You should also mention that the "one mode switch" scenario is not > covered in this implementation, due to lack of actual hardware. If I understand correctly, we should use "orientation-switch"[1] instead when the crosspoint switch on anx7625 is used to support different orientations of the Type-C connector. I'll add some explanations around this in the commit message in v10. [1]: https://docs.kernel.org/driver-api/usb/typec.html#multiplexer-demultiplexer-switches > > > Signed-off-by: Pin-yen Lin <treapking@xxxxxxxxxxxx> > > > > --- > > > > (no changes since v7) > > > > Changes in v7: > > - Fixed style issues in anx7625 driver > > - Removed DT property validation in anx7625 driver. > > - Extracted common codes to another commit. > > > > Changes in v6: > > - Squashed to a single patch > > > > drivers/gpu/drm/bridge/analogix/Kconfig | 1 + > > drivers/gpu/drm/bridge/analogix/anx7625.c | 88 +++++++++++++++++++++++ > > drivers/gpu/drm/bridge/analogix/anx7625.h | 13 ++++ > > 3 files changed, 102 insertions(+) > > > > diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig b/drivers/gpu/drm/bridge/analogix/Kconfig > > index 173dada218ec..992b43ed1dd7 100644 > > --- a/drivers/gpu/drm/bridge/analogix/Kconfig > > +++ b/drivers/gpu/drm/bridge/analogix/Kconfig > > @@ -34,6 +34,7 @@ config DRM_ANALOGIX_ANX7625 > > tristate "Analogix Anx7625 MIPI to DP interface support" > > depends on DRM > > depends on OF > > + depends on TYPEC || TYPEC=n > > select DRM_DISPLAY_DP_HELPER > > select DRM_DISPLAY_HDCP_HELPER > > select DRM_DISPLAY_HELPER > > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c > > index 1cf242130b91..2bb504a8d789 100644 > > --- a/drivers/gpu/drm/bridge/analogix/anx7625.c > > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c > > @@ -15,6 +15,8 @@ > > #include <linux/regulator/consumer.h> > > #include <linux/slab.h> > > #include <linux/types.h> > > +#include <linux/usb/typec_dp.h> > > +#include <linux/usb/typec_mux.h> > > #include <linux/workqueue.h> > > > > #include <linux/of_gpio.h> > > @@ -2572,6 +2574,86 @@ static void anx7625_runtime_disable(void *data) > > pm_runtime_disable(data); > > } > > > > +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_RX1 | SW_SEL1_DPTX0_RX2); > > + anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_1, > > + SW_SEL2_SSTX_TX1 | SW_SEL2_DPTX1_TX2); > > + } else if (orientation == TYPEC_ORIENTATION_REVERSE) { > > + anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_0, > > + SW_SEL1_SSRX_RX2 | SW_SEL1_DPTX0_RX1); > > + anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_1, > > + SW_SEL2_SSTX_TX2 | SW_SEL2_DPTX1_TX1); > > + } > > +} > > + > > +static void anx7625_typec_two_ports_update(struct anx7625_data *ctx) > > +{ > > + struct drm_dp_typec_switch_desc switch_desc = ctx->switch_desc; > > + /* Check if both ports available and do nothing to retain the current one */ > > + if (switch_desc.typec_ports[0].dp_connected && switch_desc.typec_ports[1].dp_connected) > > + return; > > + > > + if (switch_desc.typec_ports[0].dp_connected) > > + anx7625_set_crosspoint_switch(ctx, TYPEC_ORIENTATION_NORMAL); > > + else if (switch_desc.typec_ports[1].dp_connected) > > + anx7625_set_crosspoint_switch(ctx, TYPEC_ORIENTATION_REVERSE); > > +} > > + > > +static int anx7625_typec_mux_set(struct typec_mux_dev *mux, > > + struct typec_mux_state *state) > > +{ > > + struct drm_dp_typec_port_data *port_data = typec_mux_get_drvdata(mux); > > + struct anx7625_data *ctx = (struct anx7625_data *) port_data->data; > > + struct device *dev = &ctx->client->dev; > > + struct drm_dp_typec_switch_desc switch_desc = ctx->switch_desc; > > + bool new_dp_connected, old_dp_connected; > > + > > And place a TODO note here. I'll add it in v10. > > Otherwise this looks OK. > > Also, > > Tested-by: Chen-Yu Tsai <wenst@xxxxxxxxxxxx> > on MT8192 based Hayato (ASUS Chromebook Flip CM3200). > > And this also uncovered a deadlock in the unplug & disable path. > I'll send a fix for that later once I figure out all the details. Thank you so much for this!