Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <e6703585-660b-d0a2-7804-6c2724b2c095@linaro.org>
Date:   Thu, 27 Apr 2023 15:13:36 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Reply-To: neil.armstrong@linaro.org
Subject: Re: [PATCH 3/7] phy: qcom-qmp-combo: Introduce orientation variable
Content-Language: en-US
To:     Bjorn Andersson <quic_bjorande@quicinc.com>,
        Vinod Koul <vkoul@kernel.org>,
        Kishon Vijay Abraham I <kishon@kernel.org>,
        Johan Hovold <johan@kernel.org>
Cc:     Bjorn Andersson <andersson@kernel.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        linux-arm-msm@vger.kernel.org, linux-phy@lists.infradead.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20230425034010.3789376-1-quic_bjorande@quicinc.com>
 <20230425034010.3789376-4-quic_bjorande@quicinc.com>
From:   Neil Armstrong <neil.armstrong@linaro.org>
Organization: Linaro Developer Services
In-Reply-To: <20230425034010.3789376-4-quic_bjorande@quicinc.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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>