On 10 February 2016 at 15:42, Timur Tabi timur@codeaurora.org wrote:
Prashanth Prakash wrote:
+static int check_pcc_chan(void) +{
int ret = -EIO;
struct acpi_pcct_shared_memory __iomem *generic_comm_base =
pcc_comm_addr;
ktime_t next_deadline = ktime_add(ktime_get(), deadline);
/* Retry in case the remote processor was too slow to catch up. */
while (!ktime_after(ktime_get(), next_deadline)) {
if (readw_relaxed(&generic_comm_base->status) &
PCC_CMD_COMPLETE) {
ret = 0;
break;
}
/*
* Reducing the bus traffic in case this loop takes longer
than
* a few retries.
*/
udelay(3);
}
Like I said last time, you really should use readq_relaxed_poll_timeout(), which does exactly the same thing as this loop, but more elegantly.
Nope.
I think this will work:
u32 status; ret = readq_relaxed_poll_timeout(&generic_comm_base->status, status, status & PCC_CMD_COMPLETE, 3, deadline); return ret ? -EIO : 0; ... deadline = NUM_RETRIES * cppc_ss->latency;
This lets you completely eliminate all usage of ktime.
..Only to be re substituted by the macro all over again. So, there really is no value in replacing this with a macro. Also, readx_relaxed_poll_timeout() has a usleep(), which will kill all the optimization here. Its also horribly wrong in this context.
The alternative readx_poll_timeout_atomic() has a udelay() but it also adds way more conditionals than this code. So, there is no need to change things for the sake of cosmetic reasons.
You can eliminate the global variable 'deadline' also, if you can figure out how to pass the cppc_ss object to check_pcc_chan().
/* Wait for a nominal time to let platform process command. */
udelay(cmd_latency);
/* Retry in case the remote processor was too slow to catch up. */
for (retries = NUM_RETRIES; retries > 0; retries--) {
if (readw_relaxed(&generic_comm_base->status) &
PCC_CMD_COMPLETE) {
result = 0;
break;
}
}
/*
* For READs we need to ensure the cmd completed to ensure
* the ensuing read()s can proceed. For WRITEs we dont care
"don't"
Ashwin