[PATCH v11 00/10] Add sbs-manager with smbalert support

[PATCH v11 00/10] Add sbs-manager with smbalert support

[Phil Reid]

This is another go of the sbs-manager driver using smbalert for
irq support from a while ago. 

Enables the existing smbalert driver to be loaded via the device tree.
Only need to add smbus_alert interrupt to the i2c bus segement in the
devicetree and the core will then enable the alert driver.

Reorders the rquest irq call in the pca954x driver to ensure each
muxed i2c segment can handle service smbalerts on that segment before
irq's are enabled. The pca954x can't mask individual irq's routed thru
them.

Add the sbs-manager from Karl-Heinz.
Add the alert call back and gpio interface to allow the battery detect
logic in the existing sbs-battery driver to work.

Changes from v5:
- Documentation: Add sbs-manager device tree node documentation
  - Use same style as sbs-charger for compatible property. 
- power: Adds support for Smart Battery System Manager
  - reorder kconfig / makefile
  - remove errouinous le16 to cpu conversions
  - while loops to for loops
  - formating changes to error messages
  - changed sbsm_set_proprty indentation (hopefully I got it right)
  - removed CONFIG_OF conditional around of_device_id table
  - ENODEV -> EINVAL in probe function for mismatched address
  - Use BIT() macro in probe function
  - add of_node assignment in probe function
  - remove owner assignament and set of_match_table
- power: supply: sbs-battery: Add alert callback
  - Removed patch as Sebastian has queued it.
- power: supply: sbs-manager: Add alert callback and battery change notification
  - Use device_property_present instead of of_get_property
  - Add depends on GPIOLIB

Changes from v6
- Add 2 patches to remove incorrect le16_to_cpu calls in bq24735 & sbs-battery
  this was identifed in review of v6
- i2c: i2c-smbus: Use threaded irq for smbalert
  - remove alert_edge_triggered flag, see new description
  - rework the work thread and threaded irq,commit log has more details
  - Update in tree drivers where required (untested)
- i2c: i2c-smbus: add of_i2c_setup_smbus_alert
  - Add Rob's ack for doc binding
  - rework of_i2c_setup_smbus_alert so that it doesn't need to alloc memory
    addressing concern about devres allocation.
    Probe function looks up the irq number if platform data isn't defined.
- i2c: core: call of_i2c_setup_smbus_alert in i2c_register_adapter
  - investigate if the core will release the client
    It looks like it will to me, in i2c_del_adapter it iterates thru all clients
    and calls __unregister_client.
- i2c: mux: pca954x: Call request irq after adding mux segments
  - fix logic in guard for request irq
  - fix identation
  - add check to irq_create_mapping call
- Documentation: Add sbs-manager device tree node documentation
  - Remove leading 0's
  - Add Rob's ack
- power: Adds support for Smart Battery System Manager
  - remove inc header <linux/of_device.h>
  - add macro defines for various bit and masks.
  - refactor loop around i2c_mux_add_adapter 
  - Add ifdef CONFIG_OF around OF device table to save some bytes 
- power: supply: sbs-manager: Add alert callback and battery change notification
  - Add Sebastian's ack for binding
- Added new patch
  - power: supply: sbs-battery: move gpio present detect to sbs_get_property

Changes from v7
- Remove 2 patches that have been applied for incorrect le16_to_cpu
- i2c: i2c-smbus: Use threaded irq for smbalert
  - Added reviewed by Benjamin
- i2c: i2c-smbus: add of_i2c_setup_smbus_alert
  - Remove rename of variable, use adapater instead of adap in new function
  - Add CONFIG_OF guard 
- i2c: core: call of_i2c_setup_smbus_alert in i2c_register_adapter
  - Move call to just after register call and add error cleanup
  - Didn't split this as per review comments as the 1 error 
    of_property_match_string can return seems pretty unlikely.
    I don't think there's much benefit to it.
- i2c: mux: pca954x: Call request irq after adding mux segments
  - split into two patches. 
    pt1 as acked by Peter
    pt2 the extra error handling for irq_create_mapping as suggested in his review
- The rest
  - Added reviewed by Sebastian
    
  
Changes from v8
- i2c: mux: pca954x: Return error if irq_create_mapping fails
  - Add Peter's ack
  - change return check to if (!irq)

Changes from v9
- Move i2c_setup_smbbus_alert & of_i2c_setup_smbus_alert to
  i2c-core-smbus to resolve linking issues with modules.
  No functional changes.
- i2c: mux: pca954x: Call request irq after adding mux segments
  While testing above changes noticed a warning about unbalanced
  irq_enable / disable. This is resolved by remove the irq_mask
  irq_unmask functions that where an inital workaroudn to the
  problem this patch now fixes by delaying request irq.
  Removed Peter's ack on this one as it's changed.

Changes from v10
- i2c: mux: pca954x: Call request irq after adding mux segments 
  Added Peters ack
  renamed label 'fail_del_adapters' to 'fail_cleanup'

Karl-Heinz Schneider (2):
  Documentation: Add sbs-manager device tree node documentation
  power: Adds support for Smart Battery System Manager

Phil Reid (8):
  i2c: i2c-smbus: Use threaded irq for smbalert
  i2c: i2c-smbus: Move i2c_setup_smbus_alert from i2c-smbus to
    i2c-core-smbus
  i2c: i2c-smbus: add of_i2c_setup_smbus_alert
  i2c: core: call of_i2c_setup_smbus_alert in i2c_register_adapter
  i2c: mux: pca954x: call request irq after adding mux segments
  i2c: mux: pca954x: Return error if irq_create_mapping fails
  power: supply: sbs-manager: Add alert callback and battery change
    notification
  power: supply: sbs-battery: move gpio present detect to
    sbs_get_property

 Documentation/devicetree/bindings/i2c/i2c.txt      |   4 +-
 .../bindings/power/supply/sbs,sbs-manager.txt      |  66 +++
 drivers/i2c/busses/i2c-parport-light.c             |   1 -
 drivers/i2c/busses/i2c-parport.c                   |   1 -
 drivers/i2c/busses/i2c-thunderx-pcidrv.c           |   6 -
 drivers/i2c/i2c-core-base.c                        |   9 +
 drivers/i2c/i2c-core-smbus.c                       |  55 +++
 drivers/i2c/i2c-smbus.c                            |  81 ++--
 drivers/i2c/muxes/i2c-mux-pca954x.c                |  95 ++---
 drivers/power/supply/Kconfig                       |  14 +
 drivers/power/supply/Makefile                      |   1 +
 drivers/power/supply/sbs-battery.c                 |  23 +-
 drivers/power/supply/sbs-manager.c                 | 444 +++++++++++++++++++++
 include/linux/i2c-smbus.h                          |  10 +-
 14 files changed, 670 insertions(+), 140 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.txt
 create mode 100644 drivers/power/supply/sbs-manager.c

-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v11 01/10] i2c: i2c-smbus: Use threaded irq for smbalert

[Phil Reid]

Prior to this commit the smbalert_irq was handling in the hard irq
context. This change switch to using a thread irq which avoids the need
for the work thread. Using threaded irq also removes the need for the
edge_triggered flag as the enabling / disabling of the hard irq for level
triggered interrupts will be handled by the irq core.

Without this change have an irq connected to something like an i2c gpio
resulted in a null ptr deferences. Specifically handle_nested_irq calls
the threaded irq handler.

There are currently 3 in tree drivers affected by this change.

i2c-parport driver calls i2c_handle_smbus_alert in a hard irq context.
This driver use edge trigger interrupts which skip the enable / disable
calls. But it still need to handle the smbus transaction on a thread. So
the work thread is kept for this driver.

i2c-parport-light & i2c-thunderx-pcidrv provide the irq number in the
setup which will result in the thread irq being used.

i2c-parport-light is edge trigger so the enable / disable call was
skipped as well.

i2c-thunderx-pcidrv is getting the edge / level trigger setting from of
data and was setting the flag as required. However the irq core should
handle this automatically.

Signed-off-by: Phil Reid <preid@electromag.com.au>
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/i2c/busses/i2c-parport-light.c   |  1 -
 drivers/i2c/busses/i2c-parport.c         |  1 -
 drivers/i2c/busses/i2c-thunderx-pcidrv.c |  6 -----
 drivers/i2c/i2c-smbus.c                  | 41 +++++++++++++-------------------
 include/linux/i2c-smbus.h                |  1 -
 5 files changed, 17 insertions(+), 33 deletions(-)

diff --git a/drivers/i2c/busses/i2c-parport-light.c b/drivers/i2c/busses/i2c-parport-light.c
index faa8fb8..fa41ff7 100644
--- a/drivers/i2c/busses/i2c-parport-light.c
+++ b/drivers/i2c/busses/i2c-parport-light.c
@@ -123,7 +123,6 @@ static int parport_getsda(void *data)
 
 /* SMBus alert support */
 static struct i2c_smbus_alert_setup alert_data = {
-	.alert_edge_triggered	= 1,
 };
 static struct i2c_client *ara;
 static struct lineop parport_ctrl_irq = {
diff --git a/drivers/i2c/busses/i2c-parport.c b/drivers/i2c/busses/i2c-parport.c
index a8e54df..319209a 100644
--- a/drivers/i2c/busses/i2c-parport.c
+++ b/drivers/i2c/busses/i2c-parport.c
@@ -237,7 +237,6 @@ static void i2c_parport_attach(struct parport *port)
 
 	/* Setup SMBus alert if supported */
 	if (adapter_parm[type].smbus_alert) {
-		adapter->alert_data.alert_edge_triggered = 1;
 		adapter->ara = i2c_setup_smbus_alert(&adapter->adapter,
 						     &adapter->alert_data);
 		if (adapter->ara)
diff --git a/drivers/i2c/busses/i2c-thunderx-pcidrv.c b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
index ea35a895..925da84 100644
--- a/drivers/i2c/busses/i2c-thunderx-pcidrv.c
+++ b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
@@ -118,8 +118,6 @@ static void thunder_i2c_clock_disable(struct device *dev, struct clk *clk)
 static int thunder_i2c_smbus_setup_of(struct octeon_i2c *i2c,
 				      struct device_node *node)
 {
-	u32 type;
-
 	if (!node)
 		return -EINVAL;
 
@@ -127,10 +125,6 @@ static int thunder_i2c_smbus_setup_of(struct octeon_i2c *i2c,
 	if (!i2c->alert_data.irq)
 		return -EINVAL;
 
-	type = irqd_get_trigger_type(irq_get_irq_data(i2c->alert_data.irq));
-	i2c->alert_data.alert_edge_triggered =
-		(type & IRQ_TYPE_LEVEL_MASK) ? 1 : 0;
-
 	i2c->ara = i2c_setup_smbus_alert(&i2c->adap, &i2c->alert_data);
 	if (!i2c->ara)
 		return -ENODEV;
diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
index f9271c7..d4af270 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -25,8 +25,6 @@
 #include <linux/workqueue.h>
 
 struct i2c_smbus_alert {
-	unsigned int		alert_edge_triggered:1;
-	int			irq;
 	struct work_struct	alert;
 	struct i2c_client	*ara;		/* Alert response address */
 };
@@ -72,13 +70,12 @@ static int smbus_do_alert(struct device *dev, void *addrp)
  * 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 void smbus_alert(struct work_struct *work)
+static irqreturn_t smbus_alert(int irq, void *d)
 {
-	struct i2c_smbus_alert *alert;
+	struct i2c_smbus_alert *alert = d;
 	struct i2c_client *ara;
 	unsigned short prev_addr = 0;	/* Not a valid address */
 
-	alert = container_of(work, struct i2c_smbus_alert, alert);
 	ara = alert->ara;
 
 	for (;;) {
@@ -115,21 +112,17 @@ static void smbus_alert(struct work_struct *work)
 		prev_addr = data.addr;
 	}
 
-	/* We handled all alerts; re-enable level-triggered IRQs */
-	if (!alert->alert_edge_triggered)
-		enable_irq(alert->irq);
+	return IRQ_HANDLED;
 }
 
-static irqreturn_t smbalert_irq(int irq, void *d)
+static void smbalert_work(struct work_struct *work)
 {
-	struct i2c_smbus_alert *alert = d;
+	struct i2c_smbus_alert *alert;
+
+	alert = container_of(work, struct i2c_smbus_alert, alert);
 
-	/* Disable level-triggered IRQs until we handle them */
-	if (!alert->alert_edge_triggered)
-		disable_irq_nosync(irq);
+	smbus_alert(0, alert);
 
-	schedule_work(&alert->alert);
-	return IRQ_HANDLED;
 }
 
 /* Setup SMBALERT# infrastructure */
@@ -139,28 +132,28 @@ static int smbalert_probe(struct i2c_client *ara,
 	struct i2c_smbus_alert_setup *setup = dev_get_platdata(&ara->dev);
 	struct i2c_smbus_alert *alert;
 	struct i2c_adapter *adapter = ara->adapter;
-	int res;
+	int res, irq;
 
 	alert = devm_kzalloc(&ara->dev, sizeof(struct i2c_smbus_alert),
 			     GFP_KERNEL);
 	if (!alert)
 		return -ENOMEM;
 
-	alert->alert_edge_triggered = setup->alert_edge_triggered;
-	alert->irq = setup->irq;
-	INIT_WORK(&alert->alert, smbus_alert);
+	irq = setup->irq;
+	INIT_WORK(&alert->alert, smbalert_work);
 	alert->ara = ara;
 
-	if (setup->irq > 0) {
-		res = devm_request_irq(&ara->dev, setup->irq, smbalert_irq,
-				       0, "smbus_alert", alert);
+	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#, %s trigger\n",
-		 setup->alert_edge_triggered ? "edge" : "level");
+	dev_info(&adapter->dev, "supports SMBALERT#\n");
 
 	return 0;
 }
diff --git a/include/linux/i2c-smbus.h b/include/linux/i2c-smbus.h
index a138502..19efbd1 100644
--- a/include/linux/i2c-smbus.h
+++ b/include/linux/i2c-smbus.h
@@ -42,7 +42,6 @@
  * properly set.
  */
 struct i2c_smbus_alert_setup {
-	unsigned int		alert_edge_triggered:1;
 	int			irq;
 };
 
-- 
1.8.3.1

[PATCH v11 02/10] i2c: i2c-smbus: Move i2c_setup_smbus_alert from i2c-smbus to i2c-core-smbus

[Phil Reid]

In preparation to adding of_i2c_setup_smbus_alert() move
i2c_setup_smbus_alert() to core module. of_i2c_setup_smbus_alert()
will call i2c_setup_smbus_alert() and this avoid module dependecy issues.

Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
---
 drivers/i2c/i2c-core-smbus.c | 33 +++++++++++++++++++++++++++++++++
 drivers/i2c/i2c-smbus.c      | 32 --------------------------------
 2 files changed, 33 insertions(+), 32 deletions(-)

diff --git a/drivers/i2c/i2c-core-smbus.c b/drivers/i2c/i2c-core-smbus.c
index 10f00a8..7f3ec02 100644
--- a/drivers/i2c/i2c-core-smbus.c
+++ b/drivers/i2c/i2c-core-smbus.c
@@ -17,6 +17,7 @@
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/i2c.h>
+#include <linux/i2c-smbus.h>
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/smbus.h>
@@ -592,3 +593,35 @@ s32 i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client,
 	return i;
 }
 EXPORT_SYMBOL(i2c_smbus_read_i2c_block_data_or_emulated);
+
+/**
+ * i2c_setup_smbus_alert - Setup SMBus alert support
+ * @adapter: the target adapter
+ * @setup: setup data for the SMBus alert handler
+ * Context: can sleep
+ *
+ * Setup handling of the SMBus alert protocol on a given I2C bus segment.
+ *
+ * Handling can be done either through our IRQ handler, or by the
+ * adapter (from its handler, periodic polling, or whatever).
+ *
+ * NOTE that if we manage the IRQ, we *MUST* know if it's level or
+ * edge triggered in order to hand it to the workqueue correctly.
+ * If triggering the alert seems to wedge the system, you probably
+ * should have said it's level triggered.
+ *
+ * This returns the ara client, which should be saved for later use with
+ * i2c_handle_smbus_alert() and ultimately i2c_unregister_device(); or NULL
+ * to indicate an error.
+ */
+struct i2c_client *i2c_setup_smbus_alert(struct i2c_adapter *adapter,
+					 struct i2c_smbus_alert_setup *setup)
+{
+	struct i2c_board_info ara_board_info = {
+		I2C_BOARD_INFO("smbus_alert", 0x0c),
+		.platform_data = setup,
+	};
+
+	return i2c_new_device(adapter, &ara_board_info);
+}
+EXPORT_SYMBOL_GPL(i2c_setup_smbus_alert);
diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
index d4af270..d0bb035 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -183,38 +183,6 @@ static int smbalert_remove(struct i2c_client *ara)
 };
 
 /**
- * i2c_setup_smbus_alert - Setup SMBus alert support
- * @adapter: the target adapter
- * @setup: setup data for the SMBus alert handler
- * Context: can sleep
- *
- * Setup handling of the SMBus alert protocol on a given I2C bus segment.
- *
- * Handling can be done either through our IRQ handler, or by the
- * adapter (from its handler, periodic polling, or whatever).
- *
- * NOTE that if we manage the IRQ, we *MUST* know if it's level or
- * edge triggered in order to hand it to the workqueue correctly.
- * If triggering the alert seems to wedge the system, you probably
- * should have said it's level triggered.
- *
- * This returns the ara client, which should be saved for later use with
- * i2c_handle_smbus_alert() and ultimately i2c_unregister_device(); or NULL
- * to indicate an error.
- */
-struct i2c_client *i2c_setup_smbus_alert(struct i2c_adapter *adapter,
-					 struct i2c_smbus_alert_setup *setup)
-{
-	struct i2c_board_info ara_board_info = {
-		I2C_BOARD_INFO("smbus_alert", 0x0c),
-		.platform_data = setup,
-	};
-
-	return i2c_new_device(adapter, &ara_board_info);
-}
-EXPORT_SYMBOL_GPL(i2c_setup_smbus_alert);
-
-/**
  * i2c_handle_smbus_alert - Handle an SMBus alert
  * @ara: the ARA client on the relevant adapter
  * Context: can't sleep
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v11 03/10] i2c: i2c-smbus: add of_i2c_setup_smbus_alert

[Phil Reid]

This commit adds of_i2c_setup_smbus_alert which allows the smbalert
driver to be attached to an i2c adapter via the device tree.

Signed-off-by: Phil Reid <preid@electromag.com.au>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/i2c/i2c.txt |  4 ++--
 drivers/i2c/i2c-core-smbus.c                  | 22 ++++++++++++++++++++++
 drivers/i2c/i2c-smbus.c                       | 10 +++++++++-
 include/linux/i2c-smbus.h                     |  9 +++++++++
 4 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
index cee9d50..1126398 100644
--- a/Documentation/devicetree/bindings/i2c/i2c.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c.txt
@@ -59,8 +59,8 @@ wants to support one of the below features, it should adapt the bindings below.
 	interrupts used by the device.
 
 - interrupt-names
-	"irq" and "wakeup" names are recognized by I2C core, other names are
-	left to individual drivers.
+	"irq", "wakeup" and "smbus_alert" names are recognized by I2C core,
+	other names are	left to individual drivers.
 
 - host-notify
 	device uses SMBus host notify protocol instead of interrupt line.
diff --git a/drivers/i2c/i2c-core-smbus.c b/drivers/i2c/i2c-core-smbus.c
index 7f3ec02..4bb9927 100644
--- a/drivers/i2c/i2c-core-smbus.c
+++ b/drivers/i2c/i2c-core-smbus.c
@@ -625,3 +625,25 @@ 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) && 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-smbus.c b/drivers/i2c/i2c-smbus.c
index d0bb035..5a1dd7f 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -21,6 +21,7 @@
 #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>
 
@@ -139,7 +140,14 @@ static int smbalert_probe(struct i2c_client *ara,
 	if (!alert)
 		return -ENOMEM;
 
-	irq = setup->irq;
+	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;
 
diff --git a/include/linux/i2c-smbus.h b/include/linux/i2c-smbus.h
index 19efbd1..fb0e040 100644
--- a/include/linux/i2c-smbus.h
+++ b/include/linux/i2c-smbus.h
@@ -49,4 +49,13 @@ 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

[PATCH v11 04/10] i2c: core: call of_i2c_setup_smbus_alert in i2c_register_adapter

[Phil Reid]

Add a call to of_i2c_setup_smbus_alert when a i2c adapter is registered
so the the smbalert driver can be registered.

Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
---
 drivers/i2c/i2c-core-base.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 033b7b9..7006486 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -29,6 +29,7 @@
 #include <linux/errno.h>
 #include <linux/gpio.h>
 #include <linux/i2c.h>
+#include <linux/i2c-smbus.h>
 #include <linux/idr.h>
 #include <linux/init.h>
 #include <linux/irqflags.h>
@@ -1273,6 +1274,10 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
 		goto out_list;
 	}
 
+	res = of_i2c_setup_smbus_alert(adap);
+	if (res)
+		goto out_reg;
+
 	dev_dbg(&adap->dev, "adapter [%s] registered\n", adap->name);
 
 	pm_runtime_no_callbacks(&adap->dev);
@@ -1304,6 +1309,10 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
 
 	return 0;
 
+out_reg:
+	init_completion(&adap->dev_released);
+	device_unregister(&adap->dev);
+	wait_for_completion(&adap->dev_released);
 out_list:
 	mutex_lock(&core_lock);
 	idr_remove(&i2c_adapter_idr, adap->nr);
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v11 05/10] i2c: mux: pca954x: call request irq after adding mux segments

[Phil Reid]

The pca954x device do not have the ability to mask interrupts. For
i2c slave devices that also don't have masking ability (eg ltc1760
smbalert output) delay registering the irq until after the mux
segments have been configured. During the mux add_adaptor call the
core i2c system can register an smbalert handler which would then
be called immediately when the irq is registered. This smbalert
handler will then clear the pending irq.

This removes the need for the irq_mask / irq_unmask calls that were
original used to do this.

Signed-off-by: Phil Reid <preid@electromag.com.au>
Acked-by: Peter Rosin <peda@axentia.se>
---
 drivers/i2c/muxes/i2c-mux-pca954x.c | 91 ++++++++++++-------------------------
 1 file changed, 28 insertions(+), 63 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index f1751c2..5ec5020 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -246,36 +246,6 @@ static irqreturn_t pca954x_irq_handler(int irq, void *dev_id)
 	return handled ? IRQ_HANDLED : IRQ_NONE;
 }
 
-static void pca954x_irq_mask(struct irq_data *idata)
-{
-	struct pca954x *data = irq_data_get_irq_chip_data(idata);
-	unsigned int pos = idata->hwirq;
-	unsigned long flags;
-
-	raw_spin_lock_irqsave(&data->lock, flags);
-
-	data->irq_mask &= ~BIT(pos);
-	if (!data->irq_mask)
-		disable_irq(data->client->irq);
-
-	raw_spin_unlock_irqrestore(&data->lock, flags);
-}
-
-static void pca954x_irq_unmask(struct irq_data *idata)
-{
-	struct pca954x *data = irq_data_get_irq_chip_data(idata);
-	unsigned int pos = idata->hwirq;
-	unsigned long flags;
-
-	raw_spin_lock_irqsave(&data->lock, flags);
-
-	if (!data->irq_mask)
-		enable_irq(data->client->irq);
-	data->irq_mask |= BIT(pos);
-
-	raw_spin_unlock_irqrestore(&data->lock, flags);
-}
-
 static int pca954x_irq_set_type(struct irq_data *idata, unsigned int type)
 {
 	if ((type & IRQ_TYPE_SENSE_MASK) != IRQ_TYPE_LEVEL_LOW)
@@ -285,8 +255,6 @@ static int pca954x_irq_set_type(struct irq_data *idata, unsigned int type)
 
 static struct irq_chip pca954x_irq_chip = {
 	.name = "i2c-mux-pca954x",
-	.irq_mask = pca954x_irq_mask,
-	.irq_unmask = pca954x_irq_unmask,
 	.irq_set_type = pca954x_irq_set_type,
 };
 
@@ -294,7 +262,7 @@ static int pca954x_irq_setup(struct i2c_mux_core *muxc)
 {
 	struct pca954x *data = i2c_mux_priv(muxc);
 	struct i2c_client *client = data->client;
-	int c, err, irq;
+	int c, irq;
 
 	if (!data->chip->has_irq || client->irq <= 0)
 		return 0;
@@ -314,24 +282,22 @@ static int pca954x_irq_setup(struct i2c_mux_core *muxc)
 			handle_simple_irq);
 	}
 
-	err = devm_request_threaded_irq(&client->dev, data->client->irq, NULL,
-					pca954x_irq_handler,
-					IRQF_ONESHOT | IRQF_SHARED,
-					"pca954x", data);
-	if (err)
-		goto err_req_irq;
+	return 0;
+}
 
-	disable_irq(data->client->irq);
+static void pca954x_cleanup(struct i2c_mux_core *muxc)
+{
+	struct pca954x *data = i2c_mux_priv(muxc);
+	int c, irq;
 
-	return 0;
-err_req_irq:
-	for (c = 0; c < data->chip->nchans; c++) {
-		irq = irq_find_mapping(data->irq, c);
-		irq_dispose_mapping(irq);
+	if (data->irq) {
+		for (c = 0; c < data->chip->nchans; c++) {
+			irq = irq_find_mapping(data->irq, c);
+			irq_dispose_mapping(irq);
+		}
+		irq_domain_remove(data->irq);
 	}
-	irq_domain_remove(data->irq);
-
-	return err;
+	i2c_mux_del_adapters(muxc);
 }
 
 /*
@@ -391,7 +357,7 @@ static int pca954x_probe(struct i2c_client *client,
 
 	ret = pca954x_irq_setup(muxc);
 	if (ret)
-		goto fail_del_adapters;
+		goto fail_cleanup;
 
 	/* Now create an adapter for each channel */
 	for (num = 0; num < data->chip->nchans; num++) {
@@ -414,7 +380,16 @@ static int pca954x_probe(struct i2c_client *client,
 
 		ret = i2c_mux_add_adapter(muxc, force, num, class);
 		if (ret)
-			goto fail_del_adapters;
+			goto fail_cleanup;
+	}
+
+	if (data->irq) {
+		ret = devm_request_threaded_irq(&client->dev, data->client->irq,
+						NULL, pca954x_irq_handler,
+						IRQF_ONESHOT | IRQF_SHARED,
+						"pca954x", data);
+		if (ret)
+			goto fail_cleanup;
 	}
 
 	dev_info(&client->dev,
@@ -424,26 +399,16 @@ static int pca954x_probe(struct i2c_client *client,
 
 	return 0;
 
-fail_del_adapters:
-	i2c_mux_del_adapters(muxc);
+fail_cleanup:
+	pca954x_cleanup(muxc);
 	return ret;
 }
 
 static int pca954x_remove(struct i2c_client *client)
 {
 	struct i2c_mux_core *muxc = i2c_get_clientdata(client);
-	struct pca954x *data = i2c_mux_priv(muxc);
-	int c, irq;
 
-	if (data->irq) {
-		for (c = 0; c < data->chip->nchans; c++) {
-			irq = irq_find_mapping(data->irq, c);
-			irq_dispose_mapping(irq);
-		}
-		irq_domain_remove(data->irq);
-	}
-
-	i2c_mux_del_adapters(muxc);
+	pca954x_cleanup(muxc);
 	return 0;
 }
 
-- 
1.8.3.1

[PATCH v11 06/10] i2c: mux: pca954x: Return error if irq_create_mapping fails

[Phil Reid]

irq_create_mapping can return an error, report error to log and return.
Cleanup will occur in the probe function when an error is returned.

Suggested-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
Acked-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
---
 drivers/i2c/muxes/i2c-mux-pca954x.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 5ec5020..fb725de 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -277,6 +277,10 @@ static int pca954x_irq_setup(struct i2c_mux_core *muxc)
 
 	for (c = 0; c < data->chip->nchans; c++) {
 		irq = irq_create_mapping(data->irq, c);
+		if (!irq) {
+			dev_err(&client->dev, "failed irq create map\n");
+			return -EINVAL;
+		}
 		irq_set_chip_data(irq, data);
 		irq_set_chip_and_handler(irq, &pca954x_irq_chip,
 			handle_simple_irq);
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v11 07/10] Documentation: Add sbs-manager device tree node documentation

[Phil Reid]

From: Karl-Heinz Schneider <karl-heinz@schneider-inet.de>

This patch adds device tree documentation for the sbs-manager

Signed-off-by: Karl-Heinz Schneider <karl-heinz@schneider-inet.de>
Signed-off-by: Phil Reid <preid@electromag.com.au>
Acked-by: Rob Herring <robh@kernel.org>
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 .../bindings/power/supply/sbs,sbs-manager.txt      | 66 ++++++++++++++++++++++
 1 file changed, 66 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.txt

diff --git a/Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.txt b/Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.txt
new file mode 100644
index 0000000..4b219557
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.txt
@@ -0,0 +1,66 @@
+Binding for sbs-manager
+
+Required properties:
+- compatible: "<vendor>,<part-number>", "sbs,sbs-charger" as fallback. The part
+  number compatible string might be used in order to take care of vendor
+  specific registers.
+- reg: integer, i2c address of the device. Should be <0xa>.
+Optional properties:
+- gpio-controller: Marks the port as GPIO controller.
+  See "gpio-specifier" in .../devicetree/bindings/gpio/gpio.txt.
+- #gpio-cells: Should be <2>. The first cell is the pin number, the second cell
+  is used to specify optional parameters:
+  See "gpio-specifier" in .../devicetree/bindings/gpio/gpio.txt.
+
+From OS view the device is basically an i2c-mux used to communicate with up to
+four smart battery devices at address 0xb. The driver actually implements this
+behaviour. So standard i2c-mux nodes can be used to register up to four slave
+batteries. Channels will be numerated starting from 1 to 4.
+
+Example:
+
+batman@a {
+    compatible = "lltc,ltc1760", "sbs,sbs-manager";
+    reg = <0x0a>;
+    #address-cells = <1>;
+    #size-cells = <0>;
+
+    gpio-controller;
+    #gpio-cells = <2>;
+
+    i2c@1 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        reg = <1>;
+
+        battery@b {
+            compatible = "ti,bq2060", "sbs,sbs-battery";
+            reg = <0x0b>;
+            sbs,battery-detect-gpios = <&batman 1 1>;
+        };
+    };
+
+    i2c@2 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        reg = <2>;
+
+        battery@b {
+            compatible = "ti,bq2060", "sbs,sbs-battery";
+            reg = <0x0b>;
+            sbs,battery-detect-gpios = <&batman 2 1>;
+        };
+    };
+
+    i2c@3 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        reg = <3>;
+
+        battery@b {
+            compatible = "ti,bq2060", "sbs,sbs-battery";
+            reg = <0x0b>;
+            sbs,battery-detect-gpios = <&batman 3 1>;
+        };
+    };
+};
-- 
1.8.3.1

[PATCH v11 08/10] power: Adds support for Smart Battery System Manager

[Phil Reid]

From: Karl-Heinz Schneider <karl-heinz-X5L7DgJ4l23oE99TX8zNy7NAH6kLmebB@public.gmane.org>

This patch adds support for Smart Battery System Manager.
A SBSM is a device listening at I2C/SMBus address 0x0a and is capable of
communicating up to four I2C smart battery devices. All smart battery
devices are listening at address 0x0b, so the SBSM muliplexes between
them. The driver makes use of the I2C-Mux framework to allow smart
batteries to be bound via device tree, i.e. the sbs-battery driver.

Via sysfs interface the online state and charge type are presented. If
the driver is bound as ltc1760 (an implementation of a Dual Smart Battery
System Manager) the charge type can also be changed from trickle to fast.

Signed-off-by: Karl-Heinz Schneider <karl-heinz-X5L7DgJ4l23oE99TX8zNy7NAH6kLmebB@public.gmane.org>
Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
Reviewed-by: Sebastian Reichel <sebastian.reichel-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
---
 drivers/power/supply/Kconfig       |  13 ++
 drivers/power/supply/Makefile      |   1 +
 drivers/power/supply/sbs-manager.c | 323 +++++++++++++++++++++++++++++++++++++
 3 files changed, 337 insertions(+)
 create mode 100644 drivers/power/supply/sbs-manager.c

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 969f500..e7e16ec 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -184,6 +184,19 @@ config CHARGER_SBS
         help
 	  Say Y to include support for SBS compilant battery chargers.
 
+config MANAGER_SBS
+	tristate "Smart Battery System Manager"
+	depends on I2C && I2C_MUX
+	help
+	  Say Y here to include support for Smart Battery System Manager
+	  ICs. The driver reports online and charging status via sysfs.
+	  It presents itself also as I2C mux which allows to bind
+	  smart battery driver to its ports.
+	  Supported is for example LTC1760.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called sbs-manager.
+
 config BATTERY_BQ27XXX
 	tristate "BQ27xxx battery driver"
 	help
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index a41f409..2958807 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_BATTERY_IPAQ_MICRO) += ipaq_micro_battery.o
 obj-$(CONFIG_BATTERY_WM97XX)	+= wm97xx_battery.o
 obj-$(CONFIG_BATTERY_SBS)	+= sbs-battery.o
 obj-$(CONFIG_CHARGER_SBS)	+= sbs-charger.o
+obj-$(CONFIG_MANAGER_SBS)	+= sbs-manager.o
 obj-$(CONFIG_BATTERY_BQ27XXX)	+= bq27xxx_battery.o
 obj-$(CONFIG_BATTERY_BQ27XXX_I2C) += bq27xxx_battery_i2c.o
 obj-$(CONFIG_BATTERY_DA9030)	+= da9030_battery.o
diff --git a/drivers/power/supply/sbs-manager.c b/drivers/power/supply/sbs-manager.c
new file mode 100644
index 0000000..47c09a1
--- /dev/null
+++ b/drivers/power/supply/sbs-manager.c
@@ -0,0 +1,323 @@
+/*
+ * Driver for SBS compliant Smart Battery System Managers
+ *
+ * The device communicates via i2c at address 0x0a and multiplexes access to up
+ * to four smart batteries at address 0x0b.
+ *
+ * Via sysfs interface the online state and charge type are presented.
+ *
+ * Datasheet SBSM:    http://sbs-forum.org/specs/sbsm100b.pdf
+ * Datasheet LTC1760: http://cds.linear.com/docs/en/datasheet/1760fb.pdf
+ *
+ * Karl-Heinz Schneider <karl-heinz-X5L7DgJ4l23oE99TX8zNy7NAH6kLmebB@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/i2c-mux.h>
+#include <linux/power_supply.h>
+
+#define SBSM_MAX_BATS  4
+#define SBSM_RETRY_CNT 3
+
+/* registers addresses */
+#define SBSM_CMD_BATSYSSTATE     0x01
+#define SBSM_CMD_BATSYSSTATECONT 0x02
+#define SBSM_CMD_BATSYSINFO      0x04
+#define SBSM_CMD_LTC             0x3c
+
+#define SBSM_MASK_BAT_SUPPORTED  GENMASK(3, 0)
+#define SBSM_MASK_CHARGE_BAT     GENMASK(7, 4)
+#define SBSM_BIT_AC_PRESENT      BIT(0)
+#define SBSM_BIT_TURBO           BIT(7)
+
+#define SBSM_SMB_BAT_OFFSET      11
+struct sbsm_data {
+	struct i2c_client *client;
+	struct i2c_mux_core *muxc;
+
+	struct power_supply *psy;
+
+	u8 cur_chan;          /* currently selected channel */
+	bool is_ltc1760;      /* special capabilities */
+};
+
+static enum power_supply_property sbsm_props[] = {
+	POWER_SUPPLY_PROP_ONLINE,
+	POWER_SUPPLY_PROP_CHARGE_TYPE,
+};
+
+static int sbsm_read_word(struct i2c_client *client, u8 address)
+{
+	int reg, retries;
+
+	for (retries = SBSM_RETRY_CNT; retries > 0; retries--) {
+		reg = i2c_smbus_read_word_data(client, address);
+		if (reg >= 0)
+			break;
+	}
+
+	if (reg < 0) {
+		dev_err(&client->dev, "failed to read register 0x%02x\n",
+			address);
+	}
+
+	return reg;
+}
+
+static int sbsm_write_word(struct i2c_client *client, u8 address, u16 word)
+{
+	int ret, retries;
+
+	for (retries = SBSM_RETRY_CNT; retries > 0; retries--) {
+		ret = i2c_smbus_write_word_data(client, address, word);
+		if (ret >= 0)
+			break;
+	}
+	if (ret < 0)
+		dev_err(&client->dev, "failed to write to register 0x%02x\n",
+			address);
+
+	return ret;
+}
+
+static int sbsm_get_property(struct power_supply *psy,
+			     enum power_supply_property psp,
+			     union power_supply_propval *val)
+{
+	struct sbsm_data *data = power_supply_get_drvdata(psy);
+	int regval = 0;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_ONLINE:
+		regval = sbsm_read_word(data->client, SBSM_CMD_BATSYSSTATECONT);
+		if (regval < 0)
+			return regval;
+		val->intval = !!(regval & SBSM_BIT_AC_PRESENT);
+		break;
+
+	case POWER_SUPPLY_PROP_CHARGE_TYPE:
+		regval = sbsm_read_word(data->client, SBSM_CMD_BATSYSSTATE);
+		if (regval < 0)
+			return regval;
+
+		if ((regval & SBSM_MASK_CHARGE_BAT) == 0) {
+			val->intval = POWER_SUPPLY_CHARGE_TYPE_NONE;
+			return 0;
+		}
+		val->intval = POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
+
+		if (data->is_ltc1760) {
+			/* charge mode fast if turbo is active */
+			regval = sbsm_read_word(data->client, SBSM_CMD_LTC);
+			if (regval < 0)
+				return regval;
+			else if (regval & SBSM_BIT_TURBO)
+				val->intval = POWER_SUPPLY_CHARGE_TYPE_FAST;
+		}
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int sbsm_prop_is_writeable(struct power_supply *psy,
+				  enum power_supply_property psp)
+{
+	struct sbsm_data *data = power_supply_get_drvdata(psy);
+
+	return (psp == POWER_SUPPLY_PROP_CHARGE_TYPE) && data->is_ltc1760;
+}
+
+static int sbsm_set_property(struct power_supply *psy,
+			     enum power_supply_property psp,
+			     const union power_supply_propval *val)
+{
+	struct sbsm_data *data = power_supply_get_drvdata(psy);
+	int ret = -EINVAL;
+	u16 regval;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_CHARGE_TYPE:
+		/* write 1 to TURBO if type fast is given */
+		if (!data->is_ltc1760)
+			break;
+		regval = val->intval ==
+			 POWER_SUPPLY_CHARGE_TYPE_FAST ? SBSM_BIT_TURBO : 0;
+		ret = sbsm_write_word(data->client, SBSM_CMD_LTC, regval);
+		break;
+
+	default:
+		break;
+	}
+
+	return ret;
+}
+
+/*
+ * Switch to battery
+ * Parameter chan is directly the content of SMB_BAT* nibble
+ */
+static int sbsm_select(struct i2c_mux_core *muxc, u32 chan)
+{
+	struct sbsm_data *data = i2c_mux_priv(muxc);
+	struct device *dev = &data->client->dev;
+	int ret = 0;
+	u16 reg;
+
+	if (data->cur_chan == chan)
+		return ret;
+
+	/* chan goes from 1 ... 4 */
+	reg = 1 << BIT(SBSM_SMB_BAT_OFFSET + chan);
+	ret = sbsm_write_word(data->client, SBSM_CMD_BATSYSSTATE, reg);
+	if (ret)
+		dev_err(dev, "Failed to select channel %i\n", chan);
+	else
+		data->cur_chan = chan;
+
+	return ret;
+}
+
+static const struct power_supply_desc sbsm_default_psy_desc = {
+	.type = POWER_SUPPLY_TYPE_MAINS,
+	.properties = sbsm_props,
+	.num_properties = ARRAY_SIZE(sbsm_props),
+	.get_property = &sbsm_get_property,
+	.set_property = &sbsm_set_property,
+	.property_is_writeable = &sbsm_prop_is_writeable,
+};
+
+static int sbsm_probe(struct i2c_client *client,
+		      const struct i2c_device_id *id)
+{
+	struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
+	struct sbsm_data *data;
+	struct device *dev = &client->dev;
+	struct power_supply_desc *psy_desc;
+	struct power_supply_config psy_cfg = {};
+	int ret = 0, i, supported_bats;
+
+	/* Device listens only at address 0x0a */
+	if (client->addr != 0x0a)
+		return -EINVAL;
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_WORD_DATA))
+		return -EPFNOSUPPORT;
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, data);
+
+	data->client = client;
+	data->is_ltc1760 = !!strstr(id->name, "ltc1760");
+
+	ret  = sbsm_read_word(client, SBSM_CMD_BATSYSINFO);
+	if (ret < 0)
+		return ret;
+	supported_bats = ret & SBSM_MASK_BAT_SUPPORTED;
+
+	data->muxc = i2c_mux_alloc(adapter, dev, SBSM_MAX_BATS, 0,
+				   I2C_MUX_LOCKED, &sbsm_select, NULL);
+	if (!data->muxc) {
+		dev_err(dev, "failed to alloc i2c mux\n");
+		ret = -ENOMEM;
+		goto err_mux_alloc;
+	}
+	data->muxc->priv = data;
+
+	/* register muxed i2c channels. One for each supported battery */
+	for (i = 0; i < SBSM_MAX_BATS; ++i) {
+		if (supported_bats & BIT(i)) {
+			ret = i2c_mux_add_adapter(data->muxc, 0, i + 1, 0);
+			if (ret)
+				break;
+		}
+	}
+	if (ret) {
+		dev_err(dev, "failed to register i2c mux channel %d\n", i + 1);
+		goto err_mux_register;
+	}
+
+	psy_desc = devm_kmemdup(dev, &sbsm_default_psy_desc,
+				sizeof(struct power_supply_desc),
+				GFP_KERNEL);
+	if (!psy_desc) {
+		ret = -ENOMEM;
+		goto err_psy;
+	}
+
+	psy_desc->name = devm_kasprintf(dev, GFP_KERNEL, "sbsm-%s",
+					dev_name(&client->dev));
+	if (!psy_desc->name) {
+		ret = -ENOMEM;
+		goto err_psy;
+	}
+
+	psy_cfg.drv_data = data;
+	psy_cfg.of_node = dev->of_node;
+	data->psy = devm_power_supply_register(dev, psy_desc, &psy_cfg);
+	if (IS_ERR(data->psy)) {
+		ret = PTR_ERR(data->psy);
+		dev_err(dev, "failed to register power supply %s\n",
+			psy_desc->name);
+		goto err_psy;
+	}
+
+	return 0;
+
+err_psy:
+err_mux_register:
+	i2c_mux_del_adapters(data->muxc);
+
+err_mux_alloc:
+	return ret;
+}
+
+static int sbsm_remove(struct i2c_client *client)
+{
+	struct sbsm_data *data = i2c_get_clientdata(client);
+
+	i2c_mux_del_adapters(data->muxc);
+	return 0;
+}
+
+static const struct i2c_device_id sbsm_ids[] = {
+	{ "sbs-manager", 0 },
+	{ "ltc1760",     0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, sbsm_ids);
+
+#ifdef CONFIG_OF
+static const struct of_device_id sbsm_dt_ids[] = {
+	{ .compatible = "sbs,sbs-manager" },
+	{ .compatible = "lltc,ltc1760" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, sbsm_dt_ids);
+#endif
+
+static struct i2c_driver sbsm_driver = {
+	.driver = {
+		.name = "sbsm",
+		.of_match_table = of_match_ptr(sbsm_dt_ids),
+	},
+	.probe		= sbsm_probe,
+	.remove		= sbsm_remove,
+	.id_table	= sbsm_ids
+};
+module_i2c_driver(sbsm_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Karl-Heinz Schneider <karl-heinz-X5L7DgJ4l23oE99TX8zNy7NAH6kLmebB@public.gmane.org>");
+MODULE_DESCRIPTION("SBSM Smart Battery System Manager");
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v11 09/10] power: supply: sbs-manager: Add alert callback and battery change notification

[Phil Reid]

This adds smb alert support via the smbus_alert driver to generate
power_supply_changed notifications when either external power is
removed / applied or a battery inserted / removed.
Use the i2c alert callback to notify the attached battery driver that a
change has occurred.

Signed-off-by: Phil Reid <preid@electromag.com.au>
Acked-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/power/supply/Kconfig       |   3 +-
 drivers/power/supply/sbs-manager.c | 129 +++++++++++++++++++++++++++++++++++--
 2 files changed, 127 insertions(+), 5 deletions(-)

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index e7e16ec..1b05021 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -186,7 +186,8 @@ config CHARGER_SBS
 
 config MANAGER_SBS
 	tristate "Smart Battery System Manager"
-	depends on I2C && I2C_MUX
+	depends on I2C && I2C_MUX && GPIOLIB
+	select I2C_SMBUS
 	help
 	  Say Y here to include support for Smart Battery System Manager
 	  ICs. The driver reports online and charging status via sysfs.
diff --git a/drivers/power/supply/sbs-manager.c b/drivers/power/supply/sbs-manager.c
index 47c09a1..3e3ad09 100644
--- a/drivers/power/supply/sbs-manager.c
+++ b/drivers/power/supply/sbs-manager.c
@@ -16,10 +16,12 @@
  * published by the Free Software Foundation.
  */
 
+#include <linux/gpio.h>
 #include <linux/module.h>
 #include <linux/i2c.h>
 #include <linux/i2c-mux.h>
 #include <linux/power_supply.h>
+#include <linux/property.h>
 
 #define SBSM_MAX_BATS  4
 #define SBSM_RETRY_CNT 3
@@ -43,7 +45,12 @@ struct sbsm_data {
 	struct power_supply *psy;
 
 	u8 cur_chan;          /* currently selected channel */
+	struct gpio_chip chip;
 	bool is_ltc1760;      /* special capabilities */
+
+	unsigned int supported_bats;
+	unsigned int last_state;
+	unsigned int last_state_cont;
 };
 
 static enum power_supply_property sbsm_props[] = {
@@ -186,6 +193,117 @@ static int sbsm_select(struct i2c_mux_core *muxc, u32 chan)
 	return ret;
 }
 
+static int sbsm_gpio_get_value(struct gpio_chip *gc, unsigned off)
+{
+	struct sbsm_data *data = gpiochip_get_data(gc);
+	int ret;
+
+	ret = sbsm_read_word(data->client, SBSM_CMD_BATSYSSTATE);
+	if (ret < 0)
+		return ret;
+
+	return ret & BIT(off);
+}
+
+/*
+ * This needs to be defined or the GPIO lib fails to register the pin.
+ * But the 'gpio' is always an input.
+ */
+static int sbsm_gpio_direction_input(struct gpio_chip *gc, unsigned off)
+{
+	return 0;
+}
+
+static int sbsm_do_alert(struct device *dev, void *d)
+{
+	struct i2c_client *client = i2c_verify_client(dev);
+	struct i2c_driver *driver;
+
+	if (!client || client->addr != 0x0b)
+		return 0;
+
+	device_lock(dev);
+	if (client->dev.driver) {
+		driver = to_i2c_driver(client->dev.driver);
+		if (driver->alert)
+			driver->alert(client, I2C_PROTOCOL_SMBUS_ALERT, 0);
+		else
+			dev_warn(&client->dev, "no driver alert()!\n");
+	} else
+		dev_dbg(&client->dev, "alert with no driver\n");
+	device_unlock(dev);
+
+	return -EBUSY;
+}
+
+static void sbsm_alert(struct i2c_client *client, enum i2c_alert_protocol prot,
+		       unsigned int d)
+{
+	struct sbsm_data *sbsm = i2c_get_clientdata(client);
+
+	int ret, i, irq_bat = 0, state = 0;
+
+	ret = sbsm_read_word(sbsm->client, SBSM_CMD_BATSYSSTATE);
+	if (ret >= 0) {
+		irq_bat = ret ^ sbsm->last_state;
+		sbsm->last_state = ret;
+		state = ret;
+	}
+
+	ret = sbsm_read_word(sbsm->client, SBSM_CMD_BATSYSSTATECONT);
+	if ((ret >= 0) &&
+	    ((ret ^ sbsm->last_state_cont) & SBSM_BIT_AC_PRESENT)) {
+		irq_bat |= sbsm->supported_bats & state;
+		power_supply_changed(sbsm->psy);
+	}
+	sbsm->last_state_cont = ret;
+
+	for (i = 0; i < SBSM_MAX_BATS; i++) {
+		if (irq_bat & BIT(i)) {
+			device_for_each_child(&sbsm->muxc->adapter[i]->dev,
+					      NULL, sbsm_do_alert);
+		}
+	}
+}
+
+static int sbsm_gpio_setup(struct sbsm_data *data)
+{
+	struct gpio_chip *gc = &data->chip;
+	struct i2c_client *client = data->client;
+	struct device *dev = &client->dev;
+	int ret;
+
+	if (!device_property_present(dev, "gpio-controller"))
+		return 0;
+
+	ret  = sbsm_read_word(client, SBSM_CMD_BATSYSSTATE);
+	if (ret < 0)
+		return ret;
+	data->last_state = ret;
+
+	ret  = sbsm_read_word(client, SBSM_CMD_BATSYSSTATECONT);
+	if (ret < 0)
+		return ret;
+	data->last_state_cont = ret;
+
+	gc->get = sbsm_gpio_get_value;
+	gc->direction_input  = sbsm_gpio_direction_input;
+	gc->can_sleep = true;
+	gc->base = -1;
+	gc->ngpio = SBSM_MAX_BATS;
+	gc->label = client->name;
+	gc->parent = dev;
+	gc->owner = THIS_MODULE;
+
+	ret = devm_gpiochip_add_data(dev, gc, data);
+	if (ret) {
+		dev_err(dev, "devm_gpiochip_add_data failed: %d\n", ret);
+		return ret;
+	}
+
+	return ret;
+}
+
 static const struct power_supply_desc sbsm_default_psy_desc = {
 	.type = POWER_SUPPLY_TYPE_MAINS,
 	.properties = sbsm_props,
@@ -203,7 +321,7 @@ static int sbsm_probe(struct i2c_client *client,
 	struct device *dev = &client->dev;
 	struct power_supply_desc *psy_desc;
 	struct power_supply_config psy_cfg = {};
-	int ret = 0, i, supported_bats;
+	int ret = 0, i;
 
 	/* Device listens only at address 0x0a */
 	if (client->addr != 0x0a)
@@ -224,8 +342,7 @@ static int sbsm_probe(struct i2c_client *client,
 	ret  = sbsm_read_word(client, SBSM_CMD_BATSYSINFO);
 	if (ret < 0)
 		return ret;
-	supported_bats = ret & SBSM_MASK_BAT_SUPPORTED;
-
+	data->supported_bats = ret & SBSM_MASK_BAT_SUPPORTED;
 	data->muxc = i2c_mux_alloc(adapter, dev, SBSM_MAX_BATS, 0,
 				   I2C_MUX_LOCKED, &sbsm_select, NULL);
 	if (!data->muxc) {
@@ -237,7 +354,7 @@ static int sbsm_probe(struct i2c_client *client,
 
 	/* register muxed i2c channels. One for each supported battery */
 	for (i = 0; i < SBSM_MAX_BATS; ++i) {
-		if (supported_bats & BIT(i)) {
+		if (data->supported_bats & BIT(i)) {
 			ret = i2c_mux_add_adapter(data->muxc, 0, i + 1, 0);
 			if (ret)
 				break;
@@ -262,6 +379,9 @@ static int sbsm_probe(struct i2c_client *client,
 		ret = -ENOMEM;
 		goto err_psy;
 	}
+	ret = sbsm_gpio_setup(data);
+	if (ret < 0)
+		goto err_psy;
 
 	psy_cfg.drv_data = data;
 	psy_cfg.of_node = dev->of_node;
@@ -314,6 +434,7 @@ static int sbsm_remove(struct i2c_client *client)
 	},
 	.probe		= sbsm_probe,
 	.remove		= sbsm_remove,
+	.alert		= sbsm_alert,
 	.id_table	= sbsm_ids
 };
 module_i2c_driver(sbsm_driver);
-- 
1.8.3.1

[PATCH v11 10/10] power: supply: sbs-battery: move gpio present detect to sbs_get_property

[Phil Reid]

Currently when a gpio is defined for battery presence it is only used in
the sbs_get_battery_presence_and_health function for 2 properties.
All other properties currently try to read data form the battery before
returning an error if not present. We should know in advance that no
data is going to returned.

As the driver tries multiple times to access a property, this prevents
a lot of smbus accesses, which had a significant effect on device boot-up.
As when the device is registered lots of property accesses are attempted
during boot.

If no gpio is used for presence detection no change in behaviour should
occur.

Signed-off-by: Phil Reid <preid@electromag.com.au>
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/power/supply/sbs-battery.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/power/supply/sbs-battery.c b/drivers/power/supply/sbs-battery.c
index f705945..686e7cb 100644
--- a/drivers/power/supply/sbs-battery.c
+++ b/drivers/power/supply/sbs-battery.c
@@ -325,16 +325,6 @@ static int sbs_get_battery_presence_and_health(
 	union power_supply_propval *val)
 {
 	s32 ret;
-	struct sbs_info *chip = i2c_get_clientdata(client);
-
-	if (psp == POWER_SUPPLY_PROP_PRESENT && chip->gpio_detect) {
-		ret = gpiod_get_value_cansleep(chip->gpio_detect);
-		if (ret < 0)
-			return ret;
-		val->intval = ret;
-		chip->is_present = val->intval;
-		return ret;
-	}
 
 	/*
 	 * Write to ManufacturerAccess with ManufacturerAccess command
@@ -600,6 +590,19 @@ static int sbs_get_property(struct power_supply *psy,
 	struct sbs_info *chip = power_supply_get_drvdata(psy);
 	struct i2c_client *client = chip->client;
 
+	if (chip->gpio_detect) {
+		ret = gpiod_get_value_cansleep(chip->gpio_detect);
+		if (ret < 0)
+			return ret;
+		if (psp == POWER_SUPPLY_PROP_PRESENT) {
+			val->intval = ret;
+			chip->is_present = val->intval;
+			return 0;
+		}
+		if (ret == 0)
+			return -ENODATA;
+	}
+
 	switch (psp) {
 	case POWER_SUPPLY_PROP_PRESENT:
 	case POWER_SUPPLY_PROP_HEALTH:
-- 
1.8.3.1

Re: [PATCH v11 00/10] Add sbs-manager with smbalert support

[Phil Reid]

On 24/08/2017 17:31, Phil Reid wrote:
> This is another go of the sbs-manager driver using smbalert for
> irq support from a while ago.
> 
> Enables the existing smbalert driver to be loaded via the device tree.
> Only need to add smbus_alert interrupt to the i2c bus segement in the
> devicetree and the core will then enable the alert driver.
> 
> Reorders the rquest irq call in the pca954x driver to ensure each
> muxed i2c segment can handle service smbalerts on that segment before
> irq's are enabled. The pca954x can't mask individual irq's routed thru
> them.
> 
> Add the sbs-manager from Karl-Heinz.
> Add the alert call back and gpio interface to allow the battery detect
> logic in the existing sbs-battery driver to work.
> 
> Changes from v5:
> - Documentation: Add sbs-manager device tree node documentation
>    - Use same style as sbs-charger for compatible property.
> - power: Adds support for Smart Battery System Manager
>    - reorder kconfig / makefile
>    - remove errouinous le16 to cpu conversions
>    - while loops to for loops
>    - formating changes to error messages
>    - changed sbsm_set_proprty indentation (hopefully I got it right)
>    - removed CONFIG_OF conditional around of_device_id table
>    - ENODEV -> EINVAL in probe function for mismatched address
>    - Use BIT() macro in probe function
>    - add of_node assignment in probe function
>    - remove owner assignament and set of_match_table
> - power: supply: sbs-battery: Add alert callback
>    - Removed patch as Sebastian has queued it.
> - power: supply: sbs-manager: Add alert callback and battery change notification
>    - Use device_property_present instead of of_get_property
>    - Add depends on GPIOLIB
> 
> Changes from v6
> - Add 2 patches to remove incorrect le16_to_cpu calls in bq24735 & sbs-battery
>    this was identifed in review of v6
> - i2c: i2c-smbus: Use threaded irq for smbalert
>    - remove alert_edge_triggered flag, see new description
>    - rework the work thread and threaded irq,commit log has more details
>    - Update in tree drivers where required (untested)
> - i2c: i2c-smbus: add of_i2c_setup_smbus_alert
>    - Add Rob's ack for doc binding
>    - rework of_i2c_setup_smbus_alert so that it doesn't need to alloc memory
>      addressing concern about devres allocation.
>      Probe function looks up the irq number if platform data isn't defined.
> - i2c: core: call of_i2c_setup_smbus_alert in i2c_register_adapter
>    - investigate if the core will release the client
>      It looks like it will to me, in i2c_del_adapter it iterates thru all clients
>      and calls __unregister_client.
> - i2c: mux: pca954x: Call request irq after adding mux segments
>    - fix logic in guard for request irq
>    - fix identation
>    - add check to irq_create_mapping call
> - Documentation: Add sbs-manager device tree node documentation
>    - Remove leading 0's
>    - Add Rob's ack
> - power: Adds support for Smart Battery System Manager
>    - remove inc header <linux/of_device.h>
>    - add macro defines for various bit and masks.
>    - refactor loop around i2c_mux_add_adapter
>    - Add ifdef CONFIG_OF around OF device table to save some bytes
> - power: supply: sbs-manager: Add alert callback and battery change notification
>    - Add Sebastian's ack for binding
> - Added new patch
>    - power: supply: sbs-battery: move gpio present detect to sbs_get_property
> 
> Changes from v7
> - Remove 2 patches that have been applied for incorrect le16_to_cpu
> - i2c: i2c-smbus: Use threaded irq for smbalert
>    - Added reviewed by Benjamin
> - i2c: i2c-smbus: add of_i2c_setup_smbus_alert
>    - Remove rename of variable, use adapater instead of adap in new function
>    - Add CONFIG_OF guard
> - i2c: core: call of_i2c_setup_smbus_alert in i2c_register_adapter
>    - Move call to just after register call and add error cleanup
>    - Didn't split this as per review comments as the 1 error
>      of_property_match_string can return seems pretty unlikely.
>      I don't think there's much benefit to it.
> - i2c: mux: pca954x: Call request irq after adding mux segments
>    - split into two patches.
>      pt1 as acked by Peter
>      pt2 the extra error handling for irq_create_mapping as suggested in his review
> - The rest
>    - Added reviewed by Sebastian
>      
>    
> Changes from v8
> - i2c: mux: pca954x: Return error if irq_create_mapping fails
>    - Add Peter's ack
>    - change return check to if (!irq)
> 
> Changes from v9
> - Move i2c_setup_smbbus_alert & of_i2c_setup_smbus_alert to
>    i2c-core-smbus to resolve linking issues with modules.
>    No functional changes.
> - i2c: mux: pca954x: Call request irq after adding mux segments
>    While testing above changes noticed a warning about unbalanced
>    irq_enable / disable. This is resolved by remove the irq_mask
>    irq_unmask functions that where an inital workaroudn to the
>    problem this patch now fixes by delaying request irq.
>    Removed Peter's ack on this one as it's changed.
> 
> Changes from v10
> - i2c: mux: pca954x: Call request irq after adding mux segments
>    Added Peters ack
>    renamed label 'fail_del_adapters' to 'fail_cleanup'
> 
> Karl-Heinz Schneider (2):
>    Documentation: Add sbs-manager device tree node documentation
>    power: Adds support for Smart Battery System Manager
> 
> Phil Reid (8):
>    i2c: i2c-smbus: Use threaded irq for smbalert
>    i2c: i2c-smbus: Move i2c_setup_smbus_alert from i2c-smbus to
>      i2c-core-smbus
>    i2c: i2c-smbus: add of_i2c_setup_smbus_alert
>    i2c: core: call of_i2c_setup_smbus_alert in i2c_register_adapter
>    i2c: mux: pca954x: call request irq after adding mux segments
>    i2c: mux: pca954x: Return error if irq_create_mapping fails
>    power: supply: sbs-manager: Add alert callback and battery change
>      notification
>    power: supply: sbs-battery: move gpio present detect to
>      sbs_get_property
> 
>   Documentation/devicetree/bindings/i2c/i2c.txt      |   4 +-
>   .../bindings/power/supply/sbs,sbs-manager.txt      |  66 +++
>   drivers/i2c/busses/i2c-parport-light.c             |   1 -
>   drivers/i2c/busses/i2c-parport.c                   |   1 -
>   drivers/i2c/busses/i2c-thunderx-pcidrv.c           |   6 -
>   drivers/i2c/i2c-core-base.c                        |   9 +
>   drivers/i2c/i2c-core-smbus.c                       |  55 +++
>   drivers/i2c/i2c-smbus.c                            |  81 ++--
>   drivers/i2c/muxes/i2c-mux-pca954x.c                |  95 ++---
>   drivers/power/supply/Kconfig                       |  14 +
>   drivers/power/supply/Makefile                      |   1 +
>   drivers/power/supply/sbs-battery.c                 |  23 +-
>   drivers/power/supply/sbs-manager.c                 | 444 +++++++++++++++++++++
>   include/linux/i2c-smbus.h                          |  10 +-
>   14 files changed, 670 insertions(+), 140 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.txt
>   create mode 100644 drivers/power/supply/sbs-manager.c
> 
Any comments on this?


-- 
Regards
Phil Reid

Re: [PATCH v11 00/10] Add sbs-manager with smbalert support

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 496 bytes --]


> > Changes from v10
> > - i2c: mux: pca954x: Call request irq after adding mux segments
> >    Added Peters ack
> >    renamed label 'fail_del_adapters' to 'fail_cleanup'
>
> Any comments on this?

I just pushed a branch with some minor fixes to my tree [1] so that
buildbot can check it. If all goes well, I'll post the patches tomorrow
and merge this branch to for-next soon after.

Thanks for your hard work!

[1] git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/sbs-manager


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v11 00/10] Add sbs-manager with smbalert support

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1144 bytes --]

Hi,

On Sun, Oct 29, 2017 at 12:55:52AM +0200, Wolfram Sang wrote:
> 
> > > Changes from v10
> > > - i2c: mux: pca954x: Call request irq after adding mux segments
> > >    Added Peters ack
> > >    renamed label 'fail_del_adapters' to 'fail_cleanup'
> >
> > Any comments on this?
> 
> I just pushed a branch with some minor fixes to my tree [1] so that
> buildbot can check it. If all goes well, I'll post the patches tomorrow
> and merge this branch to for-next soon after.
> 
> Thanks for your hard work!
> 
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/sbs-manager

I see some additional commits:

https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git/commit/?h=i2c/sbs-manager&id=7b75a22a903e681d920f383f7c952e5420db7e19
https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git/commit/?h=i2c/sbs-manager&id=567b0a4a7997406633e26898f68a623637f525c0
https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git/commit/?h=i2c/sbs-manager&id=f2b1a502784f996ad2686d7d3f8ad62bfdc89b43

Those are

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v11 00/10] Add sbs-manager with smbalert support

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 108 bytes --]


> Those are
> 
> Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>

Cool, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v11 02/10] i2c: i2c-smbus: Move i2c_setup_smbus_alert from i2c-smbus to i2c-core-smbus

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 819 bytes --]

On Thu, Aug 24, 2017 at 05:31:02PM +0800, Phil Reid wrote:
> In preparation to adding of_i2c_setup_smbus_alert() move
> i2c_setup_smbus_alert() to core module. of_i2c_setup_smbus_alert()
> will call i2c_setup_smbus_alert() and this avoid module dependecy issues.

I am not very happy with this but don't want to cause another delay. So,
I hope we can discuss and fix it incrementally.

From what it does, I really think both setup_alert functions belong to
i2c-smbus.c. Three possibilities come to my mind (untested, though):

a) use try_then_request_module somehow

b) add to CONFIG_I2C something like:
	select I2C_SMBUS if OF

c) get rid of i2c-smbus.c entirely and move it all into the core

Dunno if a) or b) have been tried in the course of this series already?

Kind regards,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v11 02/10] i2c: i2c-smbus: Move i2c_setup_smbus_alert from i2c-smbus to i2c-core-smbus

[Phil Reid]

On 29/10/2017 23:44, Wolfram Sang wrote:
> On Thu, Aug 24, 2017 at 05:31:02PM +0800, Phil Reid wrote:
>> In preparation to adding of_i2c_setup_smbus_alert() move
>> i2c_setup_smbus_alert() to core module. of_i2c_setup_smbus_alert()
>> will call i2c_setup_smbus_alert() and this avoid module dependecy issues.
> 
> I am not very happy with this but don't want to cause another delay. So,
> I hope we can discuss and fix it incrementally.
> 
>  From what it does, I really think both setup_alert functions belong to
> i2c-smbus.c. Three possibilities come to my mind (untested, though):
> 
> a) use try_then_request_module somehow
> 
> b) add to CONFIG_I2C something like:
> 	select I2C_SMBUS if OF
> 
> c) get rid of i2c-smbus.c entirely and move it all into the core
> 
> Dunno if a) or b) have been tried in the course of this series already?
> 
G'day Wolfram, Thanks for reviewing.

Nope hadn't tried either a) or b).
a) is not something I was aware of.
Had a brief play and not sure how to make this work. This gives me the same linking problem.

A variant of b) was tried by selecting I2C_SMBUS from driver modules having smbalert support,
but that caused problems if the i2c core was builtin and the driver was a module. But this looks
to work for me.

A variant of c) with option to exclude the i2c-smbus code still.
Add i2c-smbus to the i2c-core module by:
Change Kconfig I2C_SMBUS to a bool
add i2c-smbus to i2c-core object.
fixup duplicate init_module calls etc.
Possibly also rename i2c-smbus to i2c-smbus-alert?

Let me know your preference and I'll put something together.


-- 
Regards
Phil Reid

Re: [PATCH v11 00/10] Add sbs-manager with smbalert support

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 211 bytes --]


> I just pushed a branch with some minor fixes to my tree [1] so that
> buildbot can check it. If all goes well, I'll post the patches tomorrow
> and merge this branch to for-next soon after.

And merged now!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v11 02/10] i2c: i2c-smbus: Move i2c_setup_smbus_alert from i2c-smbus to i2c-core-smbus

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 2331 bytes --]

Hey Phil,

(CCing Jean for some additional expertise)

> > > In preparation to adding of_i2c_setup_smbus_alert() move
> > > i2c_setup_smbus_alert() to core module. of_i2c_setup_smbus_alert()
> > > will call i2c_setup_smbus_alert() and this avoid module dependecy issues.
> > 
> > I am not very happy with this but don't want to cause another delay. So,
> > I hope we can discuss and fix it incrementally.
> > 
> > From what it does, I really think both setup_alert functions belong
> > to i2c-smbus.c. Three possibilities come to my mind (untested,
> > though):
> > 
> > a) use try_then_request_module somehow
> > 
> > b) add to CONFIG_I2C something like: select I2C_SMBUS if OF
> > 
> > c) get rid of i2c-smbus.c entirely and move it all into the core
> > 
> > Dunno if a) or b) have been tried in the course of this series
> > already?
> > 
> 
> Nope hadn't tried either a) or b). a) is not something I was aware of.
> Had a brief play and not sure how to make this work. This gives me the
> same linking problem.

Which one? I2C core is built-in and the driver is a module? And
i2c-smbus is a module then, too?

> A variant of b) was tried by selecting I2C_SMBUS from driver modules
> having smbalert support, but that caused problems if the i2c core was
> builtin and the driver was a module. But this looks to work for me.

It should work(tm). Yet, it will cause overhead, because OF is big these
days and I2C_SMBUS is still rare.

> A variant of c) with option to exclude the i2c-smbus code still.
> Add i2c-smbus to the i2c-core module by:
> Change Kconfig I2C_SMBUS to a bool
> add i2c-smbus to i2c-core object.
> fixup duplicate init_module calls etc.

I see. From my side, why not. But I'd really like Jean's opinion here on
why i2c-smbus.c was a seperate module and not included in the core.
Because I split the I2C core into multiple source files recently, we
could have a seperate source file for i2c-smbus-alert without ifdeffery
but with some Makefile magic.

> Possibly also rename i2c-smbus to i2c-smbus-alert?

Why the rename? I thought we put all the code into the core?

> Let me know your preference and I'll put something together.

Thanks for the assistance. I am afraid we need a bit more discussion
first, though.

All the best,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v11 02/10] i2c: i2c-smbus: Move i2c_setup_smbus_alert from i2c-smbus to i2c-core-smbus

[Phil Reid]

On 7/11/2017 05:36, Wolfram Sang wrote:
> Hey Phil,
> 
> (CCing Jean for some additional expertise)
> 
>>>> In preparation to adding of_i2c_setup_smbus_alert() move
>>>> i2c_setup_smbus_alert() to core module. of_i2c_setup_smbus_alert()
>>>> will call i2c_setup_smbus_alert() and this avoid module dependecy issues.
>>>
>>> I am not very happy with this but don't want to cause another delay. So,
>>> I hope we can discuss and fix it incrementally.
>>>
>>>  From what it does, I really think both setup_alert functions belong
>>> to i2c-smbus.c. Three possibilities come to my mind (untested,
>>> though):
>>>
>>> a) use try_then_request_module somehow
>>>
>>> b) add to CONFIG_I2C something like: select I2C_SMBUS if OF
>>>
>>> c) get rid of i2c-smbus.c entirely and move it all into the core
>>>
>>> Dunno if a) or b) have been tried in the course of this series
>>> already?
>>>
>>
>> Nope hadn't tried either a) or b). a) is not something I was aware of.
>> Had a brief play and not sure how to make this work. This gives me the
>> same linking problem.
> 
> Which one? I2C core is built-in and the driver is a module? And
> i2c-smbus is a module then, too?
Yes. I may be doing something wrong with how to use a) thou..
I do like conditional pulling the alert code in the to core if everyone else is happy with that.


> 
>> A variant of b) was tried by selecting I2C_SMBUS from driver modules
>> having smbalert support, but that caused problems if the i2c core was
>> builtin and the driver was a module. But this looks to work for me.
> 
> It should work(tm). Yet, it will cause overhead, because OF is big these
> days and I2C_SMBUS is still rare.
> 
>> A variant of c) with option to exclude the i2c-smbus code still.
>> Add i2c-smbus to the i2c-core module by:
>> Change Kconfig I2C_SMBUS to a bool
>> add i2c-smbus to i2c-core object.
>> fixup duplicate init_module calls etc.
> 
> I see. From my side, why not. But I'd really like Jean's opinion here on
> why i2c-smbus.c was a seperate module and not included in the core.
> Because I split the I2C core into multiple source files recently, we
> could have a seperate source file for i2c-smbus-alert without ifdeffery
> but with some Makefile magic.
> 
>> Possibly also rename i2c-smbus to i2c-smbus-alert?
> 
> Why the rename? I thought we put all the code into the core?

I was just thinking that all that's in the file now is smbus-alert code.
The other smbus (host notify) code was moved to i2c-core
So maybe i2c-core-smbus-alert and as you say use makefile to include it.
I've tried this and it seems to work for the various combinations.
I'll wait a bit for Jean to offer an opinion.

> 
>> Let me know your preference and I'll put something together.
> 
> Thanks for the assistance. I am afraid we need a bit more discussion
> first, though.


-- 
Regards
Phil Reid
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v11 02/10] i2c: i2c-smbus: Move i2c_setup_smbus_alert from i2c-smbus to i2c-core-smbus

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 594 bytes --]


> I was just thinking that all that's in the file now is smbus-alert code.
> The other smbus (host notify) code was moved to i2c-core
> So maybe i2c-core-smbus-alert and as you say use makefile to include it.

Aaaah, i2c-core-smbus-alert.c. Yes, that probably makes sense. I was
wondering about i2c-smbus-alert.c (without -core)...

> I've tried this and it seems to work for the various combinations.
> I'll wait a bit for Jean to offer an opinion.

Well, if you have code already, can you post it as RFC and CC Jean?
It usually is easier to talk over code, even if it is not perfect yet...


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v11 02/10] i2c: i2c-smbus: Move i2c_setup_smbus_alert from i2c-smbus to i2c-core-smbus

[Phil Reid]

On 7/11/2017 15:22, Wolfram Sang wrote:
> 
>> I was just thinking that all that's in the file now is smbus-alert code.
>> The other smbus (host notify) code was moved to i2c-core
>> So maybe i2c-core-smbus-alert and as you say use makefile to include it.
> 
> Aaaah, i2c-core-smbus-alert.c. Yes, that probably makes sense. I was
> wondering about i2c-smbus-alert.c (without -core)...
> 
>> I've tried this and it seems to work for the various combinations.
>> I'll wait a bit for Jean to offer an opinion.
> 
> Well, if you have code already, can you post it as RFC and CC Jean?
> It usually is easier to talk over code, even if it is not perfect yet...
> 

Yes will do. Was hoping to clean it up into some incremental patches and post this week.
But it looks like next week now.

-- 
Regards
Phil Reid
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html