On 25 January 2013 16:43, Wolfram Sang <w.sang@xxxxxxxxxxxxxx> wrote:
> Code-wise I am mostly satisfied, except that I need to think about
> clock-stretching a little more. We could add that later, too.
Great!!
> However, while I understand you are keen to get rid of this series ASAP,
> it seems that this eagerness results in some sloppyness which is a bit
> cumbersome (most lines changed since last time have something to comment
> on).
:(
> So, I'll try a different review style this time and let you do the
> detective work :)
>
> 1) Please be strict and consistent with spaces around operators.
Sorry, but i couldn't find any such issues in code (but were there
in a comment)
> 2) Make sure the warnings and printouts match the code
> 3) Take the printouts serious and invest a thought if they can be
> further improved.
I have looked them again and attached are the final patches.
Following are the improvements i have:
Author: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
Date: Sat Jan 26 10:31:42 2013 +0530
fixup! i2c/adapter: Add bus recovery infrastructure
---
drivers/i2c/i2c-core.c | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 24c8a1e..3ec040e 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -136,14 +136,14 @@ static int i2c_get_gpios_for_recovery(struct
i2c_adapter *adap)
ret = gpio_request_one(bri->scl_gpio, GPIOF_OPEN_DRAIN |
GPIOF_OUT_INIT_HIGH, "i2c-scl");
if (ret) {
- dev_warn(dev, "gpio request fail: %d\n", bri->scl_gpio);
+ dev_warn(dev, "Can't get SCL gpio: %d\n", bri->scl_gpio);
return ret;
}
if (bri->get_sda) {
if (gpio_request_one(bri->sda_gpio, GPIOF_IN, "i2c-sda")) {
/* work without SDA polling */
- dev_warn(dev, "can't get SDA: %d. not using
SDA polling\n",
+ dev_warn(dev, "Can't get SDA gpio: %d. Not
using SDA polling\n",
bri->sda_gpio);
bri->get_sda = NULL;
}
@@ -163,8 +163,11 @@ static void i2c_put_gpios_for_recovery(struct
i2c_adapter *adap)
}
/*
- * We need to provide delay after every half clock pulse,
- * delay in ns = (10^6/ 100 KHz)/2
+ * We are generating clock pulse. ndelay() would decide durating of clk pulse.
+ * We will generate clock with rate 100 KHz and so duration of both
clock levels
+ * would be:
+ *
+ * delay in ns = (10^6 / 100) / 2
*/
#define RECOVERY_CLK_CNT 9
#define RECOVERY_NDELAY 5000
@@ -181,13 +184,13 @@ static int i2c_generic_recovery(struct i2c_adapter *adap)
*/
while (i++ < RECOVERY_CLK_CNT * 2) {
if (val) {
- /* break if SDA got high */
+ /* Break if SDA is high */
if (bri->get_sda && bri->get_sda(adap))
break;
/* SCL shouldn't be low here */
if (!bri->get_scl(adap)) {
dev_err(&adap->dev,
- "SCL is stuck low exit recovery\n");
+ "SCL is stuck low, exit recovery\n");
ret = -EBUSY;
break;
}
@@ -229,7 +232,7 @@ int i2c_recover_bus(struct i2c_adapter *adap)
if (!adap->bus_recovery_info)
return -EOPNOTSUPP;
- dev_dbg(&adap->dev, "trying i2c bus recovery\n");
+ dev_dbg(&adap->dev, "Trying i2c bus recovery\n");
return adap->bus_recovery_info->recover_bus(adap);
}
@@ -1031,12 +1034,12 @@ static int i2c_register_adapter(struct
i2c_adapter *adap)
struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
if (!bri->recover_bus) {
- dev_err(&adap->dev, "No recover_bus(), not
using recovery\n");
+ dev_err(&adap->dev, "No recover_bus() found,
not using recovery\n");
adap->bus_recovery_info = NULL;
goto exit_recovery;
}
- /* GPIO recovery */
+ /* Generic GPIO recovery */
if (bri->recover_bus == i2c_generic_gpio_recovery) {
if (!gpio_is_valid(bri->scl_gpio)) {
dev_err(&adap->dev, "Invalid SCL gpio,
not using recovery\n");
@@ -1052,8 +1055,9 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
bri->get_scl = get_scl_gpio_value;
bri->set_scl = set_scl_gpio_value;
} else if (!bri->set_scl || !bri->get_scl) {
- dev_warn(&adap->dev,
- "set_scl() is must for SCL recovery\n");
+ /* Generic SCL recovery */
+ dev_err(&adap->dev, "No get[set]_gpio() found,
not using recovery\n");
+ adap->bus_recovery_info = NULL;
}
}
Attachment:
0001-i2c-adapter-Add-bus-recovery-infrastructure.patch
Description: Binary data
Attachment:
0002-i2c-designware-Provide-i2c-bus-recovery-support.patch
Description: Binary data
