[i2c] [PATCH] I2C bus driver for Philips ARM boards
Jean Delvare
khali at linux-fr.org
Sat Sep 30 22:24:32 CEST 2006
Hi Vitaly,
> +static inline int wait_timeout(long timeout, struct i2c_pnx_algo_data *a)
> +{
> + long tout = timeout;
> + while (tout > 0 &&
> + (ioread32(I2C_REG_STS(a)) & mstatus_active)) {
> + mdelay(1);
> + tout--;
> + }
> + return (tout <= 0);
> +}
I think there's something wrong here. You are calling this function in
several places now. It tests for mstatus_active, however in the
original code (previous patch) some code was testing for mstatus_active
and some for mstatus_reset. I doubt it's OK to change that. Shouldn't
you add a third parameter to this function to specify which bit you are
waiting for?
As a side note:
* One letter variable names are evil.
* You don't need the local "tout" variable above, you can work with
"timeout" directly.
> +static inline int i2c_pnx_bus_busy(struct i2c_pnx_algo_data *a)
> +{
> + return wait_timeout(I2C_PNX_TIMEOUT, a);
> +}
Isn't this function essantially redundant with wait_timeout?
--
Jean Delvare
More information about the i2c
mailing list