i2c: use an IRQ to report Host Notify events, not alert
authorBenjamin Tissoires <benjamin.tissoires@redhat.com>
Thu, 13 Oct 2016 12:10:40 +0000 (14:10 +0200)
committerWolfram Sang <wsa@the-dreams.de>
Thu, 24 Nov 2016 15:22:06 +0000 (16:22 +0100)
The current SMBus Host Notify implementation relies on .alert() to
relay its notifications. However, the use cases where SMBus Host
Notify is needed currently is to signal data ready on touchpads.

This is closer to an IRQ than a custom API through .alert().
Given that the 2 touchpad manufacturers (Synaptics and Elan) that
use SMBus Host Notify don't put any data in the SMBus payload, the
concept actually matches one to one.

Benefits are multiple:
- simpler code and API: the client will just have an IRQ, and
  nothing needs to be added in the adapter beside internally
  enabling it.
- no more specific workqueue, the threading is handled by IRQ core
  directly (when required)
- no more races when removing the device (the drivers are already
  required to disable irq on remove)
- simpler handling for drivers: use plain regular IRQs
- no more dependency on i2c-smbus for i2c-i801 (and any other adapter)
- the IRQ domain is created automatically when the adapter exports
  the Host Notify capability
- the IRQ are assign only if ACPI, OF and the caller did not assign
  one already
- the domain is automatically destroyed on remove
- fewer lines of code (minus 20, yeah!)

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
Documentation/i2c/smbus-protocol
drivers/i2c/Kconfig
drivers/i2c/busses/i2c-i801.c
drivers/i2c/i2c-core.c
drivers/i2c/i2c-smbus.c
include/linux/i2c-smbus.h
include/linux/i2c.h

index 14d4ec1be2457db04d0f76fe87b3ff323ece9e8a..092d474f5843d1a731834a166be8191b4a8cd8d0 100644 (file)
@@ -200,10 +200,14 @@ alerting device's address.
 [S] [HostAddr] [Wr] A [DevAddr] A [DataLow] A [DataHigh] A [P]
 
 This is implemented in the following way in the Linux kernel:
-* I2C bus drivers which support SMBus Host Notify should call
-  i2c_setup_smbus_host_notify() to setup SMBus Host Notify support.
-* I2C drivers for devices which can trigger SMBus Host Notify should implement
-  the optional alert() callback.
+* I2C bus drivers which support SMBus Host Notify should report
+  I2C_FUNC_SMBUS_HOST_NOTIFY.
+* I2C bus drivers trigger SMBus Host Notify by a call to
+  i2c_handle_smbus_host_notify().
+* I2C drivers for devices which can trigger SMBus Host Notify will have
+  client->irq assigned to a Host Notify IRQ if noone else specified an other.
+
+There is currently no way to retrieve the data parameter from the client.
 
 
 Packet Error Checking (PEC)
index d223650a97e426e582b2cfb58db3ecaedcf7cd4a..de305f89a65905fc5b16aa700c4aabf1061d9d6f 100644 (file)
@@ -7,6 +7,7 @@ menu "I2C support"
 config I2C
        tristate "I2C support"
        select RT_MUTEXES
+       select IRQ_DOMAIN
        ---help---
          I2C (pronounce: I-squared-C) is a slow serial bus protocol used in
          many micro controller applications and developed by Philips.  SMBus,
index c0c0cac9950c24c1cfb371923bef453eb4611427..e242db43774bb68fd91ce4a45a3ac73217ce2d7d 100644 (file)
@@ -269,7 +269,6 @@ struct i801_priv {
         */
        bool acpi_reserved;
        struct mutex acpi_lock;
-       struct smbus_host_notify *host_notify;
 };
 
 #define FEATURE_SMBUS_PEC      BIT(0)
@@ -585,10 +584,10 @@ static irqreturn_t i801_host_notify_isr(struct i801_priv *priv)
 
        /*
         * With the tested platforms, reading SMBNTFDDAT (22 + (p)->smba)
-        * always returns 0 and is safe to read.
-        * We just use 0 given we have no use of the data right now.
+        * always returns 0. Our current implementation doesn't provide
+        * data, so we just ignore it.
         */
-       i2c_handle_smbus_host_notify(priv->host_notify, addr, 0);
+       i2c_handle_smbus_host_notify(&priv->adapter, addr);
 
        /* clear Host Notify bit and return */
        outb_p(SMBSLVSTS_HST_NTFY_STS, SMBSLVSTS(priv));
@@ -951,17 +950,12 @@ static u32 i801_func(struct i2c_adapter *adapter)
                I2C_FUNC_SMBUS_HOST_NOTIFY : 0);
 }
 
-static int i801_enable_host_notify(struct i2c_adapter *adapter)
+static void i801_enable_host_notify(struct i2c_adapter *adapter)
 {
        struct i801_priv *priv = i2c_get_adapdata(adapter);
 
        if (!(priv->features & FEATURE_HOST_NOTIFY))
-               return -ENOTSUPP;
-
-       if (!priv->host_notify)
-               priv->host_notify = i2c_setup_smbus_host_notify(adapter);
-       if (!priv->host_notify)
-               return -ENOMEM;
+               return;
 
        priv->original_slvcmd = inb_p(SMBSLVCMD(priv));
 
@@ -971,8 +965,6 @@ static int i801_enable_host_notify(struct i2c_adapter *adapter)
 
        /* clear Host Notify bit to allow a new notification */
        outb_p(SMBSLVSTS_HST_NTFY_STS, SMBSLVSTS(priv));
-
-       return 0;
 }
 
 static void i801_disable_host_notify(struct i801_priv *priv)
@@ -1647,14 +1639,7 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
                return err;
        }
 
-       /*
-        * Enable Host Notify for chips that supports it.
-        * It is done after i2c_add_adapter() so that we are sure the work queue
-        * is not used if i2c_add_adapter() fails.
-        */
-       err = i801_enable_host_notify(&priv->adapter);
-       if (err && err != -ENOTSUPP)
-               dev_warn(&dev->dev, "Unable to enable SMBus Host Notify\n");
+       i801_enable_host_notify(&priv->adapter);
 
        i801_probe_optional_slaves(priv);
        /* We ignore errors - multiplexing is optional */
@@ -1705,11 +1690,8 @@ static int i801_resume(struct device *dev)
 {
        struct pci_dev *pci_dev = to_pci_dev(dev);
        struct i801_priv *priv = pci_get_drvdata(pci_dev);
-       int err;
 
-       err = i801_enable_host_notify(&priv->adapter);
-       if (err && err != -ENOTSUPP)
-               dev_warn(dev, "Unable to enable SMBus Host Notify\n");
+       i801_enable_host_notify(&priv->adapter);
 
        return 0;
 }
index 8b93a262e237f1f7386c11ee754bb40e97adc41d..3a1bc9c4efc7e8e3552b7c559eea19233a9c199d 100644 (file)
@@ -65,6 +65,9 @@
 #define I2C_ADDR_OFFSET_TEN_BIT        0xa000
 #define I2C_ADDR_OFFSET_SLAVE  0x1000
 
+#define I2C_ADDR_7BITS_MAX     0x77
+#define I2C_ADDR_7BITS_COUNT   (I2C_ADDR_7BITS_MAX + 1)
+
 /* core_lock protects i2c_adapter_idr, and guarantees
    that device detection, deletion of detected devices, and attach_adapter
    calls are serialized */
@@ -896,6 +899,25 @@ static void i2c_init_recovery(struct i2c_adapter *adap)
        adap->bus_recovery_info = NULL;
 }
 
+static int i2c_smbus_host_notify_to_irq(const struct i2c_client *client)
+{
+       struct i2c_adapter *adap = client->adapter;
+       unsigned int irq;
+
+       if (!adap->host_notify_domain)
+               return -ENXIO;
+
+       if (client->flags & I2C_CLIENT_TEN)
+               return -EINVAL;
+
+       irq = irq_find_mapping(adap->host_notify_domain, client->addr);
+       if (!irq)
+               irq = irq_create_mapping(adap->host_notify_domain,
+                                        client->addr);
+
+       return irq > 0 ? irq : -ENXIO;
+}
+
 static int i2c_device_probe(struct device *dev)
 {
        struct i2c_client       *client = i2c_verify_client(dev);
@@ -917,6 +939,14 @@ static int i2c_device_probe(struct device *dev)
                }
                if (irq == -EPROBE_DEFER)
                        return irq;
+               /*
+                * ACPI and OF did not find any useful IRQ, try to see
+                * if Host Notify can be used.
+                */
+               if (irq < 0) {
+                       dev_dbg(dev, "Using Host Notify IRQ\n");
+                       irq = i2c_smbus_host_notify_to_irq(client);
+               }
                if (irq < 0)
                        irq = 0;
 
@@ -1866,6 +1896,79 @@ static const struct i2c_lock_operations i2c_adapter_lock_ops = {
        .unlock_bus =  i2c_adapter_unlock_bus,
 };
 
+static void i2c_host_notify_irq_teardown(struct i2c_adapter *adap)
+{
+       struct irq_domain *domain = adap->host_notify_domain;
+       irq_hw_number_t hwirq;
+
+       if (!domain)
+               return;
+
+       for (hwirq = 0 ; hwirq < I2C_ADDR_7BITS_COUNT ; hwirq++)
+               irq_dispose_mapping(irq_find_mapping(domain, hwirq));
+
+       irq_domain_remove(domain);
+       adap->host_notify_domain = NULL;
+}
+
+static int i2c_host_notify_irq_map(struct irq_domain *h,
+                                         unsigned int virq,
+                                         irq_hw_number_t hw_irq_num)
+{
+       irq_set_chip_and_handler(virq, &dummy_irq_chip, handle_simple_irq);
+
+       return 0;
+}
+
+static const struct irq_domain_ops i2c_host_notify_irq_ops = {
+       .map = i2c_host_notify_irq_map,
+};
+
+static int i2c_setup_host_notify_irq_domain(struct i2c_adapter *adap)
+{
+       struct irq_domain *domain;
+
+       if (!i2c_check_functionality(adap, I2C_FUNC_SMBUS_HOST_NOTIFY))
+               return 0;
+
+       domain = irq_domain_create_linear(adap->dev.fwnode,
+                                         I2C_ADDR_7BITS_COUNT,
+                                         &i2c_host_notify_irq_ops, adap);
+       if (!domain)
+               return -ENOMEM;
+
+       adap->host_notify_domain = domain;
+
+       return 0;
+}
+
+/**
+ * i2c_handle_smbus_host_notify - Forward a Host Notify event to the correct
+ * I2C client.
+ * @adap: the adapter
+ * @addr: the I2C address of the notifying device
+ * Context: can't sleep
+ *
+ * Helper function to be called from an I2C bus driver's interrupt
+ * handler. It will schedule the Host Notify IRQ.
+ */
+int i2c_handle_smbus_host_notify(struct i2c_adapter *adap, unsigned short addr)
+{
+       int irq;
+
+       if (!adap)
+               return -EINVAL;
+
+       irq = irq_find_mapping(adap->host_notify_domain, addr);
+       if (irq <= 0)
+               return -ENXIO;
+
+       generic_handle_irq(irq);
+
+       return 0;
+}
+EXPORT_SYMBOL_GPL(i2c_handle_smbus_host_notify);
+
 static int i2c_register_adapter(struct i2c_adapter *adap)
 {
        int res = -EINVAL;
@@ -1897,6 +2000,14 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
        if (adap->timeout == 0)
                adap->timeout = HZ;
 
+       /* register soft irqs for Host Notify */
+       res = i2c_setup_host_notify_irq_domain(adap);
+       if (res) {
+               pr_err("adapter '%s': can't create Host Notify IRQs (%d)\n",
+                      adap->name, res);
+               goto out_list;
+       }
+
        dev_set_name(&adap->dev, "i2c-%d", adap->nr);
        adap->dev.bus = &i2c_bus_type;
        adap->dev.type = &i2c_adapter_type;
@@ -2134,6 +2245,8 @@ void i2c_del_adapter(struct i2c_adapter *adap)
 
        pm_runtime_disable(&adap->dev);
 
+       i2c_host_notify_irq_teardown(adap);
+
        /* wait until all references to the device are gone
         *
         * FIXME: This is old code and should ideally be replaced by an
index b0d2679c60d17085f2828229181f78779f5c25d0..f9271c713d202862cceac8f09d84fc30c97635cf 100644 (file)
@@ -241,108 +241,6 @@ int i2c_handle_smbus_alert(struct i2c_client *ara)
 }
 EXPORT_SYMBOL_GPL(i2c_handle_smbus_alert);
 
-static void smbus_host_notify_work(struct work_struct *work)
-{
-       struct alert_data alert;
-       struct i2c_adapter *adapter;
-       unsigned long flags;
-       u16 payload;
-       u8 addr;
-       struct smbus_host_notify *data;
-
-       data = container_of(work, struct smbus_host_notify, work);
-
-       spin_lock_irqsave(&data->lock, flags);
-       payload = data->payload;
-       addr = data->addr;
-       adapter = data->adapter;
-
-       /* clear the pending bit and release the spinlock */
-       data->pending = false;
-       spin_unlock_irqrestore(&data->lock, flags);
-
-       if (!adapter || !addr)
-               return;
-
-       alert.type = I2C_PROTOCOL_SMBUS_HOST_NOTIFY;
-       alert.addr = addr;
-       alert.data = payload;
-
-       device_for_each_child(&adapter->dev, &alert, smbus_do_alert);
-}
-
-/**
- * i2c_setup_smbus_host_notify - Allocate a new smbus_host_notify for the given
- * I2C adapter.
- * @adapter: the adapter we want to associate a Host Notify function
- *
- * Returns a struct smbus_host_notify pointer on success, and NULL on failure.
- * The resulting smbus_host_notify must not be freed afterwards, it is a
- * managed resource already.
- */
-struct smbus_host_notify *i2c_setup_smbus_host_notify(struct i2c_adapter *adap)
-{
-       struct smbus_host_notify *host_notify;
-
-       host_notify = devm_kzalloc(&adap->dev, sizeof(struct smbus_host_notify),
-                                  GFP_KERNEL);
-       if (!host_notify)
-               return NULL;
-
-       host_notify->adapter = adap;
-
-       spin_lock_init(&host_notify->lock);
-       INIT_WORK(&host_notify->work, smbus_host_notify_work);
-
-       return host_notify;
-}
-EXPORT_SYMBOL_GPL(i2c_setup_smbus_host_notify);
-
-/**
- * i2c_handle_smbus_host_notify - Forward a Host Notify event to the correct
- * I2C client.
- * @host_notify: the struct host_notify attached to the relevant adapter
- * @addr: the I2C address of the notifying device
- * @data: the payload of the notification
- * Context: can't sleep
- *
- * Helper function to be called from an I2C bus driver's interrupt
- * handler. It will schedule the Host Notify work, in turn calling the
- * corresponding I2C device driver's alert function.
- *
- * host_notify should be a valid pointer previously returned by
- * i2c_setup_smbus_host_notify().
- */
-int i2c_handle_smbus_host_notify(struct smbus_host_notify *host_notify,
-                                unsigned short addr, unsigned int data)
-{
-       unsigned long flags;
-       struct i2c_adapter *adapter;
-
-       if (!host_notify || !host_notify->adapter)
-               return -EINVAL;
-
-       adapter = host_notify->adapter;
-
-       spin_lock_irqsave(&host_notify->lock, flags);
-
-       if (host_notify->pending) {
-               spin_unlock_irqrestore(&host_notify->lock, flags);
-               dev_warn(&adapter->dev, "Host Notify already scheduled.\n");
-               return -EBUSY;
-       }
-
-       host_notify->payload = data;
-       host_notify->addr = addr;
-
-       /* Mark that there is a pending notification and release the lock */
-       host_notify->pending = true;
-       spin_unlock_irqrestore(&host_notify->lock, flags);
-
-       return schedule_work(&host_notify->work);
-}
-EXPORT_SYMBOL_GPL(i2c_handle_smbus_host_notify);
-
 module_i2c_driver(smbalert_driver);
 
 MODULE_AUTHOR("Jean Delvare <jdelvare@suse.de>");
index c2e3324f946853037d6f2ad90eacfc15781ceab1..a1385023a29b4ced7bbdd0ddc58ab7f92043f1ff 100644 (file)
@@ -50,31 +50,4 @@ struct i2c_client *i2c_setup_smbus_alert(struct i2c_adapter *adapter,
                                         struct i2c_smbus_alert_setup *setup);
 int i2c_handle_smbus_alert(struct i2c_client *ara);
 
-/**
- * smbus_host_notify - internal structure used by the Host Notify mechanism.
- * @adapter: the I2C adapter associated with this struct
- * @work: worker used to schedule the IRQ in the slave device
- * @lock: spinlock to check if a notification is already pending
- * @pending: flag set when a notification is pending (any new notification will
- *             be rejected if pending is true)
- * @payload: the actual payload of the Host Notify event
- * @addr: the address of the slave device which raised the notification
- *
- * This struct needs to be allocated by i2c_setup_smbus_host_notify() and does
- * not need to be freed. Internally, i2c_setup_smbus_host_notify() uses a
- * managed resource to clean this up when the adapter get released.
- */
-struct smbus_host_notify {
-       struct i2c_adapter      *adapter;
-       struct work_struct      work;
-       spinlock_t              lock;
-       bool                    pending;
-       u16                     payload;
-       u8                      addr;
-};
-
-struct smbus_host_notify *i2c_setup_smbus_host_notify(struct i2c_adapter *adap);
-int i2c_handle_smbus_host_notify(struct smbus_host_notify *host_notify,
-                                unsigned short addr, unsigned int data);
-
 #endif /* _LINUX_I2C_SMBUS_H */
index 82cf90945bb815dbf0edc1ef85abf17ce45ff680..b2109c522dec0726d6e6e390358a79be4bcdf891 100644 (file)
@@ -30,6 +30,7 @@
 #include <linux/device.h>      /* for struct device */
 #include <linux/sched.h>       /* for completion */
 #include <linux/mutex.h>
+#include <linux/irqdomain.h>           /* for Host Notify IRQ */
 #include <linux/of.h>          /* for struct device_node */
 #include <linux/swab.h>                /* for swab16 */
 #include <uapi/linux/i2c.h>
@@ -575,6 +576,8 @@ struct i2c_adapter {
 
        struct i2c_bus_recovery_info *bus_recovery_info;
        const struct i2c_adapter_quirks *quirks;
+
+       struct irq_domain *host_notify_domain;
 };
 #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
 
@@ -747,6 +750,7 @@ static inline u8 i2c_8bit_addr_from_msg(const struct i2c_msg *msg)
        return (msg->addr << 1) | (msg->flags & I2C_M_RD ? 1 : 0);
 }
 
+int i2c_handle_smbus_host_notify(struct i2c_adapter *adap, unsigned short addr);
 /**
  * module_i2c_driver() - Helper macro for registering a modular I2C driver
  * @__i2c_driver: i2c_driver struct