[PATCH] octeon: false positive timeout

David Daney ddaney at caviumnetworks.com
Thu Jul 16 15:36:45 UTC 2009


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.

Consider a few points:

* 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.

* For working hardware we never see the timeout, so changing from post 
to pre decrement changes the timeout by 1/1000 which really doesn't 
matter for correctness.

* This looks like needless code churn.

* You never say if you think this is causing or could potentially cause 
undesirable behavior.


That said, if the change log made sense to me and we were doing it to 
make the code look more consistent, I might change my mind.

David Daney


> ---
> diff --git a/drivers/staging/octeon/cvmx-mdio.h b/drivers/staging/octeon/cvmx-mdio.h
> index c987a75..f45dc49 100644
> --- a/drivers/staging/octeon/cvmx-mdio.h
> +++ b/drivers/staging/octeon/cvmx-mdio.h
> @@ -421,7 +421,7 @@ static inline int cvmx_mdio_45_read(int bus_id, int phy_id, int device,
>  	do {
>  		cvmx_wait(1000);
>  		smi_rd.u64 = cvmx_read_csr(CVMX_SMIX_RD_DAT(bus_id));
> -	} while (smi_rd.s.pending && timeout--);
> +	} while (smi_rd.s.pending && --timeout);
>  
>  	if (timeout <= 0) {
>  		cvmx_dprintf("cvmx_mdio_45_read: bus_id %d phy_id %2d "




More information about the devel mailing list