Re: [PATCH 5/9] omap3: pm: sr: replace get_opp with freq_to_opp

[Kevin Hilman <khilman@deeprootsystems.com>]

Nishanth Menon <nm@ti.com> writes:

> SmartReflex implements a get_opp to search through the opp table,
> replace it with the accessor function as it is a duplicate of
> freq_to_opp

SmartReflex is not quite working with this version which is in
pm-wip-opp.  My (untested) theory below...

[...]

> --- a/arch/arm/mach-omap2/smartreflex.c
> +++ b/arch/arm/mach-omap2/smartreflex.c
> @@ -146,49 +146,29 @@ static u32 cal_test_nvalue(u32 sennval, u32 senpval)
>  		(rnsenn << NVALUERECIPROCAL_RNSENN_SHIFT);
>  }
>  
> -/* determine the current OPP from the frequency
> - * we need to give this function last element of OPP rate table
> - * and the frequency
> - */
> -static u16 get_opp(struct omap_opp *opp_freq_table,
> -					unsigned long freq)
> -{
> -	struct omap_opp *prcm_config;
> -
> -	prcm_config = opp_freq_table;
> -
> -	if (prcm_config->rate <= freq)
> -		return prcm_config->opp_id; /* Return the Highest OPP */
> -	for (; prcm_config->rate; prcm_config--)
> -		if (prcm_config->rate < freq)
> -			return (prcm_config+1)->opp_id;
> -		else if (prcm_config->rate == freq)
> -			return prcm_config->opp_id;
> -	/* Return the least OPP */
> -	return (prcm_config+1)->opp_id;
> -}

The function you removed here is doing the equivalent of
opp_find_freq_ceil()


> -static u16 get_vdd1_opp(void)
> +static u8 get_vdd1_opp(void)
>  {
> -	u16 opp;
> +	u8 opp;
>  
>  	if (sr1.vdd_opp_clk == NULL || IS_ERR(sr1.vdd_opp_clk) ||
>  							mpu_opps == NULL)
>  		return 0;
>  
> -	opp = get_opp(mpu_opps + MAX_VDD1_OPP, sr1.vdd_opp_clk->rate);
> +	/* Not expected to fail.. */
> +	BUG_ON(freq_to_opp(&opp, mpu_opps, sr1.vdd_opp_clk->rate));
>  	return opp;
>  }

Ih this version you use freq_to_opp(), but in the V5 tarball you sent,
you replace get_opp with:

	opp = opp_find_freq_exact(mpu_opps, sr1.vdd_opp_clk->rate, true);

This results in never finding an OPP, and SR never scaling.

This should probably be opp_find_freq_ceil(), right?

Kevin