diff mbox

[RFC/PATCH,2/3] i2c: Fall back to emulated SMBus if the operation isn't supported natively

Message ID 20120717170243.5def41a3@endymion.delvare (mailing list archive)
State New, archived
Headers show

Commit Message

Jean Delvare July 17, 2012, 3:02 p.m. UTC
Hi Laurent,

On Tue, 26 Jun 2012 16:17:08 +0200, Laurent Pinchart wrote:
> Adapter drivers might support only a subset of the SMBus operations
> natively. Those drivers currently have to manually emulate unsupported
> operations using I2C.
> 
> Make the i2c_smbus_xfer() function fall back to
> i2c_smbus_xfer_emulated() when the adapter's .smbus_xfer() operation
> returns -EOPNOTSUPP, like it already does when the .smbus_xfer()
> operation isn't available at all.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/i2c/i2c-core.c |    9 +++++++--
>  1 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 8cfa660..16e750e 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -2113,8 +2113,8 @@ s32 i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr, unsigned short flags,
>  		   union i2c_smbus_data *data)
>  {
>  	unsigned long orig_jiffies;
> +	s32 res = -EOPNOTSUPP;
>  	int try;
> -	s32 res;
>  
>  	flags &= I2C_M_TEN | I2C_CLIENT_PEC | I2C_CLIENT_SCCB;
>  
> @@ -2134,7 +2134,12 @@ s32 i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr, unsigned short flags,
>  				break;
>  		}
>  		i2c_unlock_adapter(adapter);
> -	} else
> +	}
> +
> +	/* Fall back to i2c_smbus_xfer_emulated of the adapter doesn't implement
> +	 * native support for the SMBus operation.
> +	 */
> +	if (res == -EOPNOTSUPP && adapter->algo->master_xfer)
>  		res = i2c_smbus_xfer_emulated(adapter, addr, flags, read_write,
>  					      command, protocol, data);
>  

Looks good overall, but maybe the following variant would be preferable
from a performance perspective:


What do you think? The advantage is that we can skip the tests for
adapters which only implement adapter->algo->master_xfer().

Comments

Laurent Pinchart July 17, 2012, 3:06 p.m. UTC | #1
Hi Jean,

On Tuesday 17 July 2012 17:02:43 Jean Delvare wrote:
> On Tue, 26 Jun 2012 16:17:08 +0200, Laurent Pinchart wrote:
> > Adapter drivers might support only a subset of the SMBus operations
> > natively. Those drivers currently have to manually emulate unsupported
> > operations using I2C.
> > 
> > Make the i2c_smbus_xfer() function fall back to
> > i2c_smbus_xfer_emulated() when the adapter's .smbus_xfer() operation
> > returns -EOPNOTSUPP, like it already does when the .smbus_xfer()
> > operation isn't available at all.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > 
> >  drivers/i2c/i2c-core.c |    9 +++++++--
> >  1 files changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> > index 8cfa660..16e750e 100644
> > --- a/drivers/i2c/i2c-core.c
> > +++ b/drivers/i2c/i2c-core.c
> > @@ -2113,8 +2113,8 @@ s32 i2c_smbus_xfer(struct i2c_adapter *adapter, u16
> > addr, unsigned short flags,
> >  		   union i2c_smbus_data *data)
> >  {
> >  	unsigned long orig_jiffies;
> > +	s32 res = -EOPNOTSUPP;
> >  	int try;
> > -	s32 res;
> > 
> >  	flags &= I2C_M_TEN | I2C_CLIENT_PEC | I2C_CLIENT_SCCB;
> > 
> > @@ -2134,7 +2134,12 @@ s32 i2c_smbus_xfer(struct i2c_adapter *adapter, u16
> > addr, unsigned short flags,
> >  				break;
> >  		}
> >  		i2c_unlock_adapter(adapter);
> > -	} else
> > +	}
> > +
> > +	/* Fall back to i2c_smbus_xfer_emulated of the adapter doesn't 
implement
> > +	 * native support for the SMBus operation.
> > +	 */
> > +	if (res == -EOPNOTSUPP && adapter->algo->master_xfer)
> >  		res = i2c_smbus_xfer_emulated(adapter, addr, flags, read_write,
> >  					      command, protocol, data);
> 
> Looks good overall, but maybe the following variant would be preferable
> from a performance perspective:
> 
> --- linux-3.5-rc7.orig/drivers/i2c/i2c-core.c	2012-07-17 
16:35:42.566799611
> +0200 +++ linux-3.5-rc7/drivers/i2c/i2c-core.c	2012-07-17
> 16:59:55.334530352 +0200 @@ -2140,11 +2140,17 @@ s32 i2c_smbus_xfer(struct
> i2c_adapter *a
>  				break;
>  		}
>  		i2c_unlock_adapter(adapter);
> -	} else
> -		res = i2c_smbus_xfer_emulated(adapter, addr, flags, read_write,
> -					      command, protocol, data);
> 
> -	return res;
> +		if (res != -EOPNOTSUPP || !adapter->algo->master_xfer)
> +			return res;
> +		/*
> +		 * Fall back to i2c_smbus_xfer_emulated if the adapter doesn't 
implement
> +		 * native support for the SMBus operation.
> +		 */
> +	}
> +
> +	return i2c_smbus_xfer_emulated(adapter, addr, flags, read_write,
> +				       command, protocol, data);
>  }
>  EXPORT_SYMBOL(i2c_smbus_xfer);
> 
> 
> What do you think? The advantage is that we can skip the tests for
> adapters which only implement adapter->algo->master_xfer().

I'm fine with that.
diff mbox

Patch

--- linux-3.5-rc7.orig/drivers/i2c/i2c-core.c	2012-07-17 16:35:42.566799611 +0200
+++ linux-3.5-rc7/drivers/i2c/i2c-core.c	2012-07-17 16:59:55.334530352 +0200
@@ -2140,11 +2140,17 @@  s32 i2c_smbus_xfer(struct i2c_adapter *a
 				break;
 		}
 		i2c_unlock_adapter(adapter);
-	} else
-		res = i2c_smbus_xfer_emulated(adapter, addr, flags, read_write,
-					      command, protocol, data);
 
-	return res;
+		if (res != -EOPNOTSUPP || !adapter->algo->master_xfer)
+			return res;
+		/*
+		 * Fall back to i2c_smbus_xfer_emulated if the adapter doesn't implement
+		 * native support for the SMBus operation.
+		 */
+	}
+
+	return i2c_smbus_xfer_emulated(adapter, addr, flags, read_write,
+				       command, protocol, data);
 }
 EXPORT_SYMBOL(i2c_smbus_xfer);