Re: [PATCH v5 06/10] phy: rockchip: samsung-hdptx: Drop TMDS rate setup workaround

[Cristian Ciocaltea <cristian.ciocaltea@collabora.com>]

On 7/23/26 10:54 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The commit claims to drop the TMDS rate setup workaround but fails to remove the corresponding CCF-based rate adjustment fallback, leaving the cleanup incomplete.
> --
> 
> --- Patch [6]: [PATCH v5 06/10] phy: rockchip: samsung-hdptx: Drop TMDS rate setup workaround ---
> commit 0f64c3b5ec0d09a00473e1362200c7598b861f9d
> Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> 
> phy: rockchip: samsung-hdptx: Drop TMDS rate setup workaround
> 
> Since commit ba9c2fe18c17 ("drm/rockchip: dw_hdmi_qp: Switch to
> phy_configure()") the TMDS rate setup doesn't rely anymore on the
> unconventional usage of the bus width, instead it is managed exclusively
> through the HDMI PHY configuration API.
> 
> Drop the now obsolete workaround to retrieve the TMDS character rate via
> phy_get_bus_width() during power_on().
> 
>> diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
>> index 792eb57755eb1..31abff5ada9e3 100644
>> --- a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
>> +++ b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
>> @@ -1660,22 +1660,6 @@ static int rk_hdptx_phy_power_on(struct phy *phy)
>>  	enum phy_mode mode = phy_get_mode(phy);
>>  	int ret, lane;
>>  
>> -	if (mode != PHY_MODE_DP) {
>> -		if (!hdptx->hdmi_cfg.rate && hdptx->hdmi_cfg.mode != PHY_HDMI_MODE_FRL) {
>> -			/*
>> -			 * FIXME: Temporary workaround to setup TMDS char rate
>> -			 * from the RK DW HDMI QP bridge driver.
>> -			 * Will be removed as soon the switch to the HDMI PHY
>> -			 * configuration API has been completed on both ends.
>> -			 */
>> -			hdptx->hdmi_cfg.rate = phy_get_bus_width(hdptx->phy) & 0xfffffff;
>> -			hdptx->hdmi_cfg.rate *= 100;
>> -		}
>> -
> 
> [Severity: Medium]
> Since this patch drops the TMDS workaround here, should we also remove the
> corresponding CCF-based rate adjustment fallback and the restrict_rate_change
> state tracking?
> 
> Looking at rk_hdptx_phy_clk_determine_rate():
> 
> drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c:rk_hdptx_phy_clk_determine_rate() {
> 	...
> 	/*
> 	 * FIXME: Temporarily allow altering TMDS char rate via CCF.
> 	 * To be dropped as soon as the RK DW HDMI QP bridge driver
> 	 * switches to make use of phy_configure().
> 	 */
> 	if (!hdptx->restrict_rate_change && req->rate != hdptx->hdmi_cfg.rate) {
> 		struct phy_configure_opts_hdmi hdmi = {
> 			.tmds_char_rate = req->rate,
> 		};
> 	...
> }
> 
> Leaving this block behind seems to leave dead code, as the removal condition
> (the bridge driver switching to phy_configure()) has been met.
> 
> Does this cleanup belong in this patch as well to ensure the workaround is
> fully removed?

No, this is handled separately in [PATCH v5 08/10] phy: rockchip: samsung-hdptx:
Drop restrict_rate_change handling.

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy