Re: [PATCH 2/2] qe: add polling timeout to qe_issue_cmd()

[Timur Tabi <timur@freescale.com>]

Grant Likely wrote:

> Hmmm, the ret value is backwards from what most coders would expect
> (zero on success, non-zero on failure).  I'd personally recommend
> reversing the polarity in the macro.

The ret value is documented as being the value of the expression when the loop terminates.  The reason it appears backwards is because the expression is

	(in_be32(&qe_immr->cp.cecr) & QE_CR_FLG) == 0

That is, the loop should spin until the QE_CR_FLG is zero.  So when the loop terminates normally, the expression (x) == 0 is true, which is equal to one.

I would expect that in most cases, the loop spins until some bit is *set*.  Let's pretend that QE_CR_FLG operates this way.  In that case, the call would look like this:

spin_event_timeout(in_be32(&qe_immr->cp.cecr) & QE_CR_FLG, 100, 0, ret);

At loop termination, the result of the expression will be either QE_CR_FLG or zero.

> Otherwise, feel free to add my acked-by line to both patches.

Thanks.

Ben, would you please apply this to your 'next' branch?

-- 
Timur Tabi
Linux kernel developer at Freescale