[PATCH v5 0/9] Add sbs-manager with smbalert support

[PATCH v5 0/9] Add sbs-manager with smbalert support

[Phil Reid]

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

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

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

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



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

Phil Reid (7):
  i2c: i2c-smbus: Support threaded irqs.
  i2c: i2c-smbus: Add null ptr guard in smb_alert_probe
  i2c: i2c-smbus: add of_i2c_setup_smbus_alert
  i2c: core: call of_i2c_setup_smbus_alert in i2c_register_adapter
  i2c: mux: pca954x: Call request irq after adding mux segments
  power: supply: sbs-battery: Add alert callback
  power: supply: sbs-manager: Add alert callback and battery change
    notification

 Documentation/devicetree/bindings/i2c/i2c.txt      |   4 +-
 .../bindings/power/supply/sbs,sbs-manager.txt      |  64 +++
 drivers/i2c/i2c-core.c                             |   4 +
 drivers/i2c/i2c-smbus.c                            |  64 ++-
 drivers/i2c/muxes/i2c-mux-pca954x.c                |  52 ++-
 drivers/power/supply/Kconfig                       |  14 +
 drivers/power/supply/Makefile                      |   1 +
 drivers/power/supply/sbs-battery.c                 |  16 +-
 drivers/power/supply/sbs-manager.c                 | 448 +++++++++++++++++++++
 include/linux/i2c-smbus.h                          |   9 +
 10 files changed, 637 insertions(+), 39 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.txt
 create mode 100644 drivers/power/supply/sbs-manager.c

-- 
1.8.3.1

[PATCH v5 1/9] i2c: i2c-smbus: Support threaded irqs.

[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 dereferenced. Split
irq up into separate functions to allow thread / non thread irq to work
correctly.

Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 drivers/i2c/i2c-smbus.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
index f9271c7..b272493 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -72,13 +72,12 @@ static int smbus_do_alert(struct device *dev, void *addrp)
  * The alert IRQ handler needs to hand work off to a task which can issue
  * SMBus calls, because those sleeping calls can't be made in IRQ context.
  */
-static void smbus_alert(struct work_struct *work)
+static irqreturn_t smbus_alert(int irq, void *d)
 {
-	struct i2c_smbus_alert *alert;
+	struct i2c_smbus_alert *alert = d;
 	struct i2c_client *ara;
 	unsigned short prev_addr = 0;	/* Not a valid address */
 
-	alert = container_of(work, struct i2c_smbus_alert, alert);
 	ara = alert->ara;
 
 	for (;;) {
@@ -115,6 +114,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);
@@ -148,12 +158,14 @@ static int smbalert_probe(struct i2c_client *ara,
 
 	alert->alert_edge_triggered = setup->alert_edge_triggered;
 	alert->irq = setup->irq;
-	INIT_WORK(&alert->alert, smbus_alert);
+	INIT_WORK(&alert->alert, smbalert_work);
 	alert->ara = ara;
 
 	if (setup->irq > 0) {
-		res = devm_request_irq(&ara->dev, setup->irq, smbalert_irq,
-				       0, "smbus_alert", alert);
+		res = devm_request_threaded_irq(&ara->dev, alert->irq,
+						smbalert_irq, smbus_alert,
+						IRQF_SHARED | IRQF_ONESHOT,
+						"smbus_alert", alert);
 		if (res)
 			return res;
 	}
-- 
1.8.3.1

[PATCH v5 2/9] i2c: i2c-smbus: Add null ptr guard in smb_alert_probe

[Phil Reid]

If platdata is not correct setup before registering the device
a null ptr derefence can occur.

Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 drivers/i2c/i2c-smbus.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
index b272493..df0e2fa 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -151,6 +151,11 @@ static int smbalert_probe(struct i2c_client *ara,
 	struct i2c_adapter *adapter = ara->adapter;
 	int res;
 
+	if (!setup) {
+		dev_err(&adapter->dev, "setup not defined\n");
+		return -ENODATA;
+	}
+
 	alert = devm_kzalloc(&ara->dev, sizeof(struct i2c_smbus_alert),
 			     GFP_KERNEL);
 	if (!alert)
-- 
1.8.3.1

[PATCH v5 3/9] i2c: i2c-smbus: add of_i2c_setup_smbus_alert

[Phil Reid]

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

Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 Documentation/devicetree/bindings/i2c/i2c.txt |  4 +--
 drivers/i2c/i2c-smbus.c                       | 35 +++++++++++++++++++++++++++
 include/linux/i2c-smbus.h                     |  9 +++++++
 3 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
index cee9d50..1126398 100644
--- a/Documentation/devicetree/bindings/i2c/i2c.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c.txt
@@ -59,8 +59,8 @@ wants to support one of the below features, it should adapt the bindings below.
 	interrupts used by the device.
 
 - interrupt-names
-	"irq" and "wakeup" names are recognized by I2C core, other names are
-	left to individual drivers.
+	"irq", "wakeup" and "smbus_alert" names are recognized by I2C core,
+	other names are	left to individual drivers.
 
 - host-notify
 	device uses SMBus host notify protocol instead of interrupt line.
diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
index df0e2fa..a8f8439 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -21,6 +21,7 @@
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of_irq.h>
 #include <linux/slab.h>
 #include <linux/workqueue.h>
 
@@ -238,6 +239,40 @@ struct i2c_client *i2c_setup_smbus_alert(struct i2c_adapter *adapter,
 }
 EXPORT_SYMBOL_GPL(i2c_setup_smbus_alert);
 
+int of_i2c_setup_smbus_alert(struct i2c_adapter *adap)
+{
+	struct i2c_client *client;
+	struct i2c_smbus_alert_setup *setup;
+	struct i2c_board_info info = {
+		I2C_BOARD_INFO("smbus_alert", 0x0c),
+	};
+	int irq;
+
+	if (!adap->dev.of_node)
+		return 0;
+
+	irq = of_irq_get_byname(adap->dev.of_node, "smbus_alert");
+	if (irq == -EINVAL || irq == -ENODATA)
+		return 0;
+	else if (irq < 0)
+		return irq;
+
+	setup = devm_kzalloc(&adap->dev, sizeof(struct i2c_smbus_alert_setup),
+		GFP_KERNEL);
+	if (!setup)
+		return -ENOMEM;
+
+	setup->irq = irq;
+	info.platform_data = setup;
+
+	client = i2c_new_device(adap, &info);
+	if (!client)
+		return -ENODEV;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_i2c_setup_smbus_alert);
+
 /**
  * i2c_handle_smbus_alert - Handle an SMBus alert
  * @ara: the ARA client on the relevant adapter
diff --git a/include/linux/i2c-smbus.h b/include/linux/i2c-smbus.h
index a138502..4732d09 100644
--- a/include/linux/i2c-smbus.h
+++ b/include/linux/i2c-smbus.h
@@ -50,4 +50,13 @@ struct i2c_client *i2c_setup_smbus_alert(struct i2c_adapter *adapter,
 					 struct i2c_smbus_alert_setup *setup);
 int i2c_handle_smbus_alert(struct i2c_client *ara);
 
+#if IS_ENABLED(CONFIG_I2C_SMBUS)
+int of_i2c_setup_smbus_alert(struct i2c_adapter *adap);
+#else
+static inline int of_i2c_setup_smbus_alert(struct i2c_adapter *adap)
+{
+	return 0;
+}
+#endif
+
 #endif /* _LINUX_I2C_SMBUS_H */
-- 
1.8.3.1

[PATCH v5 4/9] i2c: core: call of_i2c_setup_smbus_alert in i2c_register_adapter

[Phil Reid]

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

Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 drivers/i2c/i2c-core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index d2402bb..626471b 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -40,6 +40,7 @@
 #include <linux/gpio.h>
 #include <linux/hardirq.h>
 #include <linux/i2c.h>
+#include <linux/i2c-smbus.h>
 #include <linux/idr.h>
 #include <linux/init.h>
 #include <linux/irqflags.h>
@@ -2045,6 +2046,9 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
 		dev_warn(&adap->dev,
 			 "Failed to create compatibility class link\n");
 #endif
+	res = of_i2c_setup_smbus_alert(adap);
+	if (res)
+		goto out_list;
 
 	i2c_init_recovery(adap);
 
-- 
1.8.3.1

Re: [PATCH v5 4/9] i2c: core: call of_i2c_setup_smbus_alert in i2c_register_adapter

[kbuild test robot]

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

Hi Phil,

[auto build test ERROR on robh/for-next]
[also build test ERROR on v4.11 next-20170501]
[cannot apply to wsa/i2c/for-next battery/master]
[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/Phil-Reid/Add-sbs-manager-with-smbalert-support/20170501-170247
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   drivers/built-in.o: In function `nsp_usb3_phy_init':
   binder.c:(.text+0x13744): undefined reference to `mdiobus_write'
   binder.c:(.text+0x13760): undefined reference to `mdiobus_write'
   binder.c:(.text+0x1377c): undefined reference to `mdiobus_write'
   binder.c:(.text+0x13798): undefined reference to `mdiobus_write'
   binder.c:(.text+0x137b4): undefined reference to `mdiobus_write'
   drivers/built-in.o:binder.c:(.text+0x137e8): more undefined references to `mdiobus_write' follow
   drivers/built-in.o: In function `i2c_register_adapter':
>> binder.c:(.text+0x21646c): undefined reference to `of_i2c_setup_smbus_alert'
   drivers/built-in.o: In function `mdio_module_init':
   binder.c:(.init.text+0x7dd4): undefined reference to `mdio_driver_register'
   drivers/built-in.o: In function `mdio_module_exit':
   binder.c:(.exit.text+0x19c): undefined reference to `mdio_driver_unregister'

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 61325 bytes --]

[PATCH v5 5/9] i2c: mux: pca954x: Call request irq after adding mux segments

[Phil Reid]

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

Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 drivers/i2c/muxes/i2c-mux-pca954x.c | 52 +++++++++++++++++--------------------
 1 file changed, 24 insertions(+), 28 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index ad31d21..4299738 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -294,7 +294,7 @@ static int pca954x_irq_setup(struct i2c_mux_core *muxc)
 {
 	struct pca954x *data = i2c_mux_priv(muxc);
 	struct i2c_client *client = data->client;
-	int c, err, irq;
+	int c, irq;
 
 	if (!data->chip->has_irq || client->irq <= 0)
 		return 0;
@@ -314,24 +314,22 @@ static int pca954x_irq_setup(struct i2c_mux_core *muxc)
 			handle_simple_irq);
 	}
 
-	err = devm_request_threaded_irq(&client->dev, data->client->irq, NULL,
-					pca954x_irq_handler,
-					IRQF_ONESHOT | IRQF_SHARED,
-					"pca954x", data);
-	if (err)
-		goto err_req_irq;
+	return 0;
+}
 
-	disable_irq(data->client->irq);
+static void pca954x_cleanup(struct i2c_mux_core *muxc)
+{
+	struct pca954x *data = i2c_mux_priv(muxc);
+	int c, irq;
 
-	return 0;
-err_req_irq:
-	for (c = 0; c < data->chip->nchans; c++) {
-		irq = irq_find_mapping(data->irq, c);
-		irq_dispose_mapping(irq);
+	if (data->irq) {
+		for (c = 0; c < data->chip->nchans; c++) {
+			irq = irq_find_mapping(data->irq, c);
+			irq_dispose_mapping(irq);
+		}
+		irq_domain_remove(data->irq);
 	}
-	irq_domain_remove(data->irq);
-
-	return err;
+	i2c_mux_del_adapters(muxc);
 }
 
 /*
@@ -422,6 +420,14 @@ static int pca954x_probe(struct i2c_client *client,
 		}
 	}
 
+	if (data->chip->has_irq || client->irq > 0) {
+		ret = devm_request_threaded_irq(&client->dev, data->client->irq,
+		NULL, pca954x_irq_handler, IRQF_ONESHOT | IRQF_SHARED,
+		"pca954x", data);
+		if (ret)
+			goto fail_del_adapters;
+	}
+
 	dev_info(&client->dev,
 		 "registered %d multiplexed busses for I2C %s %s\n",
 		 num, data->chip->muxtype == pca954x_ismux
@@ -430,25 +436,15 @@ static int pca954x_probe(struct i2c_client *client,
 	return 0;
 
 fail_del_adapters:
-	i2c_mux_del_adapters(muxc);
+	pca954x_cleanup(muxc);
 	return ret;
 }
 
 static int pca954x_remove(struct i2c_client *client)
 {
 	struct i2c_mux_core *muxc = i2c_get_clientdata(client);
-	struct pca954x *data = i2c_mux_priv(muxc);
-	int c, irq;
 
-	if (data->irq) {
-		for (c = 0; c < data->chip->nchans; c++) {
-			irq = irq_find_mapping(data->irq, c);
-			irq_dispose_mapping(irq);
-		}
-		irq_domain_remove(data->irq);
-	}
-
-	i2c_mux_del_adapters(muxc);
+	pca954x_cleanup(muxc);
 	return 0;
 }
 
-- 
1.8.3.1

[PATCH v5 6/9] 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      | 64 ++++++++++++++++++++++
 1 file changed, 64 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..c523b37
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.txt
@@ -0,0 +1,64 @@
+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. 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@0b {
+            compatible = "ti,bq2060", "sbs,sbs-battery";
+            reg = <0x0b>;
+            sbs,battery-detect-gpios = <&batman 1 1>;
+        };
+    };
+
+    i2c@2 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        reg = <2>;
+
+        battery@0b {
+            compatible = "ti,bq2060", "sbs,sbs-battery";
+            reg = <0x0b>;
+            sbs,battery-detect-gpios = <&batman 2 1>;
+        };
+    };
+
+    i2c@3 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        reg = <3>;
+
+        battery@0b {
+            compatible = "ti,bq2060", "sbs,sbs-battery";
+            reg = <0x0b>;
+            sbs,battery-detect-gpios = <&batman 3 1>;
+        };
+    };
+};
-- 
1.8.3.1

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

[Sebastian Reichel]

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

Hi,

Just two notes about compatible. Looks fine otherwise.

On Mon, May 01, 2017 at 04:49:56PM +0800, Phil Reid wrote:
> From: Karl-Heinz Schneider <karl-heinz-X5L7DgJ4l23oE99TX8zNy7NAH6kLmebB@public.gmane.org>
> 
> 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>
> Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
> ---
>  .../bindings/power/supply/sbs,sbs-manager.txt      | 64 ++++++++++++++++++++++
>  1 file changed, 64 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..c523b37
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.txt
> @@ -0,0 +1,64 @@
> +Binding for sbs-manager
> +
> +Required properties:
> +- compatible: should be "lltc,ltc1760" or use "sbs,sbs-manager" as fallback.

Use the same style as sbs-charger:

Required properties:
 - compatible: "<vendor>,<part-number>", "sbs,sbs-manager" as fallback. The part
     number compatible string might be used in order to take care of vendor
     specific registers.

> +- reg: integer, i2c address of the device. Should be <0xa>.
> +Optional properties:
> +- gpio-controller: Marks the port as GPIO controller.
> +  See "gpio-specifier" in .../devicetree/bindings/gpio/gpio.txt.
> +- #gpio-cells: Should be <2>. The first cell is the pin number, the second cell
> +  is used to specify optional parameters:
> +  See "gpio-specifier" in .../devicetree/bindings/gpio/gpio.txt.
> +
> +From OS view the device is basically an i2c-mux used to communicate with up to
> +four smart battery devices at address 0xb. The driver actually implements this
> +behaviour. So standard i2c-mux nodes can be used to register up to four slave
> +batteries. Channels will be numerated starting from 1 to 4.
> +
> +Example:
> +
> +batman@0a {
> +    compatible = "lltc,ltc1760";

This should be

compatible = "lltc,ltc1760", "sbs,sbs-manager";

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

-- Sebastian

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

[PATCH v5 7/9] 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 | 325 +++++++++++++++++++++++++++++++++++++
 3 files changed, 339 insertions(+)
 create mode 100644 drivers/power/supply/sbs-manager.c

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index da54ac8..8aa5324 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -533,4 +533,17 @@ config AXP20X_POWER
 	  This driver provides support for the power supply features of
 	  AXP20x PMIC.
 
+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.
+
 endif # POWER_SUPPLY
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index 3789a2c..4f53c98 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -74,3 +74,4 @@ obj-$(CONFIG_CHARGER_TPS65090)	+= tps65090-charger.o
 obj-$(CONFIG_CHARGER_TPS65217)	+= tps65217_charger.o
 obj-$(CONFIG_AXP288_FUEL_GAUGE) += axp288_fuel_gauge.o
 obj-$(CONFIG_AXP288_CHARGER)	+= axp288_charger.o
+obj-$(CONFIG_MANAGER_SBS)	+= sbs-manager.o
diff --git a/drivers/power/supply/sbs-manager.c b/drivers/power/supply/sbs-manager.c
new file mode 100644
index 0000000..adf9e41
--- /dev/null
+++ b/drivers/power/supply/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.8.3.1

Re: [PATCH v5 7/9] power: Adds support for Smart Battery System Manager

[Sebastian Reichel]

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

Hi,

On Mon, May 01, 2017 at 04:49:57PM +0800, Phil Reid wrote:
> From: Karl-Heinz Schneider <karl-heinz-X5L7DgJ4l23oE99TX8zNy7NAH6kLmebB@public.gmane.org>
> 
> This patch adds support for Smart Battery System Manager.
> A SBSM is a device listening at I2C/SMBus address 0x0a and is capable of
> communicating up to four I2C smart battery devices. All smart battery
> devices are listening at address 0x0b, so the SBSM muliplexes between
> them. The driver makes use of the I2C-Mux framework to allow smart
> batteries to be bound via device tree, i.e. the sbs-battery driver.
> 
> Via sysfs interface the online state and charge type are presented. If
> the driver is bound as ltc1760 (an implementation of a Dual Smart Battery
> System Manager) the charge type can also be changed from trickle to fast.
> 
> Tested-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
> Reviewed-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
> Signed-off-by: Karl-Heinz Schneider <karl-heinz-X5L7DgJ4l23oE99TX8zNy7NAH6kLmebB@public.gmane.org>
> Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
> ---
>  drivers/power/supply/Kconfig       |  13 ++
>  drivers/power/supply/Makefile      |   1 +
>  drivers/power/supply/sbs-manager.c | 325 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 339 insertions(+)
>  create mode 100644 drivers/power/supply/sbs-manager.c
> 
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index da54ac8..8aa5324 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -533,4 +533,17 @@ config AXP20X_POWER
>  	  This driver provides support for the power supply features of
>  	  AXP20x PMIC.
>  
> +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.
> +

Let's move add this directly next to CHARGER_SBS.

>  endif # POWER_SUPPLY
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index 3789a2c..4f53c98 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -74,3 +74,4 @@ obj-$(CONFIG_CHARGER_TPS65090)	+= tps65090-charger.o
>  obj-$(CONFIG_CHARGER_TPS65217)	+= tps65217_charger.o
>  obj-$(CONFIG_AXP288_FUEL_GAUGE) += axp288_fuel_gauge.o
>  obj-$(CONFIG_AXP288_CHARGER)	+= axp288_charger.o
> +obj-$(CONFIG_MANAGER_SBS)	+= sbs-manager.o

same here.

> diff --git a/drivers/power/supply/sbs-manager.c b/drivers/power/supply/sbs-manager.c
> new file mode 100644
> index 0000000..adf9e41
> --- /dev/null
> +++ b/drivers/power/supply/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-X5L7DgJ4l23oE99TX8zNy7NAH6kLmebB@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/i2c-mux.h>
> +#include <linux/power_supply.h>
> +
> +#define SBSM_MAX_BATS  4
> +#define SBSM_RETRY_CNT 3
> +
> +/* registers addresses */
> +#define SBSM_CMD_BATSYSSTATE     0x01
> +#define SBSM_CMD_BATSYSSTATECONT 0x02
> +#define SBSM_CMD_BATSYSINFO      0x04
> +#define SBSM_CMD_LTC             0x3c
> +
> +struct sbsm_data {
> +	struct i2c_client *client;
> +	struct i2c_mux_core *muxc;
> +
> +	struct power_supply *psy;
> +
> +	int cur_chan;         /* currently selected channel */

u8?

> +	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;
> +	}

for (retries = SBSM_RETRY_CNT; retries > 0; retries--)

> +
> +	if (reg < 0) {
> +		dev_err(&client->dev, "failed to read register %i\n",
> +			(int)address);

dev_err(&client->dev, "failed to read register 0x%02x\n", address);

> +		return reg;
> +	}
> +
> +	return le16_to_cpu(reg);

This is already done by i2c_smbus_read_word_data() and doing it
again will result in incorrect values on big endian architectures.

> +}
> +
> +static int sbsm_write_word(struct i2c_client *client, u8 address, u16 word)
> +{
> +	int ret, retries = SBSM_RETRY_CNT;
> +
> +	word = cpu_to_le16(word);

Same as for read_word: This is already done by the i2c-core.

> +	while (retries > 0) {
> +		ret = i2c_smbus_write_word_data(client, address, word);
> +		if (ret >= 0)
> +			break;
> +		--retries;
> +	}

for (retries = SBSM_RETRY_CNT; retries > 0; retries--)

> +	if (ret < 0)
> +		dev_err(&client->dev, "failed to write to register %i\n",
> +			(int)address);

dev_err(&client->dev, "failed to read register 0x%02x\n", address);

> +
> +	return ret;
> +}
> +
> +static int sbsm_get_property(struct power_supply *psy,
> +			     enum power_supply_property psp,
> +			     union power_supply_propval *val)
> +{
> +	struct sbsm_data *data = power_supply_get_drvdata(psy);
> +	int regval = 0;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_ONLINE:
> +		regval = sbsm_read_word(data->client, SBSM_CMD_BATSYSSTATECONT);
> +		if (regval < 0)
> +			return regval;
> +		val->intval = !!(regval & 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);
> +		}

That's not nicely indented. Try doing it this way to
reduce indention:

if (data->is_ltc1760)
    break;
u16 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);

Add

struct device *dev = &data->client->dev;

at the beginning of the function and use it here to avoid line
break.

> +	else
> +		data->cur_chan = chan;
> +
> +	return ret;
> +}
> +
> +#if defined(CONFIG_OF)
> +
> +#include <linux/of_device.h>

Move include to top of file. You can include it unconditionally.

> +static const struct of_device_id sbsm_dt_ids[] = {
> +	{ .compatible = "sbs,sbs-manager" },
> +	{ .compatible = "lltc,ltc1760" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, sbsm_dt_ids);

This can go next to the other MODULE_DEVICE_TABLE.

> +#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;

I guess EINVAL makes more sense?

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

How often do you want to convert endianess? :)
Drop this.

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

This can be written more readable as

if (supported_bats & BIT(i))

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

Please add:

psy_cfg->of_node = dev->of_node;

> +	data->psy = devm_power_supply_register(dev, psy_desc, &psy_cfg);
> +	if (IS_ERR(data->psy)) {
> +		ret = PTR_ERR(data->psy);
> +		dev_err(dev, "failed to register power supply %s\n",
> +			psy_desc->name);
> +		goto err_psy;
> +	}
> +
> +	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,

Please drop .owner assignment. It will be done by i2c-core.
But you should add the following:

.of_match_table = of_match_ptr(sbsm_dt_ids),

> +	},
> +	.probe		= sbsm_probe,
> +	.remove		= sbsm_remove,
> +	.id_table	= sbsm_ids
> +};
> +module_i2c_driver(sbsm_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Karl-Heinz Schneider <karl-heinz-X5L7DgJ4l23oE99TX8zNy7NAH6kLmebB@public.gmane.org>");
> +MODULE_DESCRIPTION("SBSM Smart Battery System Manager");

-- Sebastian

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

Re: [PATCH v5 7/9] power: Adds support for Smart Battery System Manager

[Phil Reid]

G'day Sebastian,

Thanks for the feedback. Next version coming soon.
In regards the le to cpu conversion.
grepping the supply folder finds a couple of other suspicious cases.

sbs-battery & the bq24735-cahrger do similar things.

thoughts on if they need to be corrected?



On 1/05/2017 21:41, Sebastian Reichel wrote:
> Hi,
> 
> On Mon, May 01, 2017 at 04:49:57PM +0800, Phil Reid wrote:
>> 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 | 325 +++++++++++++++++++++++++++++++++++++
>>   3 files changed, 339 insertions(+)
>>   create mode 100644 drivers/power/supply/sbs-manager.c
>>
>> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
>> index da54ac8..8aa5324 100644
>> --- a/drivers/power/supply/Kconfig
>> +++ b/drivers/power/supply/Kconfig
>> @@ -533,4 +533,17 @@ config AXP20X_POWER
>>   	  This driver provides support for the power supply features of
>>   	  AXP20x PMIC.
>>   
>> +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.
>> +
> 
> Let's move add this directly next to CHARGER_SBS.
> 
>>   endif # POWER_SUPPLY
>> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
>> index 3789a2c..4f53c98 100644
>> --- a/drivers/power/supply/Makefile
>> +++ b/drivers/power/supply/Makefile
>> @@ -74,3 +74,4 @@ obj-$(CONFIG_CHARGER_TPS65090)	+= tps65090-charger.o
>>   obj-$(CONFIG_CHARGER_TPS65217)	+= tps65217_charger.o
>>   obj-$(CONFIG_AXP288_FUEL_GAUGE) += axp288_fuel_gauge.o
>>   obj-$(CONFIG_AXP288_CHARGER)	+= axp288_charger.o
>> +obj-$(CONFIG_MANAGER_SBS)	+= sbs-manager.o
> 
> same here.
> 
>> diff --git a/drivers/power/supply/sbs-manager.c b/drivers/power/supply/sbs-manager.c
>> new file mode 100644
>> index 0000000..adf9e41
>> --- /dev/null
>> +++ b/drivers/power/supply/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 */
> 
> u8?
> 
>> +	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;
>> +	}
> 
> for (retries = SBSM_RETRY_CNT; retries > 0; retries--)
> 
>> +
>> +	if (reg < 0) {
>> +		dev_err(&client->dev, "failed to read register %i\n",
>> +			(int)address);
> 
> dev_err(&client->dev, "failed to read register 0x%02x\n", address);
> 
>> +		return reg;
>> +	}
>> +
>> +	return le16_to_cpu(reg);
> 
> This is already done by i2c_smbus_read_word_data() and doing it
> again will result in incorrect values on big endian architectures.
> 
>> +}
>> +
>> +static int sbsm_write_word(struct i2c_client *client, u8 address, u16 word)
>> +{
>> +	int ret, retries = SBSM_RETRY_CNT;
>> +
>> +	word = cpu_to_le16(word);
> 
> Same as for read_word: This is already done by the i2c-core.
> 
>> +	while (retries > 0) {
>> +		ret = i2c_smbus_write_word_data(client, address, word);
>> +		if (ret >= 0)
>> +			break;
>> +		--retries;
>> +	}
> 
> for (retries = SBSM_RETRY_CNT; retries > 0; retries--)
> 
>> +	if (ret < 0)
>> +		dev_err(&client->dev, "failed to write to register %i\n",
>> +			(int)address);
> 
> dev_err(&client->dev, "failed to read register 0x%02x\n", address);
> 
>> +
>> +	return ret;
>> +}
>> +
>> +static int sbsm_get_property(struct power_supply *psy,
>> +			     enum power_supply_property psp,
>> +			     union power_supply_propval *val)
>> +{
>> +	struct sbsm_data *data = power_supply_get_drvdata(psy);
>> +	int regval = 0;
>> +
>> +	switch (psp) {
>> +	case POWER_SUPPLY_PROP_ONLINE:
>> +		regval = sbsm_read_word(data->client, SBSM_CMD_BATSYSSTATECONT);
>> +		if (regval < 0)
>> +			return regval;
>> +		val->intval = !!(regval & 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);
>> +		}
> 
> That's not nicely indented. Try doing it this way to
> reduce indention:
> 
> if (data->is_ltc1760)
>      break;
> u16 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);
> 
> Add
> 
> struct device *dev = &data->client->dev;
> 
> at the beginning of the function and use it here to avoid line
> break.
> 
>> +	else
>> +		data->cur_chan = chan;
>> +
>> +	return ret;
>> +}
>> +
>> +#if defined(CONFIG_OF)
>> +
>> +#include <linux/of_device.h>
> 
> Move include to top of file. You can include it unconditionally.
> 
>> +static const struct of_device_id sbsm_dt_ids[] = {
>> +	{ .compatible = "sbs,sbs-manager" },
>> +	{ .compatible = "lltc,ltc1760" },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(of, sbsm_dt_ids);
> 
> This can go next to the other MODULE_DEVICE_TABLE.
> 
>> +#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;
> 
> I guess EINVAL makes more sense?
> 
>> +	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;
> 
> How often do you want to convert endianess? :)
> Drop this.
> 
>> +	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) {
> 
> This can be written more readable as
> 
> if (supported_bats & BIT(i))
> 
>> +			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;
> 
> Please add:
> 
> psy_cfg->of_node = dev->of_node;
> 
>> +	data->psy = devm_power_supply_register(dev, psy_desc, &psy_cfg);
>> +	if (IS_ERR(data->psy)) {
>> +		ret = PTR_ERR(data->psy);
>> +		dev_err(dev, "failed to register power supply %s\n",
>> +			psy_desc->name);
>> +		goto err_psy;
>> +	}
>> +
>> +	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,
> 
> Please drop .owner assignment. It will be done by i2c-core.
> But you should add the following:
> 
> .of_match_table = of_match_ptr(sbsm_dt_ids),
> 
>> +	},
>> +	.probe		= sbsm_probe,
>> +	.remove		= sbsm_remove,
>> +	.id_table	= sbsm_ids
>> +};
>> +module_i2c_driver(sbsm_driver);
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR("Karl-Heinz Schneider <karl-heinz@schneider-inet.de>");
>> +MODULE_DESCRIPTION("SBSM Smart Battery System Manager");
> 
> -- Sebastian
> 


-- 
Regards
Phil Reid

Re: [PATCH v5 7/9] power: Adds support for Smart Battery System Manager

[Sebastian Reichel]

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

Hi Phil,

On Tue, May 02, 2017 at 10:20:22AM +0800, Phil Reid wrote:
> In regards the le to cpu conversion. grepping the supply folder
> finds a couple of other suspicious cases.
> 
> sbs-battery & the bq24735-cahrger do similar things.
> 
> thoughts on if they need to be corrected?

Yes, those drivers look incorrect. I guess nobody noticed, since
most people use little endian machines nowadays (so le16_to_cpu and
cpu_to_le16 do nothing).

-- Sebastian

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

[PATCH v5 8/9] 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 8bb2eb3..a719892 100644
--- a/drivers/power/supply/sbs-battery.c
+++ b/drivers/power/supply/sbs-battery.c
@@ -675,21 +675,30 @@ static int sbs_get_property(struct power_supply *psy,
 	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;
 }
 
+static 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);
@@ -917,6 +926,7 @@ static int sbs_suspend(struct device *dev)
 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

Re: [PATCH v5 8/9] power: supply: sbs-battery: Add alert callback

[Sebastian Reichel]

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

Hi,

On Mon, May 01, 2017 at 04:49:58PM +0800, Phil Reid wrote:
> 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-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>

Thanks, queued.

-- Sebastian

>  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 8bb2eb3..a719892 100644
> --- a/drivers/power/supply/sbs-battery.c
> +++ b/drivers/power/supply/sbs-battery.c
> @@ -675,21 +675,30 @@ static int sbs_get_property(struct power_supply *psy,
>  	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;
>  }
>  
> +static 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);
> @@ -917,6 +926,7 @@ static int sbs_suspend(struct device *dev)
>  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
> 

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

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

[Phil Reid]

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

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

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 8aa5324..a25359c 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -536,6 +536,7 @@ config AXP20X_POWER
 config MANAGER_SBS
 	tristate "Smart Battery System Manager"
 	depends on I2C && I2C_MUX
+	select I2C_SMBUS
 	help
 	  Say Y here to include support for Smart Battery System Manager
 	  ICs. The driver reports online and charging status via sysfs.
diff --git a/drivers/power/supply/sbs-manager.c b/drivers/power/supply/sbs-manager.c
index adf9e41..d7f7a92 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,116 @@ static int sbsm_select(struct i2c_mux_core *muxc, u32 chan)
 	return ret;
 }
 
+static int sbsm_gpio_get_value(struct gpio_chip *gc, unsigned off)
+{
+	struct sbsm_data *data = gpiochip_get_data(gc);
+	int ret;
+
+	ret = sbsm_read_word(data->client, SBSM_CMD_BATSYSSTATE);
+	if (ret < 0)
+		return ret;
+
+	return ret & BIT(off);
+}
+
+/*
+ * This needs to be defined or the GPIO lib fails to register the pin.
+ * But the 'gpio' is always an input.
+ */
+static int sbsm_gpio_direction_input(struct gpio_chip *gc, unsigned off)
+{
+	return 0;
+}
+
+static int sbsm_do_alert(struct device *dev, void *d)
+{
+	struct i2c_client *client = i2c_verify_client(dev);
+	struct i2c_driver *driver;
+
+	if (!client || client->addr != 0x0b)
+		return 0;
+
+	device_lock(dev);
+	if (client->dev.driver) {
+		driver = to_i2c_driver(client->dev.driver);
+		if (driver->alert)
+			driver->alert(client, I2C_PROTOCOL_SMBUS_ALERT, 0);
+		else
+			dev_warn(&client->dev, "no driver alert()!\n");
+	} else
+		dev_dbg(&client->dev, "alert with no driver\n");
+	device_unlock(dev);
+
+	return -EBUSY;
+}
+
+static void sbsm_alert(struct i2c_client *client, enum i2c_alert_protocol prot,
+		       unsigned int d)
+{
+	struct sbsm_data *sbsm = i2c_get_clientdata(client);
+
+	int ret, i, irq_bat = 0;
+
+	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,
+					      NULL, sbsm_do_alert);
+		}
+	}
+}
+
+static int sbsm_gpio_setup(struct sbsm_data *data)
+{
+	struct gpio_chip *gc = &data->chip;
+	struct i2c_client *client = data->client;
+	struct device *dev = &client->dev;
+	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 +333,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 +354,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);
@@ -248,7 +367,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,
@@ -273,6 +392,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);
@@ -316,6 +438,7 @@ static int sbsm_remove(struct i2c_client *client)
 	},
 	.probe		= sbsm_probe,
 	.remove		= sbsm_remove,
+	.alert		= sbsm_alert,
 	.id_table	= sbsm_ids
 };
 module_i2c_driver(sbsm_driver);
-- 
1.8.3.1

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

[Sebastian Reichel]

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

Hi,

On Mon, May 01, 2017 at 04:49:59PM +0800, Phil Reid wrote:
> This adds smb alert support via the smbus_alert driver to generate
> power_supply_changed notifications when either external power is
> removed / applied or a battery inserted / removed.
> Use the i2c alert callback to notify the attached battery driver that a
> change has occurred.
>
> Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
> ---
>  drivers/power/supply/Kconfig       |   1 +
>  drivers/power/supply/sbs-manager.c | 129 ++++++++++++++++++++++++++++++++++++-
>  2 files changed, 127 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index 8aa5324..a25359c 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -536,6 +536,7 @@ config AXP20X_POWER
>  config MANAGER_SBS
>  	tristate "Smart Battery System Manager"
>  	depends on I2C && I2C_MUX
> +	select I2C_SMBUS

I guess we also need to depend on GPIO stuff?

>  	help
>  	  Say Y here to include support for Smart Battery System Manager
>  	  ICs. The driver reports online and charging status via sysfs.
> diff --git a/drivers/power/supply/sbs-manager.c b/drivers/power/supply/sbs-manager.c
> index adf9e41..d7f7a92 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,116 @@ static int sbsm_select(struct i2c_mux_core *muxc, u32 chan)
>  	return ret;
>  }
>  
> +static int sbsm_gpio_get_value(struct gpio_chip *gc, unsigned off)
> +{
> +	struct sbsm_data *data = gpiochip_get_data(gc);
> +	int ret;
> +
> +	ret = sbsm_read_word(data->client, SBSM_CMD_BATSYSSTATE);
> +	if (ret < 0)
> +		return ret;
> +
> +	return ret & BIT(off);
> +}
> +
> +/*
> + * This needs to be defined or the GPIO lib fails to register the pin.
> + * But the 'gpio' is always an input.
> + */
> +static int sbsm_gpio_direction_input(struct gpio_chip *gc, unsigned off)
> +{
> +	return 0;
> +}
> +
> +static int sbsm_do_alert(struct device *dev, void *d)
> +{
> +	struct i2c_client *client = i2c_verify_client(dev);
> +	struct i2c_driver *driver;
> +
> +	if (!client || client->addr != 0x0b)
> +		return 0;
> +
> +	device_lock(dev);
> +	if (client->dev.driver) {
> +		driver = to_i2c_driver(client->dev.driver);
> +		if (driver->alert)
> +			driver->alert(client, I2C_PROTOCOL_SMBUS_ALERT, 0);
> +		else
> +			dev_warn(&client->dev, "no driver alert()!\n");
> +	} else
> +		dev_dbg(&client->dev, "alert with no driver\n");
> +	device_unlock(dev);
> +
> +	return -EBUSY;
> +}
> +
> +static void sbsm_alert(struct i2c_client *client, enum i2c_alert_protocol prot,
> +		       unsigned int d)
> +{
> +	struct sbsm_data *sbsm = i2c_get_clientdata(client);
> +
> +	int ret, i, irq_bat = 0;
> +
> +	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,
> +					      NULL, sbsm_do_alert);
> +		}
> +	}
> +}
> +
> +static int sbsm_gpio_setup(struct sbsm_data *data)
> +{
> +	struct gpio_chip *gc = &data->chip;
> +	struct i2c_client *client = data->client;
> +	struct device *dev = &client->dev;
> +	struct device_node *of_node = client->dev.of_node;
> +	int ret;
> +
> +	if (!of_get_property(of_node, "gpio-controller", NULL))
> +		return 0;

#include <linux/property.h>

if (!device_property_present(dev, "gpio-controller"))
    return 0;

(and remove of_node from this function)

> +	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 +333,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 +354,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);
> @@ -248,7 +367,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,
> @@ -273,6 +392,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);
> @@ -316,6 +438,7 @@ static int sbsm_remove(struct i2c_client *client)
>  	},
>  	.probe		= sbsm_probe,
>  	.remove		= sbsm_remove,
> +	.alert		= sbsm_alert,
>  	.id_table	= sbsm_ids
>  };
>  module_i2c_driver(sbsm_driver);

-- Sebastian

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

Re: [PATCH v5 0/9] Add sbs-manager with smbalert support

[Wolfram Sang]

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

On Mon, May 01, 2017 at 04:49:50PM +0800, Phil Reid wrote:
> This is another go of the sbs-manager driver using smbalert for
> irq support from a while ago. 

Thanks for keeping at it!

Adding Benjamin Tissoires to CC, he worked lately on I2C device IRQs...
@Benjamin: do you need the patches, too, or do you read the I2C list?

> Enables the existing smbalert driver to be loaded via the device tree.
> Only need to add smbus_alert interrupt to the i2c bus segement in the
> devicetree and the core will then enable the alert driver.
> 
> Reorders the rquest irq call in the pca954x driver to ensure each
> muxed i2c segment can handle service smbalerts on that segment before
> irq's are enabled. The pca954x can't mask individual irq's routed thru
> them.
> 
> Add the sbs-manager from Karl-Heinz.
> Add the alert call back and gpio interface to allow the battery detect
> logic in the existing sbs-battery driver to work.
> 
> 
> 
> Karl-Heinz Schneider (2):
>   Documentation: Add sbs-manager device tree node documentation
>   power: Adds support for Smart Battery System Manager
> 
> Phil Reid (7):
>   i2c: i2c-smbus: Support threaded irqs.
>   i2c: i2c-smbus: Add null ptr guard in smb_alert_probe
>   i2c: i2c-smbus: add of_i2c_setup_smbus_alert
>   i2c: core: call of_i2c_setup_smbus_alert in i2c_register_adapter
>   i2c: mux: pca954x: Call request irq after adding mux segments
>   power: supply: sbs-battery: Add alert callback
>   power: supply: sbs-manager: Add alert callback and battery change
>     notification
> 
>  Documentation/devicetree/bindings/i2c/i2c.txt      |   4 +-
>  .../bindings/power/supply/sbs,sbs-manager.txt      |  64 +++
>  drivers/i2c/i2c-core.c                             |   4 +
>  drivers/i2c/i2c-smbus.c                            |  64 ++-
>  drivers/i2c/muxes/i2c-mux-pca954x.c                |  52 ++-
>  drivers/power/supply/Kconfig                       |  14 +
>  drivers/power/supply/Makefile                      |   1 +
>  drivers/power/supply/sbs-battery.c                 |  16 +-
>  drivers/power/supply/sbs-manager.c                 | 448 +++++++++++++++++++++
>  include/linux/i2c-smbus.h                          |   9 +
>  10 files changed, 637 insertions(+), 39 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.txt
>  create mode 100644 drivers/power/supply/sbs-manager.c
> 
> -- 
> 1.8.3.1
> 

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

Re: [PATCH v5 0/9] Add sbs-manager with smbalert support

[Benjamin Tissoires]

Hi Wolfram,

On May 01 2017 or thereabouts, Wolfram Sang wrote:
> On Mon, May 01, 2017 at 04:49:50PM +0800, Phil Reid wrote:
> > This is another go of the sbs-manager driver using smbalert for
> > irq support from a while ago. 
> 
> Thanks for keeping at it!
> 
> Adding Benjamin Tissoires to CC, he worked lately on I2C device IRQs...
> @Benjamin: do you need the patches, too, or do you read the I2C list?

I have already been cc-ed in the v6 of the series, so I am able to
review it properly (I do not read the I2C list, but I can work around
this through patchwork).

Cheers,
Benjamin

> 
> > Enables the existing smbalert driver to be loaded via the device tree.
> > Only need to add smbus_alert interrupt to the i2c bus segement in the
> > devicetree and the core will then enable the alert driver.
> > 
> > Reorders the rquest irq call in the pca954x driver to ensure each
> > muxed i2c segment can handle service smbalerts on that segment before
> > irq's are enabled. The pca954x can't mask individual irq's routed thru
> > them.
> > 
> > Add the sbs-manager from Karl-Heinz.
> > Add the alert call back and gpio interface to allow the battery detect
> > logic in the existing sbs-battery driver to work.
> > 
> > 
> > 
> > Karl-Heinz Schneider (2):
> >   Documentation: Add sbs-manager device tree node documentation
> >   power: Adds support for Smart Battery System Manager
> > 
> > Phil Reid (7):
> >   i2c: i2c-smbus: Support threaded irqs.
> >   i2c: i2c-smbus: Add null ptr guard in smb_alert_probe
> >   i2c: i2c-smbus: add of_i2c_setup_smbus_alert
> >   i2c: core: call of_i2c_setup_smbus_alert in i2c_register_adapter
> >   i2c: mux: pca954x: Call request irq after adding mux segments
> >   power: supply: sbs-battery: Add alert callback
> >   power: supply: sbs-manager: Add alert callback and battery change
> >     notification
> > 
> >  Documentation/devicetree/bindings/i2c/i2c.txt      |   4 +-
> >  .../bindings/power/supply/sbs,sbs-manager.txt      |  64 +++
> >  drivers/i2c/i2c-core.c                             |   4 +
> >  drivers/i2c/i2c-smbus.c                            |  64 ++-
> >  drivers/i2c/muxes/i2c-mux-pca954x.c                |  52 ++-
> >  drivers/power/supply/Kconfig                       |  14 +
> >  drivers/power/supply/Makefile                      |   1 +
> >  drivers/power/supply/sbs-battery.c                 |  16 +-
> >  drivers/power/supply/sbs-manager.c                 | 448 +++++++++++++++++++++
> >  include/linux/i2c-smbus.h                          |   9 +
> >  10 files changed, 637 insertions(+), 39 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/power/supply/sbs,sbs-manager.txt
> >  create mode 100644 drivers/power/supply/sbs-manager.c
> > 
> > -- 
> > 1.8.3.1
> > 

Re: [PATCH v5 0/9] Add sbs-manager with smbalert support

[Wolfram Sang]

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


> I have already been cc-ed in the v6 of the series, so I am able to
> review it properly (I do not read the I2C list, but I can work around
> this through patchwork).

Good. Thanks for the review, much appreciated!


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