Re: [PATCH V7 4/5] phy: freescale: fsl-samsung-hdmi: Use closest divider

[Dominique Martinet]

Adam Ford wrote on Tue, Sep 10, 2024 at 08:28:10PM -0500:
> Currently, if the clock values cannot be set to the exact rate,
> the round_rate and set_rate functions use the closest value found in
> the look-up-table.  In preparation of removing values from the LUT
> that can be calculated evenly with the integer calculator, it's
> necessary to ensure to check both the look-up-table and the integer
> divider clock values to get the closest values to the requested
> value.  It does this by measuring the difference between the
> requested clock value and the closest value in both integer divider
> calucator and the fractional clock look-up-table.
> 
> Which ever has the smallest difference between them is returned as
> the closest rate.
> 
> Signed-off-by: Adam Ford <aford173@xxxxxxxxx>

Thank you for the rework,

Reviewed-by: Dominique Martinet <dominique.martinet@xxxxxxxxxxxxxxxxx>
Tested-by: Dominique Martinet <dominique.martinet@xxxxxxxxxxxxxxxxx>

> ---
> V7:  Because of the previous patch refactoring, the flow of this patch
>      changed quite a bit to use more help functions and goto statements
>      to hopefully make the code flow better and improve comment
>      readability.  Because of the change, I removed s-o-b and r-b,
>      and t-b tags.
> 
> V6:  Simplify the calculation of the closest rate and fix
>      a situation where the integer divider values may not be properly
>      setup before they are used.
>      Fixup some comments
> V5:  No Change
> V4:  New to series
> ---
>  drivers/phy/freescale/phy-fsl-samsung-hdmi.c | 40 ++++++++++++++------
>  1 file changed, 28 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> index 49317a96f767..67a28aac9c45 100644
> --- a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> +++ b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> @@ -577,6 +577,16 @@ static void fsl_samsung_hdmi_calculate_phy(struct phy_config *cal_phy, unsigned
>  	/* pll_div_regs 3-6 are fixed and pre-defined already */
>  }
>  
> +static u32 fsl_samsung_hdmi_phy_get_closest_rate(unsigned long rate,
> +						 u32 int_div_clk, u32 frac_div_clk)
> +{
> +	/* The int_div_clk may be greater than rate, so cast it and use ABS */
> +	if (abs((long)rate - (long)int_div_clk) < (rate - frac_div_clk))
> +		return int_div_clk;
> +
> +	return frac_div_clk;
> +}
> +
>  static long phy_clk_round_rate(struct clk_hw *hw,
>  			       unsigned long rate, unsigned long *parent_rate)
>  {
> @@ -624,27 +634,33 @@ static int phy_clk_set_rate(struct clk_hw *hw,
>  		goto use_fract_div;
>  
>  	/*
> -	 * If the rate from the fractional divder is not exact, check the integer divider,
> +	 * If the rate from the fractional divider is not exact, check the integer divider,
>  	 * and use it if that value is an exact match.
>  	 */
>  	int_div_clk = fsl_samsung_hdmi_phy_find_pms(rate, &p, &m, &s);
> +	fsl_samsung_hdmi_calculate_phy(&calculated_phy_pll_cfg, int_div_clk, p, m, s);
>  	if (int_div_clk == rate) {
> -		dev_dbg(phy->dev, "fsl_samsung_hdmi_phy: integer divider rate = %u\n",
> -				   int_div_clk);
> -
> -		fsl_samsung_hdmi_calculate_phy(&calculated_phy_pll_cfg, int_div_clk, p, m, s);
> -		phy->cur_cfg  = &calculated_phy_pll_cfg;
> -		return fsl_samsung_hdmi_phy_configure(phy, phy->cur_cfg);
> +		goto use_int_div;
>  	}
>  
>  	/*
> -	 * If neither the fractional divder nor the integer divder can find an exact value
> -	 * fall back to using the fractional divider
> +	 * Compare the difference between the integer clock and the fractional clock against
> +	 * the desired clock and which whichever is closest,
>  	 */
> +	if (fsl_samsung_hdmi_phy_get_closest_rate(rate, int_div_clk,
> +						  fract_div_phy->pixclk) == fract_div_phy->pixclk)
> +		goto use_fract_div;
> +
> +use_int_div:
> +	dev_dbg(phy->dev, "fsl_samsung_hdmi_phy: integer divider rate = %u\n", int_div_clk);
> +	phy->cur_cfg  = &calculated_phy_pll_cfg;
> +	goto end;
> +
>  use_fract_div:
> -	phy->cur_cfg = fract_div_phy;
> -	dev_dbg(phy->dev, "fsl_samsung_hdmi_phy: using fractional divider rate = %u\n",
> -			   phy->cur_cfg->pixclk);
> +	 phy->cur_cfg = fract_div_phy;
> +	 dev_dbg(phy->dev, "fsl_samsung_hdmi_phy: using fractional divider rate = %u\n",
> +		   phy->cur_cfg->pixclk);
> +end:
>  	return fsl_samsung_hdmi_phy_configure(phy, phy->cur_cfg);
>  }
>