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

[Guenter Roeck <linux@xxxxxxxxxxxx>]

On 05/12/2017 01:16 PM, 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.
>
> To isolate this, create a new function to check suspend state and call it in
> every need place.
>
> As at it, move the error message from pr_err to dev_err.
>
> Signed-off-by: Rui Miguel Silva <rmfrfs@xxxxxxxxx>

Reviewed-by: Guenter Roeck <linux@xxxxxxxxxxxx>

> ---
>  drivers/staging/typec/fusb302/fusb302.c | 70 +++++++++++++++++----------------
>  1 file changed, 36 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/staging/typec/fusb302/fusb302.c b/drivers/staging/typec/fusb302/fusb302.c
> index 37fd0bd0f2a6..7e974f2244b3 100644
> --- a/drivers/staging/typec/fusb302/fusb302.c
> +++ b/drivers/staging/typec/fusb302/fusb302.c
> @@ -264,22 +264,36 @@ static void fusb302_debugfs_exit(const struct fusb302_chip *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)) {
> -			pr_err("fusb302_i2c: pm suspend, retry %d/%d\n",
> -			       retry_cnt + 1, FUSB302_RESUME_RETRY);
> +			dev_err(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)
>  		fusb302_log(chip, "cannot write 0x%02x to 0x%02x, ret=%d",
> @@ -292,21 +306,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)) {
> -			pr_err("fusb302_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)
> @@ -320,19 +330,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)) {
> -			pr_err("fusb302_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)
> @@ -345,21 +351,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)) {
> -			pr_err("fusb302_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) {

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