[PATCH] octeon: false positive timeout
roel kluin
roel.kluin at gmail.com
Fri Jul 17 10:20:09 UTC 2009
On Thu, Jul 16, 2009 at 5:36 PM, David Daney<ddaney at caviumnetworks.com> wrote:
> Roel Kluin wrote:
>>
>> Otherwise a `timeout' value of 0 would not have been a timeout, but
>> rather that `smi_rd.s.pending' caused the loop to end in the last
>> iteration.
>>
>> Signed-off-by: Roel Kluin <roel.kluin at gmail.com>
>
> NAK.
>
> I can't really parse the meaning out of your explanation, but this does not
> need to change.
Please reconsider, I think my patch is correct or prove me otherwise
> * timeout will always be a positive non-zero number because of the preceding
> if statement. Any arguments about what would happen with a zero timeout are
> moot.
I think you misunderstand my changelog. It refers to when timeout reaches the
second(!):
if (timeout <= 0) {
cvmx_dprintf("cvmx_mdio_45_read: bus_id %d phy_id %2d "
"device %2d register %2d TIME OUT(data)\n",
bus_id, phy_id, device, location);
return -1;
}
(just visible in my patch, below the change)
If we reach that with a `timeout' value of 0, this does not mean that
the timeout
caused the loop to end, but rather the `smi_rd.s.pending', in the last
iteration.
If timeout causes the loop to end, then `timeout' is -1, not 0.
> * You never say if you think this is causing or could potentially cause
> undesirable behaviour.
The undesirable behaviour would be that if timeout is 0 we currently
have a false positive cvmx_dprintf() and error return.
If still not convinced observe the output of the snippet below.
Roel
#include <stdio.h>
#include <stdlib.h>
int main()
{
int i, r, timeout;
for (i=0; i<1000; i++) {
timeout=3;
do {
r = rand();
} while (r < RAND_MAX/2 && timeout--);
if (timeout <= 0)
printf ("%d: %d, %d\n", i, r, timeout);
}
return 0;
}
More information about the devel
mailing list