[PATCH v5 0/6] power: supply: sbs-manager add driver.

[PATCH v5 0/6] power: supply: sbs-manager add driver.

[Phil Reid]

The sbs-manager driver was written by Karl-Heinz and I've added irq
support to the driver to generate power_supply_change notices.
Hope I'm not violating etticate too much with this series

I've include Andrea's smb_alert driver changes here primarily to allow
testing of the alert callback. It looks like those changes stalled.
I'd be keen to implement necessary changes to this to get dt support
for the smbus_alert driver into mainline. Just need some guidance on
how.

Patches are applied against linux-power-supply/for-next


This patch series 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 (a Dual Smart Battery System Manager)
the charge type can also be changed from trickle to fast.

Changes:
V2:
* Fixed sample node names and compatible strings in binding documentaton.
* Removed optional property "sbsm,i2c-retry-count"
* Retry count hardcoded. Removed no longer needed OF readout function
* Rebased onto v4.7rc-5

V3:
* Changed enumeration sheme
* Rebased onto v4.7rc-7

V4:
* removed .owner field as advised by kbuild test robot
* removed print on ENOMEM
* added reference to i2c-mux documentation in binding doc

V5: v2 of my (Phil) Add irq support and battery detect gpios.
* update karl's sbs-manager binding to include gpio defintions.
* rebase onto linux-power-supply/for-next
* Add alert callback to sbs-manager.
* Expose battery presence via gpio to maintain compatiblity with
  sbs-battery driver.
* Notify attached batteries of state change via alert callback.
  This was done to avoid the need for the driver to become an irq
  source. This reduces the code a fair bit.
  Hopefully the implementation meets with approval.


Andrea Merello (1):
  I2C: i2c-smbus: add device tree support

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

Phil Reid (3):
  i2c: i2c-smbus: Support threaded irq.
  power: supply: sbs-battery: Add alert callback.
  power: supply: sbs-manager: Add alert callback and battery change
    notification.

 .../devicetree/bindings/i2c/i2c-smbus.txt          |  20 +
 .../bindings/power/supply/sbs,sbs-manager.txt      |  62 +++
 drivers/i2c/i2c-smbus.c                            |  41 +-
 drivers/power/supply/Kconfig                       |  13 +
 drivers/power/supply/Makefile                      |   1 +
 drivers/power/supply/sbs-battery.c                 |  16 +-
 drivers/power/supply/sbs-manager.c                 | 451 +++++++++++++++++++++
 7 files changed, 591 insertions(+), 13 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-smbus.txt
 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

[PATCH v5 1/6] I2C: i2c-smbus: add device tree support

[Phil Reid]

From: Andrea Merello <andrea.merello-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

According to Documentation/i2c/smbus-protocol, a smbus controller driver
that wants to hook-in smbus extensions support, can call
i2c_setup_smbus_alert(). There are very few drivers that are currently
doing this.

However the i2c-smbus module can also work with any
smbus-extensions-unaware I2C controller, as long as we provide an extra
IRQ line connected to the I2C bus ALARM signal.

This patch makes it possible to go this way via DT. Note that the DT node
will indeed describe a (little) piece of HW, that is the routing of the
ALARM signal to an IRQ line (so it seems a fair DT use to me, but RFC).

Note that AFAICT, by design, i2c-smbus module pretends to be an I2C slave
with address 0x0C (that is the alarm response address), and IMHO this is
quite consistent with usage in the DT as a I2C child node.

Signed-off-by: Andrea Merello <andrea.merello-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
---
 Documentation/devicetree/bindings/i2c/i2c-smbus.txt | 20 ++++++++++++++++++++
 drivers/i2c/i2c-smbus.c                             | 20 +++++++++++++++-----
 2 files changed, 35 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-smbus.txt

diff --git a/Documentation/devicetree/bindings/i2c/i2c-smbus.txt b/Documentation/devicetree/bindings/i2c/i2c-smbus.txt
new file mode 100644
index 0000000..da83127
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-smbus.txt
@@ -0,0 +1,20 @@
+* i2c-smbus extensions
+
+Required Properties:
+  - compatible: Must contain "smbus_alert"
+  - interrupts: The irq line for smbus ALERT signal
+  - reg: I2C slave address. Set to 0x0C (alert response address).
+
+Note: The i2c-smbus module registers itself as a slave I2C device. Whenever
+a smbus controller directly support smbus extensions (and its driver supports
+this), there is no need to add anything special to the DT. Otherwise, for using
+i2c-smbus with any smbus-extensions-unaware I2C controllers, you need to
+route the smbus ALARM signal to an extra IRQ line, thus you need to describe
+this in the DT.
+
+Example:
+	alert@0x0C {
+		reg = <0x0C>;
+		compatible = "smbus_alert";
+		interrupts = <0 36 IRQ_TYPE_LEVEL_LOW>;
+	};
diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
index b0d2679..5806db3 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -23,6 +23,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/workqueue.h>
+#include <linux/of_irq.h>
 
 struct i2c_smbus_alert {
 	unsigned int		alert_edge_triggered:1;
@@ -139,20 +140,29 @@ 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;
+	struct device_node *of_node = ara->dev.of_node;
 	int res;
+	int irq_type;
 
 	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;
+	if (setup) {
+		alert->alert_edge_triggered = setup->alert_edge_triggered;
+		alert->irq = setup->irq;
+	} else if (of_node) {
+		alert->irq = irq_of_parse_and_map(of_node, 0);
+		irq_type = irq_get_trigger_type(alert->irq);
+		alert->alert_edge_triggered = (irq_type & IRQ_TYPE_EDGE_BOTH);
+	}
+
 	INIT_WORK(&alert->alert, smbus_alert);
 	alert->ara = ara;
 
-	if (setup->irq > 0) {
-		res = devm_request_irq(&ara->dev, setup->irq, smbalert_irq,
+	if (alert->irq > 0) {
+		res = devm_request_irq(&ara->dev, alert->irq, smbalert_irq,
 				       0, "smbus_alert", alert);
 		if (res)
 			return res;
@@ -160,7 +170,7 @@ static int smbalert_probe(struct i2c_client *ara,
 
 	i2c_set_clientdata(ara, alert);
 	dev_info(&adapter->dev, "supports SMBALERT#, %s trigger\n",
-		 setup->alert_edge_triggered ? "edge" : "level");
+		 alert->alert_edge_triggered ? "edge" : "level");
 
 	return 0;
 }
-- 
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 v5 2/6] i2c: i2c-smbus: Support threaded irq.

[Phil Reid]

handle_nested_irq calls the threaded irq handler. So if the smbus_alert
irq is being generated via this an null address is derefenced. Split
IRQ up into separate functions to allow thread / non thread irq to work
correctly.

Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
---
 drivers/i2c/i2c-smbus.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
index 5806db3..b641cc2 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -73,13 +73,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 (;;) {
@@ -116,6 +115,17 @@ static void smbus_alert(struct work_struct *work)
 		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(alert->irq, alert);
+
 	/* We handled all alerts; re-enable level-triggered IRQs */
 	if (!alert->alert_edge_triggered)
 		enable_irq(alert->irq);
@@ -158,12 +168,13 @@ static int smbalert_probe(struct i2c_client *ara,
 		alert->alert_edge_triggered = (irq_type & IRQ_TYPE_EDGE_BOTH);
 	}
 
-	INIT_WORK(&alert->alert, smbus_alert);
+	INIT_WORK(&alert->alert, smbalert_work);
 	alert->ara = ara;
 
 	if (alert->irq > 0) {
-		res = devm_request_irq(&ara->dev, alert->irq, smbalert_irq,
-				       0, "smbus_alert", alert);
+		res = devm_request_threaded_irq(&ara->dev, alert->irq,
+						smbalert_irq, smbus_alert,
+						0, "smbus_alert", alert);
 		if (res)
 			return res;
 	}
-- 
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 v5 3/6] 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

Reviewed-by: Phil Reid <preid@electromag.com.au>
Signed-off-by: Karl-Heinz Schneider <karl-heinz@schneider-inet.de>
Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 .../bindings/power/supply/sbs,sbs-manager.txt      | 62 ++++++++++++++++++++++
 1 file changed, 62 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..2a04b30
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.txt
@@ -0,0 +1,62 @@
+Binding for sbs-manager
+
+Required properties:
+- compatible: should be "lltc,ltc1760" or use "sbs,sbs-manager" as fallback.
+- 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. See Documentation/devicetree/bindings/i2c/i2c-mux.txt for more
+information on i2c-mux nodes. Channels will be numerated starting from 1 to 4.
+
+Example:
+
+batman@0a {
+    compatible = "lltc,ltc1760";
+    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 = <0xb>;
+        };
+    };
+
+    i2c@2 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        reg = <2>;
+
+        battery@b {
+            compatible = "ti,bq2060", "sbs,sbs-battery";
+            reg = <0xb>;
+        };
+    };
+
+    i2c@3 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        reg = <3>;
+
+        battery@b {
+            compatible = "ti,bq2060", "sbs,sbs-battery";
+            reg = <0xb>;
+        };
+    };
+};
-- 
1.8.3.1

[PATCH v5 4/6] power: Adds support for Smart Battery System Manager

[Phil Reid]

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

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.

Tested-by: Phil Reid <preid@electromag.com.au>
Reviewed-by: Phil Reid <preid@electromag.com.au>
Signed-off-by: Karl-Heinz Schneider <karl-heinz@schneider-inet.de>
Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 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 76806a0..2666ec8 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -157,6 +157,19 @@ config BATTERY_WM97XX
 	help
 	  Say Y to enable support for battery measured by WM97xx aux port.
 
+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_SBS
         tristate "SBS Compliant gas gauge"
         depends on I2C
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index 36c599d..77f4914 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_BATTERY_DS2760)	+= ds2760_battery.o
 obj-$(CONFIG_BATTERY_DS2780)	+= ds2780_battery.o
 obj-$(CONFIG_BATTERY_DS2781)	+= ds2781_battery.o
 obj-$(CONFIG_BATTERY_DS2782)	+= ds2782_battery.o
+obj-$(CONFIG_MANAGER_SBS)	+= sbs-manager.o
 obj-$(CONFIG_BATTERY_GAUGE_LTC2941)	+= ltc2941-battery-gauge.o
 obj-$(CONFIG_BATTERY_GOLDFISH)	+= goldfish_battery.o
 obj-$(CONFIG_BATTERY_PMU)	+= pmu_battery.o
diff --git a/drivers/power/supply/sbs-manager.c b/drivers/power/supply/sbs-manager.c
new file mode 100644
index 0000000..e3debd7
--- /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@schneider-inet.de>
+ *
+ * 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
+
+struct sbsm_data {
+	struct i2c_client *client;
+	struct i2c_mux_core *muxc;
+
+	struct power_supply *psy;
+
+	int 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 = SBSM_RETRY_CNT;
+
+	while (retries > 0) {
+		reg = i2c_smbus_read_word_data(client, address);
+		if (reg >= 0)
+			break;
+		--retries;
+	}
+
+	if (reg < 0) {
+		dev_err(&client->dev, "failed to read register %i\n",
+			(int)address);
+		return reg;
+	}
+
+	return le16_to_cpu(reg);
+}
+
+static int sbsm_write_word(struct i2c_client *client, u8 address, u16 word)
+{
+	int ret, retries = SBSM_RETRY_CNT;
+
+	word = cpu_to_le16(word);
+	while (retries > 0) {
+		ret = i2c_smbus_write_word_data(client, address, word);
+		if (ret >= 0)
+			break;
+		--retries;
+	}
+	if (ret < 0)
+		dev_err(&client->dev, "failed to write to register %i\n",
+			(int)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 & 0x1);
+		break;
+
+	case POWER_SUPPLY_PROP_CHARGE_TYPE:
+		regval = sbsm_read_word(data->client, SBSM_CMD_BATSYSSTATE);
+		if (regval < 0)
+			return regval;
+
+		if ((regval & 0x00f0) == 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 & 0x80)
+				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;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_CHARGE_TYPE:
+		/* write 1 to TURBO if type fast is given */
+		if (data->is_ltc1760) {
+			u16 regval = val->intval ==
+			POWER_SUPPLY_CHARGE_TYPE_FAST ? (0x1 << 7) : 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);
+	int ret = 0;
+	u16 reg;
+
+	if (data->cur_chan == chan)
+		return ret;
+
+	/* chan goes from 1 ... 4 */
+	reg = 1 << (11 + chan);
+	ret = sbsm_write_word(data->client, SBSM_CMD_BATSYSSTATE, reg);
+	if (ret)
+		dev_err(&data->client->dev, "Failed to select channel %i\n",
+			chan);
+	else
+		data->cur_chan = chan;
+
+	return ret;
+}
+
+#if defined(CONFIG_OF)
+
+#include <linux/of_device.h>
+
+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 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 -ENODEV;
+
+	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 = le16_to_cpu(ret) & 0xf;
+
+	data->muxc = i2c_mux_alloc(adapter, dev, SBSM_MAX_BATS, 0,
+				   I2C_MUX_LOCKED, &sbsm_select, NULL);
+	if (!data->muxc) {
+		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 ((1 << i) & supported_bats) {
+			ret = i2c_mux_add_adapter(data->muxc, 0, i + 1, 0);
+			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;
+	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;
+	}
+
+	dev_info(dev, "sbsm registered\n");
+	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);
+
+static struct i2c_driver sbsm_driver = {
+	.driver = {
+		.name = "sbsm",
+	},
+	.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@schneider-inet.de>");
+MODULE_DESCRIPTION("SBSM Smart Battery System Manager");
-- 
1.8.3.1

[PATCH v5 5/6] power: supply: sbs-battery: Add alert callback.

[Phil Reid]

To simplify the sbs-manager code and notification of battery removal
use the i2c alert callback to notify the sbs-battery driver that an
event has occurred.

Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 drivers/power/supply/sbs-battery.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/power/supply/sbs-battery.c b/drivers/power/supply/sbs-battery.c
index d15ce9e..b1284e6 100644
--- a/drivers/power/supply/sbs-battery.c
+++ b/drivers/power/supply/sbs-battery.c
@@ -674,21 +674,30 @@ done:
 	return 0;
 }
 
-static irqreturn_t sbs_irq(int irq, void *devid)
+static void sbs_supply_changed(struct sbs_info *chip)
 {
-	struct sbs_info *chip = devid;
 	struct power_supply *battery = chip->power_supply;
 	int ret;
 
 	ret = gpiod_get_value_cansleep(chip->gpio_detect);
 	if (ret < 0)
-		return ret;
+		return;
 	chip->is_present = ret;
 	power_supply_changed(battery);
+}
 
+static irqreturn_t sbs_irq(int irq, void *devid)
+{
+	sbs_supply_changed(devid);
 	return IRQ_HANDLED;
 }
 
+void sbs_alert(struct i2c_client *client, enum i2c_alert_protocol prot,
+	unsigned int data)
+{
+	sbs_supply_changed(i2c_get_clientdata(client));
+}
+
 static void sbs_external_power_changed(struct power_supply *psy)
 {
 	struct sbs_info *chip = power_supply_get_drvdata(psy);
@@ -916,6 +925,7 @@ MODULE_DEVICE_TABLE(of, sbs_dt_ids);
 static struct i2c_driver sbs_battery_driver = {
 	.probe		= sbs_probe,
 	.remove		= sbs_remove,
+	.alert		= sbs_alert,
 	.id_table	= sbs_id,
 	.driver = {
 		.name	= "sbs-battery",
-- 
1.8.3.1

[PATCH v5 6/6] power: supply: sbs-manager: Add alert callback and battery change notification.

[Phil Reid]

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

Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 drivers/power/supply/sbs-manager.c | 134 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 131 insertions(+), 3 deletions(-)

diff --git a/drivers/power/supply/sbs-manager.c b/drivers/power/supply/sbs-manager.c
index e3debd7..9b92b40 100644
--- a/drivers/power/supply/sbs-manager.c
+++ b/drivers/power/supply/sbs-manager.c
@@ -19,6 +19,7 @@
 #include <linux/module.h>
 #include <linux/i2c.h>
 #include <linux/i2c-mux.h>
+#include <linux/gpio.h>
 #include <linux/power_supply.h>
 
 #define SBSM_MAX_BATS  4
@@ -30,14 +31,22 @@
 #define SBSM_CMD_BATSYSINFO      0x04
 #define SBSM_CMD_LTC             0x3c
 
+#define SBSM_BIT_AC_PRESENT      BIT(0)
+
 struct sbsm_data {
 	struct i2c_client *client;
 	struct i2c_mux_core *muxc;
 
 	struct power_supply *psy;
 
+	struct gpio_chip chip;
+
 	int cur_chan;         /* currently selected channel */
 	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[] = {
@@ -184,6 +193,121 @@ 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;
+
+	/*
+	* 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, 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);
+
+	/* Stop iterating after we find the device */
+	return -EBUSY;
+}
+
+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;
+
+	ret = sbsm_read_word(sbsm->client, SBSM_CMD_BATSYSSTATE);
+	if (ret >= 0)
+		irq_bat = ret ^ sbsm->last_state;
+	sbsm->last_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;
+		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, 0,
+					      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;
+	struct device_node *of_node = client->dev.of_node;
+	int ret;
+
+	if (!of_get_property(of_node, "gpio-controller", NULL))
+		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;
+}
+
 #if defined(CONFIG_OF)
 
 #include <linux/of_device.h>
@@ -214,7 +338,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)
@@ -235,7 +359,7 @@ static int sbsm_probe(struct i2c_client *client,
 	ret  = sbsm_read_word(client, SBSM_CMD_BATSYSINFO);
 	if (ret < 0)
 		return ret;
-	supported_bats = le16_to_cpu(ret) & 0xf;
+	data->supported_bats = le16_to_cpu(ret) & 0xf;
 
 	data->muxc = i2c_mux_alloc(adapter, dev, SBSM_MAX_BATS, 0,
 				   I2C_MUX_LOCKED, &sbsm_select, NULL);
@@ -247,7 +371,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 ((1 << i) & supported_bats) {
+		if ((1 << i) & data->supported_bats) {
 			ret = i2c_mux_add_adapter(data->muxc, 0, i + 1, 0);
 			if (ret) {
 				dev_err(dev,
@@ -272,6 +396,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;
 	data->psy = devm_power_supply_register(dev, psy_desc, &psy_cfg);
@@ -314,6 +441,7 @@ static struct i2c_driver sbsm_driver = {
 	},
 	.probe		= sbsm_probe,
 	.remove		= sbsm_remove,
+	.alert		= sbsm_alert,
 	.id_table	= sbsm_ids
 };
 module_i2c_driver(sbsm_driver);
-- 
1.8.3.1

Re: [PATCH v5 1/6] I2C: i2c-smbus: add device tree support

[Mark Rutland]

On Wed, Sep 21, 2016 at 04:41:09PM +0800, Phil Reid wrote:
> From: Andrea Merello <andrea.merello@gmail.com>
> 
> According to Documentation/i2c/smbus-protocol, a smbus controller driver
> that wants to hook-in smbus extensions support, can call
> i2c_setup_smbus_alert(). There are very few drivers that are currently
> doing this.
> 
> However the i2c-smbus module can also work with any
> smbus-extensions-unaware I2C controller, as long as we provide an extra
> IRQ line connected to the I2C bus ALARM signal.
> 
> This patch makes it possible to go this way via DT. Note that the DT node
> will indeed describe a (little) piece of HW, that is the routing of the
> ALARM signal to an IRQ line (so it seems a fair DT use to me, but RFC).

Which piece of hardware actually generates this IRQ? The I2C controller?
A slave SMBus device? Or something else?

I'm not at all familiar with I2C or SMBus, and a quick scan of
Documentation/i2c/smbus-protocol, has left me none-the-wiser on that
front.

> Note that AFAICT, by design, i2c-smbus module pretends to be an I2C slave
> with address 0x0C (that is the alarm response address), and IMHO this is
> quite consistent with usage in the DT as a I2C child node.
> 
> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
> Signed-off-by: Phil Reid <preid@electromag.com.au>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-smbus.txt | 20 ++++++++++++++++++++
>  drivers/i2c/i2c-smbus.c                             | 20 +++++++++++++++-----
>  2 files changed, 35 insertions(+), 5 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-smbus.txt
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-smbus.txt b/Documentation/devicetree/bindings/i2c/i2c-smbus.txt
> new file mode 100644
> index 0000000..da83127
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/i2c-smbus.txt
> @@ -0,0 +1,20 @@
> +* i2c-smbus extensions
> +
> +Required Properties:
> +  - compatible: Must contain "smbus_alert"

Nit: s/_/-/ in compatible strings please.

> +  - interrupts: The irq line for smbus ALERT signal
> +  - reg: I2C slave address. Set to 0x0C (alert response address).
> +
> +Note: The i2c-smbus module registers itself as a slave I2C device. Whenever
> +a smbus controller directly support smbus extensions (and its driver supports
> +this), there is no need to add anything special to the DT. Otherwise, for using
> +i2c-smbus with any smbus-extensions-unaware I2C controllers, you need to
> +route the smbus ALARM signal to an extra IRQ line, thus you need to describe
> +this in the DT.

Bindings shouldn't mention driver details (e.g. the i2c-smbus module
behaviour). It feels like we're creating a virtual device for the sake
of a driver, rather than accurately capturing the hardware.

So as-is, I don't think this is the right way to describe this.

Thanks,
Mark.

Re: [PATCH v5 1/6] I2C: i2c-smbus: add device tree support

[Phil Reid]

G'day Mark

On 21/09/2016 19:34, Mark Rutland wrote:
> On Wed, Sep 21, 2016 at 04:41:09PM +0800, Phil Reid wrote:
>> From: Andrea Merello <andrea.merello@gmail.com>
>>
>> According to Documentation/i2c/smbus-protocol, a smbus controller driver
>> that wants to hook-in smbus extensions support, can call
>> i2c_setup_smbus_alert(). There are very few drivers that are currently
>> doing this.
>>
>> However the i2c-smbus module can also work with any
>> smbus-extensions-unaware I2C controller, as long as we provide an extra
>> IRQ line connected to the I2C bus ALARM signal.
>>
>> This patch makes it possible to go this way via DT. Note that the DT node
>> will indeed describe a (little) piece of HW, that is the routing of the
>> ALARM signal to an IRQ line (so it seems a fair DT use to me, but RFC).
>
> Which piece of hardware actually generates this IRQ? The I2C controller?
> A slave SMBus device? Or something else?
>
> I'm not at all familiar with I2C or SMBus, and a quick scan of
> Documentation/i2c/smbus-protocol, has left me none-the-wiser on that
> front.

The originating source is the smbus device. SMBALERT is an active low interrupt
with the addition that the SMBUS device can be polled by the controller to determine
which device is asserting by performing a read of i2c address 0xc.
Asserting devices will return their address.
See the datahsheet of the LTC1760: http://www.linear.com/docs/1958   Page 22

The i2c parport driver looks to be only controller setup for this at the moment.
However that has limitations to only poll the primary i2c segment.
If muxes are involved then each muxed bus needs to be queried.

My hardware has:

                 /- ltc1760
i2c_cntlr - mux +
                 \- ltc1760

The alert signal are routed to the mux (pca9543) irq inputs and then muxes combined
irq output to an fpga pin on an ALTERA SOC which I can route to a GPIO / IRQ

I've modified the pca954x driver to generate separate interrupts for each segment.
Submitted an RFC PATCH on this a while ago, but it needs a lot of cleaning up before
submission.

So I can create an virtual SMB_ALERT device per segment and it knows to just poll that segment.

>
>> Note that AFAICT, by design, i2c-smbus module pretends to be an I2C slave
>> with address 0x0C (that is the alarm response address), and IMHO this is
>> quite consistent with usage in the DT as a I2C child node.
>>
>> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
>> Signed-off-by: Phil Reid <preid@electromag.com.au>
>> ---
>>  Documentation/devicetree/bindings/i2c/i2c-smbus.txt | 20 ++++++++++++++++++++
>>  drivers/i2c/i2c-smbus.c                             | 20 +++++++++++++++-----
>>  2 files changed, 35 insertions(+), 5 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-smbus.txt
>>
>> diff --git a/Documentation/devicetree/bindings/i2c/i2c-smbus.txt b/Documentation/devicetree/bindings/i2c/i2c-smbus.txt
>> new file mode 100644
>> index 0000000..da83127
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/i2c/i2c-smbus.txt
>> @@ -0,0 +1,20 @@
>> +* i2c-smbus extensions
>> +
>> +Required Properties:
>> +  - compatible: Must contain "smbus_alert"
>
> Nit: s/_/-/ in compatible strings please.
>
>> +  - interrupts: The irq line for smbus ALERT signal
>> +  - reg: I2C slave address. Set to 0x0C (alert response address).
>> +
>> +Note: The i2c-smbus module registers itself as a slave I2C device. Whenever
>> +a smbus controller directly support smbus extensions (and its driver supports
>> +this), there is no need to add anything special to the DT. Otherwise, for using
>> +i2c-smbus with any smbus-extensions-unaware I2C controllers, you need to
>> +route the smbus ALARM signal to an extra IRQ line, thus you need to describe
>> +this in the DT.
>
> Bindings shouldn't mention driver details (e.g. the i2c-smbus module
> behaviour). It feels like we're creating a virtual device for the sake
> of a driver, rather than accurately capturing the hardware.
>
> So as-is, I don't think this is the right way to describe this.
>

Yes that seemed to be the discussion last time.

It's like a shared IRQ but has the extra feature of the polling to determine what device is active.
For my use case the polling is not necessary as there's only one smbalert device per segment.
So an irq would work, however that didn't seem to have much support either.

I don't know what direction to go, to get this functionality into the upstream source.


-- 
Regards
Phil Reid

Re: [PATCH v5 1/6] I2C: i2c-smbus: add device tree support

[Rob Herring]

On Wed, Sep 21, 2016 at 04:41:09PM +0800, Phil Reid wrote:
> From: Andrea Merello <andrea.merello@gmail.com>
> 
> According to Documentation/i2c/smbus-protocol, a smbus controller driver
> that wants to hook-in smbus extensions support, can call
> i2c_setup_smbus_alert(). There are very few drivers that are currently
> doing this.
> 
> However the i2c-smbus module can also work with any
> smbus-extensions-unaware I2C controller, as long as we provide an extra
> IRQ line connected to the I2C bus ALARM signal.
> 
> This patch makes it possible to go this way via DT. Note that the DT node
> will indeed describe a (little) piece of HW, that is the routing of the
> ALARM signal to an IRQ line (so it seems a fair DT use to me, but RFC).
> 
> Note that AFAICT, by design, i2c-smbus module pretends to be an I2C slave
> with address 0x0C (that is the alarm response address), and IMHO this is
> quite consistent with usage in the DT as a I2C child node.
> 
> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
> Signed-off-by: Phil Reid <preid@electromag.com.au>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-smbus.txt | 20 ++++++++++++++++++++
>  drivers/i2c/i2c-smbus.c                             | 20 +++++++++++++++-----
>  2 files changed, 35 insertions(+), 5 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-smbus.txt
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-smbus.txt b/Documentation/devicetree/bindings/i2c/i2c-smbus.txt
> new file mode 100644
> index 0000000..da83127
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/i2c-smbus.txt
> @@ -0,0 +1,20 @@
> +* i2c-smbus extensions
> +
> +Required Properties:
> +  - compatible: Must contain "smbus_alert"
> +  - interrupts: The irq line for smbus ALERT signal
> +  - reg: I2C slave address. Set to 0x0C (alert response address).

This is not right. The 0xC address is special, not an actual device 
address. The bindings should just have the actual device's compatible 
string and address.

> +
> +Note: The i2c-smbus module registers itself as a slave I2C device. Whenever
> +a smbus controller directly support smbus extensions (and its driver supports
> +this), there is no need to add anything special to the DT. Otherwise, for using
> +i2c-smbus with any smbus-extensions-unaware I2C controllers, you need to
> +route the smbus ALARM signal to an extra IRQ line, thus you need to describe
> +this in the DT.

Now, I guess what you need in the kernel is a common handler for 
SMBALERT# and to know which interrupt line is SMBALERT#.

The drivers should know this. A given h/w device will or will not handle 
the "SMB Alert Response Address". So the drivers should register their 
interrupt with the I2C/SMBus core.

If a controller handles the SMBALERT, then it should make itself an 
interrupt controller and that's what slave devices 'interrupts' property 
will point to.

Rob

Re: [PATCH v5 3/6] Documentation: Add sbs-manager device tree node documentation

[Rob Herring]

On Wed, Sep 21, 2016 at 04:41:11PM +0800, Phil Reid wrote:
> From: Karl-Heinz Schneider <karl-heinz@schneider-inet.de>
> 
> This patch adds device tree documentation for the sbs-manager
> 
> Reviewed-by: Phil Reid <preid@electromag.com.au>
> Signed-off-by: Karl-Heinz Schneider <karl-heinz@schneider-inet.de>
> Signed-off-by: Phil Reid <preid@electromag.com.au>
> ---
>  .../bindings/power/supply/sbs,sbs-manager.txt      | 62 ++++++++++++++++++++++
>  1 file changed, 62 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..2a04b30
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.txt
> @@ -0,0 +1,62 @@
> +Binding for sbs-manager
> +
> +Required properties:
> +- compatible: should be "lltc,ltc1760" or use "sbs,sbs-manager" as fallback.
> +- 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.

Is this leftover from the last version? It doesn't look like you are 
using any GPIOs in the example.

> +
> +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. See Documentation/devicetree/bindings/i2c/i2c-mux.txt for more
> +information on i2c-mux nodes. Channels will be numerated starting from 1 to 4.
> +
> +Example:
> +
> +batman@0a {
> +    compatible = "lltc,ltc1760";
> +    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 = <0xb>;
> +        };
> +    };
> +
> +    i2c@2 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        reg = <2>;
> +
> +        battery@b {
> +            compatible = "ti,bq2060", "sbs,sbs-battery";
> +            reg = <0xb>;
> +        };
> +    };
> +
> +    i2c@3 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        reg = <3>;
> +
> +        battery@b {
> +            compatible = "ti,bq2060", "sbs,sbs-battery";
> +            reg = <0xb>;
> +        };
> +    };
> +};
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/6] I2C: i2c-smbus: add device tree support

[Phil Reid]

G'day Rob,

On 24/09/2016 03:42, Rob Herring wrote:
> On Wed, Sep 21, 2016 at 04:41:09PM +0800, Phil Reid wrote:
>> From: Andrea Merello <andrea.merello@gmail.com>
>>
>> According to Documentation/i2c/smbus-protocol, a smbus controller driver
>> that wants to hook-in smbus extensions support, can call
>> i2c_setup_smbus_alert(). There are very few drivers that are currently
>> doing this.
>>
>> However the i2c-smbus module can also work with any
>> smbus-extensions-unaware I2C controller, as long as we provide an extra
>> IRQ line connected to the I2C bus ALARM signal.
>>
>> This patch makes it possible to go this way via DT. Note that the DT node
>> will indeed describe a (little) piece of HW, that is the routing of the
>> ALARM signal to an IRQ line (so it seems a fair DT use to me, but RFC).
>>
>> Note that AFAICT, by design, i2c-smbus module pretends to be an I2C slave
>> with address 0x0C (that is the alarm response address), and IMHO this is
>> quite consistent with usage in the DT as a I2C child node.
>>
>> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
>> Signed-off-by: Phil Reid <preid@electromag.com.au>
>> ---
>>  Documentation/devicetree/bindings/i2c/i2c-smbus.txt | 20 ++++++++++++++++++++
>>  drivers/i2c/i2c-smbus.c                             | 20 +++++++++++++++-----
>>  2 files changed, 35 insertions(+), 5 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-smbus.txt
>>
>> diff --git a/Documentation/devicetree/bindings/i2c/i2c-smbus.txt b/Documentation/devicetree/bindings/i2c/i2c-smbus.txt
>> new file mode 100644
>> index 0000000..da83127
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/i2c/i2c-smbus.txt
>> @@ -0,0 +1,20 @@
>> +* i2c-smbus extensions
>> +
>> +Required Properties:
>> +  - compatible: Must contain "smbus_alert"
>> +  - interrupts: The irq line for smbus ALERT signal
>> +  - reg: I2C slave address. Set to 0x0C (alert response address).
>
> This is not right. The 0xC address is special, not an actual device
> address. The bindings should just have the actual device's compatible
> string and address.
>
>> +
>> +Note: The i2c-smbus module registers itself as a slave I2C device. Whenever
>> +a smbus controller directly support smbus extensions (and its driver supports
>> +this), there is no need to add anything special to the DT. Otherwise, for using
>> +i2c-smbus with any smbus-extensions-unaware I2C controllers, you need to
>> +route the smbus ALARM signal to an extra IRQ line, thus you need to describe
>> +this in the DT.
>
> Now, I guess what you need in the kernel is a common handler for
> SMBALERT# and to know which interrupt line is SMBALERT#.
>
> The drivers should know this. A given h/w device will or will not handle
> the "SMB Alert Response Address". So the drivers should register their
> interrupt with the I2C/SMBus core.
>
> If a controller handles the SMBALERT, then it should make itself an
> interrupt controller and that's what slave devices 'interrupts' property
> will point to.
>

Could a property be added to each i2c bus segment.
Something like the following.

   interrupt-parent = <&irqsrc>;
   interrupts = <0>;
   interrupt-names = "smbalert";

Then the i2c core code could look for this in each segment, either in the parent segment
or a mux segment and create the smbalert common handler for each segment as required.




-- 
Regards
Phil Reid

Re: [PATCH v5 3/6] Documentation: Add sbs-manager device tree node documentation

[Phil Reid]

G'day Rob,
On 24/09/2016 03:44, Rob Herring wrote:
> On Wed, Sep 21, 2016 at 04:41:11PM +0800, Phil Reid wrote:
>> From: Karl-Heinz Schneider <karl-heinz@schneider-inet.de>
>>
>> This patch adds device tree documentation for the sbs-manager
>>
>> Reviewed-by: Phil Reid <preid@electromag.com.au>
>> Signed-off-by: Karl-Heinz Schneider <karl-heinz@schneider-inet.de>
>> Signed-off-by: Phil Reid <preid@electromag.com.au>
>> ---
>>  .../bindings/power/supply/sbs,sbs-manager.txt      | 62 ++++++++++++++++++++++
>>  1 file changed, 62 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..2a04b30
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.txt
>> @@ -0,0 +1,62 @@
>> +Binding for sbs-manager
>> +
>> +Required properties:
>> +- compatible: should be "lltc,ltc1760" or use "sbs,sbs-manager" as fallback.
>> +- 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.
>
> Is this leftover from the last version? It doesn't look like you are
> using any GPIOs in the example.

Sorry didn't update the example. I still need the gpio exposed to the battery.
>
>> +
>> +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. See Documentation/devicetree/bindings/i2c/i2c-mux.txt for more
>> +information on i2c-mux nodes. Channels will be numerated starting from 1 to 4.
>> +
>> +Example:
>> +
>> +batman@0a {
>> +    compatible = "lltc,ltc1760";
>> +    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 = <0xb>;
should have something like:
                 sbs,battery-detect-gpios = <&batman 0 GPIO_ACTIVE_HIGH>;

>> +        };
>> +    };
>> +
>> +    i2c@2 {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +        reg = <2>;
>> +
>> +        battery@b {
>> +            compatible = "ti,bq2060", "sbs,sbs-battery";
>> +            reg = <0xb>;
>> +        };
>> +    };
>> +
>> +    i2c@3 {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +        reg = <3>;
>> +
>> +        battery@b {
>> +            compatible = "ti,bq2060", "sbs,sbs-battery";
>> +            reg = <0xb>;
>> +        };
>> +    };
>> +};
>> --

Let me know your thoughts.

-- 
Regards
Phil Reid