Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function

[Viresh Kumar <viresh.kumar@xxxxxx>]

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