Re: [PATCH v2] phy: phy-rockchip-samsung-hdptx: Fix PHY PLL output 50.25MHz error

["Diederik de Haas" <didi.debian@cknow.org>]

[-- Attachment #1: Type: text/plain, Size: 2889 bytes --]

Hi Cristian, 

On Fri May 16, 2025 at 6:02 PM CEST, Cristian Ciocaltea wrote:
> On 5/16/25 6:36 PM, Diederik de Haas wrote:
>> On Sun Apr 27, 2025 at 11:51 AM CEST, Algea Cao wrote:
>>> When using HDMI PLL frequency division coefficient at 50.25MHz
>>> that is calculated by rk_hdptx_phy_clk_pll_calc(), it fails to
>>> get PHY LANE lock. Although the calculated values are within the
>>> allowable range of PHY PLL configuration.
>>>
>>> In order to fix the PHY LANE lock error and provide the expected
>>> 50.25MHz output, manually compute the required PHY PLL frequency
>>> division coefficient and add it to ropll_tmds_cfg configuration
>>> table.
>>>
>>> Signed-off-by: Algea Cao <algea.cao@rock-chips.com>
>>> Reviewed-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>>>
>>> ---
>>>  drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
>>> index fe7c05748356..77236f012a1f 100644
>>> --- a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
>>> +++ b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
>>> @@ -476,6 +476,8 @@ static const struct ropll_config ropll_tmds_cfg[] = {
>>>  	  1, 1, 0, 0x20, 0x0c, 1, 0x0e, 0, 0, },
>>>  	{ 650000, 162, 162, 1, 1, 11, 1, 1, 1, 1, 1, 1, 1, 54, 0, 16, 4, 1,
>>>  	  1, 1, 0, 0x20, 0x0c, 1, 0x0e, 0, 0, },
>>> +	{ 502500, 84, 84, 1, 1, 7, 1, 1, 1, 1, 1, 1, 1, 11, 1, 4, 5,
>>> +	  4, 11, 1, 0, 0x20, 0x0c, 1, 0x0e, 0, 0, },
>>>  	{ 337500, 0x70, 0x70, 1, 1, 0xf, 1, 1, 1, 1, 1, 1, 1, 0x2, 0, 0x01, 5,
>>>  	  1, 1, 1, 0, 0x20, 0x0c, 1, 0x0e, 0, 0, },
>>>  	{ 400000, 100, 100, 1, 1, 11, 1, 1, 0, 1, 0, 1, 1, 0x9, 0, 0x05, 0,
>> 
>> I found this patch in the 'fixes' branch in linux-phy:
>> https://git.kernel.org/pub/scm/linux/kernel/git/phy/linux-phy.git/commit/?h=fixes&id=f9475055b11c0c70979bd1667a76b2ebae638eb7
>> 
>> In the 'next' branch in linux-phy, I found this commit:
>> 0edf9d2bb9b4 ("phy: rockchip: samsung-hdptx: Avoid Hz<->hHz unit conversion overhead")
>> https://git.kernel.org/pub/scm/linux/kernel/git/phy/linux-phy.git/commit/?h=next&id=0edf9d2bb9b4ba7566dfdc7605883e04575129d9
>> 
>> Where the values in ropll_tmds_cfg are converted from hHz to Hz and the
>> data type changes from u32 to unsigned long long.
>> But AFAICT it does NOT convert this '502500' value, which IIUC means
>> most values are in Hz, while this one is in hHz.
>> 
>> Am I missing something or should this new value also be converted?
>
> Yeah, the conversion is necessary. FWIW, this has been already fixed by
> Stephen Rothwell in linux-next, while Vinod will handle it before
> sending the PR:
>
> https://lore.kernel.org/all/aCXEOGUDcnaoGKWW@vaman/

Excellent. Thanks for the explanation :-)

Cheers,
  Diederik

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]