Re: [PATCH 3/4] staging: typec: fusb302: refactor resume retry mechanism

[Rui Miguel Silva]

Hi,
On Thu, May 11, 2017 at 10:32:35AM -0700, Guenter Roeck wrote:

On Thu, May 11, 2017 at 03:20:22PM +0100, Rui Miguel Silva wrote:

The i2c functions need to test the pm_suspend state and do, if needed, some
retry before i2c operations. This code was repeated 4x.

The isolate this create a new function to check suspend state and call it in
every need place.

Signed-off-by: Rui Miguel Silva <rmf...@gmail.com>

This and the next patch don't apply without patch 2. Please refactor.
Also, please change the order of patch 3 and 4 since the latter is
a bug fix and this one is a cleanup (and thus probably not applicable
for v4.12).

I will refactor and reorder in v2.

Cheers,
  Rui

Thanks,
Guenter

---
 drivers/staging/typec/fusb302/fusb302.c | 66 +++++++++++++++++----------------
 1 file changed, 34 insertions(+), 32 deletions(-)

diff --git a/drivers/staging/typec/fusb302/fusb302.c 
b/drivers/staging/typec/fusb302/fusb302.c
index 40842b8075e5..26d7e2a387c3 100644
--- a/drivers/staging/typec/fusb302/fusb302.c
+++ b/drivers/staging/typec/fusb302/fusb302.c
@@ -122,22 +122,36 @@ struct fusb302_chip {

 #define FUSB302_RESUME_RETRY 10
 #define FUSB302_RESUME_RETRY_SLEEP 50
-static int fusb302_i2c_write(struct fusb302_chip *chip,
-                            u8 address, u8 data)
+
+static bool fusb302_is_suspended(struct fusb302_chip *chip)
 {
        int retry_cnt;
-       int ret = 0;

-       atomic_set(&chip->i2c_busy, 1);
        for (retry_cnt = 0; retry_cnt < FUSB302_RESUME_RETRY; retry_cnt++) {
                if (atomic_read(&chip->pm_suspend)) {
                        dev_info(chip->dev, "i2c: pm suspend, retry %d/%d\n",
                                 retry_cnt + 1, FUSB302_RESUME_RETRY);
                        msleep(FUSB302_RESUME_RETRY_SLEEP);
                } else {
-                       break;
+                       return false;
                }
        }
+
+       return true;
+}
+
+static int fusb302_i2c_write(struct fusb302_chip *chip,
+                            u8 address, u8 data)
+{
+       int ret = 0;
+
+       atomic_set(&chip->i2c_busy, 1);
+
+       if (fusb302_is_suspended(chip)) {
+               atomic_set(&chip->i2c_busy, 0);
+               return -ETIMEDOUT;
+       }
+
        ret = i2c_smbus_write_byte_data(chip->i2c_client, address, data);
        if (ret < 0)
                dev_err(chip->dev, "cannot write 0x%02x to 0x%02x: %d\n", data,
@@ -150,21 +164,17 @@ static int fusb302_i2c_write(struct fusb302_chip *chip,
 static int fusb302_i2c_block_write(struct fusb302_chip *chip, u8 address,
                                   u8 length, const u8 *data)
 {
-       int retry_cnt;
        int ret = 0;

        if (length <= 0)
                return ret;
        atomic_set(&chip->i2c_busy, 1);
-       for (retry_cnt = 0; retry_cnt < FUSB302_RESUME_RETRY; retry_cnt++) {
-               if (atomic_read(&chip->pm_suspend)) {
-                       dev_info(chip->dev, "i2c: pm suspend, retry %d/%d\n",
-                                retry_cnt + 1, FUSB302_RESUME_RETRY);
-                       msleep(FUSB302_RESUME_RETRY_SLEEP);
-               } else {
-                       break;
-               }
+
+       if (fusb302_is_suspended(chip)) {
+               atomic_set(&chip->i2c_busy, 0);
+               return -ETIMEDOUT;
        }
+
        ret = i2c_smbus_write_i2c_block_data(chip->i2c_client, address,
                                             length, data);
        if (ret < 0)
@@ -178,19 +188,15 @@ static int fusb302_i2c_block_write(struct fusb302_chip 
*chip, u8 address,
 static int fusb302_i2c_read(struct fusb302_chip *chip,
                            u8 address, u8 *data)
 {
-       int retry_cnt;
        int ret = 0;

        atomic_set(&chip->i2c_busy, 1);
-       for (retry_cnt = 0; retry_cnt < FUSB302_RESUME_RETRY; retry_cnt++) {
-               if (atomic_read(&chip->pm_suspend)) {
-                       dev_info(chip->dev, "i2c: pm suspend, retry %d/%d\n",
-                                retry_cnt + 1, FUSB302_RESUME_RETRY);
-                       msleep(FUSB302_RESUME_RETRY_SLEEP);
-               } else {
-                       break;
-               }
+
+       if (fusb302_is_suspended(chip)) {
+               atomic_set(&chip->i2c_busy, 0);
+               return -ETIMEDOUT;
        }
+
        ret = i2c_smbus_read_byte_data(chip->i2c_client, address);
        *data = (u8)ret;
        if (ret < 0)
@@ -203,21 +209,17 @@ static int fusb302_i2c_read(struct fusb302_chip *chip,
 static int fusb302_i2c_block_read(struct fusb302_chip *chip, u8 address,
                                  u8 length, u8 *data)
 {
-       int retry_cnt;
        int ret = 0;

        if (length <= 0)
                return ret;
        atomic_set(&chip->i2c_busy, 1);
-       for (retry_cnt = 0; retry_cnt < FUSB302_RESUME_RETRY; retry_cnt++) {
-               if (atomic_read(&chip->pm_suspend)) {
-                       dev_info(chip->dev, "i2c: pm suspend, retry %d/%d\n",
-                                retry_cnt + 1, FUSB302_RESUME_RETRY);
-                       msleep(FUSB302_RESUME_RETRY_SLEEP);
-               } else {
-                       break;
-               }
+
+       if (fusb302_is_suspended(chip)) {
+               atomic_set(&chip->i2c_busy, 0);
+               return -ETIMEDOUT;
        }
+
        ret = i2c_smbus_read_i2c_block_data(chip->i2c_client, address,
                                            length, data);
        if (ret < 0) {
--
2.12.2

_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel