Tero Kristo <t-kristo@xxxxxx> writes:
> Voltage code will now enable / disable auto_ret / auto_off dynamically
> according to the voltagedomain usecounts. This is accomplished via
> the usage of the voltdm callback functions for sleep / wakeup.
>
> Signed-off-by: Tero Kristo <t-kristo@xxxxxx>
[...]
> -static void __init omap3_vc_init_channel(struct voltagedomain *voltdm)
> +static void omap3_set_core_off_timings(struct voltagedomain *voltdm)
> {
> + u32 tstart, tshut;
nit: insert blank line here
> + omap_pm_get_oscillator(&tstart, &tshut);
> + omap3_set_clksetup(tstart, voltdm);
> omap3_set_off_timings(voltdm);
> }
>
> +static void omap3_vc_channel_sleep(struct voltagedomain *voltdm)
> +{
> + /* Set off timings if entering off */
> + if (voltdm->target_state == PWRDM_POWER_OFF)
> + omap3_set_off_timings(voltdm);
> + else
> + omap3_set_i2c_timings(voltdm, 0);
Comment probably applies more to patch 2 since that's where it was
introduced, but this is where I got confused, so mentioning it here:
Calling this 'set_i2c_timings' is a bit confusing IMO because reading
the code there is a choice between 'i2c' timings and 'off' timings.
Maybe just calling these 'ret' and 'off' timings will be better for
readability.
> +}
> +
> +static void omap3_vc_channel_wakeup(struct voltagedomain *voltdm)
> +{
> +}
nit: empty function not needed since code checks for non-NULL function
pointer.
> +static void omap3_vc_core_sleep(struct voltagedomain *voltdm)
> +{
> + u8 mode;
> +
> + switch (voltdm->target_state) {
> + case PWRDM_POWER_OFF:
> + mode = OMAP3430_AUTO_OFF_MASK;
> + break;
> + case PWRDM_POWER_RET:
> + mode = OMAP3430_AUTO_RET_MASK;
> + break;
> + default:
> + mode = OMAP3430_AUTO_SLEEP_MASK;
> + break;
> + }
> +
> + if (mode & OMAP3430_AUTO_OFF_MASK)
AND vs. == ?
speaking of which, this function probably needs a comment mentioning
that these bits are all mutually exclusive (with a TRM reference.)
> + omap3_set_core_off_timings(voltdm);
> + else
> + omap3_set_core_ret_timings(voltdm);
> +
> + voltdm->rmw(OMAP3430_AUTO_OFF_MASK | OMAP3430_AUTO_RET_MASK |
> + OMAP3430_AUTO_SLEEP_MASK, mode,
> + OMAP3_PRM_VOLTCTRL_OFFSET);
> +}
Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
