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

[Kevin Hilman <khilman@deeprootsystems.com>]

Thara Gopinath <thara@ti.com> writes:

> +/*
> + * vc_bypass_scale_voltage - VC bypass method of voltage scaling
> + */
> +static int vc_bypass_scale_voltage(struct omap_vdd_info *vdd,
> +		unsigned long target_volt)
> +{
> +	struct omap_volt_data *volt_data;
> +	u32 vc_bypass_value, vc_cmdval, vc_valid, vc_bypass_val_reg_offs;
> +	u32 vp_errgain_val, vc_cmd_on_mask;
> +	u32 loop_cnt = 0, retries_cnt = 0;
> +	u32 smps_steps = 0, smps_delay = 0;
> +	u8 vc_data_shift, vc_slaveaddr_shift, vc_regaddr_shift;
> +	u8 vc_cmd_on_shift;
> +	u8 target_vsel, current_vsel, sr_i2c_slave_addr;
> +
> +	if (cpu_is_omap34xx()) {
> +		vc_cmd_on_shift = OMAP3430_VC_CMD_ON_SHIFT;
> +		vc_cmd_on_mask = OMAP3430_VC_CMD_ON_MASK;
> +		vc_data_shift = OMAP3430_DATA_SHIFT;
> +		vc_slaveaddr_shift = OMAP3430_SLAVEADDR_SHIFT;
> +		vc_regaddr_shift = OMAP3430_REGADDR_SHIFT;
> +		vc_valid = OMAP3430_VALID_MASK;
> +		vc_bypass_val_reg_offs = OMAP3_PRM_VC_BYPASS_VAL_OFFSET;
> +		sr_i2c_slave_addr = OMAP3_SRI2C_SLAVE_ADDR;
> +	}

cpu_is_* should not be used every time this path is taken.  Rather, all
the SoC conditional values should be initialized once at init time.

[...]

> +/* VP force update method of voltage scaling */
> +static int vp_forceupdate_scale_voltage(struct omap_vdd_info *vdd,
> +		unsigned long target_volt)
> +{
> +	struct omap_volt_data *volt_data;
> +	u32 vc_cmd_on_mask, vc_cmdval, vpconfig;
> +	u32 smps_steps = 0, smps_delay = 0;
> +	int timeout = 0;
> +	u8 target_vsel, current_vsel;
> +	u8 vc_cmd_on_shift;
> +	u8 prm_irqst_reg_offs, ocp_mod;
> +
> +	if (cpu_is_omap34xx()) {
> +		vc_cmd_on_shift = OMAP3430_VC_CMD_ON_SHIFT;
> +		vc_cmd_on_mask = OMAP3430_VC_CMD_ON_MASK;
> +		prm_irqst_reg_offs = OMAP3_PRM_IRQSTATUS_MPU_OFFSET;
> +		ocp_mod = OCP_MOD;
> +	}

ditto.

Same goes for the OMAP4 patches which add to this.

Kevin