Re: [PATCHv2 2/8] OMAP3: PM: Adding voltage driver support for OMAP3

[Kevin Hilman <khilman@deeprootsystems.com>]

Thara Gopinath <thara@ti.com> writes:

> This patch adds voltage driver support for OMAP3. The driver
> allows  configuring the voltage controller and voltage
> processors during init and exports APIs to enable/disable
> voltage processors, scale voltage and reset voltage.
> The driver also maintains the global voltage table on a per
> VDD basis which contains the various voltages supported by the
> VDD along with per voltage dependent data like smartreflex
> n-target value, errminlimit and voltage processor errorgain.
> The driver allows scaling of VDD voltages either through
> "vc bypass method" or through "vp forceupdate method" the
> choice being configurable through the board file.
>
> This patch contains code originally in linux omap pm branch
> smartreflex driver.  Major contributors to this driver are
> Lesly A M, Rajendra Nayak, Kalle Jokiniemi, Paul Walmsley,
> Nishant Menon, Kevin Hilman.
>
> Signed-off-by: Thara Gopinath <thara@ti.com>

[...]

> +unsigned long omap_voltage_get_nom_volt(struct voltagedomain *voltdm)
> +{
> +	struct omap_opp *opp;
> +	struct omap_vdd_info *vdd;
> +	unsigned long freq;
> +
> +	if (!voltdm || IS_ERR(voltdm)) {
> +		pr_warning("%s: VDD specified does not exist!\n", __func__);
> +		return 0;
> +	}
> +
> +	vdd = container_of(voltdm, struct omap_vdd_info, voltdm);
> +
> +	freq = vdd->volt_clk->rate;
> +	opp = opp_find_freq_ceil(vdd->opp_dev, &freq);
> +	if (IS_ERR(opp)) {
> +		pr_warning("%s: Unable to find OPP for vdd_%s freq%ld\n",
> +			__func__, voltdm->name, freq);
> +		return 0;
> +	}
> +
> +	/*
> +	 * Use higher freq voltage even if an exact match is not available
> +	 * we are probably masking a clock framework bug, so warn
> +	 */
> +	if (unlikely(freq != vdd->volt_clk->rate))
> +		pr_warning("%s: Available freq %ld != dpll freq %ld.\n",
> +			__func__, freq, vdd->volt_clk->rate);
> +
> +	return opp_get_voltage(opp);
> +}

Minor nit...

Rather than having to look this up in the OPP table every time, couldn't
this be initialized and stored in struct voltagedomain? 

Whenever the voltage is changed, it's updated in struct voltagedomain
and doesn't have to be continually looked up in the OPP layer.

> +/**
> + * omap_vp_get_curr_volt : API to get the current vp voltage.
> + * @voltdm: pointer to the VDD.
> + *
> + * This API returns the current voltage for the specified voltage processor
> + */
> +unsigned long omap_vp_get_curr_volt(struct voltagedomain *voltdm)
> +{
> +	struct omap_vdd_info *vdd;
> +	u8 curr_vsel;
> +
> +	if (!voltdm || IS_ERR(voltdm)) {
> +		pr_warning("%s: VDD specified does not exist!\n", __func__);
> +		return 0;
> +	}
> +
> +	vdd = container_of(voltdm, struct omap_vdd_info, voltdm);
> +
> +	curr_vsel = voltage_read_reg(vdd->vp_offs.voltage);
> +	return omap_twl_vsel_to_uv(curr_vsel);
> +}

This too.  No need to read from the HW, if we just keep track of all
the changes in 'struct voltagedomain'.

Kevin