diff mbox series

[3/7] phy: qcom-qmp-combo: Introduce orientation variable

Message ID 20230425034010.3789376-4-quic_bjorande@quicinc.com
State New
Headers
Series phy: qcom-qmp-combo: Support orientation switching |

Commit Message

Bjorn Andersson April 25, 2023, 3:40 a.m. UTC 
  In multiple places throughout the driver code has been written in
prepration for handling of orientation switching.

Introduce a typec_orientation in qmp_combo and fill out the various
"placeholders" with the associated logic. By initializing the
orientation to "normal" this change has no functional impact, but
reduces the size of the upcoming introduction of dynamic orientation
switching.

Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 54 +++++++++++++----------
 1 file changed, 30 insertions(+), 24 deletions(-)

Comments

Neil Armstrong April 27, 2023, 1:13 p.m. UTC | #1 
On 25/04/2023 05:40, Bjorn Andersson wrote:
> In multiple places throughout the driver code has been written in
> prepration for handling of orientation switching.
> 
> Introduce a typec_orientation in qmp_combo and fill out the various
> "placeholders" with the associated logic. By initializing the
> orientation to "normal" this change has no functional impact, but
> reduces the size of the upcoming introduction of dynamic orientation
> switching.
> 
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
>   drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 54 +++++++++++++----------
>   1 file changed, 30 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index 7280f7141961..6748f31da7a3 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -19,6 +19,7 @@
>   #include <linux/regulator/consumer.h>
>   #include <linux/reset.h>
>   #include <linux/slab.h>
> +#include <linux/usb/typec.h>
>   
>   #include <dt-bindings/phy/phy-qcom-qmp.h>
>   
> @@ -63,6 +64,10 @@
>   /* QPHY_V3_PCS_MISC_CLAMP_ENABLE register bits */
>   #define CLAMP_EN				BIT(0) /* enables i/o clamp_n */
>   
> +/* QPHY_V3_DP_COM_TYPEC_CTRL register bits */
> +#define SW_PORTSELECT_VAL			BIT(0)
> +#define SW_PORTSELECT_MUX			BIT(1)
> +
>   #define PHY_INIT_COMPLETE_TIMEOUT		10000
>   
>   struct qmp_phy_init_tbl {
> @@ -1323,6 +1328,8 @@ struct qmp_combo {
>   	struct clk_fixed_rate pipe_clk_fixed;
>   	struct clk_hw dp_link_hw;
>   	struct clk_hw dp_pixel_hw;
> +
> +	enum typec_orientation orientation;
>   };
>   
>   static void qmp_v3_dp_aux_init(struct qmp_combo *qmp);
> @@ -1955,29 +1962,23 @@ static void qmp_v3_configure_dp_tx(struct qmp_combo *qmp)
>   static bool qmp_combo_configure_dp_mode(struct qmp_combo *qmp)
>   {
>   	u32 val;
> -	bool reverse = false;
> +	bool reverse = qmp->orientation == TYPEC_ORIENTATION_REVERSE;
> +	const struct phy_configure_opts_dp *dp_opts = &qmp->dp_opts;
>   
>   	val = DP_PHY_PD_CTL_PWRDN | DP_PHY_PD_CTL_AUX_PWRDN |
>   	      DP_PHY_PD_CTL_PLL_PWRDN | DP_PHY_PD_CTL_DP_CLAMP_EN;
>   
> -	/*
> -	 * TODO: Assume orientation is CC1 for now and two lanes, need to
> -	 * use type-c connector to understand orientation and lanes.
> -	 *
> -	 * Otherwise val changes to be like below if this code understood
> -	 * the orientation of the type-c cable.
> -	 *
> -	 * if (lane_cnt == 4 || orientation == ORIENTATION_CC2)
> -	 *	val |= DP_PHY_PD_CTL_LANE_0_1_PWRDN;
> -	 * if (lane_cnt == 4 || orientation == ORIENTATION_CC1)
> -	 *	val |= DP_PHY_PD_CTL_LANE_2_3_PWRDN;
> -	 * if (orientation == ORIENTATION_CC2)
> -	 *	writel(0x4c, qmp->dp_dp_phy + QSERDES_V3_DP_PHY_MODE);
> -	 */
> -	val |= DP_PHY_PD_CTL_LANE_2_3_PWRDN;
> +	if (dp_opts->lanes == 4 || reverse)
> +		val |= DP_PHY_PD_CTL_LANE_0_1_PWRDN;
> +	if (dp_opts->lanes == 4 || !reverse)
> +		val |= DP_PHY_PD_CTL_LANE_2_3_PWRDN;
> +
>   	writel(val, qmp->dp_dp_phy + QSERDES_DP_PHY_PD_CTL);
>   
> -	writel(0x5c, qmp->dp_dp_phy + QSERDES_DP_PHY_MODE);
> +	if (reverse)
> +		writel(0x4c, qmp->pcs + QSERDES_DP_PHY_MODE);
> +	else
> +		writel(0x5c, qmp->pcs + QSERDES_DP_PHY_MODE);
>   
>   	return reverse;
>   }
> @@ -2235,7 +2236,7 @@ static int qmp_v4_configure_dp_phy(struct qmp_combo *qmp)
>   {
>   	const struct phy_configure_opts_dp *dp_opts = &qmp->dp_opts;
>   	u32 bias0_en, drvr0_en, bias1_en, drvr1_en;
> -	bool reverse = false;
> +	bool reverse = qmp->orientation == TYPEC_ORIENTATION_REVERSE;
>   	u32 status;
>   	int ret;
>   
> @@ -2299,7 +2300,7 @@ static int qmp_v5_configure_dp_phy(struct qmp_combo *qmp)
>   {
>   	const struct phy_configure_opts_dp *dp_opts = &qmp->dp_opts;
>   	u32 bias0_en, drvr0_en, bias1_en, drvr1_en;
> -	bool reverse = false;
> +	bool reverse = qmp->orientation == TYPEC_ORIENTATION_REVERSE;
>   	u32 status;
>   	int ret;
>   
> @@ -2358,7 +2359,7 @@ static int qmp_v6_configure_dp_phy(struct qmp_combo *qmp)
>   {
>   	const struct phy_configure_opts_dp *dp_opts = &qmp->dp_opts;
>   	u32 bias0_en, drvr0_en, bias1_en, drvr1_en;
> -	bool reverse = false;
> +	bool reverse = qmp->orientation == TYPEC_ORIENTATION_REVERSE;
>   	u32 status;
>   	int ret;
>   
> @@ -2462,6 +2463,7 @@ static int qmp_combo_com_init(struct qmp_combo *qmp)
>   	const struct qmp_phy_cfg *cfg = qmp->cfg;
>   	void __iomem *com = qmp->com;
>   	int ret;
> +	u32 val;
>   
>   	if (qmp->init_count++)
>   		return 0;
> @@ -2495,10 +2497,12 @@ static int qmp_combo_com_init(struct qmp_combo *qmp)
>   			SW_DPPHY_RESET_MUX | SW_DPPHY_RESET |
>   			SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
>   
> -	/* Default type-c orientation, i.e CC1 */
> -	qphy_setbits(com, QPHY_V3_DP_COM_TYPEC_CTRL, 0x02);
> -
> -	qphy_setbits(com, QPHY_V3_DP_COM_PHY_MODE_CTRL, USB3_MODE | DP_MODE);
> +	/* Use software based port select and switch on typec orientation */
> +	val = SW_PORTSELECT_MUX;
> +	if (qmp->orientation == TYPEC_ORIENTATION_REVERSE)
> +		val |= SW_PORTSELECT_VAL;
> +	writel(val, com + QPHY_V3_DP_COM_TYPEC_CTRL);
> +	writel(USB3_MODE | DP_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
>   
>   	/* bring both QMP USB and QMP DP PHYs PCS block out of reset */
>   	qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
> @@ -3361,6 +3365,8 @@ static int qmp_combo_probe(struct platform_device *pdev)
>   
>   	qmp->dev = dev;
>   
> +	qmp->orientation = TYPEC_ORIENTATION_NORMAL;
> +
>   	qmp->cfg = of_device_get_match_data(dev);
>   	if (!qmp->cfg)
>   		return -EINVAL;

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Johan Hovold May 2, 2023, 11:48 a.m. UTC | #2 
On Mon, Apr 24, 2023 at 08:40:06PM -0700, Bjorn Andersson wrote:
> In multiple places throughout the driver code has been written in
> prepration for handling of orientation switching.
> 
> Introduce a typec_orientation in qmp_combo and fill out the various
> "placeholders" with the associated logic. By initializing the
> orientation to "normal" this change has no functional impact, but
> reduces the size of the upcoming introduction of dynamic orientation
> switching.
> 
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 54 +++++++++++++----------
>  1 file changed, 30 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index 7280f7141961..6748f31da7a3 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -19,6 +19,7 @@
>  #include <linux/regulator/consumer.h>
>  #include <linux/reset.h>
>  #include <linux/slab.h>
> +#include <linux/usb/typec.h>
>  
>  #include <dt-bindings/phy/phy-qcom-qmp.h>
>  
> @@ -63,6 +64,10 @@
>  /* QPHY_V3_PCS_MISC_CLAMP_ENABLE register bits */
>  #define CLAMP_EN				BIT(0) /* enables i/o clamp_n */
>  
> +/* QPHY_V3_DP_COM_TYPEC_CTRL register bits */
> +#define SW_PORTSELECT_VAL			BIT(0)
> +#define SW_PORTSELECT_MUX			BIT(1)
> +
>  #define PHY_INIT_COMPLETE_TIMEOUT		10000
>  
>  struct qmp_phy_init_tbl {
> @@ -1323,6 +1328,8 @@ struct qmp_combo {
>  	struct clk_fixed_rate pipe_clk_fixed;
>  	struct clk_hw dp_link_hw;
>  	struct clk_hw dp_pixel_hw;
> +
> +	enum typec_orientation orientation;
>  };
>  
>  static void qmp_v3_dp_aux_init(struct qmp_combo *qmp);
> @@ -1955,29 +1962,23 @@ static void qmp_v3_configure_dp_tx(struct qmp_combo *qmp)
>  static bool qmp_combo_configure_dp_mode(struct qmp_combo *qmp)
>  {
>  	u32 val;
> -	bool reverse = false;
> +	bool reverse = qmp->orientation == TYPEC_ORIENTATION_REVERSE;

Adding parentheses around the right-hand side should make this a little
easier to parse.

It also looks like these callbacks end up being called without holding
the qmp->phy_mutex via phy->power_on(). Perhaps there is no risk for a
concurrent switch notification and dp phy power-on but it's not that
obvious.

> +	const struct phy_configure_opts_dp *dp_opts = &qmp->dp_opts;

Also could you add these before u32 val to maintain an approximation of
reverse xmas style?

And similar below.

Johan
Bjorn Andersson May 4, 2023, 3:29 a.m. UTC | #3 
On Tue, May 02, 2023 at 01:48:16PM +0200, Johan Hovold wrote:
> On Mon, Apr 24, 2023 at 08:40:06PM -0700, Bjorn Andersson wrote:
> > In multiple places throughout the driver code has been written in
> > prepration for handling of orientation switching.
> > 
> > Introduce a typec_orientation in qmp_combo and fill out the various
> > "placeholders" with the associated logic. By initializing the
> > orientation to "normal" this change has no functional impact, but
> > reduces the size of the upcoming introduction of dynamic orientation
> > switching.
> > 
> > Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> > ---
> >  drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 54 +++++++++++++----------
> >  1 file changed, 30 insertions(+), 24 deletions(-)
> > 
> > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> > index 7280f7141961..6748f31da7a3 100644
> > --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> > @@ -19,6 +19,7 @@
> >  #include <linux/regulator/consumer.h>
> >  #include <linux/reset.h>
> >  #include <linux/slab.h>
> > +#include <linux/usb/typec.h>
> >  
> >  #include <dt-bindings/phy/phy-qcom-qmp.h>
> >  
> > @@ -63,6 +64,10 @@
> >  /* QPHY_V3_PCS_MISC_CLAMP_ENABLE register bits */
> >  #define CLAMP_EN				BIT(0) /* enables i/o clamp_n */
> >  
> > +/* QPHY_V3_DP_COM_TYPEC_CTRL register bits */
> > +#define SW_PORTSELECT_VAL			BIT(0)
> > +#define SW_PORTSELECT_MUX			BIT(1)
> > +
> >  #define PHY_INIT_COMPLETE_TIMEOUT		10000
> >  
> >  struct qmp_phy_init_tbl {
> > @@ -1323,6 +1328,8 @@ struct qmp_combo {
> >  	struct clk_fixed_rate pipe_clk_fixed;
> >  	struct clk_hw dp_link_hw;
> >  	struct clk_hw dp_pixel_hw;
> > +
> > +	enum typec_orientation orientation;
> >  };
> >  
> >  static void qmp_v3_dp_aux_init(struct qmp_combo *qmp);
> > @@ -1955,29 +1962,23 @@ static void qmp_v3_configure_dp_tx(struct qmp_combo *qmp)
> >  static bool qmp_combo_configure_dp_mode(struct qmp_combo *qmp)
> >  {
> >  	u32 val;
> > -	bool reverse = false;
> > +	bool reverse = qmp->orientation == TYPEC_ORIENTATION_REVERSE;
> 
> Adding parentheses around the right-hand side should make this a little
> easier to parse.
> 
> It also looks like these callbacks end up being called without holding
> the qmp->phy_mutex via phy->power_on(). Perhaps there is no risk for a
> concurrent switch notification and dp phy power-on but it's not that
> obvious.
> 

It seems we're arriving here from hpd_event_thread(), while
phy_power_on() and phy_power_off() will be called in some other context.
I've not been able to convince myself if DP driver ensures ordering, or
if we have an existing race here...

Unless you insist, I would prefer to follow up with an additional patch
once we've landed this series. The fix will depend on the phy_mutex
shuffling patch anyways...

> > +	const struct phy_configure_opts_dp *dp_opts = &qmp->dp_opts;
> 
> Also could you add these before u32 val to maintain an approximation of
> reverse xmas style?
> 

I'd be happy to do so :)

Regards,
Bjorn

> And similar below.
> 
> Johan
Johan Hovold May 4, 2023, 1:44 p.m. UTC | #4 
On Wed, May 03, 2023 at 08:29:07PM -0700, Bjorn Andersson wrote:
> On Tue, May 02, 2023 at 01:48:16PM +0200, Johan Hovold wrote:
> > On Mon, Apr 24, 2023 at 08:40:06PM -0700, Bjorn Andersson wrote:

> > >  static void qmp_v3_dp_aux_init(struct qmp_combo *qmp);
> > > @@ -1955,29 +1962,23 @@ static void qmp_v3_configure_dp_tx(struct qmp_combo *qmp)
> > >  static bool qmp_combo_configure_dp_mode(struct qmp_combo *qmp)
> > >  {
> > >  	u32 val;
> > > -	bool reverse = false;
> > > +	bool reverse = qmp->orientation == TYPEC_ORIENTATION_REVERSE;

> > It also looks like these callbacks end up being called without holding
> > the qmp->phy_mutex via phy->power_on(). Perhaps there is no risk for a
> > concurrent switch notification and dp phy power-on but it's not that
> > obvious.

> It seems we're arriving here from hpd_event_thread(), while
> phy_power_on() and phy_power_off() will be called in some other context.
> I've not been able to convince myself if DP driver ensures ordering, or
> if we have an existing race here...

> Unless you insist, I would prefer to follow up with an additional patch
> once we've landed this series. The fix will depend on the phy_mutex
> shuffling patch anyways...

Sure.

But perhaps you can just move the orientation == qmp->orientation check
under the mutex in qmp_combo_typec_switch_set() for now (in case I
forgot to point that out earlier).

Johan
Bjorn Andersson May 4, 2023, 3:16 p.m. UTC | #5 
On Thu, May 04, 2023 at 03:44:53PM +0200, Johan Hovold wrote:
> On Wed, May 03, 2023 at 08:29:07PM -0700, Bjorn Andersson wrote:
> > On Tue, May 02, 2023 at 01:48:16PM +0200, Johan Hovold wrote:
> > > On Mon, Apr 24, 2023 at 08:40:06PM -0700, Bjorn Andersson wrote:
> 
> > > >  static void qmp_v3_dp_aux_init(struct qmp_combo *qmp);
> > > > @@ -1955,29 +1962,23 @@ static void qmp_v3_configure_dp_tx(struct qmp_combo *qmp)
> > > >  static bool qmp_combo_configure_dp_mode(struct qmp_combo *qmp)
> > > >  {
> > > >  	u32 val;
> > > > -	bool reverse = false;
> > > > +	bool reverse = qmp->orientation == TYPEC_ORIENTATION_REVERSE;
> 
> > > It also looks like these callbacks end up being called without holding
> > > the qmp->phy_mutex via phy->power_on(). Perhaps there is no risk for a
> > > concurrent switch notification and dp phy power-on but it's not that
> > > obvious.
> 
> > It seems we're arriving here from hpd_event_thread(), while
> > phy_power_on() and phy_power_off() will be called in some other context.
> > I've not been able to convince myself if DP driver ensures ordering, or
> > if we have an existing race here...
> 
> > Unless you insist, I would prefer to follow up with an additional patch
> > once we've landed this series. The fix will depend on the phy_mutex
> > shuffling patch anyways...
> 
> Sure.
> 
> But perhaps you can just move the orientation == qmp->orientation check
> under the mutex in qmp_combo_typec_switch_set() for now (in case I
> forgot to point that out earlier).
> 

qmp_combo_probe() and qmp_combo_typec_switch_set() are the only writers
to qmp->orientation, so that check can't race with any updates and hence
doesn't need to be protected.

Reading the code again, qmp_combo_configure_dp_mode() is invoked from
phy_power_on(), not the hpd_event_thread(), as I claimed yesterday.

But we shouldn't do qmp_combo_dp_power_on() in parallel with the
reinitialization following a switch in orientation, qmp->orientation
might change, but we definitely would have two contexts reconfiguring
the hardware simultaneously - perhaps this was the cause for the 10%
crashes I hit when trying to extend this to handle typec_mux as well...

I will grab the phy_mux in qmp_combo_configure_dp_mode() as well, thanks
for "insisting" :)

Regards,
Bjorn
Johan Hovold May 4, 2023, 3:41 p.m. UTC | #6 
On Thu, May 04, 2023 at 08:16:33AM -0700, Bjorn Andersson wrote:
> On Thu, May 04, 2023 at 03:44:53PM +0200, Johan Hovold wrote:
> > On Wed, May 03, 2023 at 08:29:07PM -0700, Bjorn Andersson wrote:
> > > On Tue, May 02, 2023 at 01:48:16PM +0200, Johan Hovold wrote:
> > > > On Mon, Apr 24, 2023 at 08:40:06PM -0700, Bjorn Andersson wrote:
> > 
> > > > >  static void qmp_v3_dp_aux_init(struct qmp_combo *qmp);
> > > > > @@ -1955,29 +1962,23 @@ static void qmp_v3_configure_dp_tx(struct qmp_combo *qmp)
> > > > >  static bool qmp_combo_configure_dp_mode(struct qmp_combo *qmp)
> > > > >  {
> > > > >  	u32 val;
> > > > > -	bool reverse = false;
> > > > > +	bool reverse = qmp->orientation == TYPEC_ORIENTATION_REVERSE;
> > 
> > > > It also looks like these callbacks end up being called without holding
> > > > the qmp->phy_mutex via phy->power_on(). Perhaps there is no risk for a
> > > > concurrent switch notification and dp phy power-on but it's not that
> > > > obvious.
> > 
> > > It seems we're arriving here from hpd_event_thread(), while
> > > phy_power_on() and phy_power_off() will be called in some other context.
> > > I've not been able to convince myself if DP driver ensures ordering, or
> > > if we have an existing race here...
> > 
> > > Unless you insist, I would prefer to follow up with an additional patch
> > > once we've landed this series. The fix will depend on the phy_mutex
> > > shuffling patch anyways...
> > 
> > Sure.
> > 
> > But perhaps you can just move the orientation == qmp->orientation check
> > under the mutex in qmp_combo_typec_switch_set() for now (in case I
> > forgot to point that out earlier).
> > 
> 
> qmp_combo_probe() and qmp_combo_typec_switch_set() are the only writers
> to qmp->orientation, so that check can't race with any updates and hence
> doesn't need to be protected.

Only if you happen to know that the callers of
qmp_combo_typec_switch_set() are serialised, right? That happens to be
the case for pmic_glink, but it may not be the case generally.
 
> Reading the code again, qmp_combo_configure_dp_mode() is invoked from
> phy_power_on(), not the hpd_event_thread(), as I claimed yesterday.

Yeah, but phy_power_on() is typically called from that thread. But
perhaps not only from there.

> But we shouldn't do qmp_combo_dp_power_on() in parallel with the
> reinitialization following a switch in orientation, qmp->orientation
> might change, but we definitely would have two contexts reconfiguring
> the hardware simultaneously - perhaps this was the cause for the 10%
> crashes I hit when trying to extend this to handle typec_mux as well...
> 
> I will grab the phy_mux in qmp_combo_configure_dp_mode() as well, thanks
> for "insisting" :)

:)

Johan
diff mbox series

Patch

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index 7280f7141961..6748f31da7a3 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -19,6 +19,7 @@ 
 #include <linux/regulator/consumer.h>
 #include <linux/reset.h>
 #include <linux/slab.h>
+#include <linux/usb/typec.h>
 
 #include <dt-bindings/phy/phy-qcom-qmp.h>
 
@@ -63,6 +64,10 @@ 
 /* QPHY_V3_PCS_MISC_CLAMP_ENABLE register bits */
 #define CLAMP_EN				BIT(0) /* enables i/o clamp_n */
 
+/* QPHY_V3_DP_COM_TYPEC_CTRL register bits */
+#define SW_PORTSELECT_VAL			BIT(0)
+#define SW_PORTSELECT_MUX			BIT(1)
+
 #define PHY_INIT_COMPLETE_TIMEOUT		10000
 
 struct qmp_phy_init_tbl {
@@ -1323,6 +1328,8 @@  struct qmp_combo {
 	struct clk_fixed_rate pipe_clk_fixed;
 	struct clk_hw dp_link_hw;
 	struct clk_hw dp_pixel_hw;
+
+	enum typec_orientation orientation;
 };
 
 static void qmp_v3_dp_aux_init(struct qmp_combo *qmp);
@@ -1955,29 +1962,23 @@  static void qmp_v3_configure_dp_tx(struct qmp_combo *qmp)
 static bool qmp_combo_configure_dp_mode(struct qmp_combo *qmp)
 {
 	u32 val;
-	bool reverse = false;
+	bool reverse = qmp->orientation == TYPEC_ORIENTATION_REVERSE;
+	const struct phy_configure_opts_dp *dp_opts = &qmp->dp_opts;
 
 	val = DP_PHY_PD_CTL_PWRDN | DP_PHY_PD_CTL_AUX_PWRDN |
 	      DP_PHY_PD_CTL_PLL_PWRDN | DP_PHY_PD_CTL_DP_CLAMP_EN;
 
-	/*
-	 * TODO: Assume orientation is CC1 for now and two lanes, need to
-	 * use type-c connector to understand orientation and lanes.
-	 *
-	 * Otherwise val changes to be like below if this code understood
-	 * the orientation of the type-c cable.
-	 *
-	 * if (lane_cnt == 4 || orientation == ORIENTATION_CC2)
-	 *	val |= DP_PHY_PD_CTL_LANE_0_1_PWRDN;
-	 * if (lane_cnt == 4 || orientation == ORIENTATION_CC1)
-	 *	val |= DP_PHY_PD_CTL_LANE_2_3_PWRDN;
-	 * if (orientation == ORIENTATION_CC2)
-	 *	writel(0x4c, qmp->dp_dp_phy + QSERDES_V3_DP_PHY_MODE);
-	 */
-	val |= DP_PHY_PD_CTL_LANE_2_3_PWRDN;
+	if (dp_opts->lanes == 4 || reverse)
+		val |= DP_PHY_PD_CTL_LANE_0_1_PWRDN;
+	if (dp_opts->lanes == 4 || !reverse)
+		val |= DP_PHY_PD_CTL_LANE_2_3_PWRDN;
+
 	writel(val, qmp->dp_dp_phy + QSERDES_DP_PHY_PD_CTL);
 
-	writel(0x5c, qmp->dp_dp_phy + QSERDES_DP_PHY_MODE);
+	if (reverse)
+		writel(0x4c, qmp->pcs + QSERDES_DP_PHY_MODE);
+	else
+		writel(0x5c, qmp->pcs + QSERDES_DP_PHY_MODE);
 
 	return reverse;
 }
@@ -2235,7 +2236,7 @@  static int qmp_v4_configure_dp_phy(struct qmp_combo *qmp)
 {
 	const struct phy_configure_opts_dp *dp_opts = &qmp->dp_opts;
 	u32 bias0_en, drvr0_en, bias1_en, drvr1_en;
-	bool reverse = false;
+	bool reverse = qmp->orientation == TYPEC_ORIENTATION_REVERSE;
 	u32 status;
 	int ret;
 
@@ -2299,7 +2300,7 @@  static int qmp_v5_configure_dp_phy(struct qmp_combo *qmp)
 {
 	const struct phy_configure_opts_dp *dp_opts = &qmp->dp_opts;
 	u32 bias0_en, drvr0_en, bias1_en, drvr1_en;
-	bool reverse = false;
+	bool reverse = qmp->orientation == TYPEC_ORIENTATION_REVERSE;
 	u32 status;
 	int ret;
 
@@ -2358,7 +2359,7 @@  static int qmp_v6_configure_dp_phy(struct qmp_combo *qmp)
 {
 	const struct phy_configure_opts_dp *dp_opts = &qmp->dp_opts;
 	u32 bias0_en, drvr0_en, bias1_en, drvr1_en;
-	bool reverse = false;
+	bool reverse = qmp->orientation == TYPEC_ORIENTATION_REVERSE;
 	u32 status;
 	int ret;
 
@@ -2462,6 +2463,7 @@  static int qmp_combo_com_init(struct qmp_combo *qmp)
 	const struct qmp_phy_cfg *cfg = qmp->cfg;
 	void __iomem *com = qmp->com;
 	int ret;
+	u32 val;
 
 	if (qmp->init_count++)
 		return 0;
@@ -2495,10 +2497,12 @@  static int qmp_combo_com_init(struct qmp_combo *qmp)
 			SW_DPPHY_RESET_MUX | SW_DPPHY_RESET |
 			SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
 
-	/* Default type-c orientation, i.e CC1 */
-	qphy_setbits(com, QPHY_V3_DP_COM_TYPEC_CTRL, 0x02);
-
-	qphy_setbits(com, QPHY_V3_DP_COM_PHY_MODE_CTRL, USB3_MODE | DP_MODE);
+	/* Use software based port select and switch on typec orientation */
+	val = SW_PORTSELECT_MUX;
+	if (qmp->orientation == TYPEC_ORIENTATION_REVERSE)
+		val |= SW_PORTSELECT_VAL;
+	writel(val, com + QPHY_V3_DP_COM_TYPEC_CTRL);
+	writel(USB3_MODE | DP_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
 
 	/* bring both QMP USB and QMP DP PHYs PCS block out of reset */
 	qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
@@ -3361,6 +3365,8 @@  static int qmp_combo_probe(struct platform_device *pdev)
 
 	qmp->dev = dev;
 
+	qmp->orientation = TYPEC_ORIENTATION_NORMAL;
+
 	qmp->cfg = of_device_get_match_data(dev);
 	if (!qmp->cfg)
 		return -EINVAL;