diff mbox series

[RFC,1/2] i2c: core: Add adapter transfer callback

Message ID 20220518204119.38943-2-eajames@linux.ibm.com (mailing list archive)
State Changes Requested
Headers show
Series i2c: core and si7020: Add adapter transfer callback | expand

Commit Message

Eddie James May 18, 2022, 8:41 p.m. UTC
Add a callback function pointer to be executed after the adapter
performs a transfer. The purpose of such a callback is for a client
to execute some code while "owning" the bus entirely. Holding the
adapter lock is insufficient in the case where the client is behind a
mux, as the mux driver could perform mux selection operations on the
bus while locked.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 drivers/i2c/i2c-core-base.c |  3 +++
 include/linux/i2c.h         | 25 +++++++++++++++++++++++++
 2 files changed, 28 insertions(+)

Comments

Peter Rosin May 20, 2022, 11:42 a.m. UTC | #1
Hi!

2022-05-18 at 22:41, Eddie James wrote:
> Add a callback function pointer to be executed after the adapter
> performs a transfer. The purpose of such a callback is for a client
> to execute some code while "owning" the bus entirely. Holding the
> adapter lock is insufficient in the case where the client is behind a
> mux, as the mux driver could perform mux selection operations on the
> bus while locked.
> 
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
>  drivers/i2c/i2c-core-base.c |  3 +++
>  include/linux/i2c.h         | 25 +++++++++++++++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index d43db2c3876e..a46bfee2d845 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -2128,6 +2128,9 @@ int __i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
>  		trace_i2c_result(adap, num, ret);
>  	}
>  
> +	if (adap->xfer_callback)
> +		adap->xfer_callback(adap->xfer_data, ret);
> +

Have you actually tested this_ Because it is not handling your issue
AFAICT. The master_xfer for muxes include the select/deselect which
(potentially) does xfers on the parent bus, so your hook is in the
wrong place when a I2C mux is involved.

Also, the hook should probably trigger for SMBus xfers.

Cheers,
Peter

>  	return ret;
>  }
>  EXPORT_SYMBOL(__i2c_transfer);
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index fbda5ada2afc..ea773f2ee9c8 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -747,6 +747,9 @@ struct i2c_adapter {
>  
>  	struct irq_domain *host_notify_domain;
>  	struct regulator *bus_regulator;
> +
> +	void (*xfer_callback)(void *data, int xfer_rc);
> +	void *xfer_data;
>  };
>  #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
>  
> @@ -814,6 +817,7 @@ i2c_trylock_bus(struct i2c_adapter *adapter, unsigned int flags)
>  static inline void
>  i2c_unlock_bus(struct i2c_adapter *adapter, unsigned int flags)
>  {
> +	adapter->xfer_callback = NULL;
>  	adapter->lock_ops->unlock_bus(adapter, flags);
>  }
>  
> @@ -849,6 +853,27 @@ static inline void i2c_mark_adapter_resumed(struct i2c_adapter *adap)
>  	i2c_unlock_bus(adap, I2C_LOCK_ROOT_ADAPTER);
>  }
>  
> +/**
> + * i2c_adapter_xfer_callback - Register a callback function that is executed
> + *			       when a transfer completes.
> + * @adap: Adapter to which the callback function will be registered
> + * @cb: The callback function pointer
> + * @data: The data to pass to the callback function
> + *
> + * This function should be called with the adapter locked with
> + * I2C_LOCK_ROOT_ADAPTER to ensure that the whole bus is idle while the
> + * callback executes.
> + * The callback is automatically removed when the bus is unlocked to avoid
> + * spurious executions of the callback.
> + */
> +static inline void i2c_adapter_xfer_callback(struct i2c_adapter *adap,
> +					     void (*cb)(void *data, int rc),
> +					     void *data)
> +{
> +	adap->xfer_callback = cb;
> +	adap->xfer_data = data;
> +}
> +
>  /* i2c adapter classes (bitmask) */
>  #define I2C_CLASS_HWMON		(1<<0)	/* lm_sensors, ... */
>  #define I2C_CLASS_DDC		(1<<3)	/* DDC bus on graphics adapters */
diff mbox series

Patch

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index d43db2c3876e..a46bfee2d845 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -2128,6 +2128,9 @@  int __i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 		trace_i2c_result(adap, num, ret);
 	}
 
+	if (adap->xfer_callback)
+		adap->xfer_callback(adap->xfer_data, ret);
+
 	return ret;
 }
 EXPORT_SYMBOL(__i2c_transfer);
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index fbda5ada2afc..ea773f2ee9c8 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -747,6 +747,9 @@  struct i2c_adapter {
 
 	struct irq_domain *host_notify_domain;
 	struct regulator *bus_regulator;
+
+	void (*xfer_callback)(void *data, int xfer_rc);
+	void *xfer_data;
 };
 #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
 
@@ -814,6 +817,7 @@  i2c_trylock_bus(struct i2c_adapter *adapter, unsigned int flags)
 static inline void
 i2c_unlock_bus(struct i2c_adapter *adapter, unsigned int flags)
 {
+	adapter->xfer_callback = NULL;
 	adapter->lock_ops->unlock_bus(adapter, flags);
 }
 
@@ -849,6 +853,27 @@  static inline void i2c_mark_adapter_resumed(struct i2c_adapter *adap)
 	i2c_unlock_bus(adap, I2C_LOCK_ROOT_ADAPTER);
 }
 
+/**
+ * i2c_adapter_xfer_callback - Register a callback function that is executed
+ *			       when a transfer completes.
+ * @adap: Adapter to which the callback function will be registered
+ * @cb: The callback function pointer
+ * @data: The data to pass to the callback function
+ *
+ * This function should be called with the adapter locked with
+ * I2C_LOCK_ROOT_ADAPTER to ensure that the whole bus is idle while the
+ * callback executes.
+ * The callback is automatically removed when the bus is unlocked to avoid
+ * spurious executions of the callback.
+ */
+static inline void i2c_adapter_xfer_callback(struct i2c_adapter *adap,
+					     void (*cb)(void *data, int rc),
+					     void *data)
+{
+	adap->xfer_callback = cb;
+	adap->xfer_data = data;
+}
+
 /* i2c adapter classes (bitmask) */
 #define I2C_CLASS_HWMON		(1<<0)	/* lm_sensors, ... */
 #define I2C_CLASS_DDC		(1<<3)	/* DDC bus on graphics adapters */