[RFC v2 0/3] i2c: core: move smbus_alert into i2c-core

[RFC v2 0/3] i2c: core: move smbus_alert into i2c-core

[Phil Reid]

A part of the review of previous accepted series 
"Add sbs-manager with smbalert support" Wolfram suggested moving
the smbus_alert driver module into the i2c-core.

This is a proposed implementation of that move.

I've also rename Kconfig and file name to reflect that this only
relates the smbus alert feature.

I think I've compile tested all the possible permutations of compiling 
things as either builtin or module.
eg:
core Builtin, Driver Builin
core Builtin, Driver Module
core Module,  Driver Module

Changes from v1:
- Add guard around of_i2c_setup_smbus_alert for builds
  that include smbus-alert but are not CONFIG_OF


Phil Reid (3):
  i2c: smbus: move of_i2c_setup_smbus_alert declaration to i2c-core.h
  i2c: Kconfig: rename variable I2C_SMBUS to I2C_SMBUS_ALERT
  i2c: core: move smbus_alert module into i2c-core

 arch/blackfin/configs/BF527-TLL6527M_defconfig |   2 +-
 drivers/i2c/Kconfig                            |  14 +-
 drivers/i2c/Makefile                           |   2 +-
 drivers/i2c/busses/Kconfig                     |   8 +-
 drivers/i2c/i2c-core-base.c                    |   7 +
 drivers/i2c/i2c-core-smbus-alert.c             | 243 +++++++++++++++++++++++++
 drivers/i2c/i2c-core-smbus.c                   |  22 ---
 drivers/i2c/i2c-core.h                         |  22 +++
 drivers/i2c/i2c-smbus.c                        | 217 ----------------------
 drivers/power/supply/Kconfig                   |   2 +-
 include/linux/i2c-smbus.h                      |  10 -
 11 files changed, 284 insertions(+), 265 deletions(-)
 create mode 100644 drivers/i2c/i2c-core-smbus-alert.c
 delete mode 100644 drivers/i2c/i2c-smbus.c

-- 
1.8.3.1

[RFC v2 1/3] i2c: smbus: move of_i2c_setup_smbus_alert declaration to i2c-core.h

[Phil Reid]

This declaration isn't used by anything outside of the i2c-core at
present. In preparation for moving the smbus_alert code from it's
own module into the i2c-core move to i2c-core.h

Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 drivers/i2c/i2c-core.h    |  9 +++++++++
 drivers/i2c/i2c-smbus.c   |  3 ++-
 include/linux/i2c-smbus.h | 10 ----------
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/i2c/i2c-core.h b/drivers/i2c/i2c-core.h
index 3d3d9bf..8ef0402 100644
--- a/drivers/i2c/i2c-core.h
+++ b/drivers/i2c/i2c-core.h
@@ -60,3 +60,12 @@ static inline void i2c_acpi_remove_space_handler(struct i2c_adapter *adapter) {
 static inline void of_i2c_register_devices(struct i2c_adapter *adap) { }
 #endif
 extern struct notifier_block i2c_of_notifier;
+
+#if IS_ENABLED(CONFIG_I2C_SMBUS) && IS_ENABLED(CONFIG_OF)
+int of_i2c_setup_smbus_alert(struct i2c_adapter *adap);
+#else
+static inline int of_i2c_setup_smbus_alert(struct i2c_adapter *adap)
+{
+	return 0;
+}
+#endif
diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
index 5a1dd7f..c11a50e 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -25,6 +25,8 @@
 #include <linux/slab.h>
 #include <linux/workqueue.h>
 
+#include "i2c-core.h"
+
 struct i2c_smbus_alert {
 	struct work_struct	alert;
 	struct i2c_client	*ara;		/* Alert response address */
@@ -123,7 +125,6 @@ static void smbalert_work(struct work_struct *work)
 	alert = container_of(work, struct i2c_smbus_alert, alert);
 
 	smbus_alert(0, alert);
-
 }
 
 /* Setup SMBALERT# infrastructure */
diff --git a/include/linux/i2c-smbus.h b/include/linux/i2c-smbus.h
index fb0e040..5f16a63 100644
--- a/include/linux/i2c-smbus.h
+++ b/include/linux/i2c-smbus.h
@@ -26,7 +26,6 @@
 #include <linux/spinlock.h>
 #include <linux/workqueue.h>
 
-
 /**
  * i2c_smbus_alert_setup - platform data for the smbus_alert i2c client
  * @alert_edge_triggered: whether the alert interrupt is edge (1) or level (0)
@@ -49,13 +48,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);
 
-#if IS_ENABLED(CONFIG_I2C_SMBUS) && IS_ENABLED(CONFIG_OF)
-int of_i2c_setup_smbus_alert(struct i2c_adapter *adap);
-#else
-static inline int of_i2c_setup_smbus_alert(struct i2c_adapter *adap)
-{
-	return 0;
-}
-#endif
-
 #endif /* _LINUX_I2C_SMBUS_H */
-- 
1.8.3.1

[RFC v2 2/3] i2c: Kconfig: rename variable I2C_SMBUS to I2C_SMBUS_ALERT

[Phil Reid]

i2c-smbus now only contains code related to the smbus_alert driver.
Other smbus protocols have been moved from this into the i2c-core-smbus.
Change the Kconfig variable name to reflect this.

Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 arch/blackfin/configs/BF527-TLL6527M_defconfig |  2 +-
 drivers/i2c/Kconfig                            | 11 +++++------
 drivers/i2c/Makefile                           |  2 +-
 drivers/i2c/busses/Kconfig                     |  8 ++++----
 drivers/i2c/i2c-core-smbus.c                   |  2 +-
 drivers/i2c/i2c-core.h                         |  2 +-
 drivers/power/supply/Kconfig                   |  2 +-
 7 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/arch/blackfin/configs/BF527-TLL6527M_defconfig b/arch/blackfin/configs/BF527-TLL6527M_defconfig
index cdeb518..30f3b5c 100644
--- a/arch/blackfin/configs/BF527-TLL6527M_defconfig
+++ b/arch/blackfin/configs/BF527-TLL6527M_defconfig
@@ -109,7 +109,7 @@ CONFIG_SERIAL_BFIN_UART1=y
 # CONFIG_HW_RANDOM is not set
 CONFIG_I2C_CHARDEV=y
 # CONFIG_I2C_HELPER_AUTO is not set
-CONFIG_I2C_SMBUS=y
+CONFIG_I2C_SMBUS_ALERT=y
 CONFIG_I2C_BLACKFIN_TWI=y
 CONFIG_I2C_BLACKFIN_TWI_CLK_KHZ=100
 CONFIG_GPIOLIB=y
diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
index efc3354..fcd4bea 100644
--- a/drivers/i2c/Kconfig
+++ b/drivers/i2c/Kconfig
@@ -55,7 +55,7 @@ config I2C_CHARDEV
 	  programs use the I2C bus.  Information on how to do this is
 	  contained in the file <file:Documentation/i2c/dev-interface>.
 
-	  This support is also available as a module.  If so, the module 
+	  This support is also available as a module.  If so, the module
 	  will be called i2c-dev.
 
 config I2C_MUX
@@ -84,12 +84,11 @@ config I2C_HELPER_AUTO
 
 	  In doubt, say Y.
 
-config I2C_SMBUS
-	tristate "SMBus-specific protocols" if !I2C_HELPER_AUTO
+config I2C_SMBUS_ALERT
+	tristate "SMBus-alert protocol" if !I2C_HELPER_AUTO
 	help
-	  Say Y here if you want support for SMBus extensions to the I2C
-	  specification. At the moment, two extensions are supported:
-	  the SMBus Alert protocol and the SMBus Host Notify protocol.
+	  Say Y here if you want support for SMBus alert extensions to the I2C
+	  specification.
 
 	  This support is also available as a module.  If so, the module
 	  will be called i2c-smbus.
diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
index 7bb65a4..b0116a1 100644
--- a/drivers/i2c/Makefile
+++ b/drivers/i2c/Makefile
@@ -9,7 +9,7 @@ i2c-core-$(CONFIG_ACPI)		+= i2c-core-acpi.o
 i2c-core-$(CONFIG_I2C_SLAVE) 	+= i2c-core-slave.o
 i2c-core-$(CONFIG_OF) 		+= i2c-core-of.o
 
-obj-$(CONFIG_I2C_SMBUS)		+= i2c-smbus.o
+obj-$(CONFIG_I2C_SMBUS_ALERT)	+= i2c-smbus.o
 obj-$(CONFIG_I2C_CHARDEV)	+= i2c-dev.o
 obj-$(CONFIG_I2C_MUX)		+= i2c-mux.o
 obj-y				+= algos/ busses/ muxes/
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 009345d..310d5ec 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -91,7 +91,7 @@ config I2C_I801
 	tristate "Intel 82801 (ICH/PCH)"
 	depends on PCI
 	select CHECK_SIGNATURE if X86 && DMI
-	select I2C_SMBUS
+	select I2C_SMBUS_ALERT
 	help
 	  If you say yes to this option, support will be included for the Intel
 	  801 family of mainboard I2C interfaces.  Specifically, the following
@@ -1056,7 +1056,7 @@ config I2C_OCTEON
 config I2C_THUNDERX
 	tristate "Cavium ThunderX I2C bus support"
 	depends on 64BIT && PCI && (ARM64 || COMPILE_TEST)
-	select I2C_SMBUS
+	select I2C_SMBUS_ALERT
 	help
 	  Say yes if you want to support the I2C serial bus on Cavium
 	  ThunderX SOC.
@@ -1132,7 +1132,7 @@ config I2C_PARPORT
 	tristate "Parallel port adapter"
 	depends on PARPORT
 	select I2C_ALGOBIT
-	select I2C_SMBUS
+	select I2C_SMBUS_ALERT
 	help
 	  This supports parallel port I2C adapters such as the ones made by
 	  Philips or Velleman, Analog Devices evaluation boards, and more.
@@ -1156,7 +1156,7 @@ config I2C_PARPORT
 config I2C_PARPORT_LIGHT
 	tristate "Parallel port adapter (light)"
 	select I2C_ALGOBIT
-	select I2C_SMBUS
+	select I2C_SMBUS_ALERT
 	help
 	  This supports parallel port I2C adapters such as the ones made by
 	  Philips or Velleman, Analog Devices evaluation boards, and more.
diff --git a/drivers/i2c/i2c-core-smbus.c b/drivers/i2c/i2c-core-smbus.c
index 4bb9927..f4c3b18 100644
--- a/drivers/i2c/i2c-core-smbus.c
+++ b/drivers/i2c/i2c-core-smbus.c
@@ -626,7 +626,7 @@ struct i2c_client *i2c_setup_smbus_alert(struct i2c_adapter *adapter,
 }
 EXPORT_SYMBOL_GPL(i2c_setup_smbus_alert);
 
-#if IS_ENABLED(CONFIG_I2C_SMBUS) && IS_ENABLED(CONFIG_OF)
+#if IS_ENABLED(CONFIG_I2C_SMBUS_ALERT) && IS_ENABLED(CONFIG_OF)
 int of_i2c_setup_smbus_alert(struct i2c_adapter *adapter)
 {
 	struct i2c_client *client;
diff --git a/drivers/i2c/i2c-core.h b/drivers/i2c/i2c-core.h
index 8ef0402..2ee5396 100644
--- a/drivers/i2c/i2c-core.h
+++ b/drivers/i2c/i2c-core.h
@@ -61,7 +61,7 @@ static inline void of_i2c_register_devices(struct i2c_adapter *adap) { }
 #endif
 extern struct notifier_block i2c_of_notifier;
 
-#if IS_ENABLED(CONFIG_I2C_SMBUS) && IS_ENABLED(CONFIG_OF)
+#if IS_ENABLED(CONFIG_I2C_SMBUS_ALERT) && IS_ENABLED(CONFIG_OF)
 int of_i2c_setup_smbus_alert(struct i2c_adapter *adap);
 #else
 static inline int of_i2c_setup_smbus_alert(struct i2c_adapter *adap)
diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index fbca0ba..fac8252 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -187,7 +187,7 @@ config CHARGER_SBS
 config MANAGER_SBS
 	tristate "Smart Battery System Manager"
 	depends on I2C && I2C_MUX && GPIOLIB
-	select I2C_SMBUS
+	select I2C_SMBUS_ALERT
 	help
 	  Say Y here to include support for Smart Battery System Manager
 	  ICs. The driver reports online and charging status via sysfs.
-- 
1.8.3.1

[RFC v2 3/3] i2c: core: move smbus_alert module into i2c-core

[Phil Reid]

Remove the i2c-smbus module and move the smbus_alert driver into the
i2c-core. This simplifies the build logic for when i2c-core is builtin
and drivers selecting I2C_SMBUS_ALERT are modules. Also smbus_alert code
is moved from i2c-core-smbus.c to i2c-core-smbus-alert.c

Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 drivers/i2c/Kconfig                |   5 +-
 drivers/i2c/Makefile               |   2 +-
 drivers/i2c/i2c-core-base.c        |   7 ++
 drivers/i2c/i2c-core-smbus-alert.c | 243 +++++++++++++++++++++++++++++++++++++
 drivers/i2c/i2c-core-smbus.c       |  22 ----
 drivers/i2c/i2c-core.h             |  13 ++
 drivers/i2c/i2c-smbus.c            | 218 ---------------------------------
 7 files changed, 265 insertions(+), 245 deletions(-)
 create mode 100644 drivers/i2c/i2c-core-smbus-alert.c
 delete mode 100644 drivers/i2c/i2c-smbus.c

diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
index fcd4bea..486a1f4 100644
--- a/drivers/i2c/Kconfig
+++ b/drivers/i2c/Kconfig
@@ -85,14 +85,11 @@ config I2C_HELPER_AUTO
 	  In doubt, say Y.
 
 config I2C_SMBUS_ALERT
-	tristate "SMBus-alert protocol" if !I2C_HELPER_AUTO
+	bool "SMBus-alert protocol" if !I2C_HELPER_AUTO
 	help
 	  Say Y here if you want support for SMBus alert extensions to the I2C
 	  specification.
 
-	  This support is also available as a module.  If so, the module
-	  will be called i2c-smbus.
-
 source drivers/i2c/algos/Kconfig
 source drivers/i2c/busses/Kconfig
 
diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
index b0116a1..efc3c54c 100644
--- a/drivers/i2c/Makefile
+++ b/drivers/i2c/Makefile
@@ -8,8 +8,8 @@ i2c-core-objs 			:= i2c-core-base.o i2c-core-smbus.o
 i2c-core-$(CONFIG_ACPI)		+= i2c-core-acpi.o
 i2c-core-$(CONFIG_I2C_SLAVE) 	+= i2c-core-slave.o
 i2c-core-$(CONFIG_OF) 		+= i2c-core-of.o
+i2c-core-$(CONFIG_I2C_SMBUS_ALERT)	+= i2c-core-smbus-alert.o
 
-obj-$(CONFIG_I2C_SMBUS_ALERT)	+= i2c-smbus.o
 obj-$(CONFIG_I2C_CHARDEV)	+= i2c-dev.o
 obj-$(CONFIG_I2C_MUX)		+= i2c-mux.o
 obj-y				+= algos/ busses/ muxes/
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 54ffc8d..5788c80 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -1690,6 +1690,10 @@ static int __init i2c_init(void)
 	if (retval)
 		goto class_err;
 
+	retval = i2c_smbus_alert_add_driver();
+	if (retval)
+		goto dummy_err;
+
 	if (IS_ENABLED(CONFIG_OF_DYNAMIC))
 		WARN_ON(of_reconfig_notifier_register(&i2c_of_notifier));
 	if (IS_ENABLED(CONFIG_ACPI))
@@ -1697,6 +1701,8 @@ static int __init i2c_init(void)
 
 	return 0;
 
+dummy_err:
+	i2c_del_driver(&dummy_driver);
 class_err:
 #ifdef CONFIG_I2C_COMPAT
 	class_compat_unregister(i2c_adapter_compat_class);
@@ -1713,6 +1719,7 @@ static void __exit i2c_exit(void)
 		WARN_ON(acpi_reconfig_notifier_unregister(&i2c_acpi_notifier));
 	if (IS_ENABLED(CONFIG_OF_DYNAMIC))
 		WARN_ON(of_reconfig_notifier_unregister(&i2c_of_notifier));
+	i2c_smbus_alert_del_driver();
 	i2c_del_driver(&dummy_driver);
 #ifdef CONFIG_I2C_COMPAT
 	class_compat_unregister(i2c_adapter_compat_class);
diff --git a/drivers/i2c/i2c-core-smbus-alert.c b/drivers/i2c/i2c-core-smbus-alert.c
new file mode 100644
index 0000000..fb5aae6
--- /dev/null
+++ b/drivers/i2c/i2c-core-smbus-alert.c
@@ -0,0 +1,243 @@
+/*
+ * i2c-smbus.c - SMBus extensions to the I2C protocol
+ *
+ * Copyright (C) 2008 David Brownell
+ * Copyright (C) 2010 Jean Delvare <jdelvare@suse.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/i2c-smbus.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_irq.h>
+#include <linux/slab.h>
+#include <linux/workqueue.h>
+
+#include "i2c-core.h"
+
+struct i2c_smbus_alert {
+	struct work_struct	alert;
+	struct i2c_client	*ara;		/* Alert response address */
+};
+
+struct alert_data {
+	unsigned short		addr;
+	enum i2c_alert_protocol	type;
+	unsigned int		data;
+};
+
+/* If this is the alerting device, notify its driver */
+static int smbus_do_alert(struct device *dev, void *addrp)
+{
+	struct i2c_client *client = i2c_verify_client(dev);
+	struct alert_data *data = addrp;
+	struct i2c_driver *driver;
+
+	if (!client || client->addr != data->addr)
+		return 0;
+	if (client->flags & I2C_CLIENT_TEN)
+		return 0;
+
+	/*
+	 * Drivers should either disable alerts, or provide at least
+	 * a minimal handler.  Lock so the driver won't change.
+	 */
+	device_lock(dev);
+	if (client->dev.driver) {
+		driver = to_i2c_driver(client->dev.driver);
+		if (driver->alert)
+			driver->alert(client, data->type, data->data);
+		else
+			dev_warn(&client->dev, "no driver alert()!\n");
+	} else
+		dev_dbg(&client->dev, "alert with no driver\n");
+	device_unlock(dev);
+
+	/* Stop iterating after we find the device */
+	return -EBUSY;
+}
+
+/*
+ * The alert IRQ handler needs to hand work off to a task which can issue
+ * SMBus calls, because those sleeping calls can't be made in IRQ context.
+ */
+static irqreturn_t smbus_alert(int irq, void *d)
+{
+	struct i2c_smbus_alert *alert = d;
+	struct i2c_client *ara;
+	unsigned short prev_addr = 0;	/* Not a valid address */
+
+	ara = alert->ara;
+
+	for (;;) {
+		s32 status;
+		struct alert_data data;
+
+		/*
+		 * Devices with pending alerts reply in address order, low
+		 * to high, because of slave transmit arbitration.  After
+		 * responding, an SMBus device stops asserting SMBALERT#.
+		 *
+		 * Note that SMBus 2.0 reserves 10-bit addresses for future
+		 * use.  We neither handle them, nor try to use PEC here.
+		 */
+		status = i2c_smbus_read_byte(ara);
+		if (status < 0)
+			break;
+
+		data.data = status & 1;
+		data.addr = status >> 1;
+		data.type = I2C_PROTOCOL_SMBUS_ALERT;
+
+		if (data.addr == prev_addr) {
+			dev_warn(&ara->dev, "Duplicate SMBALERT# from dev "
+				"0x%02x, skipping\n", data.addr);
+			break;
+		}
+		dev_dbg(&ara->dev, "SMBALERT# from dev 0x%02x, flag %d\n",
+			data.addr, data.data);
+
+		/* Notify driver for the device which issued the alert */
+		device_for_each_child(&ara->adapter->dev, &data,
+				      smbus_do_alert);
+		prev_addr = data.addr;
+	}
+
+	return IRQ_HANDLED;
+}
+
+static void smbalert_work(struct work_struct *work)
+{
+	struct i2c_smbus_alert *alert;
+
+	alert = container_of(work, struct i2c_smbus_alert, alert);
+
+	smbus_alert(0, alert);
+}
+
+/* Setup SMBALERT# infrastructure */
+static int smbalert_probe(struct i2c_client *ara,
+			  const struct i2c_device_id *id)
+{
+	struct i2c_smbus_alert_setup *setup = dev_get_platdata(&ara->dev);
+	struct i2c_smbus_alert *alert;
+	struct i2c_adapter *adapter = ara->adapter;
+	int res, irq;
+
+	alert = devm_kzalloc(&ara->dev, sizeof(struct i2c_smbus_alert),
+			     GFP_KERNEL);
+	if (!alert)
+		return -ENOMEM;
+
+	if (setup) {
+		irq = setup->irq;
+	} else {
+		irq = of_irq_get_byname(adapter->dev.of_node, "smbus_alert");
+		if (irq <= 0)
+			return irq;
+	}
+
+	INIT_WORK(&alert->alert, smbalert_work);
+	alert->ara = ara;
+
+	if (irq > 0) {
+		res = devm_request_threaded_irq(&ara->dev, irq,
+						NULL, smbus_alert,
+						IRQF_SHARED | IRQF_ONESHOT,
+						"smbus_alert", alert);
+		if (res)
+			return res;
+	}
+
+	i2c_set_clientdata(ara, alert);
+	dev_info(&adapter->dev, "supports SMBALERT#\n");
+
+	return 0;
+}
+
+/* IRQ and memory resources are managed so they are freed automatically */
+static int smbalert_remove(struct i2c_client *ara)
+{
+	struct i2c_smbus_alert *alert = i2c_get_clientdata(ara);
+
+	cancel_work_sync(&alert->alert);
+	return 0;
+}
+
+static const struct i2c_device_id smbalert_ids[] = {
+	{ "smbus_alert", 0 },
+	{ /* LIST END */ }
+};
+MODULE_DEVICE_TABLE(i2c, smbalert_ids);
+
+static struct i2c_driver smbalert_driver = {
+	.driver = {
+		.name	= "smbus_alert",
+	},
+	.probe		= smbalert_probe,
+	.remove		= smbalert_remove,
+	.id_table	= smbalert_ids,
+};
+
+#if IS_ENABLED(CONFIG_OF)
+int of_i2c_setup_smbus_alert(struct i2c_adapter *adapter)
+{
+	struct i2c_client *client;
+	int irq;
+
+	irq = of_property_match_string(adapter->dev.of_node, "interrupt-names",
+				       "smbus_alert");
+	if (irq == -EINVAL || irq == -ENODATA)
+		return 0;
+	else if (irq < 0)
+		return irq;
+
+	client = i2c_setup_smbus_alert(adapter, NULL);
+	if (!client)
+		return -ENODEV;
+
+	return 0;
+}
+#endif
+
+/**
+ * i2c_handle_smbus_alert - Handle an SMBus alert
+ * @ara: the ARA client on the relevant adapter
+ * Context: can't sleep
+ *
+ * Helper function to be called from an I2C bus driver's interrupt
+ * handler. It will schedule the alert work, in turn calling the
+ * corresponding I2C device driver's alert function.
+ *
+ * It is assumed that ara is a valid i2c client previously returned by
+ * i2c_setup_smbus_alert().
+ */
+int i2c_handle_smbus_alert(struct i2c_client *ara)
+{
+	struct i2c_smbus_alert *alert = i2c_get_clientdata(ara);
+
+	return schedule_work(&alert->alert);
+}
+EXPORT_SYMBOL_GPL(i2c_handle_smbus_alert);
+
+int i2c_smbus_alert_add_driver(void)
+{
+	return i2c_add_driver(&smbalert_driver);
+}
+
+void i2c_smbus_alert_del_driver(void)
+{
+	i2c_del_driver(&smbalert_driver);
+}
diff --git a/drivers/i2c/i2c-core-smbus.c b/drivers/i2c/i2c-core-smbus.c
index f4c3b18..7f3ec02 100644
--- a/drivers/i2c/i2c-core-smbus.c
+++ b/drivers/i2c/i2c-core-smbus.c
@@ -625,25 +625,3 @@ struct i2c_client *i2c_setup_smbus_alert(struct i2c_adapter *adapter,
 	return i2c_new_device(adapter, &ara_board_info);
 }
 EXPORT_SYMBOL_GPL(i2c_setup_smbus_alert);
-
-#if IS_ENABLED(CONFIG_I2C_SMBUS_ALERT) && IS_ENABLED(CONFIG_OF)
-int of_i2c_setup_smbus_alert(struct i2c_adapter *adapter)
-{
-	struct i2c_client *client;
-	int irq;
-
-	irq = of_property_match_string(adapter->dev.of_node, "interrupt-names",
-				       "smbus_alert");
-	if (irq == -EINVAL || irq == -ENODATA)
-		return 0;
-	else if (irq < 0)
-		return irq;
-
-	client = i2c_setup_smbus_alert(adapter, NULL);
-	if (!client)
-		return -ENODEV;
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(of_i2c_setup_smbus_alert);
-#endif
diff --git a/drivers/i2c/i2c-core.h b/drivers/i2c/i2c-core.h
index 2ee5396..c6873f4 100644
--- a/drivers/i2c/i2c-core.h
+++ b/drivers/i2c/i2c-core.h
@@ -69,3 +69,16 @@ static inline int of_i2c_setup_smbus_alert(struct i2c_adapter *adap)
 	return 0;
 }
 #endif
+
+#if IS_ENABLED(CONFIG_I2C_SMBUS_ALERT)
+int i2c_smbus_alert_add_driver(void);
+void i2c_smbus_alert_del_driver(void);
+#else
+static inline int i2c_smbus_alert_add_driver(void)
+{
+	return 0;
+}
+static inline void i2c_smbus_alert_del_driver(void)
+{
+}
+#endif
diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
deleted file mode 100644
index c11a50e..0000000
--- a/drivers/i2c/i2c-smbus.c
+++ /dev/null
@@ -1,218 +0,0 @@
-/*
- * i2c-smbus.c - SMBus extensions to the I2C protocol
- *
- * Copyright (C) 2008 David Brownell
- * Copyright (C) 2010 Jean Delvare <jdelvare@suse.de>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-
-#include <linux/device.h>
-#include <linux/i2c.h>
-#include <linux/i2c-smbus.h>
-#include <linux/interrupt.h>
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/of_irq.h>
-#include <linux/slab.h>
-#include <linux/workqueue.h>
-
-#include "i2c-core.h"
-
-struct i2c_smbus_alert {
-	struct work_struct	alert;
-	struct i2c_client	*ara;		/* Alert response address */
-};
-
-struct alert_data {
-	unsigned short		addr;
-	enum i2c_alert_protocol	type;
-	unsigned int		data;
-};
-
-/* If this is the alerting device, notify its driver */
-static int smbus_do_alert(struct device *dev, void *addrp)
-{
-	struct i2c_client *client = i2c_verify_client(dev);
-	struct alert_data *data = addrp;
-	struct i2c_driver *driver;
-
-	if (!client || client->addr != data->addr)
-		return 0;
-	if (client->flags & I2C_CLIENT_TEN)
-		return 0;
-
-	/*
-	 * Drivers should either disable alerts, or provide at least
-	 * a minimal handler.  Lock so the driver won't change.
-	 */
-	device_lock(dev);
-	if (client->dev.driver) {
-		driver = to_i2c_driver(client->dev.driver);
-		if (driver->alert)
-			driver->alert(client, data->type, data->data);
-		else
-			dev_warn(&client->dev, "no driver alert()!\n");
-	} else
-		dev_dbg(&client->dev, "alert with no driver\n");
-	device_unlock(dev);
-
-	/* Stop iterating after we find the device */
-	return -EBUSY;
-}
-
-/*
- * The alert IRQ handler needs to hand work off to a task which can issue
- * SMBus calls, because those sleeping calls can't be made in IRQ context.
- */
-static irqreturn_t smbus_alert(int irq, void *d)
-{
-	struct i2c_smbus_alert *alert = d;
-	struct i2c_client *ara;
-	unsigned short prev_addr = 0;	/* Not a valid address */
-
-	ara = alert->ara;
-
-	for (;;) {
-		s32 status;
-		struct alert_data data;
-
-		/*
-		 * Devices with pending alerts reply in address order, low
-		 * to high, because of slave transmit arbitration.  After
-		 * responding, an SMBus device stops asserting SMBALERT#.
-		 *
-		 * Note that SMBus 2.0 reserves 10-bit addresses for future
-		 * use.  We neither handle them, nor try to use PEC here.
-		 */
-		status = i2c_smbus_read_byte(ara);
-		if (status < 0)
-			break;
-
-		data.data = status & 1;
-		data.addr = status >> 1;
-		data.type = I2C_PROTOCOL_SMBUS_ALERT;
-
-		if (data.addr == prev_addr) {
-			dev_warn(&ara->dev, "Duplicate SMBALERT# from dev "
-				"0x%02x, skipping\n", data.addr);
-			break;
-		}
-		dev_dbg(&ara->dev, "SMBALERT# from dev 0x%02x, flag %d\n",
-			data.addr, data.data);
-
-		/* Notify driver for the device which issued the alert */
-		device_for_each_child(&ara->adapter->dev, &data,
-				      smbus_do_alert);
-		prev_addr = data.addr;
-	}
-
-	return IRQ_HANDLED;
-}
-
-static void smbalert_work(struct work_struct *work)
-{
-	struct i2c_smbus_alert *alert;
-
-	alert = container_of(work, struct i2c_smbus_alert, alert);
-
-	smbus_alert(0, alert);
-}
-
-/* Setup SMBALERT# infrastructure */
-static int smbalert_probe(struct i2c_client *ara,
-			  const struct i2c_device_id *id)
-{
-	struct i2c_smbus_alert_setup *setup = dev_get_platdata(&ara->dev);
-	struct i2c_smbus_alert *alert;
-	struct i2c_adapter *adapter = ara->adapter;
-	int res, irq;
-
-	alert = devm_kzalloc(&ara->dev, sizeof(struct i2c_smbus_alert),
-			     GFP_KERNEL);
-	if (!alert)
-		return -ENOMEM;
-
-	if (setup) {
-		irq = setup->irq;
-	} else {
-		irq = of_irq_get_byname(adapter->dev.of_node, "smbus_alert");
-		if (irq <= 0)
-			return irq;
-	}
-
-	INIT_WORK(&alert->alert, smbalert_work);
-	alert->ara = ara;
-
-	if (irq > 0) {
-		res = devm_request_threaded_irq(&ara->dev, irq,
-						NULL, smbus_alert,
-						IRQF_SHARED | IRQF_ONESHOT,
-						"smbus_alert", alert);
-		if (res)
-			return res;
-	}
-
-	i2c_set_clientdata(ara, alert);
-	dev_info(&adapter->dev, "supports SMBALERT#\n");
-
-	return 0;
-}
-
-/* IRQ and memory resources are managed so they are freed automatically */
-static int smbalert_remove(struct i2c_client *ara)
-{
-	struct i2c_smbus_alert *alert = i2c_get_clientdata(ara);
-
-	cancel_work_sync(&alert->alert);
-	return 0;
-}
-
-static const struct i2c_device_id smbalert_ids[] = {
-	{ "smbus_alert", 0 },
-	{ /* LIST END */ }
-};
-MODULE_DEVICE_TABLE(i2c, smbalert_ids);
-
-static struct i2c_driver smbalert_driver = {
-	.driver = {
-		.name	= "smbus_alert",
-	},
-	.probe		= smbalert_probe,
-	.remove		= smbalert_remove,
-	.id_table	= smbalert_ids,
-};
-
-/**
- * i2c_handle_smbus_alert - Handle an SMBus alert
- * @ara: the ARA client on the relevant adapter
- * Context: can't sleep
- *
- * Helper function to be called from an I2C bus driver's interrupt
- * handler. It will schedule the alert work, in turn calling the
- * corresponding I2C device driver's alert function.
- *
- * It is assumed that ara is a valid i2c client previously returned by
- * i2c_setup_smbus_alert().
- */
-int i2c_handle_smbus_alert(struct i2c_client *ara)
-{
-	struct i2c_smbus_alert *alert = i2c_get_clientdata(ara);
-
-	return schedule_work(&alert->alert);
-}
-EXPORT_SYMBOL_GPL(i2c_handle_smbus_alert);
-
-module_i2c_driver(smbalert_driver);
-
-MODULE_AUTHOR("Jean Delvare <jdelvare@suse.de>");
-MODULE_DESCRIPTION("SMBus protocol extensions support");
-MODULE_LICENSE("GPL");
-- 
1.8.3.1

Re: [RFC v2 2/3] i2c: Kconfig: rename variable I2C_SMBUS to I2C_SMBUS_ALERT

[Jean Delvare]

Hi Phil,

On Tue, 28 Nov 2017 11:38:28 +0800, Phil Reid wrote:
> i2c-smbus now only contains code related to the smbus_alert driver.
> Other smbus protocols have been moved from this into the i2c-core-smbus.
> Change the Kconfig variable name to reflect this.

Not really. i2c-core-smbus.c contains essentially SMBus transaction
helpers, which have never been in i2c-smbus.c. They aren't really SMBus
protocols, more like a subset of I2C transactions suitable for general
purpose. The only really SMBus protocol specific portion is relative to
SMBus Alert, and that's only a small part of it. The other supported
SMBus-specific protocol, Host Notify, is in i2c-core-base.c.

My original intent was to have all the SMBus protocols specific code in
one file, controlled by one Kconfig option, which could be built as a
separate module. I wasn't certain it would be viable, it was a bet
which I lost, as it turns out there are too many dependencies.

If the code can no longer build as a separate module, there is no
benefit in having it in a separate file. Let's just merge it into
i2c-code-smbus.c, where the rest of the SMBus alert code already is.
That would make things more simple.

I also don't think renaming this configuration option and moving code
to a separate file (as your patch 3/3 does) makes sense. Having a
separate option for each SMBus-specific protocol seems overkill to me,
having one for all of them was and still is more reasonable. I wonder
why the SMBus Notify code does NOT depend on CONFIG_I2C_SMBUS, and why
it is in i2c-core-base.c instead of i2c-core-smbus.c. Wolfram?

And more generally, renaming a Kconfig option has a non-trivial cost
for distribution kernel maintainers, so it shall not be done lightly.
So you need a clear and strong rationale.

> Signed-off-by: Phil Reid <preid@electromag.com.au>
> ---
>  arch/blackfin/configs/BF527-TLL6527M_defconfig |  2 +-
>  drivers/i2c/Kconfig                            | 11 +++++------
>  drivers/i2c/Makefile                           |  2 +-
>  drivers/i2c/busses/Kconfig                     |  8 ++++----
>  drivers/i2c/i2c-core-smbus.c                   |  2 +-
>  drivers/i2c/i2c-core.h                         |  2 +-
>  drivers/power/supply/Kconfig                   |  2 +-
>  7 files changed, 14 insertions(+), 15 deletions(-)
> (...)
> diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
> index efc3354..fcd4bea 100644
> --- a/drivers/i2c/Kconfig
> +++ b/drivers/i2c/Kconfig
> @@ -55,7 +55,7 @@ config I2C_CHARDEV
>  	  programs use the I2C bus.  Information on how to do this is
>  	  contained in the file <file:Documentation/i2c/dev-interface>.
>  
> -	  This support is also available as a module.  If so, the module 
> +	  This support is also available as a module.  If so, the module
>  	  will be called i2c-dev.
>  
>  config I2C_MUX

Please don't mix white-space clean-ups with actual changes. It might
be tolerated by some maintainers if within a chunk you are already
touching. But if such a change creates a new patch chunk then it's a
no-go.

Your patch 1/3 suffers from similar issues.

> @@ -84,12 +84,11 @@ config I2C_HELPER_AUTO
>  
>  	  In doubt, say Y.
>  
> -config I2C_SMBUS
> -	tristate "SMBus-specific protocols" if !I2C_HELPER_AUTO
> +config I2C_SMBUS_ALERT
> +	tristate "SMBus-alert protocol" if !I2C_HELPER_AUTO

"SMBus Alert" is the correct spelling.

>  	help
> -	  Say Y here if you want support for SMBus extensions to the I2C
> -	  specification. At the moment, two extensions are supported:
> -	  the SMBus Alert protocol and the SMBus Host Notify protocol.
> +	  Say Y here if you want support for SMBus alert extensions to the I2C
> +	  specification.

"extension" without "s".

>  
>  	  This support is also available as a module.  If so, the module
>  	  will be called i2c-smbus.

-- 
Jean Delvare
SUSE L3 Support

Re: [RFC v2 2/3] i2c: Kconfig: rename variable I2C_SMBUS to I2C_SMBUS_ALERT

[Phil Reid]

G'day Jean,

Thanks for looking at this.

On 29/11/2017 06:08, Jean Delvare wrote:
> Hi Phil,
> 
> On Tue, 28 Nov 2017 11:38:28 +0800, Phil Reid wrote:
>> i2c-smbus now only contains code related to the smbus_alert driver.
>> Other smbus protocols have been moved from this into the i2c-core-smbus.
>> Change the Kconfig variable name to reflect this.
> 
> Not really. i2c-core-smbus.c contains essentially SMBus transaction
> helpers, which have never been in i2c-smbus.c. They aren't really SMBus
> protocols, more like a subset of I2C transactions suitable for general
> purpose. The only really SMBus protocol specific portion is relative to
> SMBus Alert, and that's only a small part of it. The other supported
> SMBus-specific protocol, Host Notify, is in i2c-core-base.c.
> 
> My original intent was to have all the SMBus protocols specific code in
> one file, controlled by one Kconfig option, which could be built as a
> separate module. I wasn't certain it would be viable, it was a bet
> which I lost, as it turns out there are too many dependencies.
> 
> If the code can no longer build as a separate module, there is no
> benefit in having it in a separate file. Let's just merge it into
> i2c-code-smbus.c, where the rest of the SMBus alert code already is.
> That would make things more simple.
I had to put of_i2c_setup_smbus_alert in there to deal with dependencies.
This was seen as a little messy at the time. It was accepted provided
that I clean things up somehow.

The question is how. This series is just one proposal for that to stir some
discussion. Your approach is certainly the simpler one.

> 
> I also don't think renaming this configuration option and moving code
> to a separate file (as your patch 3/3 does) makes sense. Having a
> separate option for each SMBus-specific protocol seems overkill to me,
> having one for all of them was and still is more reasonable. I wonder
> why the SMBus Notify code does NOT depend on CONFIG_I2C_SMBUS, and why
> it is in i2c-core-base.c instead of i2c-core-smbus.c. Wolfram?
> 
> And more generally, renaming a Kconfig option has a non-trivial cost
> for distribution kernel maintainers, so it shall not be done lightly.
> So you need a clear and strong rationale.

My only rational is that it didn't encompass all the SMBUS specific protocols.
It currently seems a little ambiguous to me. Especially with regard the current
Kconfig help description is not accurate with regard the Host Notify protocol.

> 
>> Signed-off-by: Phil Reid <preid@electromag.com.au>
>> ---
>>   arch/blackfin/configs/BF527-TLL6527M_defconfig |  2 +-
>>   drivers/i2c/Kconfig                            | 11 +++++------
>>   drivers/i2c/Makefile                           |  2 +-
>>   drivers/i2c/busses/Kconfig                     |  8 ++++----
>>   drivers/i2c/i2c-core-smbus.c                   |  2 +-
>>   drivers/i2c/i2c-core.h                         |  2 +-
>>   drivers/power/supply/Kconfig                   |  2 +-
>>   7 files changed, 14 insertions(+), 15 deletions(-)
>> (...)
>> diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
>> index efc3354..fcd4bea 100644
>> --- a/drivers/i2c/Kconfig
>> +++ b/drivers/i2c/Kconfig
>> @@ -55,7 +55,7 @@ config I2C_CHARDEV
>>   	  programs use the I2C bus.  Information on how to do this is
>>   	  contained in the file <file:Documentation/i2c/dev-interface>.
>>   
>> -	  This support is also available as a module.  If so, the module
>> +	  This support is also available as a module.  If so, the module
>>   	  will be called i2c-dev.
>>   
>>   config I2C_MUX
> 
> Please don't mix white-space clean-ups with actual changes. It might
> be tolerated by some maintainers if within a chunk you are already
> touching. But if such a change creates a new patch chunk then it's a
> no-go.
> 
> Your patch 1/3 suffers from similar issues.
> 
>> @@ -84,12 +84,11 @@ config I2C_HELPER_AUTO
>>   
>>   	  In doubt, say Y.
>>   
>> -config I2C_SMBUS
>> -	tristate "SMBus-specific protocols" if !I2C_HELPER_AUTO
>> +config I2C_SMBUS_ALERT
>> +	tristate "SMBus-alert protocol" if !I2C_HELPER_AUTO
> 
> "SMBus Alert" is the correct spelling.
> 
>>   	help
>> -	  Say Y here if you want support for SMBus extensions to the I2C
>> -	  specification. At the moment, two extensions are supported:
>> -	  the SMBus Alert protocol and the SMBus Host Notify protocol.
>> +	  Say Y here if you want support for SMBus alert extensions to the I2C
>> +	  specification.
> 
> "extension" without "s".
> 
>>   
>>   	  This support is also available as a module.  If so, the module
>>   	  will be called i2c-smbus.
> 


-- 
Regards
Phil Reid

Re: [RFC v2 2/3] i2c: Kconfig: rename variable I2C_SMBUS to I2C_SMBUS_ALERT

[Jean Delvare]

On Wed, 29 Nov 2017 09:58:44 +0800, Phil Reid wrote:
> G'day Jean,
> 
> Thanks for looking at this.
> 
> On 29/11/2017 06:08, Jean Delvare wrote:
> > Hi Phil,
> > 
> > On Tue, 28 Nov 2017 11:38:28 +0800, Phil Reid wrote:  
> >> i2c-smbus now only contains code related to the smbus_alert driver.
> >> Other smbus protocols have been moved from this into the i2c-core-smbus.
> >> Change the Kconfig variable name to reflect this.  
> > 
> > Not really. i2c-core-smbus.c contains essentially SMBus transaction
> > helpers, which have never been in i2c-smbus.c. They aren't really SMBus
> > protocols, more like a subset of I2C transactions suitable for general
> > purpose. The only really SMBus protocol specific portion is relative to
> > SMBus Alert, and that's only a small part of it. The other supported
> > SMBus-specific protocol, Host Notify, is in i2c-core-base.c.
> > 
> > My original intent was to have all the SMBus protocols specific code in
> > one file, controlled by one Kconfig option, which could be built as a
> > separate module. I wasn't certain it would be viable, it was a bet
> > which I lost, as it turns out there are too many dependencies.
> > 
> > If the code can no longer build as a separate module, there is no
> > benefit in having it in a separate file. Let's just merge it into
> > i2c-code-smbus.c, where the rest of the SMBus alert code already is.
> > That would make things more simple.  
> I had to put of_i2c_setup_smbus_alert in there to deal with dependencies.
> This was seen as a little messy at the time. It was accepted provided
> that I clean things up somehow.
> 
> The question is how. This series is just one proposal for that to stir some
> discussion. Your approach is certainly the simpler one.

OK, can you please give it a try and wee how it compares with your own
approach?

> > I also don't think renaming this configuration option and moving code
> > to a separate file (as your patch 3/3 does) makes sense. Having a
> > separate option for each SMBus-specific protocol seems overkill to me,
> > having one for all of them was and still is more reasonable. I wonder
> > why the SMBus Notify code does NOT depend on CONFIG_I2C_SMBUS, and why
> > it is in i2c-core-base.c instead of i2c-core-smbus.c. Wolfram?
> > 
> > And more generally, renaming a Kconfig option has a non-trivial cost
> > for distribution kernel maintainers, so it shall not be done lightly.
> > So you need a clear and strong rationale.  
> 
> My only rational is that it didn't encompass all the SMBUS specific protocols.
> It currently seems a little ambiguous to me. Especially with regard the current
> Kconfig help description is not accurate with regard the Host Notify protocol.

I agree, the I2C_SMBUS Kconfig help text no longer reflects the reality
since over a year now. But I think the text is how things should be and
the code is what should be fixed (that is, all the SMBus Host Notify
code should be left out when I2C_SMBUS isn't set, and it should live in
i2c-core-smbus.c.) But I did not look at the details, maybe this isn't
possible for some reason. Maybe you can look into it after you are done
with moving the SMBus Alert code into the i2c core? If you don't, I
will.

Thanks,
-- 
Jean Delvare
SUSE L3 Support

Re: [RFC v2 2/3] i2c: Kconfig: rename variable I2C_SMBUS to I2C_SMBUS_ALERT

[Phil Reid]

On 29/11/2017 20:48, Jean Delvare wrote:
> On Wed, 29 Nov 2017 09:58:44 +0800, Phil Reid wrote:
>> G'day Jean,
>>
>> Thanks for looking at this.
>>
>> On 29/11/2017 06:08, Jean Delvare wrote:
>>> Hi Phil,
>>>
>>> On Tue, 28 Nov 2017 11:38:28 +0800, Phil Reid wrote:
>>>> i2c-smbus now only contains code related to the smbus_alert driver.
>>>> Other smbus protocols have been moved from this into the i2c-core-smbus.
>>>> Change the Kconfig variable name to reflect this.
>>>
>>> Not really. i2c-core-smbus.c contains essentially SMBus transaction
>>> helpers, which have never been in i2c-smbus.c. They aren't really SMBus
>>> protocols, more like a subset of I2C transactions suitable for general
>>> purpose. The only really SMBus protocol specific portion is relative to
>>> SMBus Alert, and that's only a small part of it. The other supported
>>> SMBus-specific protocol, Host Notify, is in i2c-core-base.c.
>>>
>>> My original intent was to have all the SMBus protocols specific code in
>>> one file, controlled by one Kconfig option, which could be built as a
>>> separate module. I wasn't certain it would be viable, it was a bet
>>> which I lost, as it turns out there are too many dependencies.
>>>
>>> If the code can no longer build as a separate module, there is no
>>> benefit in having it in a separate file. Let's just merge it into
>>> i2c-code-smbus.c, where the rest of the SMBus alert code already is.
>>> That would make things more simple.
>> I had to put of_i2c_setup_smbus_alert in there to deal with dependencies.
>> This was seen as a little messy at the time. It was accepted provided
>> that I clean things up somehow.
>>
>> The question is how. This series is just one proposal for that to stir some
>> discussion. Your approach is certainly the simpler one.
> 
> OK, can you please give it a try and wee how it compares with your own
> approach?

Should be able to get to it next week.

> 
>>> I also don't think renaming this configuration option and moving code
>>> to a separate file (as your patch 3/3 does) makes sense. Having a
>>> separate option for each SMBus-specific protocol seems overkill to me,
>>> having one for all of them was and still is more reasonable. I wonder
>>> why the SMBus Notify code does NOT depend on CONFIG_I2C_SMBUS, and why
>>> it is in i2c-core-base.c instead of i2c-core-smbus.c. Wolfram?
>>>
>>> And more generally, renaming a Kconfig option has a non-trivial cost
>>> for distribution kernel maintainers, so it shall not be done lightly.
>>> So you need a clear and strong rationale.
>>
>> My only rational is that it didn't encompass all the SMBUS specific protocols.
>> It currently seems a little ambiguous to me. Especially with regard the current
>> Kconfig help description is not accurate with regard the Host Notify protocol.
> 
> I agree, the I2C_SMBUS Kconfig help text no longer reflects the reality
> since over a year now. But I think the text is how things should be and
> the code is what should be fixed (that is, all the SMBus Host Notify
> code should be left out when I2C_SMBUS isn't set, and it should live in
> i2c-core-smbus.c.) But I did not look at the details, maybe this isn't
> possible for some reason. Maybe you can look into it after you are done
> with moving the SMBus Alert code into the i2c core? If you don't, I
> will.
> 
> Thanks,
> 


-- 
Regards
Phil Reid