On 2/29/2012 5:22 PM, Laxman Dewangan wrote: > On Tuesday 28 February 2012 06:53 PM, viresh kumar wrote: >> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c >> +static int i2c_gpio_recover_bus(struct i2c_adapter *adap) >> +{ >> + int tmp, val = 1; >> + unsigned long delay = 1000000; >> + >> + tmp = gpio_request_one(adap->scl_gpio, GPIOF_DIR_OUT | >> + GPIOF_INIT_LOW, "i2c-bus-recover"); > > Should rename tmp to ret or status. tmp does not looks appropriate. > Ok. >> + if (tmp< 0) { >> + dev_warn(&adap->dev, "gpio request one fail: %d\n", >> + adap->scl_gpio); >> + return tmp; >> + } >> + >> + delay /= adap->clock_rate * 2; > Here delay is turning as micor sec and function used as the nano sec. clock_rate is in KHz, mentioned in comment of clock_rate. Makes sense now or am i missing something? >> + >> + for (tmp = 0; tmp< adap->clock_cnt * 2; tmp++, val = !val) { >> + ndelay(delay); > should be udelay()? Please add blank lines before and after your comment. That makes it more readable. >> + gpio_set_value(adap->scl_gpio, val); > > I think it should check for the sda line for coming out of the loop. > There may be possibility that we may not need 9 clock pulses. > I asked this in another mail, how to be sure that it will work. >> + u8 scl_gpio; > gpio can be more than 256. better to use int. > Take scl_gpio_flag and use in the gpio_request_one. Ok. -- viresh -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html