Linux-ARM-MSM Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] phy: qcom-qmp: Correct ready status, again
@ 2019-08-06  0:42 Bjorn Andersson
  2019-08-20 14:07 ` Marc Gonzalez
       [not found] ` <20190806155040.0B54520C01@mail.kernel.org>
  0 siblings, 2 replies; 4+ messages in thread
From: Bjorn Andersson @ 2019-08-06  0:42 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Marc Gonzalez
  Cc: linux-arm-msm, linux-kernel, stable, Vivek Gautam, Evan Green,
	Niklas Cassel

Despite extensive testing of 885bd765963b ("phy: qcom-qmp: Correct
READY_STATUS poll break condition") I failed to conclude that the
PHYSTATUS bit of the PCS_STATUS register used in PCIe and USB3 falls as
the PHY gets ready. Similar to the prior bug with UFS the code will
generally get past the check before the transition and thereby
"succeed".

Correct the name of the register used PCIe and USB3 PHYs, replace
mask_pcs_ready with a constant expression depending on the type of the
PHY and check for the appropriate ready state.

Cc: stable@vger.kernel.org
Cc: Vivek Gautam <vivek.gautam@codeaurora.org>
Cc: Evan Green <evgreen@chromium.org>
Cc: Niklas Cassel <niklas.cassel@linaro.org>
Reported-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
Fixes: 885bd765963b ("phy: qcom-qmp: Correct READY_STATUS poll break condition")
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/phy/qualcomm/phy-qcom-qmp.c | 33 ++++++++++++++---------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c
index 34ff6434da8f..6bb49cc25c63 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
@@ -35,7 +35,7 @@
 #define PLL_READY_GATE_EN			BIT(3)
 /* QPHY_PCS_STATUS bit */
 #define PHYSTATUS				BIT(6)
-/* QPHY_COM_PCS_READY_STATUS bit */
+/* QPHY_PCS_READY_STATUS & QPHY_COM_PCS_READY_STATUS bit */
 #define PCS_READY				BIT(0)
 
 /* QPHY_V3_DP_COM_RESET_OVRD_CTRL register bits */
@@ -115,6 +115,7 @@ enum qphy_reg_layout {
 	QPHY_SW_RESET,
 	QPHY_START_CTRL,
 	QPHY_PCS_READY_STATUS,
+	QPHY_PCS_STATUS,
 	QPHY_PCS_AUTONOMOUS_MODE_CTRL,
 	QPHY_PCS_LFPS_RXTERM_IRQ_CLEAR,
 	QPHY_PCS_LFPS_RXTERM_IRQ_STATUS,
@@ -133,7 +134,7 @@ static const unsigned int pciephy_regs_layout[] = {
 	[QPHY_FLL_MAN_CODE]		= 0xd4,
 	[QPHY_SW_RESET]			= 0x00,
 	[QPHY_START_CTRL]		= 0x08,
-	[QPHY_PCS_READY_STATUS]		= 0x174,
+	[QPHY_PCS_STATUS]		= 0x174,
 };
 
 static const unsigned int usb3phy_regs_layout[] = {
@@ -144,7 +145,7 @@ static const unsigned int usb3phy_regs_layout[] = {
 	[QPHY_FLL_MAN_CODE]		= 0xd0,
 	[QPHY_SW_RESET]			= 0x00,
 	[QPHY_START_CTRL]		= 0x08,
-	[QPHY_PCS_READY_STATUS]		= 0x17c,
+	[QPHY_PCS_STATUS]		= 0x17c,
 	[QPHY_PCS_AUTONOMOUS_MODE_CTRL]	= 0x0d4,
 	[QPHY_PCS_LFPS_RXTERM_IRQ_CLEAR]  = 0x0d8,
 	[QPHY_PCS_LFPS_RXTERM_IRQ_STATUS] = 0x178,
@@ -153,7 +154,7 @@ static const unsigned int usb3phy_regs_layout[] = {
 static const unsigned int qmp_v3_usb3phy_regs_layout[] = {
 	[QPHY_SW_RESET]			= 0x00,
 	[QPHY_START_CTRL]		= 0x08,
-	[QPHY_PCS_READY_STATUS]		= 0x174,
+	[QPHY_PCS_STATUS]		= 0x174,
 	[QPHY_PCS_AUTONOMOUS_MODE_CTRL]	= 0x0d8,
 	[QPHY_PCS_LFPS_RXTERM_IRQ_CLEAR]  = 0x0dc,
 	[QPHY_PCS_LFPS_RXTERM_IRQ_STATUS] = 0x170,
@@ -911,7 +912,6 @@ struct qmp_phy_cfg {
 
 	unsigned int start_ctrl;
 	unsigned int pwrdn_ctrl;
-	unsigned int mask_pcs_ready;
 	unsigned int mask_com_pcs_ready;
 
 	/* true, if PHY has a separate PHY_COM control block */
@@ -1074,7 +1074,6 @@ static const struct qmp_phy_cfg msm8996_pciephy_cfg = {
 
 	.start_ctrl		= PCS_START | PLL_READY_GATE_EN,
 	.pwrdn_ctrl		= SW_PWRDN | REFCLK_DRV_DSBL,
-	.mask_pcs_ready		= PHYSTATUS,
 	.mask_com_pcs_ready	= PCS_READY,
 
 	.has_phy_com_ctrl	= true,
@@ -1106,7 +1105,6 @@ static const struct qmp_phy_cfg msm8996_usb3phy_cfg = {
 
 	.start_ctrl		= SERDES_START | PCS_START,
 	.pwrdn_ctrl		= SW_PWRDN,
-	.mask_pcs_ready		= PHYSTATUS,
 };
 
 /* list of resets */
@@ -1136,7 +1134,6 @@ static const struct qmp_phy_cfg ipq8074_pciephy_cfg = {
 
 	.start_ctrl		= SERDES_START | PCS_START,
 	.pwrdn_ctrl		= SW_PWRDN | REFCLK_DRV_DSBL,
-	.mask_pcs_ready		= PHYSTATUS,
 
 	.has_phy_com_ctrl	= false,
 	.has_lane_rst		= false,
@@ -1167,7 +1164,6 @@ static const struct qmp_phy_cfg qmp_v3_usb3phy_cfg = {
 
 	.start_ctrl		= SERDES_START | PCS_START,
 	.pwrdn_ctrl		= SW_PWRDN,
-	.mask_pcs_ready		= PHYSTATUS,
 
 	.has_pwrdn_delay	= true,
 	.pwrdn_delay_min	= POWER_DOWN_DELAY_US_MIN,
@@ -1199,7 +1195,6 @@ static const struct qmp_phy_cfg qmp_v3_usb3_uniphy_cfg = {
 
 	.start_ctrl		= SERDES_START | PCS_START,
 	.pwrdn_ctrl		= SW_PWRDN,
-	.mask_pcs_ready		= PHYSTATUS,
 
 	.has_pwrdn_delay	= true,
 	.pwrdn_delay_min	= POWER_DOWN_DELAY_US_MIN,
@@ -1226,7 +1221,6 @@ static const struct qmp_phy_cfg sdm845_ufsphy_cfg = {
 
 	.start_ctrl		= SERDES_START,
 	.pwrdn_ctrl		= SW_PWRDN,
-	.mask_pcs_ready		= PCS_READY,
 
 	.is_dual_lane_phy	= true,
 	.no_pcs_sw_reset	= true,
@@ -1254,7 +1248,6 @@ static const struct qmp_phy_cfg msm8998_pciephy_cfg = {
 
 	.start_ctrl             = SERDES_START | PCS_START,
 	.pwrdn_ctrl		= SW_PWRDN | REFCLK_DRV_DSBL,
-	.mask_pcs_ready		= PHYSTATUS,
 };
 
 static const struct qmp_phy_cfg msm8998_usb3phy_cfg = {
@@ -1279,7 +1272,6 @@ static const struct qmp_phy_cfg msm8998_usb3phy_cfg = {
 
 	.start_ctrl             = SERDES_START | PCS_START,
 	.pwrdn_ctrl             = SW_PWRDN,
-	.mask_pcs_ready         = PHYSTATUS,
 
 	.is_dual_lane_phy       = true,
 };
@@ -1457,7 +1449,7 @@ static int qcom_qmp_phy_enable(struct phy *phy)
 	void __iomem *pcs = qphy->pcs;
 	void __iomem *dp_com = qmp->dp_com;
 	void __iomem *status;
-	unsigned int mask, val;
+	unsigned int mask, val, ready;
 	int ret;
 
 	dev_vdbg(qmp->dev, "Initializing QMP phy\n");
@@ -1545,10 +1537,17 @@ static int qcom_qmp_phy_enable(struct phy *phy)
 	/* start SerDes and Phy-Coding-Sublayer */
 	qphy_setbits(pcs, cfg->regs[QPHY_START_CTRL], cfg->start_ctrl);
 
-	status = pcs + cfg->regs[QPHY_PCS_READY_STATUS];
-	mask = cfg->mask_pcs_ready;
+	if (cfg->type == PHY_TYPE_UFS) {
+		status = pcs + cfg->regs[QPHY_PCS_READY_STATUS];
+		mask = PCS_READY;
+		ready = PCS_READY;
+	} else {
+		status = pcs + cfg->regs[QPHY_PCS_STATUS];
+		mask = PHYSTATUS;
+		ready = 0;
+	}
 
-	ret = readl_poll_timeout(status, val, val & mask, 10,
+	ret = readl_poll_timeout(status, val, (val & mask) == ready, 10,
 				 PHY_INIT_COMPLETE_TIMEOUT);
 	if (ret) {
 		dev_err(qmp->dev, "phy initialization timed-out\n");
-- 
2.18.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] phy: qcom-qmp: Correct ready status, again
  2019-08-06  0:42 [PATCH] phy: qcom-qmp: Correct ready status, again Bjorn Andersson
@ 2019-08-20 14:07 ` Marc Gonzalez
       [not found] ` <20190806155040.0B54520C01@mail.kernel.org>
  1 sibling, 0 replies; 4+ messages in thread
From: Marc Gonzalez @ 2019-08-20 14:07 UTC (permalink / raw)
  To: Bjorn Andersson, Kishon Vijay Abraham I
  Cc: MSM, LKML, stable, Vivek Gautam, Evan Green, Niklas Cassel

On 06/08/2019 02:42, Bjorn Andersson wrote:

> Despite extensive testing of 885bd765963b ("phy: qcom-qmp: Correct
> READY_STATUS poll break condition") I failed to conclude that the
> PHYSTATUS bit of the PCS_STATUS register used in PCIe and USB3 falls as
> the PHY gets ready. Similar to the prior bug with UFS the code will
> generally get past the check before the transition and thereby
> "succeed".
> 
> Correct the name of the register used PCIe and USB3 PHYs, replace
> mask_pcs_ready with a constant expression depending on the type of the
> PHY and check for the appropriate ready state.
> 
> Cc: stable@vger.kernel.org
> Cc: Vivek Gautam <vivek.gautam@codeaurora.org>
> Cc: Evan Green <evgreen@chromium.org>
> Cc: Niklas Cassel <niklas.cassel@linaro.org>
> Reported-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
> Fixes: 885bd765963b ("phy: qcom-qmp: Correct READY_STATUS poll break condition")
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp.c | 33 ++++++++++++++---------------
>  1 file changed, 16 insertions(+), 17 deletions(-)

FWIW, for msm8998:

Tested-by: Marc Gonzalez <marc.w.gonzalez@free.fr>

Regards.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] phy: qcom-qmp: Correct ready status, again
       [not found] ` <20190806155040.0B54520C01@mail.kernel.org>
@ 2019-08-21  0:23   ` Kishon Vijay Abraham I
  2019-08-26 15:15     ` Bjorn Andersson
  0 siblings, 1 reply; 4+ messages in thread
From: Kishon Vijay Abraham I @ 2019-08-21  0:23 UTC (permalink / raw)
  To: Sasha Levin, Bjorn Andersson
  Cc: linux-arm-msm, linux-kernel, stable, Vivek Gautam, Evan Green,
	Niklas Cassel

Hi Sasha,

On 06/08/19 9:20 PM, Sasha Levin wrote:
> Hi,
> 
> [This is an automated email]
> 
> This commit has been processed because it contains a "Fixes:" tag,
> fixing commit: 885bd765963b phy: qcom-qmp: Correct READY_STATUS poll break condition.
> 
> The bot has tested the following trees: v5.2.6.
> 
> v5.2.6: Failed to apply! Possible dependencies:
>     520602640419 ("phy: qcom-qmp: Raise qcom_qmp_phy_enable() polling delay")
> 
> 
> NOTE: The patch will not be queued to stable trees until it is upstream.
> 
> How should we proceed with this patch?

Merging of this patch got delayed. Bjorn, Is it okay if this patch gets merged
in the next merge window and backported to stable releases then?

Thanks
Kishon

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] phy: qcom-qmp: Correct ready status, again
  2019-08-21  0:23   ` Kishon Vijay Abraham I
@ 2019-08-26 15:15     ` Bjorn Andersson
  0 siblings, 0 replies; 4+ messages in thread
From: Bjorn Andersson @ 2019-08-26 15:15 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Sasha Levin, linux-arm-msm, linux-kernel, stable, Vivek Gautam,
	Evan Green, Niklas Cassel

On Tue 20 Aug 17:23 PDT 2019, Kishon Vijay Abraham I wrote:

> Hi Sasha,
> 
> On 06/08/19 9:20 PM, Sasha Levin wrote:
> > Hi,
> > 
> > [This is an automated email]
> > 
> > This commit has been processed because it contains a "Fixes:" tag,
> > fixing commit: 885bd765963b phy: qcom-qmp: Correct READY_STATUS poll break condition.
> > 
> > The bot has tested the following trees: v5.2.6.
> > 
> > v5.2.6: Failed to apply! Possible dependencies:
> >     520602640419 ("phy: qcom-qmp: Raise qcom_qmp_phy_enable() polling delay")
> > 
> > 
> > NOTE: The patch will not be queued to stable trees until it is upstream.
> > 
> > How should we proceed with this patch?
> 
> Merging of this patch got delayed. Bjorn, Is it okay if this patch gets merged
> in the next merge window and backported to stable releases then?
> 

That's fine, thanks for picking it up Kishon

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-06  0:42 [PATCH] phy: qcom-qmp: Correct ready status, again Bjorn Andersson
2019-08-20 14:07 ` Marc Gonzalez
     [not found] ` <20190806155040.0B54520C01@mail.kernel.org>
2019-08-21  0:23   ` Kishon Vijay Abraham I
2019-08-26 15:15     ` Bjorn Andersson

Linux-ARM-MSM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-msm/0 linux-arm-msm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-msm linux-arm-msm/ https://lore.kernel.org/linux-arm-msm \
		linux-arm-msm@vger.kernel.org linux-arm-msm@archiver.kernel.org
	public-inbox-index linux-arm-msm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-arm-msm


AGPL code for this site: git clone https://public-inbox.org/ public-inbox