[PATCH v2 0/2] Fix PLL lock timeout and calibration wait time

[PATCH v2 0/2] Fix PLL lock timeout and calibration wait time

[Devarsh Thakkar]

This fixes PLL lockup and O_CMN_READY timeout by moving the polling
function after common state machine gets enabled. Also fix the
calibration wait time to optimize the polling time.

Changelog:
V1: https://lore.kernel.org/all/20241230125319.941372-1-devarsht@ti.com/

Test logs:
Link: https://gist.github.com/devarsht/89e4830e886774fcd50aa6e29cce3a79

Devarsh Thakkar (2):
  phy: cadence: cdns-dphy: Fix PLL lock and O_CMN_READY polling
  phy: cadence: cdns-dphy: Update calibration wait time for startup
    state machine

 drivers/phy/cadence/cdns-dphy.c | 60 ++++++++++++++++++++++++++++-----
 1 file changed, 52 insertions(+), 8 deletions(-)

-- 
2.39.1

[PATCH v2 1/2] phy: cadence: cdns-dphy: Fix PLL lock and O_CMN_READY polling

[Devarsh Thakkar]

PLL lockup and O_CMN_READY assertion can only happen after common state
machine gets enabled, but driver was polling them before the common state
machine was enabled. To fix this, add new function callbacks polling on PLL
lock and O_CMN_READY assertion and call them only after common state
machine gets enabled.

Cc: stable@vger.kernel.org
Fixes: 7a343c8bf4b5 ("phy: Add Cadence D-PHY support")
Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
---
V2: 
- Return error code on polling timeout
- Moved out calibration logic to separate patch

 drivers/phy/cadence/cdns-dphy.c | 57 ++++++++++++++++++++++++++++-----
 1 file changed, 49 insertions(+), 8 deletions(-)

diff --git a/drivers/phy/cadence/cdns-dphy.c b/drivers/phy/cadence/cdns-dphy.c
index ed87a3970f83..c4de9e4d3e93 100644
--- a/drivers/phy/cadence/cdns-dphy.c
+++ b/drivers/phy/cadence/cdns-dphy.c
@@ -99,6 +99,8 @@ struct cdns_dphy_ops {
 	void (*set_pll_cfg)(struct cdns_dphy *dphy,
 			    const struct cdns_dphy_cfg *cfg);
 	unsigned long (*get_wakeup_time_ns)(struct cdns_dphy *dphy);
+	int (*wait_for_pll_lock)(struct cdns_dphy *dphy);
+	int (*wait_for_cmn_ready)(struct cdns_dphy *dphy);
 };
 
 struct cdns_dphy {
@@ -191,6 +193,26 @@ static unsigned long cdns_dphy_get_wakeup_time_ns(struct cdns_dphy *dphy)
 	return dphy->ops->get_wakeup_time_ns(dphy);
 }
 
+static int cdns_dphy_wait_for_pll_lock(struct cdns_dphy *dphy)
+{
+	int ret = 0;
+
+	if (dphy->ops->wait_for_pll_lock)
+		ret = dphy->ops->wait_for_pll_lock(dphy);
+
+	return ret;
+}
+
+static int cdns_dphy_wait_for_cmn_ready(struct cdns_dphy *dphy)
+{
+	int ret = 0;
+
+	if (dphy->ops->wait_for_cmn_ready)
+		ret = dphy->ops->wait_for_cmn_ready(dphy);
+
+	return ret;
+}
+
 static unsigned long cdns_dphy_ref_get_wakeup_time_ns(struct cdns_dphy *dphy)
 {
 	/* Default wakeup time is 800 ns (in a simulated environment). */
@@ -232,7 +254,6 @@ static unsigned long cdns_dphy_j721e_get_wakeup_time_ns(struct cdns_dphy *dphy)
 static void cdns_dphy_j721e_set_pll_cfg(struct cdns_dphy *dphy,
 					const struct cdns_dphy_cfg *cfg)
 {
-	u32 status;
 
 	/*
 	 * set the PWM and PLL Byteclk divider settings to recommended values
@@ -249,13 +270,6 @@ static void cdns_dphy_j721e_set_pll_cfg(struct cdns_dphy *dphy,
 
 	writel(DPHY_TX_J721E_WIZ_LANE_RSTB,
 	       dphy->regs + DPHY_TX_J721E_WIZ_RST_CTRL);
-
-	readl_poll_timeout(dphy->regs + DPHY_TX_J721E_WIZ_PLL_CTRL, status,
-			   (status & DPHY_TX_WIZ_PLL_LOCK), 0, POLL_TIMEOUT_US);
-
-	readl_poll_timeout(dphy->regs + DPHY_TX_J721E_WIZ_STATUS, status,
-			   (status & DPHY_TX_WIZ_O_CMN_READY), 0,
-			   POLL_TIMEOUT_US);
 }
 
 static void cdns_dphy_j721e_set_psm_div(struct cdns_dphy *dphy, u8 div)
@@ -263,6 +277,23 @@ static void cdns_dphy_j721e_set_psm_div(struct cdns_dphy *dphy, u8 div)
 	writel(div, dphy->regs + DPHY_TX_J721E_WIZ_PSM_FREQ);
 }
 
+static int cdns_dphy_j721e_wait_for_pll_lock(struct cdns_dphy *dphy)
+{
+	u32 status;
+
+	return readl_poll_timeout(dphy->regs + DPHY_TX_J721E_WIZ_PLL_CTRL, status,
+			       status & DPHY_TX_WIZ_PLL_LOCK, 0, POLL_TIMEOUT_US);
+}
+
+static int cdns_dphy_j721e_wait_for_cmn_ready(struct cdns_dphy *dphy)
+{
+	u32 status;
+
+	return readl_poll_timeout(dphy->regs + DPHY_TX_J721E_WIZ_STATUS, status,
+			       status & DPHY_TX_WIZ_O_CMN_READY, 0,
+			       POLL_TIMEOUT_US);
+}
+
 /*
  * This is the reference implementation of DPHY hooks. Specific integration of
  * this IP may have to re-implement some of them depending on how they decided
@@ -278,6 +309,8 @@ static const struct cdns_dphy_ops j721e_dphy_ops = {
 	.get_wakeup_time_ns = cdns_dphy_j721e_get_wakeup_time_ns,
 	.set_pll_cfg = cdns_dphy_j721e_set_pll_cfg,
 	.set_psm_div = cdns_dphy_j721e_set_psm_div,
+	.wait_for_pll_lock = cdns_dphy_j721e_wait_for_pll_lock,
+	.wait_for_cmn_ready = cdns_dphy_j721e_wait_for_cmn_ready,
 };
 
 static int cdns_dphy_config_from_opts(struct phy *phy,
@@ -373,6 +406,14 @@ static int cdns_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
 	      FIELD_PREP(DPHY_BAND_CFG_RIGHT_BAND, band_ctrl);
 	writel(reg, dphy->regs + DPHY_BAND_CFG);
 
+	ret = cdns_dphy_wait_for_pll_lock(dphy);
+	if (ret)
+		dev_err(&dphy->phy->dev, "Failed to lock PLL with err %d\n", ret);
+
+	ret = cdns_dphy_wait_for_cmn_ready(dphy);
+	if (ret)
+		dev_err(&dphy->phy->dev, "O_CMN_READY signal failed to assert with err %d\n", ret);
+
 	return 0;
 }
 
-- 
2.39.1

Re: [PATCH v2 1/2] phy: cadence: cdns-dphy: Fix PLL lock and O_CMN_READY polling

[Tomi Valkeinen]

Hi,

On 26/03/2025 17:23, Devarsh Thakkar wrote:
> PLL lockup and O_CMN_READY assertion can only happen after common state
> machine gets enabled, but driver was polling them before the common state
> machine was enabled. To fix this, add new function callbacks polling on PLL
> lock and O_CMN_READY assertion and call them only after common state
> machine gets enabled.
> 
> Cc: stable@vger.kernel.org
> Fixes: 7a343c8bf4b5 ("phy: Add Cadence D-PHY support")
> Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
> ---
> V2:
> - Return error code on polling timeout
> - Moved out calibration logic to separate patch
> 
>   drivers/phy/cadence/cdns-dphy.c | 57 ++++++++++++++++++++++++++++-----
>   1 file changed, 49 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/phy/cadence/cdns-dphy.c b/drivers/phy/cadence/cdns-dphy.c
> index ed87a3970f83..c4de9e4d3e93 100644
> --- a/drivers/phy/cadence/cdns-dphy.c
> +++ b/drivers/phy/cadence/cdns-dphy.c
> @@ -99,6 +99,8 @@ struct cdns_dphy_ops {
>   	void (*set_pll_cfg)(struct cdns_dphy *dphy,
>   			    const struct cdns_dphy_cfg *cfg);
>   	unsigned long (*get_wakeup_time_ns)(struct cdns_dphy *dphy);
> +	int (*wait_for_pll_lock)(struct cdns_dphy *dphy);
> +	int (*wait_for_cmn_ready)(struct cdns_dphy *dphy);
>   };
>   
>   struct cdns_dphy {
> @@ -191,6 +193,26 @@ static unsigned long cdns_dphy_get_wakeup_time_ns(struct cdns_dphy *dphy)
>   	return dphy->ops->get_wakeup_time_ns(dphy);
>   }
>   
> +static int cdns_dphy_wait_for_pll_lock(struct cdns_dphy *dphy)
> +{
> +	int ret = 0;
> +
> +	if (dphy->ops->wait_for_pll_lock)
> +		ret = dphy->ops->wait_for_pll_lock(dphy);
> +
> +	return ret;
> +}
> +
> +static int cdns_dphy_wait_for_cmn_ready(struct cdns_dphy *dphy)
> +{
> +	int ret = 0;
> +
> +	if (dphy->ops->wait_for_cmn_ready)
> +		ret = dphy->ops->wait_for_cmn_ready(dphy);
> +
> +	return ret;
> +}
> +
>   static unsigned long cdns_dphy_ref_get_wakeup_time_ns(struct cdns_dphy *dphy)
>   {
>   	/* Default wakeup time is 800 ns (in a simulated environment). */
> @@ -232,7 +254,6 @@ static unsigned long cdns_dphy_j721e_get_wakeup_time_ns(struct cdns_dphy *dphy)
>   static void cdns_dphy_j721e_set_pll_cfg(struct cdns_dphy *dphy,
>   					const struct cdns_dphy_cfg *cfg)
>   {
> -	u32 status;
>   
>   	/*
>   	 * set the PWM and PLL Byteclk divider settings to recommended values
> @@ -249,13 +270,6 @@ static void cdns_dphy_j721e_set_pll_cfg(struct cdns_dphy *dphy,
>   
>   	writel(DPHY_TX_J721E_WIZ_LANE_RSTB,
>   	       dphy->regs + DPHY_TX_J721E_WIZ_RST_CTRL);
> -
> -	readl_poll_timeout(dphy->regs + DPHY_TX_J721E_WIZ_PLL_CTRL, status,
> -			   (status & DPHY_TX_WIZ_PLL_LOCK), 0, POLL_TIMEOUT_US);
> -
> -	readl_poll_timeout(dphy->regs + DPHY_TX_J721E_WIZ_STATUS, status,
> -			   (status & DPHY_TX_WIZ_O_CMN_READY), 0,
> -			   POLL_TIMEOUT_US);
>   }
>   
>   static void cdns_dphy_j721e_set_psm_div(struct cdns_dphy *dphy, u8 div)
> @@ -263,6 +277,23 @@ static void cdns_dphy_j721e_set_psm_div(struct cdns_dphy *dphy, u8 div)
>   	writel(div, dphy->regs + DPHY_TX_J721E_WIZ_PSM_FREQ);
>   }
>   
> +static int cdns_dphy_j721e_wait_for_pll_lock(struct cdns_dphy *dphy)
> +{
> +	u32 status;
> +
> +	return readl_poll_timeout(dphy->regs + DPHY_TX_J721E_WIZ_PLL_CTRL, status,
> +			       status & DPHY_TX_WIZ_PLL_LOCK, 0, POLL_TIMEOUT_US);
> +}
> +
> +static int cdns_dphy_j721e_wait_for_cmn_ready(struct cdns_dphy *dphy)
> +{
> +	u32 status;
> +
> +	return readl_poll_timeout(dphy->regs + DPHY_TX_J721E_WIZ_STATUS, status,
> +			       status & DPHY_TX_WIZ_O_CMN_READY, 0,
> +			       POLL_TIMEOUT_US);
> +}
> +
>   /*
>    * This is the reference implementation of DPHY hooks. Specific integration of
>    * this IP may have to re-implement some of them depending on how they decided
> @@ -278,6 +309,8 @@ static const struct cdns_dphy_ops j721e_dphy_ops = {
>   	.get_wakeup_time_ns = cdns_dphy_j721e_get_wakeup_time_ns,
>   	.set_pll_cfg = cdns_dphy_j721e_set_pll_cfg,
>   	.set_psm_div = cdns_dphy_j721e_set_psm_div,
> +	.wait_for_pll_lock = cdns_dphy_j721e_wait_for_pll_lock,
> +	.wait_for_cmn_ready = cdns_dphy_j721e_wait_for_cmn_ready,
>   };
>   
>   static int cdns_dphy_config_from_opts(struct phy *phy,
> @@ -373,6 +406,14 @@ static int cdns_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
>   	      FIELD_PREP(DPHY_BAND_CFG_RIGHT_BAND, band_ctrl);
>   	writel(reg, dphy->regs + DPHY_BAND_CFG);
>   
> +	ret = cdns_dphy_wait_for_pll_lock(dphy);
> +	if (ret)
> +		dev_err(&dphy->phy->dev, "Failed to lock PLL with err %d\n", ret);
> +
> +	ret = cdns_dphy_wait_for_cmn_ready(dphy);
> +	if (ret)
> +		dev_err(&dphy->phy->dev, "O_CMN_READY signal failed to assert with err %d\n", ret);
> +

Shouldn't these return an error? Or what's the reason these are ok (and 
if so, should the prints be dev_dbg?)

  Tomi

>   	return 0;
>   }
>   

Re: [PATCH v2 1/2] phy: cadence: cdns-dphy: Fix PLL lock and O_CMN_READY polling

[Devarsh Thakkar]

Hi Tomi,

Thanks for the review.

On 02/04/25 17:25, Tomi Valkeinen wrote:
<snip>
>>   /*
>>    * This is the reference implementation of DPHY hooks. Specific 
>> integration of
>>    * this IP may have to re-implement some of them depending on how 
>> they decided
>> @@ -278,6 +309,8 @@ static const struct cdns_dphy_ops j721e_dphy_ops = {
>>       .get_wakeup_time_ns = cdns_dphy_j721e_get_wakeup_time_ns,
>>       .set_pll_cfg = cdns_dphy_j721e_set_pll_cfg,
>>       .set_psm_div = cdns_dphy_j721e_set_psm_div,
>> +    .wait_for_pll_lock = cdns_dphy_j721e_wait_for_pll_lock,
>> +    .wait_for_cmn_ready = cdns_dphy_j721e_wait_for_cmn_ready,
>>   };
>>   static int cdns_dphy_config_from_opts(struct phy *phy,
>> @@ -373,6 +406,14 @@ static int cdns_dphy_configure(struct phy *phy, 
>> union phy_configure_opts *opts)
>>             FIELD_PREP(DPHY_BAND_CFG_RIGHT_BAND, band_ctrl);
>>       writel(reg, dphy->regs + DPHY_BAND_CFG);
>> +    ret = cdns_dphy_wait_for_pll_lock(dphy);
>> +    if (ret)
>> +        dev_err(&dphy->phy->dev, "Failed to lock PLL with err %d\n", 
>> ret);
>> +
>> +    ret = cdns_dphy_wait_for_cmn_ready(dphy);
>> +    if (ret)
>> +        dev_err(&dphy->phy->dev, "O_CMN_READY signal failed to assert 
>> with err %d\n", ret);
>> +
> 
> Shouldn't these return an error? Or what's the reason these are ok (and 
> if so, should the prints be dev_dbg?)
> 

Yes, I think it would be better if we can return error and fail if 
timeout happen. I will fix it in next revision.

Regards
Devarsh

>   Tomi
> 
>>       return 0;
>>   }
> 

[PATCH v2 2/2] phy: cadence: cdns-dphy: Update calibration wait time for startup state machine

[Devarsh Thakkar]

Use system characterized reset value specified in TRM [1] to program
calibration wait time which defines number of cycles to wait for after
startup state machine is in bandgap enable state.

This fixes PLL lock timeout error faced while using RPi DSI Panel on TI's
AM62L and J721E SoC [2].

[1] AM62P TRM (Section ):
https://www.ti.com/lit/pdf/spruj83

[2]:
Link: https://gist.github.com/devarsht/89e4830e886774fcd50aa6e29cce3a79

Cc: stable@vger.kernel.org
Fixes: 7a343c8bf4b5 ("phy: Add Cadence D-PHY support")
Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
---
V2: Introduced this as as separate patch

 drivers/phy/cadence/cdns-dphy.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/phy/cadence/cdns-dphy.c b/drivers/phy/cadence/cdns-dphy.c
index c4de9e4d3e93..11fbffe5aafd 100644
--- a/drivers/phy/cadence/cdns-dphy.c
+++ b/drivers/phy/cadence/cdns-dphy.c
@@ -30,6 +30,7 @@
 
 #define DPHY_CMN_SSM			DPHY_PMA_CMN(0x20)
 #define DPHY_CMN_SSM_EN			BIT(0)
+#define DPHY_CMN_SSM_CAL_WAIT_TIME	GENMASK(8, 1)
 #define DPHY_CMN_TX_MODE_EN		BIT(9)
 
 #define DPHY_CMN_PWM			DPHY_PMA_CMN(0x40)
@@ -405,6 +406,8 @@ static int cdns_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
 	reg = FIELD_PREP(DPHY_BAND_CFG_LEFT_BAND, band_ctrl) |
 	      FIELD_PREP(DPHY_BAND_CFG_RIGHT_BAND, band_ctrl);
 	writel(reg, dphy->regs + DPHY_BAND_CFG);
+	writel(FIELD_PREP(DPHY_CMN_SSM_CAL_WAIT_TIME, 0x14) | DPHY_CMN_SSM_EN | DPHY_CMN_TX_MODE_EN,
+	       dphy->regs + DPHY_CMN_SSM);
 
 	ret = cdns_dphy_wait_for_pll_lock(dphy);
 	if (ret)
-- 
2.39.1

Re: [PATCH v2 2/2] phy: cadence: cdns-dphy: Update calibration wait time for startup state machine

[Tomi Valkeinen]

Hi,

On 26/03/2025 17:23, Devarsh Thakkar wrote:
> Use system characterized reset value specified in TRM [1] to program
> calibration wait time which defines number of cycles to wait for after
> startup state machine is in bandgap enable state.
> 
> This fixes PLL lock timeout error faced while using RPi DSI Panel on TI's
> AM62L and J721E SoC [2].
> 
> [1] AM62P TRM (Section ):
> https://www.ti.com/lit/pdf/spruj83
> 
> [2]:
> Link: https://gist.github.com/devarsht/89e4830e886774fcd50aa6e29cce3a79
> 
> Cc: stable@vger.kernel.org
> Fixes: 7a343c8bf4b5 ("phy: Add Cadence D-PHY support")
> Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
> ---
> V2: Introduced this as as separate patch
> 
>   drivers/phy/cadence/cdns-dphy.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/phy/cadence/cdns-dphy.c b/drivers/phy/cadence/cdns-dphy.c
> index c4de9e4d3e93..11fbffe5aafd 100644
> --- a/drivers/phy/cadence/cdns-dphy.c
> +++ b/drivers/phy/cadence/cdns-dphy.c
> @@ -30,6 +30,7 @@
>   
>   #define DPHY_CMN_SSM			DPHY_PMA_CMN(0x20)
>   #define DPHY_CMN_SSM_EN			BIT(0)
> +#define DPHY_CMN_SSM_CAL_WAIT_TIME	GENMASK(8, 1)
>   #define DPHY_CMN_TX_MODE_EN		BIT(9)
>   
>   #define DPHY_CMN_PWM			DPHY_PMA_CMN(0x40)
> @@ -405,6 +406,8 @@ static int cdns_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
>   	reg = FIELD_PREP(DPHY_BAND_CFG_LEFT_BAND, band_ctrl) |
>   	      FIELD_PREP(DPHY_BAND_CFG_RIGHT_BAND, band_ctrl);
>   	writel(reg, dphy->regs + DPHY_BAND_CFG);
> +	writel(FIELD_PREP(DPHY_CMN_SSM_CAL_WAIT_TIME, 0x14) | DPHY_CMN_SSM_EN | DPHY_CMN_TX_MODE_EN,
> +	       dphy->regs + DPHY_CMN_SSM);

This sounds like a TI specific characterized value, but the function 
here is a generic one. Also, is the value same for all TI SoCs? Or is it 
per-soc?

  Tomi

Re: [PATCH v2 2/2] phy: cadence: cdns-dphy: Update calibration wait time for startup state machine

[Devarsh Thakkar]

Hi Tomi,

Thanks for the review.

On 02/04/25 17:12, Tomi Valkeinen wrote:
> Hi,
> 
> On 26/03/2025 17:23, Devarsh Thakkar wrote:
>> Use system characterized reset value specified in TRM [1] to program
>> calibration wait time which defines number of cycles to wait for after
>> startup state machine is in bandgap enable state.
>>
>> This fixes PLL lock timeout error faced while using RPi DSI Panel on TI's
>> AM62L and J721E SoC [2].
>>
>> [1] AM62P TRM (Section ):
>> https://www.ti.com/lit/pdf/spruj83
>>
>> [2]:
>> Link: https://gist.github.com/devarsht/89e4830e886774fcd50aa6e29cce3a79
>>
>> Cc: stable@vger.kernel.org
>> Fixes: 7a343c8bf4b5 ("phy: Add Cadence D-PHY support")
>> Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
>> ---
>> V2: Introduced this as as separate patch
>>
>>   drivers/phy/cadence/cdns-dphy.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/phy/cadence/cdns-dphy.c 
>> b/drivers/phy/cadence/cdns-dphy.c
>> index c4de9e4d3e93..11fbffe5aafd 100644
>> --- a/drivers/phy/cadence/cdns-dphy.c
>> +++ b/drivers/phy/cadence/cdns-dphy.c
>> @@ -30,6 +30,7 @@
>>   #define DPHY_CMN_SSM            DPHY_PMA_CMN(0x20)
>>   #define DPHY_CMN_SSM_EN            BIT(0)
>> +#define DPHY_CMN_SSM_CAL_WAIT_TIME    GENMASK(8, 1)
>>   #define DPHY_CMN_TX_MODE_EN        BIT(9)
>>   #define DPHY_CMN_PWM            DPHY_PMA_CMN(0x40)
>> @@ -405,6 +406,8 @@ static int cdns_dphy_configure(struct phy *phy, 
>> union phy_configure_opts *opts)
>>       reg = FIELD_PREP(DPHY_BAND_CFG_LEFT_BAND, band_ctrl) |
>>             FIELD_PREP(DPHY_BAND_CFG_RIGHT_BAND, band_ctrl);
>>       writel(reg, dphy->regs + DPHY_BAND_CFG);
>> +    writel(FIELD_PREP(DPHY_CMN_SSM_CAL_WAIT_TIME, 0x14) | 
>> DPHY_CMN_SSM_EN | DPHY_CMN_TX_MODE_EN,
>> +           dphy->regs + DPHY_CMN_SSM);
> 
> This sounds like a TI specific characterized value, but the function 
> here is a generic one. Also, is the value same for all TI SoCs? Or is it 
> per-soc?
> 

No this is not TI specific value. As mentioned in commit message, 0x14 
is the cadence characterized default value for calibration wait time 
which they put in as reset value in the IP (if you reset the IP you will 
see calibration wait time as 0x14) to be used by software for optimal 
initialization.

Regards
Devarsh

Re: [PATCH v2 2/2] phy: cadence: cdns-dphy: Update calibration wait time for startup state machine

[Tomi Valkeinen]

Hi,

On 02/04/2025 16:35, Devarsh Thakkar wrote:
> Hi Tomi,
> 
> Thanks for the review.
> 
> On 02/04/25 17:12, Tomi Valkeinen wrote:
>> Hi,
>>
>> On 26/03/2025 17:23, Devarsh Thakkar wrote:
>>> Use system characterized reset value specified in TRM [1] to program
>>> calibration wait time which defines number of cycles to wait for after
>>> startup state machine is in bandgap enable state.
>>>
>>> This fixes PLL lock timeout error faced while using RPi DSI Panel on 
>>> TI's
>>> AM62L and J721E SoC [2].
>>>
>>> [1] AM62P TRM (Section ):
>>> https://www.ti.com/lit/pdf/spruj83
>>>
>>> [2]:
>>> Link: https://gist.github.com/devarsht/89e4830e886774fcd50aa6e29cce3a79
>>>
>>> Cc: stable@vger.kernel.org
>>> Fixes: 7a343c8bf4b5 ("phy: Add Cadence D-PHY support")
>>> Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
>>> ---
>>> V2: Introduced this as as separate patch
>>>
>>>   drivers/phy/cadence/cdns-dphy.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/phy/cadence/cdns-dphy.c b/drivers/phy/cadence/ 
>>> cdns-dphy.c
>>> index c4de9e4d3e93..11fbffe5aafd 100644
>>> --- a/drivers/phy/cadence/cdns-dphy.c
>>> +++ b/drivers/phy/cadence/cdns-dphy.c
>>> @@ -30,6 +30,7 @@
>>>   #define DPHY_CMN_SSM            DPHY_PMA_CMN(0x20)
>>>   #define DPHY_CMN_SSM_EN            BIT(0)
>>> +#define DPHY_CMN_SSM_CAL_WAIT_TIME    GENMASK(8, 1)
>>>   #define DPHY_CMN_TX_MODE_EN        BIT(9)
>>>   #define DPHY_CMN_PWM            DPHY_PMA_CMN(0x40)
>>> @@ -405,6 +406,8 @@ static int cdns_dphy_configure(struct phy *phy, 
>>> union phy_configure_opts *opts)
>>>       reg = FIELD_PREP(DPHY_BAND_CFG_LEFT_BAND, band_ctrl) |
>>>             FIELD_PREP(DPHY_BAND_CFG_RIGHT_BAND, band_ctrl);
>>>       writel(reg, dphy->regs + DPHY_BAND_CFG);
>>> +    writel(FIELD_PREP(DPHY_CMN_SSM_CAL_WAIT_TIME, 0x14) | 
>>> DPHY_CMN_SSM_EN | DPHY_CMN_TX_MODE_EN,
>>> +           dphy->regs + DPHY_CMN_SSM);
>>
>> This sounds like a TI specific characterized value, but the function 
>> here is a generic one. Also, is the value same for all TI SoCs? Or is 
>> it per-soc?
>>
> 
> No this is not TI specific value. As mentioned in commit message, 0x14 
> is the cadence characterized default value for calibration wait time 
> which they put in as reset value in the IP (if you reset the IP you will 
> see calibration wait time as 0x14) to be used by software for optimal 
> initialization.

If it's a register default value, why do you need to write it? Also, why 
do you say "characterized reset value"? I can't find any reference to 
this in the TRM (in the patch you point to the TRM, but not where to 
find the data there). The reset value of 0x14 could be just a default 
value, not characterized at all. And if this is a bandgad related wait 
timeout, shouldn't it be set before the BAND_CFG registers? Although 
It's not clear what those do.

  Tomi

Re: [PATCH v2 2/2] phy: cadence: cdns-dphy: Update calibration wait time for startup state machine

[Devarsh Thakkar]

Hi Tomi,

On 02/04/25 17:12, Tomi Valkeinen wrote:
> Hi,
> 
Thanks for the review.

> On 26/03/2025 17:23, Devarsh Thakkar wrote:
>> @@ -405,6 +406,8 @@ static int cdns_dphy_configure(struct phy *phy,
>> union phy_configure_opts *opts)
>>       reg = FIELD_PREP(DPHY_BAND_CFG_LEFT_BAND, band_ctrl) |
>>             FIELD_PREP(DPHY_BAND_CFG_RIGHT_BAND, band_ctrl);
>>       writel(reg, dphy->regs + DPHY_BAND_CFG);
>> +    writel(FIELD_PREP(DPHY_CMN_SSM_CAL_WAIT_TIME, 0x14) |
>> DPHY_CMN_SSM_EN | DPHY_CMN_TX_MODE_EN,
>> +           dphy->regs + DPHY_CMN_SSM);
> 
> This sounds like a TI specific characterized value, but the function
> here is a generic one. Also, is the value same for all TI SoCs? Or is it
> per-soc?
> 

I see same characterized value for calibration wait time for each TI SoC
but as discussed offline, I will use read-modify-write to preserve the
reset value which was characterized for the corresponding platform so
that if any other platform is using a different characterized reset
value for calibration wait time, it would handle that case as well.

Regards
Devarsh