[1/4] phy: qcom-qmp: Increase the phy init timeout

Message ID 20191219150433.2785427-2-vkoul@kernel.org
State Superseded
Headers show
Series
  • phy: qcom-qmp: Fixes and updates for sm8150
Related show

Commit Message

Vinod Koul Dec. 19, 2019, 3:04 p.m.
If we do full reset of the phy, it seems to take a couple of ms to come
up on my system so increase the timeout to 10ms.

This was found by full reset addition by commit 870b1279c7a0
("scsi: ufs-qcom: Add reset control support for host controller") and
fixes the regression to platforms by this commit.

Suggested-by: Can Guo <cang@codeaurora.org>
Signed-off-by: Vinod Koul <vkoul@kernel.org>

---
 drivers/phy/qualcomm/phy-qcom-qmp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.23.0

Comments

Jeffrey Hugo Dec. 19, 2019, 3:29 p.m. | #1
On Thu, Dec 19, 2019 at 8:04 AM Vinod Koul <vkoul@kernel.org> wrote:
>

> If we do full reset of the phy, it seems to take a couple of ms to come

> up on my system so increase the timeout to 10ms.

>

> This was found by full reset addition by commit 870b1279c7a0

> ("scsi: ufs-qcom: Add reset control support for host controller") and

> fixes the regression to platforms by this commit.

>

> Suggested-by: Can Guo <cang@codeaurora.org>

> Signed-off-by: Vinod Koul <vkoul@kernel.org>


Reviewed-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>

Tested-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>


Tested on the Lenovo Miix 630 laptop (a msm8998 based system).  This
addresses the regression.

> ---

>  drivers/phy/qualcomm/phy-qcom-qmp.c | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

>

> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c

> index 091e20303a14..c2e800a3825a 100644

> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c

> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c

> @@ -66,7 +66,7 @@

>  /* QPHY_V3_PCS_MISC_CLAMP_ENABLE register bits */

>  #define CLAMP_EN                               BIT(0) /* enables i/o clamp_n */

>

> -#define PHY_INIT_COMPLETE_TIMEOUT              1000

> +#define PHY_INIT_COMPLETE_TIMEOUT              100000

>  #define POWER_DOWN_DELAY_US_MIN                        10

>  #define POWER_DOWN_DELAY_US_MAX                        11

>

> --

> 2.23.0

>
Vinod Koul Dec. 19, 2019, 3:40 p.m. | #2
On 19-12-19, 08:29, Jeffrey Hugo wrote:
> On Thu, Dec 19, 2019 at 8:04 AM Vinod Koul <vkoul@kernel.org> wrote:

> >

> > If we do full reset of the phy, it seems to take a couple of ms to come

> > up on my system so increase the timeout to 10ms.

> >

> > This was found by full reset addition by commit 870b1279c7a0

> > ("scsi: ufs-qcom: Add reset control support for host controller") and

> > fixes the regression to platforms by this commit.

> >

> > Suggested-by: Can Guo <cang@codeaurora.org>

> > Signed-off-by: Vinod Koul <vkoul@kernel.org>

> 

> Reviewed-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>

> Tested-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>

> 

> Tested on the Lenovo Miix 630 laptop (a msm8998 based system).  This

> addresses the regression.


Thanks Jeff for quick test and reviews! Appreciate it.

> > ---

> >  drivers/phy/qualcomm/phy-qcom-qmp.c | 2 +-

> >  1 file changed, 1 insertion(+), 1 deletion(-)

> >

> > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c

> > index 091e20303a14..c2e800a3825a 100644

> > --- a/drivers/phy/qualcomm/phy-qcom-qmp.c

> > +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c

> > @@ -66,7 +66,7 @@

> >  /* QPHY_V3_PCS_MISC_CLAMP_ENABLE register bits */

> >  #define CLAMP_EN                               BIT(0) /* enables i/o clamp_n */

> >

> > -#define PHY_INIT_COMPLETE_TIMEOUT              1000

> > +#define PHY_INIT_COMPLETE_TIMEOUT              100000

> >  #define POWER_DOWN_DELAY_US_MIN                        10

> >  #define POWER_DOWN_DELAY_US_MAX                        11

> >

> > --

> > 2.23.0

> >


-- 
~Vinod
Bjorn Andersson Dec. 20, 2019, 2:08 a.m. | #3
On Thu 19 Dec 07:04 PST 2019, Vinod Koul wrote:

> If we do full reset of the phy, it seems to take a couple of ms to come

> up on my system so increase the timeout to 10ms.

> 

> This was found by full reset addition by commit 870b1279c7a0

> ("scsi: ufs-qcom: Add reset control support for host controller") and

> fixes the regression to platforms by this commit.

> 

> Suggested-by: Can Guo <cang@codeaurora.org>

> Signed-off-by: Vinod Koul <vkoul@kernel.org>


This does look familiar...

https://lore.kernel.org/linux-arm-msm/20191107000917.1092409-3-bjorn.andersson@linaro.org/

> ---

>  drivers/phy/qualcomm/phy-qcom-qmp.c | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

> 

> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c

> index 091e20303a14..c2e800a3825a 100644

> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c

> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c

> @@ -66,7 +66,7 @@

>  /* QPHY_V3_PCS_MISC_CLAMP_ENABLE register bits */

>  #define CLAMP_EN				BIT(0) /* enables i/o clamp_n */

>  

> -#define PHY_INIT_COMPLETE_TIMEOUT		1000

> +#define PHY_INIT_COMPLETE_TIMEOUT		100000


100ms seems a little bit excessive, and we do end up waiting this long
when we have PCIe links without an attached device...

Do you need >10ms or could we just have my patch merged?

Regards,
Bjorn

>  #define POWER_DOWN_DELAY_US_MIN			10

>  #define POWER_DOWN_DELAY_US_MAX			11

>  

> -- 

> 2.23.0

>
Vinod Koul Dec. 20, 2019, 4:21 a.m. | #4
On 19-12-19, 18:08, Bjorn Andersson wrote:
> On Thu 19 Dec 07:04 PST 2019, Vinod Koul wrote:

> 

> > If we do full reset of the phy, it seems to take a couple of ms to come

> > up on my system so increase the timeout to 10ms.

> > 

> > This was found by full reset addition by commit 870b1279c7a0

> > ("scsi: ufs-qcom: Add reset control support for host controller") and

> > fixes the regression to platforms by this commit.

> > 

> > Suggested-by: Can Guo <cang@codeaurora.org>

> > Signed-off-by: Vinod Koul <vkoul@kernel.org>

> 

> This does look familiar...

> 

> https://lore.kernel.org/linux-arm-msm/20191107000917.1092409-3-bjorn.andersson@linaro.org/

> 

> > ---

> >  drivers/phy/qualcomm/phy-qcom-qmp.c | 2 +-

> >  1 file changed, 1 insertion(+), 1 deletion(-)

> > 

> > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c

> > index 091e20303a14..c2e800a3825a 100644

> > --- a/drivers/phy/qualcomm/phy-qcom-qmp.c

> > +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c

> > @@ -66,7 +66,7 @@

> >  /* QPHY_V3_PCS_MISC_CLAMP_ENABLE register bits */

> >  #define CLAMP_EN				BIT(0) /* enables i/o clamp_n */

> >  

> > -#define PHY_INIT_COMPLETE_TIMEOUT		1000

> > +#define PHY_INIT_COMPLETE_TIMEOUT		100000

> 

> 100ms seems a little bit excessive, and we do end up waiting this long

> when we have PCIe links without an attached device...

> 

> Do you need >10ms or could we just have my patch merged?


Yeah I quick tested 10ms as well and seems good for me, so either ways
if fine, but lets get it applied quickly :-)

-- 
~Vinod

Patch

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c
index 091e20303a14..c2e800a3825a 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
@@ -66,7 +66,7 @@ 
 /* QPHY_V3_PCS_MISC_CLAMP_ENABLE register bits */
 #define CLAMP_EN				BIT(0) /* enables i/o clamp_n */
 
-#define PHY_INIT_COMPLETE_TIMEOUT		1000
+#define PHY_INIT_COMPLETE_TIMEOUT		100000
 #define POWER_DOWN_DELAY_US_MIN			10
 #define POWER_DOWN_DELAY_US_MAX			11