diff mbox

[4/5] i2c-i801: clean up block transaction

Message ID 1453223377-20608-5-git-send-email-minyard@acm.org
State Superseded
Headers show

Commit Message

Corey Minyard Jan. 19, 2016, 5:09 p.m. UTC
From: Corey Minyard <cminyard@mvista.com>

The block transaction code looked a little messy, pull out the
code to copy to/from the buffer into separate functions to make
it a little neater.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/i2c/busses/i2c-i801.c | 54 +++++++++++++++++++++++++++----------------
 1 file changed, 34 insertions(+), 20 deletions(-)

Comments

Jean Delvare May 25, 2016, 12:31 p.m. UTC | #1
On Tue, 19 Jan 2016 11:09:36 -0600, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
> 
> The block transaction code looked a little messy, pull out the
> code to copy to/from the buffer into separate functions to make
> it a little neater.

A matter of taste I suppose. I don't see anything messy in the code as
it exists. A 32-line function doing a single thing is just fine with me.

> 
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
>  drivers/i2c/busses/i2c-i801.c | 54 +++++++++++++++++++++++++++----------------
>  1 file changed, 34 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
> index 7567a96..840656c 100644
> --- a/drivers/i2c/busses/i2c-i801.c
> +++ b/drivers/i2c/busses/i2c-i801.c
> @@ -425,37 +425,51 @@ static int i801_transaction(struct i801_priv *priv, int xact)
>  	return i801_check_post(priv, status);
>  }
>  
> +static void i801_write_block(struct i801_priv *priv, union i2c_smbus_data *data)
> +{
> +	int i, len;
> +
> +	len = data->block[0];
> +	outb_p(len, SMBHSTDAT0(priv));
> +	for (i = 0; i < len; i++)
> +		outb_p(data->block[i+1], SMBBLKDAT(priv));
> +}
> +
> +static int i801_read_block(struct i801_priv *priv, union i2c_smbus_data *data)
> +{
> +	int i, len;
> +
> +	len = inb_p(SMBHSTDAT0(priv));
> +	if (len < 1 || len > I2C_SMBUS_BLOCK_MAX)
> +		return -EPROTO;
> +
> +	data->block[0] = len;
> +	for (i = 0; i < len; i++)
> +		data->block[i + 1] = inb_p(SMBBLKDAT(priv));
> +
> +	return 0;
> +}
> +
>  static int i801_block_transaction_by_block(struct i801_priv *priv,
>  					   union i2c_smbus_data *data,
>  					   char read_write)
>  {
> -	int i, len;
> -	int status;
> +	int result;
>  
>  	inb_p(SMBHSTCNT(priv)); /* reset the data buffer index */
>  
>  	/* Use 32-byte buffer to process this transaction */
> -	if (read_write == I2C_SMBUS_WRITE) {
> -		len = data->block[0];
> -		outb_p(len, SMBHSTDAT0(priv));
> -		for (i = 0; i < len; i++)
> -			outb_p(data->block[i+1], SMBBLKDAT(priv));
> -	}
> +	if (read_write == I2C_SMBUS_WRITE)
> +		i801_write_block(priv, data);
>  
> -	status = i801_transaction(priv, I801_BLOCK_DATA);
> -	if (status)
> -		return status;
> +	result = i801_transaction(priv, I801_BLOCK_DATA);
> +	if (result)
> +		return result;
>  
> -	if (read_write == I2C_SMBUS_READ) {
> -		len = inb_p(SMBHSTDAT0(priv));
> -		if (len < 1 || len > I2C_SMBUS_BLOCK_MAX)
> -			return -EPROTO;
> +	if (read_write == I2C_SMBUS_READ)
> +		result = i801_read_block(priv, data);
>  
> -		data->block[0] = len;
> -		for (i = 0; i < len; i++)
> -			data->block[i + 1] = inb_p(SMBBLKDAT(priv));
> -	}
> -	return 0;
> +	return result;
>  }
>  
>  static void i801_isr_byte_done(struct i801_priv *priv)
diff mbox

Patch

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 7567a96..840656c 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -425,37 +425,51 @@  static int i801_transaction(struct i801_priv *priv, int xact)
 	return i801_check_post(priv, status);
 }
 
+static void i801_write_block(struct i801_priv *priv, union i2c_smbus_data *data)
+{
+	int i, len;
+
+	len = data->block[0];
+	outb_p(len, SMBHSTDAT0(priv));
+	for (i = 0; i < len; i++)
+		outb_p(data->block[i+1], SMBBLKDAT(priv));
+}
+
+static int i801_read_block(struct i801_priv *priv, union i2c_smbus_data *data)
+{
+	int i, len;
+
+	len = inb_p(SMBHSTDAT0(priv));
+	if (len < 1 || len > I2C_SMBUS_BLOCK_MAX)
+		return -EPROTO;
+
+	data->block[0] = len;
+	for (i = 0; i < len; i++)
+		data->block[i + 1] = inb_p(SMBBLKDAT(priv));
+
+	return 0;
+}
+
 static int i801_block_transaction_by_block(struct i801_priv *priv,
 					   union i2c_smbus_data *data,
 					   char read_write)
 {
-	int i, len;
-	int status;
+	int result;
 
 	inb_p(SMBHSTCNT(priv)); /* reset the data buffer index */
 
 	/* Use 32-byte buffer to process this transaction */
-	if (read_write == I2C_SMBUS_WRITE) {
-		len = data->block[0];
-		outb_p(len, SMBHSTDAT0(priv));
-		for (i = 0; i < len; i++)
-			outb_p(data->block[i+1], SMBBLKDAT(priv));
-	}
+	if (read_write == I2C_SMBUS_WRITE)
+		i801_write_block(priv, data);
 
-	status = i801_transaction(priv, I801_BLOCK_DATA);
-	if (status)
-		return status;
+	result = i801_transaction(priv, I801_BLOCK_DATA);
+	if (result)
+		return result;
 
-	if (read_write == I2C_SMBUS_READ) {
-		len = inb_p(SMBHSTDAT0(priv));
-		if (len < 1 || len > I2C_SMBUS_BLOCK_MAX)
-			return -EPROTO;
+	if (read_write == I2C_SMBUS_READ)
+		result = i801_read_block(priv, data);
 
-		data->block[0] = len;
-		for (i = 0; i < len; i++)
-			data->block[i + 1] = inb_p(SMBBLKDAT(priv));
-	}
-	return 0;
+	return result;
 }
 
 static void i801_isr_byte_done(struct i801_priv *priv)