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

Devarsh Thakkar posted 2 patches 8 months, 3 weeks ago
There is a newer version of this series
[PATCH v2 2/2] phy: cadence: cdns-dphy: Update calibration wait time for startup state machine
Posted by Devarsh Thakkar 8 months, 3 weeks ago
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
Posted by Tomi Valkeinen 8 months, 2 weeks ago
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
Posted by Devarsh Thakkar 7 months, 2 weeks ago
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
Re: [PATCH v2 2/2] phy: cadence: cdns-dphy: Update calibration wait time for startup state machine
Posted by Devarsh Thakkar 8 months, 2 weeks ago
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
Posted by Tomi Valkeinen 8 months, 2 weeks ago
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