[PATCH v3 0/2] Add support for Smart Battery System Manager

[PATCH v3 0/2] Add support for Smart Battery System Manager

[Karl-Heinz Schneider]

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

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

 .../devicetree/bindings/power/sbs,sbs-manager.txt  |  52 ++++
 drivers/power/Kconfig                              |  13 +
 drivers/power/Makefile                             |   1 +
 drivers/power/sbs-manager.c                        | 325 +++++++++++++++++++++
 4 files changed, 391 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/sbs,sbs-manager.txt
 create mode 100644 drivers/power/sbs-manager.c

--
1.9.1

[PATCH v3 1/2] Documentation: Add sbs-manager device tree node documentation

[Karl-Heinz Schneider]

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>
---
 .../devicetree/bindings/power/sbs,sbs-manager.txt  | 52 ++++++++++++++++++++++
 1 file changed, 52 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/sbs,sbs-manager.txt

diff --git a/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt b/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt
new file mode 100644
index 0000000..f0e2253
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt
@@ -0,0 +1,52 @@
+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>.
+
+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@0a {
+    compatible = "lltc,ltc1760";
+    reg = <0x0a>;
+    #address-cells = <1>;
+    #size-cells = <0>;
+
+    i2c@1 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        reg = <1>;
+
+        battery@0b {
+            compatible = "ti,bq2060", "sbs,sbs-battery";
+            reg = <0x0b>;
+        };
+    };
+
+    i2c@2 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        reg = <2>;
+
+        battery@0b {
+            compatible = "ti,bq2060", "sbs,sbs-battery";
+            reg = <0x0b>;
+        };
+    };
+
+    i2c@3 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        reg = <3>;
+
+        battery@0b {
+            compatible = "ti,bq2060", "sbs,sbs-battery";
+            reg = <0x0b>;
+        };
+    };
+};
--
1.9.1

[PATCH v3 2/2] power: Adds support for Smart Battery System Manager

[Karl-Heinz Schneider]

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>
---
 drivers/power/Kconfig       |  13 ++
 drivers/power/Makefile      |   1 +
 drivers/power/sbs-manager.c | 325 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 339 insertions(+)
 create mode 100644 drivers/power/sbs-manager.c

diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 421770d..7c561bf 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/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/Makefile b/drivers/power/Makefile
index e46b75d..5378e65 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/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/sbs-manager.c b/drivers/power/sbs-manager.c
new file mode 100644
index 0000000..adf9e41
--- /dev/null
+++ b/drivers/power/sbs-manager.c
@@ -0,0 +1,325 @@
+/*
+ * 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) {
+		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 ((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",
+		.owner = THIS_MODULE,
+	},
+	.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.9.1

Re: [PATCH v3 2/2] power: Adds support for Smart Battery System Manager

[kbuild test robot]

Hi,

[auto build test WARNING on battery/master]
[also build test WARNING on v4.7-rc7 next-20160712]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Karl-Heinz-Schneider/Documentation-Add-sbs-manager-device-tree-node-documentation/20160713-025704
base:   git://git.infradead.org/battery-2.6.git master


coccinelle warnings: (new ones prefixed by >>)

>> drivers/power/sbs-manager.c:315:3-8: No need to set .owner here. The core will do it.

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[PATCH] power: fix platform_no_drv_owner.cocci warnings

[kbuild test robot]

drivers/power/sbs-manager.c:315:3-8: No need to set .owner here. The core will do it.

 Remove .owner field if calls are used which set it automatically

Generated by: scripts/coccinelle/api/platform_no_drv_owner.cocci

CC: Karl-Heinz Schneider <karl-heinz@schneider-inet.de>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---

 sbs-manager.c |    1 -
 1 file changed, 1 deletion(-)

--- a/drivers/power/sbs-manager.c
+++ b/drivers/power/sbs-manager.c
@@ -312,7 +312,6 @@ MODULE_DEVICE_TABLE(i2c, sbsm_ids);
 static struct i2c_driver sbsm_driver = {
 	.driver = {
 		.name = "sbsm",
-		.owner = THIS_MODULE,
 	},
 	.probe		= sbsm_probe,
 	.remove		= sbsm_remove,

Re: [PATCH v3 1/2] Documentation: Add sbs-manager device tree node documentation

[Rob Herring]

On Tue, Jul 12, 2016 at 08:48:14PM +0200, Karl-Heinz Schneider wrote:
> 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>
> ---
>  .../devicetree/bindings/power/sbs,sbs-manager.txt  | 52 ++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/sbs,sbs-manager.txt

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v3 1/2] Documentation: Add sbs-manager device tree node documentation

[Karl-Heinz Schneider]

Am Samstag, den 16.07.2016, 17:05 -0500 schrieb Rob Herring:
> On Tue, Jul 12, 2016 at 08:48:14PM +0200, Karl-Heinz Schneider wrote:
> > 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>
> > ---
> >  .../devicetree/bindings/power/sbs,sbs-manager.txt  | 52 ++++++++++++++++++++++
> >  1 file changed, 52 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/power/sbs,sbs-manager.txt
> 
> Acked-by: Rob Herring <robh@kernel.org>
Thanks!
-- 
Karl-Heinz

Re: [PATCH v3 1/2] Documentation: Add sbs-manager device tree node documentation

[Wolfram Sang]

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

> +batman@0a {
> +    compatible = "lltc,ltc1760";
> +    reg = <0x0a>;
> +    #address-cells = <1>;
> +    #size-cells = <0>;
> +
> +    i2c@1 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;

Rob, aren't those two lines inherited from above?


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

Re: [PATCH v3 2/2] power: Adds support for Smart Battery System Manager

[Wolfram Sang]

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


> +	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");

Don't print errors on ENOMEM.

Other than that, looks good to me, yet Peter has more insight to
i2c-muxes these days.


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

Re: [PATCH v3 2/2] power: Adds support for Smart Battery System Manager

[Karl-Heinz Schneider]

Hi Wolfram,

Am Dienstag, den 19.07.2016, 06:17 +0200 schrieb Wolfram Sang:
> > +	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");
> 
> Don't print errors on ENOMEM.

Will be fixed on next revision.

> 
> Other than that, looks good to me, yet Peter has more insight to
> i2c-muxes these days.
> 
Tanks for review. 

Will wait another few days before sending a new revision, hence changes
are rather small.
-- 
Karl-Heinz

Re: [PATCH v3 1/2] Documentation: Add sbs-manager device tree node documentation

[Peter Rosin]

On 2016-07-19 06:14, Wolfram Sang wrote:
>> +batman@0a {
>> +    compatible = "lltc,ltc1760";
>> +    reg = <0x0a>;
>> +    #address-cells = <1>;
>> +    #size-cells = <0>;
>> +
>> +    i2c@1 {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
> 
> Rob, aren't those two lines inherited from above?

I too think they are inherited, but am a bit undecided whether I want
that fact to be explored or not, as they just happen to match. They do
not express the same thing (it is for the i2c-mux number in the parent,
and for the i2c device address in the children).

But the repetitions are indeed tedious when there are many muxed
buses...

Cheers,
Peter

Re: [PATCH v3 2/2] power: Adds support for Smart Battery System Manager

[Peter Rosin]

On 2016-07-19 20:51, Karl-Heinz Schneider wrote:
> Hi Wolfram,
> 
> Am Dienstag, den 19.07.2016, 06:17 +0200 schrieb Wolfram Sang:
>>> +	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");
>>
>> Don't print errors on ENOMEM.
> 
> Will be fixed on next revision.
> 
>>
>> Other than that, looks good to me, yet Peter has more insight to
>> i2c-muxes these days.
>>
> Tanks for review. 
> 
> Will wait another few days before sending a new revision, hence changes
> are rather small.

Looks good to me from a muxing perspective, with very a minor nit that the
devicetree bindings do not point to i2c-mux.txt for where "standard i2c-mux
nodes" are described.

Cheers,
Peter

Re: [PATCH v3 1/2] Documentation: Add sbs-manager device tree node documentation

[Wolfram Sang]

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

On Mon, Jul 25, 2016 at 12:33:20PM +0200, Peter Rosin wrote:
> On 2016-07-19 06:14, Wolfram Sang wrote:
> >> +batman@0a {
> >> +    compatible = "lltc,ltc1760";
> >> +    reg = <0x0a>;
> >> +    #address-cells = <1>;
> >> +    #size-cells = <0>;
> >> +
> >> +    i2c@1 {
> >> +        #address-cells = <1>;
> >> +        #size-cells = <0>;
> > 
> > Rob, aren't those two lines inherited from above?
> 
> I too think they are inherited, but am a bit undecided whether I want
> that fact to be explored or not, as they just happen to match. They do
> not express the same thing (it is for the i2c-mux number in the parent,
> and for the i2c device address in the children).

Ah, I see. If they don't mean the same thing, then we should be rather
safe than sorry, I guess.

> But the repetitions are indeed tedious when there are many muxed
> buses...

Yes.


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

Re: [PATCH v3 2/2] power: Adds support for Smart Battery System Manager

[Karl-Heinz Schneider]

Hi Peter, Hi Rob,

Am Montag, den 25.07.2016, 12:44 +0200 schrieb Peter Rosin:
> On 2016-07-19 20:51, Karl-Heinz Schneider wrote:
> >> Other than that, looks good to me, yet Peter has more insight to
> >> i2c-muxes these days.
> >>
> > Tanks for review. 
> > 
> > Will wait another few days before sending a new revision, hence changes
> > are rather small.
> 
> Looks good to me from a muxing perspective, with very a minor nit that the
> devicetree bindings do not point to i2c-mux.txt for where "standard i2c-mux
> nodes" are described.
Right, will add it.

like this: 
@@ -7,7 +7,8 @@ Required properties:
 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.
+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:

Rob, are you OK with that change?
> 
> Cheers,
> Peter
tanks.
-- 
Karl-Heinz

[PATCH v4 0/2] Add support for Smart Battery System Manager

[Karl-Heinz Schneider]

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

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

 .../devicetree/bindings/power/sbs,sbs-manager.txt  |  53 ++++
 drivers/power/Kconfig                              |  13 +
 drivers/power/Makefile                             |   1 +
 drivers/power/sbs-manager.c                        | 323 +++++++++++++++++++++
 4 files changed, 390 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/sbs,sbs-manager.txt
 create mode 100644 drivers/power/sbs-manager.c

--
2.7.4

[PATCH v4 1/2] Documentation: Add sbs-manager device tree node documentation

[Karl-Heinz Schneider]

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>
---
 .../devicetree/bindings/power/sbs,sbs-manager.txt  | 53 ++++++++++++++++++++++
 1 file changed, 53 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/sbs,sbs-manager.txt

diff --git a/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt b/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt
new file mode 100644
index 0000000..6b1a87ce
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt
@@ -0,0 +1,53 @@
+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>.
+
+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>;
+
+    i2c@1 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        reg = <1>;
+
+        battery@0b {
+            compatible = "ti,bq2060", "sbs,sbs-battery";
+            reg = <0x0b>;
+        };
+    };
+
+    i2c@2 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        reg = <2>;
+
+        battery@0b {
+            compatible = "ti,bq2060", "sbs,sbs-battery";
+            reg = <0x0b>;
+        };
+    };
+
+    i2c@3 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        reg = <3>;
+
+        battery@0b {
+            compatible = "ti,bq2060", "sbs,sbs-battery";
+            reg = <0x0b>;
+        };
+    };
+};
-- 
2.7.4

[PATCH v4 2/2] power: Adds support for Smart Battery System Manager

[Karl-Heinz Schneider]

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>
---
 drivers/power/Kconfig       |  13 ++
 drivers/power/Makefile      |   1 +
 drivers/power/sbs-manager.c | 323 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 337 insertions(+)
 create mode 100644 drivers/power/sbs-manager.c

diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index acd4a15..6816153 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/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/Makefile b/drivers/power/Makefile
index e46b75d..5378e65 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/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/sbs-manager.c b/drivers/power/sbs-manager.c
new file mode 100644
index 0000000..e3debd7
--- /dev/null
+++ b/drivers/power/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");
-- 
2.7.4

Re: [PATCH v4 1/2] Documentation: Add sbs-manager device tree node documentation

[Rob Herring]

On Thu, Aug 25, 2016 at 10:21:00PM +0200, Karl-Heinz Schneider wrote:
> 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>
> ---
>  .../devicetree/bindings/power/sbs,sbs-manager.txt  | 53 ++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/sbs,sbs-manager.txt
> 
> diff --git a/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt b/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt
> new file mode 100644
> index 0000000..6b1a87ce
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt
> @@ -0,0 +1,53 @@
> +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>.

What happened to adding Phil's interrupt support into this? You don't 
have to add the whole patch, just the binding part. The driver support 
can come later.

> +
> +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 {

drop leading 0.

> +    compatible = "lltc,ltc1760";
> +    reg = <0x0a>;
> +    #address-cells = <1>;
> +    #size-cells = <0>;
> +
> +    i2c@1 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        reg = <1>;
> +
> +        battery@0b {

and here...

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

Re: [PATCH v4 0/2] Add support for Smart Battery System Manager

[Sebastian Reichel]

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

Hi,

On Thu, Aug 25, 2016 at 10:20:59PM +0200, Karl-Heinz Schneider wrote:
> 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.

Driver looks fine to me. Waiting for Rob's ACK on the DT bindings.

-- Sebastian

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

Re: [PATCH v4 1/2] Documentation: Add sbs-manager device tree node documentation

[Karl-Heinz Schneider]

Hi Rob,

Am Mittwoch, den 31.08.2016, 09:43 -0500 schrieb Rob Herring:
> On Thu, Aug 25, 2016 at 10:21:00PM +0200, Karl-Heinz Schneider wrote:
> > 
> > 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>
> > ---
> >  .../devicetree/bindings/power/sbs,sbs-manager.txt  | 53
> > ++++++++++++++++++++++
> >  1 file changed, 53 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/power/sbs,sbs-manager.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/power/sbs,sbs-
> > manager.txt b/Documentation/devicetree/bindings/power/sbs,sbs-
> > manager.txt
> > new file mode 100644
> > index 0000000..6b1a87ce
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt
> > @@ -0,0 +1,53 @@
> > +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>.
> What happened to adding Phil's interrupt support into this? You
> don't 
> have to add the whole patch, just the binding part. The driver
> support 
> can come later.

Phil revoked his patch. Right now it's not clear how the interrupt/gpio
solution will look like and if they will need an device tree binding at
all...

> 
> > 
> > +
> > +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 {
> drop leading 0.

OK.

> > 
> > +    compatible = "lltc,ltc1760";
> > +    reg = <0x0a>;

Will remove leading 0 at this places too.

> > +    #address-cells = <1>;
> > +    #size-cells = <0>;
> > +
> > +    i2c@1 {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +        reg = <1>;
> > +
> > +        battery@0b {
> and here...

OK.

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


Thanks for review Rob. 
-- 
Karl-Heinz

Re: [PATCH v4 0/2] Add support for Smart Battery System Manager

[Karl-Heinz Schneider]

Hi Sebastian,

Am Mittwoch, den 31.08.2016, 17:10 +0200 schrieb Sebastian Reichel:
> Hi,
> 
> On Thu, Aug 25, 2016 at 10:20:59PM +0200, Karl-Heinz Schneider wrote:
> > 
> > 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.
> Driver looks fine to me. Waiting for Rob's ACK on the DT bindings.
> 
> -- Sebastian
Thanks
-- 
Karl-Heinz

Re: [PATCH v4 1/2] Documentation: Add sbs-manager device tree node documentation

[Karl-Heinz Schneider]

Hi Rob,

Sorry for resending this mail, evolution mixed it up...

On Wed, 31 Aug 2016 09:43:49 -0500, Rob Herring wrote:
> On Thu, Aug 25, 2016 at 10:21:00PM +0200, Karl-Heinz Schneider wrote:
>> 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>
>> ---
>>  .../devicetree/bindings/power/sbs,sbs-manager.txt  | 53 
>> ++++++++++++++++++++++
>>  1 file changed, 53 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/power/sbs,sbs-manager.txt
>>
>> diff --git 
>> a/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt 
>> b/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt
>> new file mode 100644
>> index 0000000..6b1a87ce
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt
>> @@ -0,0 +1,53 @@
>> +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>.
>
> What happened to adding Phil's interrupt support into this? You don't
> have to add the whole patch, just the binding part. The driver 
> support
> can come later.

Phil revoked his patch. Right now it's not clear how the interrupt/gpio
solution will look like and if they will need an device tree binding at
all...

>
>> +
>> +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 {
>
> drop leading 0.

OK.

>> +    compatible = "lltc,ltc1760";
>> +    reg = <0x0a>;


Will remove leading 0 at this places too.

>> +    #address-cells = <1>;
>> +    #size-cells = <0>;
>> +
>> +    i2c@1 {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +        reg = <1>;
>> +
>> +        battery@0b {
>
> and here...

OK.

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

Thanks for review Rob.
-- 
Karl-Heinz

Re: [PATCH v4 1/2] Documentation: Add sbs-manager device tree node documentation

[Phil Reid]

On 1/09/2016 03:52, Karl-Heinz Schneider wrote:
> Hi Rob,
>
> Am Mittwoch, den 31.08.2016, 09:43 -0500 schrieb Rob Herring:
>> On Thu, Aug 25, 2016 at 10:21:00PM +0200, Karl-Heinz Schneider wrote:
>>>
>>> This patch adds device tree documentation for the sbs-manager
>>>
>>> Reviewed-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
>>> Signed-off-by: Karl-Heinz Schneider <karl-heinz-X5L7DgJ4l23oE99TX8zNy7NAH6kLmebB@public.gmane.org>
>>> ---
>>>  .../devicetree/bindings/power/sbs,sbs-manager.txt  | 53
>>> ++++++++++++++++++++++
>>>  1 file changed, 53 insertions(+)
>>>  create mode 100644
>>> Documentation/devicetree/bindings/power/sbs,sbs-manager.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/power/sbs,sbs-
>>> manager.txt b/Documentation/devicetree/bindings/power/sbs,sbs-
>>> manager.txt
>>> new file mode 100644
>>> index 0000000..6b1a87ce
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt
>>> @@ -0,0 +1,53 @@
>>> +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>.
>> What happened to adding Phil's interrupt support into this? You
>> don't
>> have to add the whole patch, just the binding part. The driver
>> support
>> can come later.
>
> Phil revoked his patch. Right now it's not clear how the interrupt/gpio
> solution will look like and if they will need an device tree binding at
> all...

I'm getting back to this now.
I think we can use the smbalert driver to handle the source interrupt.
Which I think doesn't need anything in the dt binding. Driver just needs to
implement the alert callback.

I haven't figured out if the smb_alert could be used for the sbs-battery
driver interrupt or if this would make sense. However I think it would still need
to be a gpio controller in either case so that it can report the presence of the
batteries in a manner suitable for the sbs-battery driver to use.
I think the gpio controller approach is the simpler method.

It looks like the smbalert_driver driver needs some updates to work with device trees.
I need a bit more time to investigate this option.


>
>>
>>>
>>> +
>>> +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 {
>> drop leading 0.
>
> OK.
>
>>>
>>> +    compatible = "lltc,ltc1760";
>>> +    reg = <0x0a>;
>
> Will remove leading 0 at this places too.
>
>>> +    #address-cells = <1>;
>>> +    #size-cells = <0>;
>>> +
>>> +    i2c@1 {
>>> +        #address-cells = <1>;
>>> +        #size-cells = <0>;
>>> +        reg = <1>;
>>> +
>>> +        battery@0b {
>> and here...
>
> OK.
>
>>>
>>> +            compatible = "ti,bq2060", "sbs,sbs-battery";
>>> +            reg = <0x0b>;
>>> +        };
>>> +    };
>>> +
>>> +    i2c@2 {
>>> +        #address-cells = <1>;
>>> +        #size-cells = <0>;
>>> +        reg = <2>;
>>> +
>>> +        battery@0b {
>>> +            compatible = "ti,bq2060", "sbs,sbs-battery";
>>> +            reg = <0x0b>;
>>> +        };
>>> +    };
>>> +
>>> +    i2c@3 {
>>> +        #address-cells = <1>;
>>> +        #size-cells = <0>;
>>> +        reg = <3>;
>>> +
>>> +        battery@0b {
>>> +            compatible = "ti,bq2060", "sbs,sbs-battery";
>>> +            reg = <0x0b>;
>>> +        };
>>> +    };
>>> +};
>
>
> Thanks for review Rob.
>


-- 
Regards
Phil Reid

ElectroMagnetic Imaging Technology Pty Ltd
Development of Geophysical Instrumentation & Software
www.electromag.com.au

3 The Avenue, Midland WA 6056, AUSTRALIA
Ph: +61 8 9250 8100
Fax: +61 8 9250 7100
Email: preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org
--
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 v4 1/2] Documentation: Add sbs-manager device tree node documentation

[Phil Reid]

Sorry for the resend, I noticed my reply also got horrible mangled.

On 1/09/2016 06:34, Karl-Heinz Schneider wrote:
> Hi Rob,
>
> Sorry for resending this mail, evolution mixed it up...
>
> On Wed, 31 Aug 2016 09:43:49 -0500, Rob Herring wrote:
>> On Thu, Aug 25, 2016 at 10:21:00PM +0200, Karl-Heinz Schneider wrote:
>>> 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>
>>> ---
>>>  .../devicetree/bindings/power/sbs,sbs-manager.txt  | 53 ++++++++++++++++++++++
>>>  1 file changed, 53 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/power/sbs,sbs-manager.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt b/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt
>>> new file mode 100644
>>> index 0000000..6b1a87ce
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt
>>> @@ -0,0 +1,53 @@
>>> +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>.
>>
>> What happened to adding Phil's interrupt support into this? You don't
>> have to add the whole patch, just the binding part. The driver support
>> can come later.
>
> Phil revoked his patch. Right now it's not clear how the interrupt/gpio
> solution will look like and if they will need an device tree binding at
> all...
>


I'm getting back to this now.
I think we can use the smbalert driver to handle the source interrupt.
Which I think doesn't need anything in the dt binding. Driver just needs to
implement the alert callback.
I haven't figured out if the smb_alert could be used for the sbs-battery
driver interrupt or if this would make sense. However I think it would still need
to be a gpio controller in either case so that it can report the presence of the
batteries in a manner suitable for the sbs-battery driver to use.
I think the gpio controller approach is the simpler method.
It looks like the smbalert_driver driver needs some updates to work with device trees.
I need a bit more time to investigate this option.

-- 
Regards
Phil Reid