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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <D9XQ6FEVLTFF.6FEVFO840FYQ@cknow.org>
Date: Fri, 16 May 2025 18:28:27 +0200
From: "Diederik de Haas" <didi.debian@...ow.org>
To: "Cristian Ciocaltea" <cristian.ciocaltea@...labora.com>, "Algea Cao"
 <algea.cao@...k-chips.com>, <vkoul@...nel.org>, <kishon@...nel.org>,
 <heiko@...ech.de>, <andy.yan@...k-chips.com>
Cc: <linux-phy@...ts.infradead.org>, <linux-arm-kernel@...ts.infradead.org>,
 <linux-rockchip@...ts.infradead.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] phy: phy-rockchip-samsung-hdptx: Fix PHY PLL output
 50.25MHz error

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@...k-chips.com>
>>> Reviewed-by: Cristian Ciocaltea <cristian.ciocaltea@...labora.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

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)