Re: [PATCH] phy: freescale: fsl-samsung-hdmi: return closest rate instead LUT

[Uwe Kleine-König]

Hello,

On Mon, Mar 10, 2025 at 10:21:32AM +0900, Dominique Martinet wrote:
> From: Makoto Sato <makoto.sato@xxxxxxxxxxxxxxxxx>
> 
> If the requested rate is not an exact match of the integer divider
> phy_clk_round_rate() would return the look up table value,
> but phy_clk_set_rate() can still use the integer divider if it results
> in a frequency that is closer than the look up table.
> 
> In particular, not returning the actually used value here made the hdmi
> bridge driver reject a frequency that has an integer divider rate
> within 0.5% of the target:
> for 83.5mHz, the integer divider generates 83.2mHz (-0.36%), but the
> next LUT value (82.5mHz) is 1.2% off which incorrectly rejects modes
> requiring this frequency.

Is the unit here MHz or mHz? I suspect the former?

Without having looked in detail, I think it would be nice to reduce code
duplication between phy_clk_round_rate() and phy_clk_set_rate(). The
former has

	if (rate > 297000000 || rate < 22250000)
		return -EINVAL;

which seems to be lacking from the latter so I suspect there are more
differences between the two functions than fixed here?

Ideally the implementation would look conceptually like:

	static long phy_clk_round_rate(..., unsigned long rate, ...)
	{
		hw = phy_determine_register_settings_for(rate);
		if (hw_is_error(hw))
			return someerror;

		return phy_get_rate_from(hw);
	}

	static int phy_clk_set_rate(..., unsigned long rate, ...)
	{
		hw = phy_determine_register_settings_for(rate);
		if (hw_is_error(hw))
			return someerror;

		return phy_apply(hw);
	}

Best regards
Uwe

Attachment: signature.asc
Description: PGP signature