* [PATCH v3 01/10] i2c: i2c-smbus: prevent races on remove when Host Notify is used
2016-09-28 8:15 [PATCH v3 00/10] i2c: Host Notify / i801 fixes Benjamin Tissoires
@ 2016-09-28 8:15 ` Benjamin Tissoires
2016-09-28 8:15 ` [PATCH v3 02/10] i2c: i801: store and restore the SLVCMD register at load and unload Benjamin Tissoires
` (8 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Benjamin Tissoires @ 2016-09-28 8:15 UTC (permalink / raw)
To: Jean Delvare, Wolfram Sang; +Cc: linux-i2c, linux-kernel
struct host_notify contains its own workqueue, so there is a race when
the adapter gets removed:
- the adapter schedules a notification
- the notification is on hold
- the adapter gets removed and all its children too
- the worker fires and access illegal memory
Add an API to actually kill the workqueue and prevent it to access such
illegal memory. I couldn't find a reliable way of automatically calling
this, so it's the responsibility of the adapter driver to clean up after
itself.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
no changes in v3
changes in v2:
- changed i801_disable_host_notify() parameter
- changed the comments to actually match the behavior
---
drivers/i2c/busses/i2c-i801.c | 13 +++++++++++++
drivers/i2c/i2c-smbus.c | 19 +++++++++++++++++++
include/linux/i2c-smbus.h | 1 +
3 files changed, 33 insertions(+)
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 22a0ed4..b494a85 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -959,6 +959,18 @@ static int i801_enable_host_notify(struct i2c_adapter *adapter)
return 0;
}
+static void i801_disable_host_notify(struct i801_priv *priv)
+{
+
+ if (!(priv->features & FEATURE_HOST_NOTIFY))
+ return;
+
+ /* disable Host Notify... */
+ outb_p(0, SMBSLVCMD(priv));
+ /* ...and process the already queued notifications */
+ i2c_cancel_smbus_host_notify(priv->host_notify);
+}
+
static const struct i2c_algorithm smbus_algorithm = {
.smbus_xfer = i801_access,
.functionality = i801_func,
@@ -1647,6 +1659,7 @@ static void i801_remove(struct pci_dev *dev)
pm_runtime_forbid(&dev->dev);
pm_runtime_get_noresume(&dev->dev);
+ i801_disable_host_notify(priv);
i801_del_mux(priv);
i2c_del_adapter(&priv->adapter);
i801_acpi_remove(priv);
diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
index b0d2679..35e4f1a 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -279,6 +279,8 @@ static void smbus_host_notify_work(struct work_struct *work)
* 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.
+ * To prevent races on remove, the caller needs to stop the embedded worker
+ * by calling i2c_cancel_smbus_host_notify().
*/
struct smbus_host_notify *i2c_setup_smbus_host_notify(struct i2c_adapter *adap)
{
@@ -299,6 +301,23 @@ struct smbus_host_notify *i2c_setup_smbus_host_notify(struct i2c_adapter *adap)
EXPORT_SYMBOL_GPL(i2c_setup_smbus_host_notify);
/**
+ * i2c_cancel_smbus_host_notify - Terminate any active Host Notification.
+ * @host_notify: the host_notify object to terminate
+ *
+ * Process any pending Host Notifcation and prevent new ones to be added.
+ * Must be called to ensure no races between the adaptor being removed and
+ * the Host Notification being processed.
+ */
+void i2c_cancel_smbus_host_notify(struct smbus_host_notify *host_notify)
+{
+ if (!host_notify)
+ return;
+
+ cancel_work_sync(&host_notify->work);
+}
+EXPORT_SYMBOL_GPL(i2c_cancel_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
diff --git a/include/linux/i2c-smbus.h b/include/linux/i2c-smbus.h
index c2e3324..ac02827 100644
--- a/include/linux/i2c-smbus.h
+++ b/include/linux/i2c-smbus.h
@@ -76,5 +76,6 @@ struct smbus_host_notify {
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);
+void i2c_cancel_smbus_host_notify(struct smbus_host_notify *host_notify);
#endif /* _LINUX_I2C_SMBUS_H */
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 02/10] i2c: i801: store and restore the SLVCMD register at load and unload
2016-09-28 8:15 [PATCH v3 00/10] i2c: Host Notify / i801 fixes Benjamin Tissoires
2016-09-28 8:15 ` [PATCH v3 01/10] i2c: i2c-smbus: prevent races on remove when Host Notify is used Benjamin Tissoires
@ 2016-09-28 8:15 ` Benjamin Tissoires
2016-09-28 8:15 ` [PATCH v3 03/10] i2c: i801: minor formatting issues Benjamin Tissoires
` (7 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Benjamin Tissoires @ 2016-09-28 8:15 UTC (permalink / raw)
To: Jean Delvare, Wolfram Sang; +Cc: linux-i2c, linux-kernel
Also do not override any other configuration in this register.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
no changes in v3
new in v2
---
drivers/i2c/busses/i2c-i801.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index b494a85..7334831 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -240,6 +240,7 @@ struct i801_priv {
struct i2c_adapter adapter;
unsigned long smba;
unsigned char original_hstcfg;
+ unsigned char original_slvcmd;
struct pci_dev *pci_dev;
unsigned int features;
@@ -952,7 +953,12 @@ static int i801_enable_host_notify(struct i2c_adapter *adapter)
if (!priv->host_notify)
return -ENOMEM;
- outb_p(SMBSLVCMD_HST_NTFY_INTREN, SMBSLVCMD(priv));
+ priv->original_slvcmd = inb_p(SMBSLVCMD(priv));
+
+ if (!(SMBSLVCMD_HST_NTFY_INTREN & priv->original_slvcmd))
+ outb_p(SMBSLVCMD_HST_NTFY_INTREN | priv->original_slvcmd,
+ SMBSLVCMD(priv));
+
/* clear Host Notify bit to allow a new notification */
outb_p(SMBSLVSTS_HST_NTFY_STS, SMBSLVSTS(priv));
@@ -961,12 +967,11 @@ static int i801_enable_host_notify(struct i2c_adapter *adapter)
static void i801_disable_host_notify(struct i801_priv *priv)
{
-
if (!(priv->features & FEATURE_HOST_NOTIFY))
return;
/* disable Host Notify... */
- outb_p(0, SMBSLVCMD(priv));
+ outb_p(priv->original_slvcmd, SMBSLVCMD(priv));
/* ...and process the already queued notifications */
i2c_cancel_smbus_host_notify(priv->host_notify);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 03/10] i2c: i801: minor formatting issues
2016-09-28 8:15 [PATCH v3 00/10] i2c: Host Notify / i801 fixes Benjamin Tissoires
2016-09-28 8:15 ` [PATCH v3 01/10] i2c: i2c-smbus: prevent races on remove when Host Notify is used Benjamin Tissoires
2016-09-28 8:15 ` [PATCH v3 02/10] i2c: i801: store and restore the SLVCMD register at load and unload Benjamin Tissoires
@ 2016-09-28 8:15 ` Benjamin Tissoires
2016-09-28 8:15 ` [PATCH v3 04/10] i2c: i801: use BIT() macro for bits definition Benjamin Tissoires
` (6 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Benjamin Tissoires @ 2016-09-28 8:15 UTC (permalink / raw)
To: Jean Delvare, Wolfram Sang; +Cc: linux-i2c, linux-kernel
No functional changes, just typos and remove unused #define.
Reviewed-by: Jean Delvare <jdelvare@suse.de>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
no changes in v3
no changes in v2
---
drivers/i2c/busses/i2c-i801.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 7334831..4696319bf 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -186,10 +186,10 @@
#define SMBHSTSTS_INTR 0x02
#define SMBHSTSTS_HOST_BUSY 0x01
-/* Host Notify Status registers bits */
+/* Host Notify Status register bits */
#define SMBSLVSTS_HST_NTFY_STS 1
-/* Host Notify Command registers bits */
+/* Host Notify Command register bits */
#define SMBSLVCMD_HST_NTFY_INTREN 0x01
#define STATUS_ERROR_FLAGS (SMBHSTSTS_FAILED | SMBHSTSTS_BUS_ERR | \
@@ -270,8 +270,6 @@ struct i801_priv {
struct smbus_host_notify *host_notify;
};
-#define SMBHSTNTFY_SIZE 8
-
#define FEATURE_SMBUS_PEC (1 << 0)
#define FEATURE_BLOCK_BUFFER (1 << 1)
#define FEATURE_BLOCK_PROC (1 << 2)
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 04/10] i2c: i801: use BIT() macro for bits definition
2016-09-28 8:15 [PATCH v3 00/10] i2c: Host Notify / i801 fixes Benjamin Tissoires
` (2 preceding siblings ...)
2016-09-28 8:15 ` [PATCH v3 03/10] i2c: i801: minor formatting issues Benjamin Tissoires
@ 2016-09-28 8:15 ` Benjamin Tissoires
2016-09-28 8:15 ` [PATCH v3 05/10] i2c: i801: use the BIT() macro for FEATURES_* also Benjamin Tissoires
` (5 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Benjamin Tissoires @ 2016-09-28 8:15 UTC (permalink / raw)
To: Jean Delvare, Wolfram Sang; +Cc: linux-i2c, linux-kernel
i801 mixes hexadecimal and decimal values for defining bits. However,
we have a nice BIT() macro for this exact purpose.
No functional changes, cleanup only.
Reviewed-by: Jean Delvare <jdelvare@suse.de>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
no changes in v3
no changes in v2
---
drivers/i2c/busses/i2c-i801.c | 50 +++++++++++++++++++++----------------------
1 file changed, 25 insertions(+), 25 deletions(-)
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 4696319bf..05ce265 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -136,26 +136,26 @@
#define SBREG_SMBCTRL 0xc6000c
/* Host status bits for SMBPCISTS */
-#define SMBPCISTS_INTS 0x08
+#define SMBPCISTS_INTS BIT(3)
/* Control bits for SMBPCICTL */
-#define SMBPCICTL_INTDIS 0x0400
+#define SMBPCICTL_INTDIS BIT(10)
/* Host configuration bits for SMBHSTCFG */
-#define SMBHSTCFG_HST_EN 1
-#define SMBHSTCFG_SMB_SMI_EN 2
-#define SMBHSTCFG_I2C_EN 4
+#define SMBHSTCFG_HST_EN BIT(0)
+#define SMBHSTCFG_SMB_SMI_EN BIT(1)
+#define SMBHSTCFG_I2C_EN BIT(2)
/* TCO configuration bits for TCOCTL */
-#define TCOCTL_EN 0x0100
+#define TCOCTL_EN BIT(8)
/* Auxiliary status register bits, ICH4+ only */
-#define SMBAUXSTS_CRCE 1
-#define SMBAUXSTS_STCO 2
+#define SMBAUXSTS_CRCE BIT(0)
+#define SMBAUXSTS_STCO BIT(1)
/* Auxiliary control register bits, ICH4+ only */
-#define SMBAUXCTL_CRC 1
-#define SMBAUXCTL_E32B 2
+#define SMBAUXCTL_CRC BIT(0)
+#define SMBAUXCTL_E32B BIT(1)
/* Other settings */
#define MAX_RETRIES 400
@@ -170,27 +170,27 @@
#define I801_I2C_BLOCK_DATA 0x18 /* ICH5 and later */
/* I801 Host Control register bits */
-#define SMBHSTCNT_INTREN 0x01
-#define SMBHSTCNT_KILL 0x02
-#define SMBHSTCNT_LAST_BYTE 0x20
-#define SMBHSTCNT_START 0x40
-#define SMBHSTCNT_PEC_EN 0x80 /* ICH3 and later */
+#define SMBHSTCNT_INTREN BIT(0)
+#define SMBHSTCNT_KILL BIT(1)
+#define SMBHSTCNT_LAST_BYTE BIT(5)
+#define SMBHSTCNT_START BIT(6)
+#define SMBHSTCNT_PEC_EN BIT(7) /* ICH3 and later */
/* I801 Hosts Status register bits */
-#define SMBHSTSTS_BYTE_DONE 0x80
-#define SMBHSTSTS_INUSE_STS 0x40
-#define SMBHSTSTS_SMBALERT_STS 0x20
-#define SMBHSTSTS_FAILED 0x10
-#define SMBHSTSTS_BUS_ERR 0x08
-#define SMBHSTSTS_DEV_ERR 0x04
-#define SMBHSTSTS_INTR 0x02
-#define SMBHSTSTS_HOST_BUSY 0x01
+#define SMBHSTSTS_BYTE_DONE BIT(7)
+#define SMBHSTSTS_INUSE_STS BIT(6)
+#define SMBHSTSTS_SMBALERT_STS BIT(5)
+#define SMBHSTSTS_FAILED BIT(4)
+#define SMBHSTSTS_BUS_ERR BIT(3)
+#define SMBHSTSTS_DEV_ERR BIT(2)
+#define SMBHSTSTS_INTR BIT(1)
+#define SMBHSTSTS_HOST_BUSY BIT(0)
/* Host Notify Status register bits */
-#define SMBSLVSTS_HST_NTFY_STS 1
+#define SMBSLVSTS_HST_NTFY_STS BIT(0)
/* Host Notify Command register bits */
-#define SMBSLVCMD_HST_NTFY_INTREN 0x01
+#define SMBSLVCMD_HST_NTFY_INTREN BIT(0)
#define STATUS_ERROR_FLAGS (SMBHSTSTS_FAILED | SMBHSTSTS_BUS_ERR | \
SMBHSTSTS_DEV_ERR)
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 05/10] i2c: i801: use the BIT() macro for FEATURES_* also
2016-09-28 8:15 [PATCH v3 00/10] i2c: Host Notify / i801 fixes Benjamin Tissoires
` (3 preceding siblings ...)
2016-09-28 8:15 ` [PATCH v3 04/10] i2c: i801: use BIT() macro for bits definition Benjamin Tissoires
@ 2016-09-28 8:15 ` Benjamin Tissoires
2016-09-28 8:15 ` [PATCH v3 06/10] i2c: i801: do not report an error if FEATURE_HOST_NOTIFY is not set Benjamin Tissoires
` (4 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Benjamin Tissoires @ 2016-09-28 8:15 UTC (permalink / raw)
To: Jean Delvare, Wolfram Sang; +Cc: linux-i2c, linux-kernel
no functional changes
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
no changes in v3
new in v2
---
drivers/i2c/busses/i2c-i801.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 05ce265..55fa6e1 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -270,15 +270,15 @@ struct i801_priv {
struct smbus_host_notify *host_notify;
};
-#define FEATURE_SMBUS_PEC (1 << 0)
-#define FEATURE_BLOCK_BUFFER (1 << 1)
-#define FEATURE_BLOCK_PROC (1 << 2)
-#define FEATURE_I2C_BLOCK_READ (1 << 3)
-#define FEATURE_IRQ (1 << 4)
-#define FEATURE_HOST_NOTIFY (1 << 5)
+#define FEATURE_SMBUS_PEC BIT(0)
+#define FEATURE_BLOCK_BUFFER BIT(1)
+#define FEATURE_BLOCK_PROC BIT(2)
+#define FEATURE_I2C_BLOCK_READ BIT(3)
+#define FEATURE_IRQ BIT(4)
+#define FEATURE_HOST_NOTIFY BIT(5)
/* Not really a feature, but it's convenient to handle it as such */
-#define FEATURE_IDF (1 << 15)
-#define FEATURE_TCO (1 << 16)
+#define FEATURE_IDF BIT(15)
+#define FEATURE_TCO BIT(16)
static const char *i801_feature_names[] = {
"SMBus PEC",
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 06/10] i2c: i801: do not report an error if FEATURE_HOST_NOTIFY is not set
2016-09-28 8:15 [PATCH v3 00/10] i2c: Host Notify / i801 fixes Benjamin Tissoires
` (4 preceding siblings ...)
2016-09-28 8:15 ` [PATCH v3 05/10] i2c: i801: use the BIT() macro for FEATURES_* also Benjamin Tissoires
@ 2016-09-28 8:15 ` Benjamin Tissoires
2016-09-28 8:15 ` [PATCH v3 07/10] i2c: i2c-smbus: remove double warning message Benjamin Tissoires
` (3 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Benjamin Tissoires @ 2016-09-28 8:15 UTC (permalink / raw)
To: Jean Delvare, Wolfram Sang; +Cc: linux-i2c, linux-kernel
we can skip one test when calling i801_enable_host_notify(). Given
that we call it all the time, it's better to consider the fact that
the adapter doesn't support Host Notify as not an error.
Reviewed-by: Jean Delvare <jdelvare@suse.de>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
no changes in v3
no changes in v2
---
drivers/i2c/busses/i2c-i801.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 55fa6e1..422cce2 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -944,12 +944,13 @@ static int i801_enable_host_notify(struct i2c_adapter *adapter)
struct i801_priv *priv = i2c_get_adapdata(adapter);
if (!(priv->features & FEATURE_HOST_NOTIFY))
- return -ENOTSUPP;
+ return 0; /* not an error actually */
- if (!priv->host_notify)
+ if (!priv->host_notify) {
priv->host_notify = i2c_setup_smbus_host_notify(adapter);
- if (!priv->host_notify)
- return -ENOMEM;
+ if (!priv->host_notify)
+ return -ENOMEM;
+ }
priv->original_slvcmd = inb_p(SMBSLVCMD(priv));
@@ -1638,7 +1639,7 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
* is not used if i2c_add_adapter() fails.
*/
err = i801_enable_host_notify(&priv->adapter);
- if (err && err != -ENOTSUPP)
+ if (err)
dev_warn(&dev->dev, "Unable to enable SMBus Host Notify\n");
i801_probe_optional_slaves(priv);
@@ -1693,7 +1694,7 @@ static int i801_resume(struct device *dev)
int err;
err = i801_enable_host_notify(&priv->adapter);
- if (err && err != -ENOTSUPP)
+ if (err)
dev_warn(dev, "Unable to enable SMBus Host Notify\n");
return 0;
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 07/10] i2c: i2c-smbus: remove double warning message
2016-09-28 8:15 [PATCH v3 00/10] i2c: Host Notify / i801 fixes Benjamin Tissoires
` (5 preceding siblings ...)
2016-09-28 8:15 ` [PATCH v3 06/10] i2c: i801: do not report an error if FEATURE_HOST_NOTIFY is not set Benjamin Tissoires
@ 2016-09-28 8:15 ` Benjamin Tissoires
2016-09-28 8:15 ` [PATCH v3 08/10] i2c: i2c-smbus: fix return value of i2c_handle_smbus_host_notify() Benjamin Tissoires
` (2 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Benjamin Tissoires @ 2016-09-28 8:15 UTC (permalink / raw)
To: Jean Delvare, Wolfram Sang; +Cc: linux-i2c, linux-kernel
i2c_handle_smbus_host_notify() returns an error code when it fails.
If the caller wants to warn when an error is returned, we should not
partially warn in the function itself.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
no changes in v3
new in v2
---
drivers/i2c/i2c-smbus.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
index 35e4f1a..819e5f2 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -347,7 +347,6 @@ int i2c_handle_smbus_host_notify(struct smbus_host_notify *host_notify,
if (host_notify->pending) {
spin_unlock_irqrestore(&host_notify->lock, flags);
- dev_warn(&adapter->dev, "Host Notify already scheduled.\n");
return -EBUSY;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 08/10] i2c: i2c-smbus: fix return value of i2c_handle_smbus_host_notify()
2016-09-28 8:15 [PATCH v3 00/10] i2c: Host Notify / i801 fixes Benjamin Tissoires
` (6 preceding siblings ...)
2016-09-28 8:15 ` [PATCH v3 07/10] i2c: i2c-smbus: remove double warning message Benjamin Tissoires
@ 2016-09-28 8:15 ` Benjamin Tissoires
2016-09-28 8:15 ` [PATCH v3 09/10] i2c: i801: check if Host Notify is available during interrupt Benjamin Tissoires
2016-09-28 8:15 ` [PATCH v3 10/10] i2c: i801: remove SMBNTFDDAT reads as they always seem to return 0 Benjamin Tissoires
9 siblings, 0 replies; 11+ messages in thread
From: Benjamin Tissoires @ 2016-09-28 8:15 UTC (permalink / raw)
To: Jean Delvare, Wolfram Sang; +Cc: linux-i2c, linux-kernel
schedule_work() returns a boolean (true on success, false if the work
was already queued).
This doesn't match the "negative number on error" return model that the
function exports.
Given that schedule_work() will always return true (we have an internal
.pending check protected by a spinlock), just return false and ignore
the return value of schedule_work().
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
no changes in v3
new in v2
---
drivers/i2c/i2c-smbus.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
index 819e5f2..f6780de 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -357,7 +357,10 @@ int i2c_handle_smbus_host_notify(struct smbus_host_notify *host_notify,
host_notify->pending = true;
spin_unlock_irqrestore(&host_notify->lock, flags);
- return schedule_work(&host_notify->work);
+ /* schedule_work is called if .pending is false, so it can't fail. */
+ schedule_work(&host_notify->work);
+
+ return 0;
}
EXPORT_SYMBOL_GPL(i2c_handle_smbus_host_notify);
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 09/10] i2c: i801: check if Host Notify is available during interrupt
2016-09-28 8:15 [PATCH v3 00/10] i2c: Host Notify / i801 fixes Benjamin Tissoires
` (7 preceding siblings ...)
2016-09-28 8:15 ` [PATCH v3 08/10] i2c: i2c-smbus: fix return value of i2c_handle_smbus_host_notify() Benjamin Tissoires
@ 2016-09-28 8:15 ` Benjamin Tissoires
2016-09-28 8:15 ` [PATCH v3 10/10] i2c: i801: remove SMBNTFDDAT reads as they always seem to return 0 Benjamin Tissoires
9 siblings, 0 replies; 11+ messages in thread
From: Benjamin Tissoires @ 2016-09-28 8:15 UTC (permalink / raw)
To: Jean Delvare, Wolfram Sang; +Cc: linux-i2c, linux-kernel
There is a chance we get called while Host Notify is not available (yet),
so we need to clear the Host Notify bit in those rare case.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
changes in v3:
- renamed from "i2c: i801: warn on i2c_handle_smbus_host_notify() errors"
- removed the warnings and just kept the test
- added some comments on why the errors are not treated
no changes in v2 (comments addressed in the previous patches)
i2c: i2c-i801: remove warnings on the handle host notify
add a comment on i2c-i801
---
drivers/i2c/busses/i2c-i801.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 422cce2..38e2421 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -580,11 +580,20 @@ static irqreturn_t i801_host_notify_isr(struct i801_priv *priv)
unsigned short addr;
unsigned int data;
+ if (unlikely(!priv->host_notify))
+ goto out;
+
addr = inb_p(SMBNTFDADD(priv)) >> 1;
data = inw_p(SMBNTFDDAT(priv));
+ /*
+ * We don't notify about missed host notify events when the adapter
+ * lags behind. The operation is still scheduled and will get
+ * eventually processed.
+ */
i2c_handle_smbus_host_notify(priv->host_notify, addr, data);
+out:
/* clear Host Notify bit and return */
outb_p(SMBSLVSTS_HST_NTFY_STS, SMBSLVSTS(priv));
return IRQ_HANDLED;
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 10/10] i2c: i801: remove SMBNTFDDAT reads as they always seem to return 0
2016-09-28 8:15 [PATCH v3 00/10] i2c: Host Notify / i801 fixes Benjamin Tissoires
` (8 preceding siblings ...)
2016-09-28 8:15 ` [PATCH v3 09/10] i2c: i801: check if Host Notify is available during interrupt Benjamin Tissoires
@ 2016-09-28 8:15 ` Benjamin Tissoires
9 siblings, 0 replies; 11+ messages in thread
From: Benjamin Tissoires @ 2016-09-28 8:15 UTC (permalink / raw)
To: Jean Delvare, Wolfram Sang; +Cc: linux-i2c, linux-kernel
On the platform tested, reading SMBNTFDDAT always returns 0 (using 1 read
of a word or 2 of 2 bytes). Given that we are not sure why and that we
don't need to rely on the data parameter in the current users of Host
Notify, remove this part of the code.
If someone wants to re-enable it, just revert this commit and data should
be available.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
no changes in v3
new in v2
---
drivers/i2c/busses/i2c-i801.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 38e2421..0e3aa44 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -117,7 +117,6 @@
#define SMBSLVSTS(p) (16 + (p)->smba) /* ICH3 and later */
#define SMBSLVCMD(p) (17 + (p)->smba) /* ICH3 and later */
#define SMBNTFDADD(p) (20 + (p)->smba) /* ICH3 and later */
-#define SMBNTFDDAT(p) (22 + (p)->smba) /* ICH3 and later */
/* PCI Address Constants */
#define SMBBAR 4
@@ -578,20 +577,22 @@ static void i801_isr_byte_done(struct i801_priv *priv)
static irqreturn_t i801_host_notify_isr(struct i801_priv *priv)
{
unsigned short addr;
- unsigned int data;
if (unlikely(!priv->host_notify))
goto out;
addr = inb_p(SMBNTFDADD(priv)) >> 1;
- data = inw_p(SMBNTFDDAT(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.
+ *
* We don't notify about missed host notify events when the adapter
* lags behind. The operation is still scheduled and will get
* eventually processed.
*/
- i2c_handle_smbus_host_notify(priv->host_notify, addr, data);
+ i2c_handle_smbus_host_notify(priv->host_notify, addr, 0);
out:
/* clear Host Notify bit and return */
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread