On 10 February 2016 at 15:42, Timur Tabi <timur@xxxxxxxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
