Re: [PATCH] phy: qcom-qmp-combo: correct DP register offsets

From: Johan Hovold <johan@kernel.org>
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: Andy Gross <agross@kernel.org>,
	Bjorn Andersson <andersson@kernel.org>,
	Konrad Dybcio <konrad.dybcio@linaro.org>,
	Vinod Koul <vkoul@kernel.org>,
	Kishon Vijay Abraham I <kishon@kernel.org>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	linux-arm-msm@vger.kernel.org, linux-phy@lists.infradead.org,
	Johan Hovold <johan+linaro@kernel.org>
Subject: Re: [PATCH] phy: qcom-qmp-combo: correct DP register offsets
Date: Mon, 23 Jan 2023 15:00:45 +0100	[thread overview]
Message-ID: <Y86TDYOKtpcLdZYx@hovoldconsulting.com> (raw)
In-Reply-To: <20230123120807.3101313-1-dmitry.baryshkov@linaro.org>

On Mon, Jan 23, 2023 at 02:08:07PM +0200, Dmitry Baryshkov wrote:
> Correct DP register offsets used with new DT bindings scheme. First, DP
> TX registers reside in separate regions, not in the same regions as USB
> TX registers do. Second, correct DP_PHY region offset to follow the
> offset used for earlier generations/bindings.

No, this patch is broken. SC8280XP is different, doesn't seem to have
separate DP TX regions and the DP_PHY registers lies at a different
offset than on previous generations.

You can't just pull these numbers out of your ... ;)

This is the only platform that I can test the DP part on and it is
working. If that happens to be by chance, then you need to blame the
commit adding support for sc8280xp (i.e. not the one that fixed the
binding). And note that this was added by Bjorn who do have access to
the documentation for this SoC (as well as actual hardware).

> Cc: Johan Hovold <johan+linaro@kernel.org>
> Fixes: 83a0bbe39b17 ("phy: qcom-qmp-combo: add support for updated sc8280xp binding")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index 1f022e580407..c6634f92de19 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -807,6 +807,8 @@ struct qmp_combo_offsets {
>  	u16 usb3_pcs;
>  	u16 usb3_pcs_usb;
>  	u16 dp_serdes;
> +	u16 dp_txa;
> +	u16 dp_txb;
>  	u16 dp_dp_phy;
>  };
>  
> @@ -984,7 +986,9 @@ static const struct qmp_combo_offsets qmp_combo_offsets_v5 = {
>  	.usb3_pcs	= 0x1400,
>  	.usb3_pcs_usb	= 0x1700,
>  	.dp_serdes	= 0x2000,
> -	.dp_dp_phy	= 0x2200,
> +	.dp_txa		= 0x2200,
> +	.dp_txa		= 0x2600,

You added dp_txa twice.

> +	.dp_dp_phy	= 0x2a00,
>  };
>  
>  static const struct qmp_phy_cfg sc7180_usb3dpphy_cfg = {
> @@ -2639,8 +2643,8 @@ static int qmp_combo_parse_dt(struct qmp_combo *qmp)
>  	qmp->pcs_usb = base + offs->usb3_pcs_usb;
>  
>  	qmp->dp_serdes = base + offs->dp_serdes;
> -	qmp->dp_tx = base + offs->txa;
> -	qmp->dp_tx2 = base + offs->txb;
> +	qmp->dp_tx = base + offs->dp_txa;
> +	qmp->dp_tx2 = base + offs->dp_txb;
>  	qmp->dp_dp_phy = base + offs->dp_dp_phy;
>  
>  	qmp->pipe_clk = devm_clk_get(dev, "usb3_pipe");

Johan

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy