Santosh Shilimkar <santosh.shilimkar@xxxxxx> writes:
> The OMAP5 idle driver can re-use OMAP4 CPUidle driver implementation thanks
> to compatible MPUSS design.
>
> Though unlike OMAP4, on OMAP5 devices, MPUSS CSWR (Close Switch
> Retention) power states can be achieved with respective power states
> on CPU0 and CPU1 power domain. This mode was broken on OMAP4 devices
> because of hardware limitation.
I'm a bit confused by the description here.
I gather from the code that this means that CPU0 and CPU1 can hit CSWR
independently, correct?
> Also there is no CPU low power entry order requirement like
> master CPU etc for CSWR C-state, which is icing on the cake.
Even on secure devices?
> Code makes use of voting scheme for cluster low power state to support
> MPUSS CSWR C-state.
The voting scheme and associated locking should be better described
here, or commented in the code itself.
> Supported OMAP5 CPUidle C-states:
> C1 - CPU0 ON(WFI) + CPU1 ON(WFI) + MPUSS ON
> C2 - CPU0 CSWR + CPU1 CSWR + MPUSS CSWR
> C3 - CPU0 OFF + CPU1 OFF + MPUSS OSWR
>
> Acked-by: Nishanth Menon <nm@xxxxxx>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@xxxxxx>
[...]
> +static int omap_enter_idle_smp(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv,
> + int index)
> +{
> + struct idle_statedata *cx = state_ptr + index;
> + int cpu_id = smp_processor_id();
> + unsigned long flag;
> +
> + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu_id);
I think the CPUidle core handles the broadcast notification now.
> + raw_spin_lock_irqsave(&mpu_lock, flag);
> + cx->mpu_state_vote++;
How about using an atomic_t and atomic_inc()/atomic_dec() instead, which
will avoid the need for a spinlock.
Even with that, it still seems potentially racy. Do you need a memory
barrier here to be sure that any changes from another CPU are visible
here, and vice versa?
> + if (cx->mpu_state_vote == num_online_cpus()) {
> + pwrdm_set_logic_retst(mpu_pd, cx->mpu_logic_state);
> + omap_set_pwrdm_state(mpu_pd, cx->mpu_state);
> + }
> + raw_spin_unlock_irqrestore(&mpu_lock, flag);
> +
> + omap4_enter_lowpower(dev->cpu, cx->cpu_state);
> +
> + raw_spin_lock_irqsave(&mpu_lock, flag);
> + if (cx->mpu_state_vote == num_online_cpus())
> + omap_set_pwrdm_state(mpu_pd, PWRDM_POWER_ON);
> + cx->mpu_state_vote--;
> + raw_spin_unlock_irqrestore(&mpu_lock, flag);
> +
> + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &cpu_id);
> +
> + return index;
> +}
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
