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

[Dominique Martinet]

Uwe Kleine-König wrote on Mon, Mar 10, 2025 at 10:14:23AM +0100:
> > 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?

Err, yes MHz; I was still half asleep when I added that example to the
commit message..

> 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?

For this particular rate check, I believe that if phy_clk_round_rate()
rejected the frequency then phy_clk_set_rate() cannot be called at all
with the said value (at least on this particular setup going through the
imx8mp-hdmi-tx bridge validating frequencies first before allowing
modes), not that it'd hurt to recheck.


In general though I agree these are doing pretty much the same thing,
with clk_round_rate() throwing away the pms values and there's more
duplication than ideal...
Unfortunately the code that computes registers for the integer divider
does it in a global variable so just computing everything in
round_rate() would forget what last setting was actually applied and
break e.g. resume, but yes that's just refactoring that should be done.

While we're here I also have no idea what recalc_rate() is supposed to
do but that 'return 74250000;' is definitely odd, and I'm sure there are
other improvements that could be made at the edges.


That's quite rude of me given I just sent the patch, but we probably
won't have time to rework this until mid-april after some urgent work
ends (this has actually been waiting for testing for 3 months
already...)
If this doesn't bother anyone else we can wait for a v2 then, but
otherwise it might be worth considering getting as is until refactoring
happens (and I pinky promise I'll find time before this summer -- I can
send a v2 with commit message fixed up as Frieder suggested if this is
acceptable to you)



Thanks for the quick feedback either way, and sorry for the long delay.
-- 
Dominique