Re: [PATCH 6/7] memory: tegra: Move compare/update current delay values to a function

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 09/04/2024 11:46, Diogo Ivo wrote:
> Separate the comparison/updating of the measured delay values with the
> values currently programmed into a separate function to simplify the
> code.
> 
> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
> ---
>  drivers/memory/tegra/tegra210-emc-cc-r21021.c | 84 +++++++++----------
>  1 file changed, 38 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/memory/tegra/tegra210-emc-cc-r21021.c b/drivers/memory/tegra/tegra210-emc-cc-r21021.c
> index 566e5c65c854..ec2f84758d55 100644
> --- a/drivers/memory/tegra/tegra210-emc-cc-r21021.c
> +++ b/drivers/memory/tegra/tegra210-emc-cc-r21021.c
> @@ -113,19 +113,35 @@ enum {
>  #define __MOVAVG(timing, dev)                      \
>  	((timing)->ptfv_list[dev])
>  
> +static bool tegra210_emc_compare_update_delay(struct tegra210_emc_timing *timing,
> +					      u32 measured, u32 idx)
> +{
> +	u32 *curr = &timing->current_dram_clktree[idx];
> +	u32 rate_mhz = timing->rate / 1000;
> +	u32 tmdel;
> +
> +	tmdel = abs(*curr - measured);
> +
> +	if (tmdel * 128 * rate_mhz / 1000000 > timing->tree_margin) {
> +		*curr = measured;
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
>  static u32 update_clock_tree_delay(struct tegra210_emc *emc, int type)
>  {
>  	bool periodic_training_update = type == PERIODIC_TRAINING_UPDATE;
>  	struct tegra210_emc_timing *last = emc->last;
>  	struct tegra210_emc_timing *next = emc->next;
>  	u32 last_timing_rate_mhz = last->rate / 1000;
> -	u32 next_timing_rate_mhz = next->rate / 1000;
>  	bool dvfs_update = type == DVFS_UPDATE;
> -	s32 tdel = 0, tmdel = 0, adel = 0;
>  	bool dvfs_pt1 = type == DVFS_PT1;
>  	u32 temp[2][2], value, udelay;
>  	unsigned long cval = 0;
>  	unsigned int c, d, idx;
> +	bool over = false;
>  
>  	if (dvfs_pt1 || periodic_training_update) {
>  		udelay = tegra210_emc_actual_osc_clocks(last->run_clocks);
> @@ -174,17 +190,9 @@ static u32 update_clock_tree_delay(struct tegra210_emc *emc, int type)
>  			else if (periodic_training_update)
>  				__WEIGHTED_UPDATE_PTFV(idx, cval);
>  
> -			if (dvfs_update || periodic_training_update) {
> -				tdel = next->current_dram_clktree[idx] -
> -						__MOVAVG_AC(next, idx);
> -				tmdel = (tdel < 0) ? -1 * tdel : tdel;
> -				adel = tmdel;
> -
> -				if (tmdel * 128 * next_timing_rate_mhz / 1000000 >
> -				    next->tree_margin)
> -					next->current_dram_clktree[idx] =
> -						__MOVAVG_AC(next, idx);
> -			}
> +			if (dvfs_update || periodic_training_update)
> +				over |= tegra210_emc_compare_update_delay(next,
> +							__MOVAVG_AC(next, idx), idx);
>  
>  			/* C[c]D[d]U[1] */
>  			idx++;
> @@ -202,35 +210,26 @@ static u32 update_clock_tree_delay(struct tegra210_emc *emc, int type)
>  			else if (periodic_training_update)
>  				__WEIGHTED_UPDATE_PTFV(idx, cval);
>  
> -			if (dvfs_update || periodic_training_update) {
> -				tdel = next->current_dram_clktree[idx] -
> -						__MOVAVG_AC(next, idx);
> -				tmdel = (tdel < 0) ? -1 * tdel : tdel;
> -
> -				if (tmdel > adel)
> -					adel = tmdel;
> -
> -				if (tmdel * 128 * next_timing_rate_mhz / 1000000 >
> -				    next->tree_margin)
> -					next->current_dram_clktree[idx] =
> -						__MOVAVG_AC(next, idx);
> -			}
> +			if (dvfs_update || periodic_training_update)
> +				over |= tegra210_emc_compare_update_delay(next,
> +							__MOVAVG_AC(next, idx), idx);
>  		}
>  	}
>  
> -	return adel;
> +	return over;

You are now returning always 0 or 1, while previously it was tmdel,
which I suppose is not 0/1.

This looks odd, especially that function prototype did not change.



Best regards,
Krzysztof