On Tue, Dec 02 2014 at 04:10 -0700, Catalin Marinas wrote:
On Mon, Dec 01, 2014 at 06:57:05PM +0000, Lina Iyer wrote:On Thu, Nov 27 2014 at 08:01 -0700, Lorenzo Pieralisi wrote: >On Thu, Nov 27, 2014 at 05:24:07AM +0000, Lina Iyer wrote: > >[...] > >> +static int spm_set_low_power_mode(enum pm_sleep_mode mode) >> +{ >> + struct spm_driver_data *drv = per_cpu(cpu_spm_drv, >> + smp_processor_id()); >> + u32 start_index; >> + u32 ctl_val; >> + >> + if (!drv) >> + return -ENXIO; >> + >> + start_index = drv->reg_data->start_index[mode]; >> + >> + ctl_val = spm_register_read(drv, SPM_REG_SPM_CTL); >> + ctl_val &= ~(SPM_CTL_INDEX << SPM_CTL_INDEX_SHIFT); >> + ctl_val |= start_index << SPM_CTL_INDEX_SHIFT; >> + ctl_val |= SPM_CTL_EN; >> + spm_register_write(drv, SPM_REG_SPM_CTL, ctl_val); >> + >> + /* Ensure we have written the start address */ >> + wmb(); > >Can you explain please what this wmb is meant to achieve ? If it is there >to make sure the write made it to the SPM that's not a proper way >to achieve it, the barrier ensures ordering, not write completion. A write completion would be great, but assuming that the context switch using SCM or WFI could ensure the flush, then ordering is probably good enough for this.I assume you want the write to the device to complete (side-effects are observed) before the WFI (which would raise a StandbyWFI signal). The safer way is to read back from the device (or check/poll for the device state change) rather than a barrier.
Polling back to see if the register write was indeed successful, seems a bit more than what I would like on a low latency mode like Standby. Let me check with the hardware engineers, if a write ordering is required. It seems to me like I am not getting much out of the wmb() anyways. I will find out what kind of guarantees I would need to ensure the SPM write is successful. Thanks for your time, Catalin. Thanks, Lina. -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
