[PATCH v2 0/9] Summit SMB3xx driver & device-tree

[PATCH v2 0/9] Summit SMB3xx driver & device-tree

[Dmitry Osipenko]

We gathered existing patches, fixed and improved what we could and
final result is an working charging driver with device-tree support
for Nexus 7.

At this moment charging works with:
 - Nexus 7 2012 (grouper and tilapia)
 - Nexus 7 2013 (flo and deb)
 - ... and there are more devices equipped with these chargers.

Changelog:

v2: - Addressed v1 review comments from Rob Herring and Sebastian Reichel
      by moving out common battery properties from the charger node into the
      battery-cell node.

    - power_supply_register() of the SMB driver converted to resource-managed
      API variant.

    - Improved DT property names of the SMB binding by making them to follow
      the generic power-supply naming scheme (-microvolts at the end, etc).

David Heidelberg (6):
  dt-bindings: power: supply: Add device-tree binding for Summit SMB3xx
  power: supply: smb347-charger: Use resource-managed API
  power: supply: smb347-charger: Implement device-tree support
  power: supply: smb347-charger: Support SMB345 and SMB358
  power: supply: smb347-charger: Remove virtual smb347-battery
  ARM: dts: qcom: apq8064-nexus7: Add SMB345 charger node

Dmitry Osipenko (3):
  dt-bindings: battery: Add temperature properties
  power: supply: Support battery temperature device-tree properties
  power: supply: smb347-charger: Replace mutex with IRQ disable/enable

 .../bindings/power/supply/battery.txt         |  10 +
 .../power/supply/summit,smb347-charger.yaml   | 165 +++++
 .../boot/dts/qcom-apq8064-asus-nexus7-flo.dts |  23 +
 drivers/power/supply/Kconfig                  |   6 +-
 drivers/power/supply/power_supply_core.c      |  18 +
 drivers/power/supply/smb347-charger.c         | 565 +++++++++---------
 .../dt-bindings/power/summit,smb347-charger.h |  19 +
 include/linux/power_supply.h                  |   6 +
 8 files changed, 542 insertions(+), 270 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/supply/summit,smb347-charger.yaml
 create mode 100644 include/dt-bindings/power/summit,smb347-charger.h

-- 
2.26.0

[PATCH v2 1/9] dt-bindings: power: supply: Add device-tree binding for Summit SMB3xx

[Dmitry Osipenko]

From: David Heidelberg <david@ixit.cz>

Summit SMB3xx series is a Programmable Switching Li+ Battery Charger.
This patch adds device-tree binding for Summit SMB345, SMB347 and SMB358
chargers.

Signed-off-by: David Heidelberg <david@ixit.cz>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 .../power/supply/summit,smb347-charger.yaml   | 165 ++++++++++++++++++
 .../dt-bindings/power/summit,smb347-charger.h |  19 ++
 2 files changed, 184 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/summit,smb347-charger.yaml
 create mode 100644 include/dt-bindings/power/summit,smb347-charger.h

diff --git a/Documentation/devicetree/bindings/power/supply/summit,smb347-charger.yaml b/Documentation/devicetree/bindings/power/supply/summit,smb347-charger.yaml
new file mode 100644
index 000000000000..eea0a6398c95
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/summit,smb347-charger.yaml
@@ -0,0 +1,165 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/power/supply/summit,smb347-charger.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Battery charger driver for SMB345, SMB347 and SMB358
+
+maintainers:
+  - David Heidelberg <david@ixit.cz>
+  - Dmitry Osipenko <digetx@gmail.com>
+
+properties:
+  compatible:
+    enum:
+      - summit,smb345
+      - summit,smb347
+      - summit,smb358
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  monitored-battery:
+    description: phandle to the battery node
+    $ref: /schemas/types.yaml#/definitions/phandle
+
+  summit,enable-usb-charging:
+    type: boolean
+    description: Enable charging through USB.
+
+  summit,enable-otg-charging:
+    type: boolean
+    description: Provide power for USB OTG
+
+  summit,enable-mains-charging:
+    type: boolean
+    description: Enable charging through mains
+
+  summit,enable-charge-control:
+    description: Enable charging control
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+      - enum:
+          - 0 # SMB3XX_CHG_ENABLE_SW SW (I2C interface)
+          - 1 # SMB3XX_CHG_ENABLE_PIN_ACTIVE_LOW Pin control (Active Low)
+          - 2 # SMB3XX_CHG_ENABLE_PIN_ACTIVE_HIGH Pin control (Active High)
+
+  summit,fast-voltage-threshold-microvolt:
+    description: Voltage threshold to transit to fast charge mode (in uV)
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 2400000
+    maximum: 3000000
+
+  summit,mains-current-limit-microamp:
+    description: Maximum input current from AC/DC input (in uA)
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+
+  summit,usb-current-limit-microamp:
+    description: Maximum input current from USB input (in uA)
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+
+  summit,charge-current-compensation-microamp:
+    description: Charge current compensation (in uA)
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+
+  summit,chip-temperature-threshold-celsius:
+    description: Chip temperature for thermal regulation in °C.
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [100, 110, 120, 130]
+
+  summit,soft-compensation-method:
+    description: Soft temperature limit compensation method
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+      - enum:
+          - 0 # SMB3XX_SOFT_TEMP_COMPENSATE_NONE Compensation none
+          - 1 # SMB3XX_SOFT_TEMP_COMPENSATE_CURRENT Current compensation
+          - 2 # SMB3XX_SOFT_TEMP_COMPENSATE_VOLTAGE Voltage compensation
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          enum:
+            - summit,smb345
+            - summit,smb358
+
+    then:
+      properties:
+        summit,mains-current-limit-microamp:
+          enum: [ 300000,  500000,  700000, 1000000,
+                 1500000, 1800000, 2000000]
+
+        summit,usb-current-limit-microamp:
+          enum: [ 300000,  500000,  700000, 1000000,
+                 1500000, 1800000, 2000000]
+
+        summit,charge-current-compensation-microamp:
+          enum: [200000, 450000, 600000, 900000]
+
+    else:
+      properties:
+        summit,mains-current-limit-microamp:
+          enum: [ 300000,  500000,  700000,  900000, 1200000,
+                 1500000, 1800000, 2000000, 2200000, 2500000]
+
+        summit,usb-current-limit-microamp:
+          enum: [ 300000,  500000,  700000,  900000, 1200000,
+                 1500000, 1800000, 2000000, 2200000, 2500000]
+
+        summit,charge-current-compensation-microamp:
+          enum: [250000, 700000, 900000, 1200000]
+
+required:
+  - compatible
+  - reg
+
+anyOf:
+  - required:
+      - summit,enable-usb-charging
+  - required:
+      - summit,enable-otg-charging
+  - required:
+      - summit,enable-mains-charging
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/power/summit,smb347-charger.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        charger@7f {
+            compatible = "summit,smb347";
+            reg = <0x7f>;
+            status = "okay";
+
+            summit,enable-charge-control = <SMB3XX_CHG_ENABLE_PIN_ACTIVE_HIGH>;
+            summit,chip-temperature-threshold-celsius = <110>;
+            summit,mains-current-limit-microamp = <2000000>;
+            summit,usb-current-limit-microamp = <500000>;
+            summit,enable-usb-charging;
+            summit,enable-mains-charging;
+
+            monitored-battery = <&battery>;
+        };
+    };
+
+    battery: battery-cell {
+        compatible = "simple-battery";
+        constant-charge-current-max-microamp = <1800000>;
+        temperature-min-alert-celsius = <5>;
+        temperature-max-alert-celsius = <40>;
+    };
diff --git a/include/dt-bindings/power/summit,smb347-charger.h b/include/dt-bindings/power/summit,smb347-charger.h
new file mode 100644
index 000000000000..d918bf321a71
--- /dev/null
+++ b/include/dt-bindings/power/summit,smb347-charger.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: (GPL-2.0-or-later or MIT) */
+/*
+ * Author: David Heidelberg <david@ixit.cz>
+ */
+
+#ifndef _DT_BINDINGS_SMB347_CHARGER_H
+#define _DT_BINDINGS_SMB347_CHARGER_H
+
+/* Charging compensation method */
+#define SMB3XX_SOFT_TEMP_COMPENSATE_NONE	0
+#define SMB3XX_SOFT_TEMP_COMPENSATE_CURRENT	1
+#define SMB3XX_SOFT_TEMP_COMPENSATE_VOLTAGE	2
+
+/* Charging enable control */
+#define SMB3XX_CHG_ENABLE_SW			0
+#define SMB3XX_CHG_ENABLE_PIN_ACTIVE_LOW	1
+#define SMB3XX_CHG_ENABLE_PIN_ACTIVE_HIGH	2
+
+#endif
-- 
2.26.0

[PATCH v2 2/9] dt-bindings: battery: Add temperature properties

[Dmitry Osipenko]

Document generic battery temperature properties.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 .../devicetree/bindings/power/supply/battery.txt       | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt
index 5e29595edd74..e0c35eff9d3f 100644
--- a/Documentation/devicetree/bindings/power/supply/battery.txt
+++ b/Documentation/devicetree/bindings/power/supply/battery.txt
@@ -45,6 +45,16 @@ Optional Properties:
    and corresponding battery internal resistance percent, which is used to look
    up the resistance percent according to current temperature to get a accurate
    batterty internal resistance in different temperatures.
+ - temperature-ambient-min-alert-celsius: Alert when ambient temperature of a
+   battery is lower than threshold value.
+ - temperature-ambient-max-alert-celsius: Alert when ambient temperature of a
+   battery is higher than threshold value.
+ - temperature-min-alert-celsius: Alert when battery temperature is lower
+   than threshold value.
+ - temperature-max-alert-celsius: Alert when battery temperature is higher
+   than threshold value.
+ - temperature-min-celsius: minimum temperature at which battery can operate
+ - temperature-max-celsius: maximum temperature at which battery can operate
 
 Battery properties are named, where possible, for the corresponding
 elements in enum power_supply_property, defined in
-- 
2.26.0

[PATCH v2 3/9] power: supply: Support battery temperature device-tree properties

[Dmitry Osipenko]

The generic battery temperature properties are already supported by the
power-supply core. Let's support parsing of the common battery temperature
properties from a device-tree.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/power/supply/power_supply_core.c | 18 ++++++++++++++++++
 include/linux/power_supply.h             |  6 ++++++
 2 files changed, 24 insertions(+)

diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index 02b37fe6061c..8b54a5a107d7 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -579,6 +579,12 @@ int power_supply_get_battery_info(struct power_supply *psy,
 	info->charge_term_current_ua         = -EINVAL;
 	info->constant_charge_current_max_ua = -EINVAL;
 	info->constant_charge_voltage_max_uv = -EINVAL;
+	info->temp_ambient_alert_min         = INT_MIN;
+	info->temp_ambient_alert_max         = INT_MAX;
+	info->temp_alert_min                 = INT_MIN;
+	info->temp_alert_max                 = INT_MAX;
+	info->temp_min                       = INT_MIN;
+	info->temp_max                       = INT_MAX;
 	info->factory_internal_resistance_uohm  = -EINVAL;
 	info->resist_table = NULL;
 
@@ -638,6 +644,18 @@ int power_supply_get_battery_info(struct power_supply *psy,
 			     &info->constant_charge_voltage_max_uv);
 	of_property_read_u32(battery_np, "factory-internal-resistance-micro-ohms",
 			     &info->factory_internal_resistance_uohm);
+	of_property_read_u32(battery_np, "temperature-ambient-min-alert-celsius",
+			     &info->temp_ambient_alert_min);
+	of_property_read_u32(battery_np, "temperature-ambient-max-alert-celsius",
+			     &info->temp_ambient_alert_max);
+	of_property_read_u32(battery_np, "temperature-min-alert-celsius",
+			     &info->temp_alert_min);
+	of_property_read_u32(battery_np, "temperature-max-alert-celsius",
+			     &info->temp_alert_max);
+	of_property_read_u32(battery_np, "temperature-min-celsius",
+			     &info->temp_min);
+	of_property_read_u32(battery_np, "temperature-max-celsius",
+			     &info->temp_max);
 
 	len = of_property_count_u32_elems(battery_np, "ocv-capacity-celsius");
 	if (len < 0 && len != -EINVAL) {
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index ac1345a48ad0..31fcadd81d63 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -361,6 +361,12 @@ struct power_supply_battery_info {
 	int constant_charge_voltage_max_uv; /* microVolts */
 	int factory_internal_resistance_uohm;   /* microOhms */
 	int ocv_temp[POWER_SUPPLY_OCV_TEMP_MAX];/* celsius */
+	int temp_ambient_alert_min;             /* celsius */
+	int temp_ambient_alert_max;             /* celsius */
+	int temp_alert_min;                     /* celsius */
+	int temp_alert_max;                     /* celsius */
+	int temp_min;                           /* celsius */
+	int temp_max;                           /* celsius */
 	struct power_supply_battery_ocv_table *ocv_table[POWER_SUPPLY_OCV_TEMP_MAX];
 	int ocv_table_size[POWER_SUPPLY_OCV_TEMP_MAX];
 	struct power_supply_resistance_temp_table *resist_table;
-- 
2.26.0

[PATCH v2 4/9] power: supply: smb347-charger: Use resource-managed API

[Dmitry Osipenko]

From: David Heidelberg <david@ixit.cz>

Simplify code, more convenient to use with Device Tree.

Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
 drivers/power/supply/smb347-charger.c | 75 +++++++++++----------------
 1 file changed, 29 insertions(+), 46 deletions(-)

diff --git a/drivers/power/supply/smb347-charger.c b/drivers/power/supply/smb347-charger.c
index f99026d81f2a..60894105fcbd 100644
--- a/drivers/power/supply/smb347-charger.c
+++ b/drivers/power/supply/smb347-charger.c
@@ -836,21 +836,31 @@ static int smb347_irq_init(struct smb347_charger *smb,
 			   struct i2c_client *client)
 {
 	const struct smb347_charger_platform_data *pdata = smb->pdata;
-	int ret, irq = gpio_to_irq(pdata->irq_gpio);
+	unsigned long irqflags = IRQF_ONESHOT;
+	int ret;
 
-	ret = gpio_request_one(pdata->irq_gpio, GPIOF_IN, client->name);
-	if (ret < 0)
-		goto fail;
+	/* Requesting GPIO for IRQ is only needed in non-DT way */
+	if (!client->irq) {
+		int irq = gpio_to_irq(pdata->irq_gpio);
+
+		ret = devm_gpio_request_one(smb->dev, pdata->irq_gpio,
+					    GPIOF_IN, client->name);
+		if (ret < 0)
+			return ret;
+
+		irqflags |= IRQF_TRIGGER_FALLING;
+		client->irq = irq;
+	}
 
-	ret = request_threaded_irq(irq, NULL, smb347_interrupt,
-				   IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
-				   client->name, smb);
+	ret = devm_request_threaded_irq(smb->dev, client->irq, NULL,
+					smb347_interrupt, irqflags,
+					client->name, smb);
 	if (ret < 0)
-		goto fail_gpio;
+		return ret;
 
 	ret = smb347_set_writable(smb, true);
 	if (ret < 0)
-		goto fail_irq;
+		return ret;
 
 	/*
 	 * Configure the STAT output to be suitable for interrupts: disable
@@ -860,20 +870,10 @@ static int smb347_irq_init(struct smb347_charger *smb,
 				 CFG_STAT_ACTIVE_HIGH | CFG_STAT_DISABLED,
 				 CFG_STAT_DISABLED);
 	if (ret < 0)
-		goto fail_readonly;
+		client->irq = 0;
 
 	smb347_set_writable(smb, false);
-	client->irq = irq;
-	return 0;
 
-fail_readonly:
-	smb347_set_writable(smb, false);
-fail_irq:
-	free_irq(irq, smb);
-fail_gpio:
-	gpio_free(pdata->irq_gpio);
-fail:
-	client->irq = 0;
 	return ret;
 }
 
@@ -1251,32 +1251,24 @@ static int smb347_probe(struct i2c_client *client,
 	mains_usb_cfg.num_supplicants = ARRAY_SIZE(battery);
 	mains_usb_cfg.drv_data = smb;
 	if (smb->pdata->use_mains) {
-		smb->mains = power_supply_register(dev, &smb347_mains_desc,
-						   &mains_usb_cfg);
+		smb->mains = devm_power_supply_register(dev, &smb347_mains_desc,
+							&mains_usb_cfg);
 		if (IS_ERR(smb->mains))
 			return PTR_ERR(smb->mains);
 	}
 
 	if (smb->pdata->use_usb) {
-		smb->usb = power_supply_register(dev, &smb347_usb_desc,
-						 &mains_usb_cfg);
-		if (IS_ERR(smb->usb)) {
-			if (smb->pdata->use_mains)
-				power_supply_unregister(smb->mains);
+		smb->usb = devm_power_supply_register(dev, &smb347_usb_desc,
+						      &mains_usb_cfg);
+		if (IS_ERR(smb->usb))
 			return PTR_ERR(smb->usb);
-		}
 	}
 
 	battery_cfg.drv_data = smb;
-	smb->battery = power_supply_register(dev, &smb347_battery_desc,
-					     &battery_cfg);
-	if (IS_ERR(smb->battery)) {
-		if (smb->pdata->use_usb)
-			power_supply_unregister(smb->usb);
-		if (smb->pdata->use_mains)
-			power_supply_unregister(smb->mains);
+	smb->battery = devm_power_supply_register(dev, &smb347_battery_desc,
+						  &battery_cfg);
+	if (IS_ERR(smb->battery))
 		return PTR_ERR(smb->battery);
-	}
 
 	/*
 	 * Interrupt pin is optional. If it is connected, we setup the
@@ -1299,17 +1291,8 @@ static int smb347_remove(struct i2c_client *client)
 {
 	struct smb347_charger *smb = i2c_get_clientdata(client);
 
-	if (client->irq) {
+	if (client->irq)
 		smb347_irq_disable(smb);
-		free_irq(client->irq, smb);
-		gpio_free(smb->pdata->irq_gpio);
-	}
-
-	power_supply_unregister(smb->battery);
-	if (smb->pdata->use_usb)
-		power_supply_unregister(smb->usb);
-	if (smb->pdata->use_mains)
-		power_supply_unregister(smb->mains);
 	return 0;
 }
 
-- 
2.26.0

[PATCH v2 5/9] power: supply: smb347-charger: Implement device-tree support

[Dmitry Osipenko]

From: David Heidelberg <david@ixit.cz>

This patch adds device-tree support to the SMB347 charger driver. All
legacy platform data now can be parsed from DT. Because of that and since
SMB347 is an I2C client driver, the IRQ number can be passed automatically
through client's IRQ variable if it's defined in DT. There is no need to
map GPIO to IRQ manually in the case of DT.

This patch is based on the original work made by:
Jonghwa Lee <jonghwa3.lee@samsung.com>
Link: https://patchwork.kernel.org/patch/4284731/

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
 drivers/power/supply/smb347-charger.c | 151 +++++++++++++++++++++++---
 1 file changed, 137 insertions(+), 14 deletions(-)

diff --git a/drivers/power/supply/smb347-charger.c b/drivers/power/supply/smb347-charger.c
index 60894105fcbd..da2c337107bf 100644
--- a/drivers/power/supply/smb347-charger.c
+++ b/drivers/power/supply/smb347-charger.c
@@ -1180,6 +1180,119 @@ static bool smb347_readable_reg(struct device *dev, unsigned int reg)
 	return smb347_volatile_reg(dev, reg);
 }
 
+static void smb347_dt_parse_pdata(struct device_node *np,
+				  struct smb347_charger_platform_data *pdata)
+{
+	pdata->soft_temp_limit_compensation =
+					SMB347_SOFT_TEMP_COMPENSATE_DEFAULT;
+	/*
+	 * These properties come from the battery info, still we need to
+	 * pre-initialize the values. See smb347_get_battery_info() below.
+	 */
+	pdata->soft_cold_temp_limit = SMB347_TEMP_USE_DEFAULT;
+	pdata->hard_cold_temp_limit = SMB347_TEMP_USE_DEFAULT;
+	pdata->soft_hot_temp_limit  = SMB347_TEMP_USE_DEFAULT;
+	pdata->hard_hot_temp_limit  = SMB347_TEMP_USE_DEFAULT;
+
+	/* Charging constraints */
+	of_property_read_u32(np, "summit,fast-voltage-threshold-microvolt",
+			     &pdata->pre_to_fast_voltage);
+	of_property_read_u32(np, "summit,mains-current-limit-microamp",
+			     &pdata->mains_current_limit);
+	of_property_read_u32(np, "summit,usb-current-limit-microamp",
+			     &pdata->usb_hc_current_limit);
+
+	/* For thermometer monitoring */
+	of_property_read_u32(np, "summit,chip-temperature-threshold-celsius",
+			     &pdata->chip_temp_threshold);
+	of_property_read_u32(np, "summit,soft-compensation-method",
+			     &pdata->soft_temp_limit_compensation);
+	of_property_read_u32(np, "summit,charge-current-compensation-microamp",
+			     &pdata->charge_current_compensation);
+
+	/* Supported charging mode */
+	pdata->use_mains =
+		of_property_read_bool(np, "summit,enable-mains-charging");
+	pdata->use_usb =
+		of_property_read_bool(np, "summit,enable-usb-charging");
+	pdata->use_usb_otg =
+		of_property_read_bool(np, "summit,enable-otg-charging");
+
+	/* Select charging control */
+	of_property_read_u32(np, "summit,enable-charge-control",
+			     &pdata->enable_control);
+
+	/* Interrupt support is optional */
+	if (!of_find_property(np, "interrupts", NULL))
+		pdata->irq_gpio = -1;
+}
+
+static int smb347_get_battery_info(struct smb347_charger *smb)
+{
+	struct smb347_charger_platform_data *pdata = (void *)smb->pdata;
+	struct power_supply_battery_info info = {};
+	struct power_supply *supply;
+	int err;
+
+	if (smb->mains)
+		supply = smb->mains;
+	else
+		supply = smb->usb;
+
+	err = power_supply_get_battery_info(supply, &info);
+	if (err == -ENXIO || err == -ENODEV)
+		return 0;
+	if (err)
+		return err;
+
+	if (info.constant_charge_current_max_ua != -EINVAL)
+		pdata->max_charge_current = info.constant_charge_current_max_ua;
+
+	if (info.constant_charge_voltage_max_uv != -EINVAL)
+		pdata->max_charge_voltage = info.constant_charge_voltage_max_uv;
+
+	if (info.precharge_current_ua != -EINVAL)
+		pdata->pre_charge_current = info.precharge_current_ua;
+
+	if (info.charge_term_current_ua != -EINVAL)
+		pdata->termination_current = info.charge_term_current_ua;
+
+	if (info.temp_alert_min != INT_MIN)
+		pdata->soft_cold_temp_limit = info.temp_alert_min;
+
+	if (info.temp_alert_max != INT_MAX)
+		pdata->soft_hot_temp_limit = info.temp_alert_max;
+
+	if (info.temp_min != INT_MIN)
+		pdata->hard_cold_temp_limit = info.temp_min;
+
+	if (info.temp_max != INT_MAX)
+		pdata->hard_hot_temp_limit = info.temp_max;
+
+	/* Suspend when battery temperature is outside hard limits */
+	if (pdata->hard_cold_temp_limit != SMB347_TEMP_USE_DEFAULT ||
+	    pdata->hard_hot_temp_limit != SMB347_TEMP_USE_DEFAULT)
+		pdata->suspend_on_hard_temp_limit = true;
+
+	return 0;
+}
+
+static struct smb347_charger_platform_data
+			*smb347_get_platdata(struct device *dev)
+{
+	struct smb347_charger_platform_data *pdata;
+
+	if (dev->of_node) {
+		pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+		if (pdata)
+			smb347_dt_parse_pdata(dev->of_node, pdata);
+	} else {
+		pdata = dev_get_platdata(dev);
+	}
+
+	return pdata;
+}
+
 static const struct regmap_config smb347_regmap = {
 	.reg_bits	= 8,
 	.val_bits	= 8,
@@ -1216,40 +1329,35 @@ static int smb347_probe(struct i2c_client *client,
 			const struct i2c_device_id *id)
 {
 	static char *battery[] = { "smb347-battery" };
-	const struct smb347_charger_platform_data *pdata;
 	struct power_supply_config mains_usb_cfg = {}, battery_cfg = {};
 	struct device *dev = &client->dev;
 	struct smb347_charger *smb;
 	int ret;
 
-	pdata = dev->platform_data;
-	if (!pdata)
-		return -EINVAL;
-
-	if (!pdata->use_mains && !pdata->use_usb)
-		return -EINVAL;
-
 	smb = devm_kzalloc(dev, sizeof(*smb), GFP_KERNEL);
 	if (!smb)
 		return -ENOMEM;
 
+	smb->pdata = smb347_get_platdata(dev);
+	if (!smb->pdata)
+		return -ENODEV;
+
+	if (!smb->pdata->use_mains && !smb->pdata->use_usb)
+		return -EINVAL;
+
 	i2c_set_clientdata(client, smb);
 
 	mutex_init(&smb->lock);
 	smb->dev = &client->dev;
-	smb->pdata = pdata;
 
 	smb->regmap = devm_regmap_init_i2c(client, &smb347_regmap);
 	if (IS_ERR(smb->regmap))
 		return PTR_ERR(smb->regmap);
 
-	ret = smb347_hw_init(smb);
-	if (ret < 0)
-		return ret;
-
 	mains_usb_cfg.supplied_to = battery;
 	mains_usb_cfg.num_supplicants = ARRAY_SIZE(battery);
 	mains_usb_cfg.drv_data = smb;
+	mains_usb_cfg.of_node = dev->of_node;
 	if (smb->pdata->use_mains) {
 		smb->mains = devm_power_supply_register(dev, &smb347_mains_desc,
 							&mains_usb_cfg);
@@ -1270,11 +1378,19 @@ static int smb347_probe(struct i2c_client *client,
 	if (IS_ERR(smb->battery))
 		return PTR_ERR(smb->battery);
 
+	ret = smb347_get_battery_info(smb);
+	if (ret)
+		return ret;
+
+	ret = smb347_hw_init(smb);
+	if (ret < 0)
+		return ret;
+
 	/*
 	 * Interrupt pin is optional. If it is connected, we setup the
 	 * interrupt support here.
 	 */
-	if (pdata->irq_gpio >= 0) {
+	if (smb->pdata->irq_gpio >= 0) {
 		ret = smb347_irq_init(smb, client);
 		if (ret < 0) {
 			dev_warn(dev, "failed to initialize IRQ: %d\n", ret);
@@ -1302,9 +1418,16 @@ static const struct i2c_device_id smb347_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, smb347_id);
 
+static const struct of_device_id smb3xx_of_match[] = {
+	{ .compatible = "summit,smb347" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, smb3xx_of_match);
+
 static struct i2c_driver smb347_driver = {
 	.driver = {
 		.name = "smb347",
+		.of_match_table = smb3xx_of_match,
 	},
 	.probe        = smb347_probe,
 	.remove       = smb347_remove,
-- 
2.26.0

[PATCH v2 6/9] power: supply: smb347-charger: Support SMB345 and SMB358

[Dmitry Osipenko]

From: David Heidelberg <david@ixit.cz>

SMB345 tested on Nexus 7 2013.

Based on:
- https://patchwork.kernel.org/patch/4922431/
- https://patchwork.ozlabs.org/patch/666877/

Signed-off-by: David Heidelberg <david@ixit.cz>
---
 drivers/power/supply/Kconfig          |   6 +-
 drivers/power/supply/smb347-charger.c | 109 ++++++++++++++------------
 2 files changed, 62 insertions(+), 53 deletions(-)

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 5dde0e2de232..3ace33b116a8 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -626,12 +626,12 @@ config CHARGER_BQ25890
 	  Say Y to enable support for the TI BQ25890 battery charger.
 
 config CHARGER_SMB347
-	tristate "Summit Microelectronics SMB347 Battery Charger"
+	tristate "Summit Microelectronics SMB3XX Battery Charger"
 	depends on I2C
 	select REGMAP_I2C
 	help
-	  Say Y to include support for Summit Microelectronics SMB347
-	  Battery Charger.
+	  Say Y to include support for Summit Microelectronics SMB345,
+	  SMB347 or SMB358 Battery Charger.
 
 config CHARGER_TPS65090
 	tristate "TPS65090 battery charger driver"
diff --git a/drivers/power/supply/smb347-charger.c b/drivers/power/supply/smb347-charger.c
index da2c337107bf..90a87e0624a6 100644
--- a/drivers/power/supply/smb347-charger.c
+++ b/drivers/power/supply/smb347-charger.c
@@ -128,6 +128,7 @@
  * @mains: power_supply instance for AC/DC power
  * @usb: power_supply instance for USB power
  * @battery: power_supply instance for battery
+ * @id: SMB charger ID
  * @mains_online: is AC/DC input connected
  * @usb_online: is USB input connected
  * @charging_enabled: is charging enabled
@@ -140,64 +141,61 @@ struct smb347_charger {
 	struct power_supply	*mains;
 	struct power_supply	*usb;
 	struct power_supply	*battery;
+	unsigned int		id;
 	bool			mains_online;
 	bool			usb_online;
 	bool			charging_enabled;
 	const struct smb347_charger_platform_data *pdata;
 };
 
-/* Fast charge current in uA */
-static const unsigned int fcc_tbl[] = {
-	700000,
-	900000,
-	1200000,
-	1500000,
-	1800000,
-	2000000,
-	2200000,
-	2500000,
+enum smb_charger_chipid {
+	SMB345,
+	SMB347,
+	SMB358,
+	NUM_CHIP_TYPES,
 };
 
+/* Fast charge current in uA */
+static const unsigned int fcc_tbl[NUM_CHIP_TYPES][8] = {
+	[SMB345] = {  200000,  450000,  600000,  900000,
+		     1300000, 1500000, 1800000, 2000000 },
+	[SMB347] = {  700000,  900000, 1200000, 1500000,
+		     1800000, 2000000, 2200000, 2500000 },
+	[SMB358] = {  200000,  450000,  600000,  900000,
+		     1300000, 1500000, 1800000, 2000000 },
+};
 /* Pre-charge current in uA */
-static const unsigned int pcc_tbl[] = {
-	100000,
-	150000,
-	200000,
-	250000,
+static const unsigned int pcc_tbl[NUM_CHIP_TYPES][4] = {
+	[SMB345] = { 150000, 250000, 350000, 450000 },
+	[SMB347] = { 100000, 150000, 200000, 250000 },
+	[SMB358] = { 150000, 250000, 350000, 450000 },
 };
 
 /* Termination current in uA */
-static const unsigned int tc_tbl[] = {
-	37500,
-	50000,
-	100000,
-	150000,
-	200000,
-	250000,
-	500000,
-	600000,
+static const unsigned int tc_tbl[NUM_CHIP_TYPES][8] = {
+	[SMB345] = {  30000,  40000,  60000,  80000,
+		     100000, 125000, 150000, 200000 },
+	[SMB347] = {  37500,  50000, 100000, 150000,
+		     200000, 250000, 500000, 600000 },
+	[SMB358] = {  30000,  40000,  60000,  80000,
+		     100000, 125000, 150000, 200000 },
 };
 
 /* Input current limit in uA */
-static const unsigned int icl_tbl[] = {
-	300000,
-	500000,
-	700000,
-	900000,
-	1200000,
-	1500000,
-	1800000,
-	2000000,
-	2200000,
-	2500000,
+static const unsigned int icl_tbl[NUM_CHIP_TYPES][10] = {
+	[SMB345] = {  300000,  500000,  700000, 1000000, 1500000,
+		     1800000, 2000000, 2000000, 2000000, 2000000 },
+	[SMB347] = {  300000,  500000,  700000,  900000, 1200000,
+		     1500000, 1800000, 2000000, 2200000, 2500000 },
+	[SMB358] = {  300000,  500000,  700000, 1000000, 1500000,
+		     1800000, 2000000, 2000000, 2000000, 2000000 },
 };
 
 /* Charge current compensation in uA */
-static const unsigned int ccc_tbl[] = {
-	250000,
-	700000,
-	900000,
-	1200000,
+static const unsigned int ccc_tbl[NUM_CHIP_TYPES][4] = {
+	[SMB345] = {  200000,  450000,  600000,  900000 },
+	[SMB347] = {  250000,  700000,  900000, 1200000 },
+	[SMB358] = {  200000,  450000,  600000,  900000 },
 };
 
 /* Convert register value to current using lookup table */
@@ -352,10 +350,11 @@ static int smb347_start_stop_charging(struct smb347_charger *smb)
 
 static int smb347_set_charge_current(struct smb347_charger *smb)
 {
+	unsigned int id = smb->id;
 	int ret;
 
 	if (smb->pdata->max_charge_current) {
-		ret = current_to_hw(fcc_tbl, ARRAY_SIZE(fcc_tbl),
+		ret = current_to_hw(fcc_tbl[id], ARRAY_SIZE(fcc_tbl[id]),
 				    smb->pdata->max_charge_current);
 		if (ret < 0)
 			return ret;
@@ -368,7 +367,7 @@ static int smb347_set_charge_current(struct smb347_charger *smb)
 	}
 
 	if (smb->pdata->pre_charge_current) {
-		ret = current_to_hw(pcc_tbl, ARRAY_SIZE(pcc_tbl),
+		ret = current_to_hw(pcc_tbl[id], ARRAY_SIZE(pcc_tbl[id]),
 				    smb->pdata->pre_charge_current);
 		if (ret < 0)
 			return ret;
@@ -381,7 +380,7 @@ static int smb347_set_charge_current(struct smb347_charger *smb)
 	}
 
 	if (smb->pdata->termination_current) {
-		ret = current_to_hw(tc_tbl, ARRAY_SIZE(tc_tbl),
+		ret = current_to_hw(tc_tbl[id], ARRAY_SIZE(tc_tbl[id]),
 				    smb->pdata->termination_current);
 		if (ret < 0)
 			return ret;
@@ -397,10 +396,11 @@ static int smb347_set_charge_current(struct smb347_charger *smb)
 
 static int smb347_set_current_limits(struct smb347_charger *smb)
 {
+	unsigned int id = smb->id;
 	int ret;
 
 	if (smb->pdata->mains_current_limit) {
-		ret = current_to_hw(icl_tbl, ARRAY_SIZE(icl_tbl),
+		ret = current_to_hw(icl_tbl[id], ARRAY_SIZE(icl_tbl[id]),
 				    smb->pdata->mains_current_limit);
 		if (ret < 0)
 			return ret;
@@ -413,7 +413,7 @@ static int smb347_set_current_limits(struct smb347_charger *smb)
 	}
 
 	if (smb->pdata->usb_hc_current_limit) {
-		ret = current_to_hw(icl_tbl, ARRAY_SIZE(icl_tbl),
+		ret = current_to_hw(icl_tbl[id], ARRAY_SIZE(icl_tbl[id]),
 				    smb->pdata->usb_hc_current_limit);
 		if (ret < 0)
 			return ret;
@@ -463,6 +463,7 @@ static int smb347_set_voltage_limits(struct smb347_charger *smb)
 
 static int smb347_set_temp_limits(struct smb347_charger *smb)
 {
+	unsigned int id = smb->id;
 	bool enable_therm_monitor = false;
 	int ret = 0;
 	int val;
@@ -587,7 +588,7 @@ static int smb347_set_temp_limits(struct smb347_charger *smb)
 	}
 
 	if (smb->pdata->charge_current_compensation) {
-		val = current_to_hw(ccc_tbl, ARRAY_SIZE(ccc_tbl),
+		val = current_to_hw(ccc_tbl[id], ARRAY_SIZE(ccc_tbl[id]),
 				    smb->pdata->charge_current_compensation);
 		if (val < 0)
 			return val;
@@ -883,6 +884,7 @@ static int smb347_irq_init(struct smb347_charger *smb,
  */
 static int get_const_charge_current(struct smb347_charger *smb)
 {
+	unsigned int id = smb->id;
 	int ret, intval;
 	unsigned int v;
 
@@ -898,10 +900,12 @@ static int get_const_charge_current(struct smb347_charger *smb)
 	 * and we can detect which table to use from bit 5.
 	 */
 	if (v & 0x20) {
-		intval = hw_to_current(fcc_tbl, ARRAY_SIZE(fcc_tbl), v & 7);
+		intval = hw_to_current(fcc_tbl[id],
+				       ARRAY_SIZE(fcc_tbl[id]), v & 7);
 	} else {
 		v >>= 3;
-		intval = hw_to_current(pcc_tbl, ARRAY_SIZE(pcc_tbl), v & 7);
+		intval = hw_to_current(pcc_tbl[id],
+				       ARRAY_SIZE(pcc_tbl[id]), v & 7);
 	}
 
 	return intval;
@@ -1349,6 +1353,7 @@ static int smb347_probe(struct i2c_client *client,
 
 	mutex_init(&smb->lock);
 	smb->dev = &client->dev;
+	smb->id = id->driver_data;
 
 	smb->regmap = devm_regmap_init_i2c(client, &smb347_regmap);
 	if (IS_ERR(smb->regmap))
@@ -1413,13 +1418,17 @@ static int smb347_remove(struct i2c_client *client)
 }
 
 static const struct i2c_device_id smb347_id[] = {
-	{ "smb347", 0 },
-	{ }
+	{ "smb345", SMB345 },
+	{ "smb347", SMB347 },
+	{ "smb358", SMB358 },
+	{ },
 };
 MODULE_DEVICE_TABLE(i2c, smb347_id);
 
 static const struct of_device_id smb3xx_of_match[] = {
+	{ .compatible = "summit,smb345" },
 	{ .compatible = "summit,smb347" },
+	{ .compatible = "summit,smb358" },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, smb3xx_of_match);
-- 
2.26.0

[PATCH v2 7/9] power: supply: smb347-charger: Remove virtual smb347-battery

[Dmitry Osipenko]

From: David Heidelberg <david@ixit.cz>

SMB347 is a charger and not a battery driver. Secondly, power-supply core
now supports monitored-battery. So the 'fake' battery doesn't do anything
useful for us, and thus, it should be removed.

Transfer smb347-battery functionality into smb347-mains and smb347-usb.

Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
 drivers/power/supply/smb347-charger.c | 206 ++++++++------------------
 1 file changed, 60 insertions(+), 146 deletions(-)

diff --git a/drivers/power/supply/smb347-charger.c b/drivers/power/supply/smb347-charger.c
index 90a87e0624a6..335b6ee494e4 100644
--- a/drivers/power/supply/smb347-charger.c
+++ b/drivers/power/supply/smb347-charger.c
@@ -127,7 +127,6 @@
  * @regmap: pointer to driver regmap
  * @mains: power_supply instance for AC/DC power
  * @usb: power_supply instance for USB power
- * @battery: power_supply instance for battery
  * @id: SMB charger ID
  * @mains_online: is AC/DC input connected
  * @usb_online: is USB input connected
@@ -140,7 +139,6 @@ struct smb347_charger {
 	struct regmap		*regmap;
 	struct power_supply	*mains;
 	struct power_supply	*usb;
-	struct power_supply	*battery;
 	unsigned int		id;
 	bool			mains_online;
 	bool			usb_online;
@@ -743,7 +741,10 @@ static irqreturn_t smb347_interrupt(int irq, void *data)
 	 */
 	if (stat_c & STAT_C_CHARGER_ERROR) {
 		dev_err(smb->dev, "charging stopped due to charger error\n");
-		power_supply_changed(smb->battery);
+		if (smb->pdata->use_mains)
+			power_supply_changed(smb->mains);
+		if (smb->pdata->use_usb)
+			power_supply_changed(smb->usb);
 		handled = true;
 	}
 
@@ -753,8 +754,12 @@ static irqreturn_t smb347_interrupt(int irq, void *data)
 	 * disabled by the hardware.
 	 */
 	if (irqstat_c & (IRQSTAT_C_TERMINATION_IRQ | IRQSTAT_C_TAPER_IRQ)) {
-		if (irqstat_c & IRQSTAT_C_TERMINATION_STAT)
-			power_supply_changed(smb->battery);
+		if (irqstat_c & IRQSTAT_C_TERMINATION_STAT) {
+			if (smb->pdata->use_mains)
+				power_supply_changed(smb->mains);
+			if (smb->pdata->use_usb)
+				power_supply_changed(smb->usb);
+		}
 		dev_dbg(smb->dev, "going to HW maintenance mode\n");
 		handled = true;
 	}
@@ -768,7 +773,10 @@ static irqreturn_t smb347_interrupt(int irq, void *data)
 
 		if (irqstat_d & IRQSTAT_D_CHARGE_TIMEOUT_STAT)
 			dev_warn(smb->dev, "charging stopped due to timeout\n");
-		power_supply_changed(smb->battery);
+		if (smb->pdata->use_mains)
+			power_supply_changed(smb->mains);
+		if (smb->pdata->use_usb)
+			power_supply_changed(smb->usb);
 		handled = true;
 	}
 
@@ -936,95 +944,19 @@ static int get_const_charge_voltage(struct smb347_charger *smb)
 	return intval;
 }
 
-static int smb347_mains_get_property(struct power_supply *psy,
-				     enum power_supply_property prop,
-				     union power_supply_propval *val)
-{
-	struct smb347_charger *smb = power_supply_get_drvdata(psy);
-	int ret;
-
-	switch (prop) {
-	case POWER_SUPPLY_PROP_ONLINE:
-		val->intval = smb->mains_online;
-		break;
-
-	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
-		ret = get_const_charge_voltage(smb);
-		if (ret < 0)
-			return ret;
-		else
-			val->intval = ret;
-		break;
-
-	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
-		ret = get_const_charge_current(smb);
-		if (ret < 0)
-			return ret;
-		else
-			val->intval = ret;
-		break;
-
-	default:
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
-static enum power_supply_property smb347_mains_properties[] = {
-	POWER_SUPPLY_PROP_ONLINE,
-	POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT,
-	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
-};
-
-static int smb347_usb_get_property(struct power_supply *psy,
-				   enum power_supply_property prop,
-				   union power_supply_propval *val)
-{
-	struct smb347_charger *smb = power_supply_get_drvdata(psy);
-	int ret;
-
-	switch (prop) {
-	case POWER_SUPPLY_PROP_ONLINE:
-		val->intval = smb->usb_online;
-		break;
-
-	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
-		ret = get_const_charge_voltage(smb);
-		if (ret < 0)
-			return ret;
-		else
-			val->intval = ret;
-		break;
-
-	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
-		ret = get_const_charge_current(smb);
-		if (ret < 0)
-			return ret;
-		else
-			val->intval = ret;
-		break;
-
-	default:
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
-static enum power_supply_property smb347_usb_properties[] = {
-	POWER_SUPPLY_PROP_ONLINE,
-	POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT,
-	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
-};
-
-static int smb347_get_charging_status(struct smb347_charger *smb)
+static int smb347_get_charging_status(struct smb347_charger *smb,
+				      struct power_supply *psy)
 {
 	int ret, status;
 	unsigned int val;
 
-	if (!smb347_is_ps_online(smb))
-		return POWER_SUPPLY_STATUS_DISCHARGING;
+	if (psy->desc->type == POWER_SUPPLY_TYPE_USB) {
+		if (!smb->usb_online)
+			return POWER_SUPPLY_STATUS_DISCHARGING;
+	} else {
+		if (!smb->mains_online)
+			return POWER_SUPPLY_STATUS_DISCHARGING;
+	}
 
 	ret = regmap_read(smb->regmap, STAT_C, &val);
 	if (ret < 0)
@@ -1063,29 +995,29 @@ static int smb347_get_charging_status(struct smb347_charger *smb)
 	return status;
 }
 
-static int smb347_battery_get_property(struct power_supply *psy,
-				       enum power_supply_property prop,
-				       union power_supply_propval *val)
+static int smb347_get_property(struct power_supply *psy,
+			       enum power_supply_property prop,
+			       union power_supply_propval *val)
 {
 	struct smb347_charger *smb = power_supply_get_drvdata(psy);
-	const struct smb347_charger_platform_data *pdata = smb->pdata;
 	int ret;
 
-	ret = smb347_update_ps_status(smb);
-	if (ret < 0)
-		return ret;
-
 	switch (prop) {
 	case POWER_SUPPLY_PROP_STATUS:
-		ret = smb347_get_charging_status(smb);
+		ret = smb347_get_charging_status(smb, psy);
 		if (ret < 0)
 			return ret;
 		val->intval = ret;
 		break;
 
 	case POWER_SUPPLY_PROP_CHARGE_TYPE:
-		if (!smb347_is_ps_online(smb))
-			return -ENODATA;
+		if (psy->desc->type == POWER_SUPPLY_TYPE_USB) {
+			if (!smb->usb_online)
+				return -ENODATA;
+		} else {
+			if (!smb->mains_online)
+				return -ENODATA;
+		}
 
 		/*
 		 * We handle trickle and pre-charging the same, and taper
@@ -1104,24 +1036,25 @@ static int smb347_battery_get_property(struct power_supply *psy,
 		}
 		break;
 
-	case POWER_SUPPLY_PROP_TECHNOLOGY:
-		val->intval = pdata->battery_info.technology;
-		break;
-
-	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
-		val->intval = pdata->battery_info.voltage_min_design;
-		break;
-
-	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
-		val->intval = pdata->battery_info.voltage_max_design;
+	case POWER_SUPPLY_PROP_ONLINE:
+		if (psy->desc->type == POWER_SUPPLY_TYPE_USB)
+			val->intval = smb->usb_online;
+		else
+			val->intval = smb->mains_online;
 		break;
 
-	case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
-		val->intval = pdata->battery_info.charge_full_design;
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
+		ret = get_const_charge_voltage(smb);
+		if (ret < 0)
+			return ret;
+		val->intval = ret;
 		break;
 
-	case POWER_SUPPLY_PROP_MODEL_NAME:
-		val->strval = pdata->battery_info.name;
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
+		ret = get_const_charge_current(smb);
+		if (ret < 0)
+			return ret;
+		val->intval = ret;
 		break;
 
 	default:
@@ -1131,14 +1064,12 @@ static int smb347_battery_get_property(struct power_supply *psy,
 	return 0;
 }
 
-static enum power_supply_property smb347_battery_properties[] = {
+static enum power_supply_property smb347_properties[] = {
 	POWER_SUPPLY_PROP_STATUS,
 	POWER_SUPPLY_PROP_CHARGE_TYPE,
-	POWER_SUPPLY_PROP_TECHNOLOGY,
-	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
-	POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
-	POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
-	POWER_SUPPLY_PROP_MODEL_NAME,
+	POWER_SUPPLY_PROP_ONLINE,
+	POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT,
+	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
 };
 
 static bool smb347_volatile_reg(struct device *dev, unsigned int reg)
@@ -1308,32 +1239,23 @@ static const struct regmap_config smb347_regmap = {
 static const struct power_supply_desc smb347_mains_desc = {
 	.name		= "smb347-mains",
 	.type		= POWER_SUPPLY_TYPE_MAINS,
-	.get_property	= smb347_mains_get_property,
-	.properties	= smb347_mains_properties,
-	.num_properties	= ARRAY_SIZE(smb347_mains_properties),
+	.get_property	= smb347_get_property,
+	.properties	= smb347_properties,
+	.num_properties	= ARRAY_SIZE(smb347_properties),
 };
 
 static const struct power_supply_desc smb347_usb_desc = {
 	.name		= "smb347-usb",
 	.type		= POWER_SUPPLY_TYPE_USB,
-	.get_property	= smb347_usb_get_property,
-	.properties	= smb347_usb_properties,
-	.num_properties	= ARRAY_SIZE(smb347_usb_properties),
-};
-
-static const struct power_supply_desc smb347_battery_desc = {
-	.name		= "smb347-battery",
-	.type		= POWER_SUPPLY_TYPE_BATTERY,
-	.get_property	= smb347_battery_get_property,
-	.properties	= smb347_battery_properties,
-	.num_properties	= ARRAY_SIZE(smb347_battery_properties),
+	.get_property	= smb347_get_property,
+	.properties	= smb347_properties,
+	.num_properties	= ARRAY_SIZE(smb347_properties),
 };
 
 static int smb347_probe(struct i2c_client *client,
 			const struct i2c_device_id *id)
 {
-	static char *battery[] = { "smb347-battery" };
-	struct power_supply_config mains_usb_cfg = {}, battery_cfg = {};
+	struct power_supply_config mains_usb_cfg = {};
 	struct device *dev = &client->dev;
 	struct smb347_charger *smb;
 	int ret;
@@ -1359,8 +1281,6 @@ static int smb347_probe(struct i2c_client *client,
 	if (IS_ERR(smb->regmap))
 		return PTR_ERR(smb->regmap);
 
-	mains_usb_cfg.supplied_to = battery;
-	mains_usb_cfg.num_supplicants = ARRAY_SIZE(battery);
 	mains_usb_cfg.drv_data = smb;
 	mains_usb_cfg.of_node = dev->of_node;
 	if (smb->pdata->use_mains) {
@@ -1377,12 +1297,6 @@ static int smb347_probe(struct i2c_client *client,
 			return PTR_ERR(smb->usb);
 	}
 
-	battery_cfg.drv_data = smb;
-	smb->battery = devm_power_supply_register(dev, &smb347_battery_desc,
-						  &battery_cfg);
-	if (IS_ERR(smb->battery))
-		return PTR_ERR(smb->battery);
-
 	ret = smb347_get_battery_info(smb);
 	if (ret)
 		return ret;
-- 
2.26.0

[PATCH v2 8/9] power: supply: smb347-charger: Replace mutex with IRQ disable/enable

[Dmitry Osipenko]

Let's simply disable/enable IRQ rather than use a mutex that protects from
racing with the interrupt handler. The result of this patch is that it's a
bit easier now to follow the driver's code.

Tested-by: David Heidelberg <david@ixit.cz>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/power/supply/smb347-charger.c | 38 ++++++++++++++-------------
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/drivers/power/supply/smb347-charger.c b/drivers/power/supply/smb347-charger.c
index 335b6ee494e4..ec68ab2bce27 100644
--- a/drivers/power/supply/smb347-charger.c
+++ b/drivers/power/supply/smb347-charger.c
@@ -16,7 +16,6 @@
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/i2c.h>
-#include <linux/mutex.h>
 #include <linux/power_supply.h>
 #include <linux/power/smb347-charger.h>
 #include <linux/regmap.h>
@@ -122,7 +121,6 @@
 
 /**
  * struct smb347_charger - smb347 charger instance
- * @lock: protects concurrent access to online variables
  * @dev: pointer to device
  * @regmap: pointer to driver regmap
  * @mains: power_supply instance for AC/DC power
@@ -134,7 +132,6 @@
  * @pdata: pointer to platform data
  */
 struct smb347_charger {
-	struct mutex		lock;
 	struct device		*dev;
 	struct regmap		*regmap;
 	struct power_supply	*mains;
@@ -243,11 +240,9 @@ static int smb347_update_ps_status(struct smb347_charger *smb)
 	if (smb->pdata->use_usb)
 		usb = !(val & IRQSTAT_E_USBIN_UV_STAT);
 
-	mutex_lock(&smb->lock);
 	ret = smb->mains_online != dc || smb->usb_online != usb;
 	smb->mains_online = dc;
 	smb->usb_online = usb;
-	mutex_unlock(&smb->lock);
 
 	return ret;
 }
@@ -263,13 +258,7 @@ static int smb347_update_ps_status(struct smb347_charger *smb)
  */
 static bool smb347_is_ps_online(struct smb347_charger *smb)
 {
-	bool ret;
-
-	mutex_lock(&smb->lock);
-	ret = smb->usb_online || smb->mains_online;
-	mutex_unlock(&smb->lock);
-
-	return ret;
+	return smb->usb_online || smb->mains_online;
 }
 
 /**
@@ -303,14 +292,13 @@ static int smb347_charging_set(struct smb347_charger *smb, bool enable)
 		return 0;
 	}
 
-	mutex_lock(&smb->lock);
 	if (smb->charging_enabled != enable) {
 		ret = regmap_update_bits(smb->regmap, CMD_A, CMD_A_CHG_ENABLED,
 					 enable ? CMD_A_CHG_ENABLED : 0);
 		if (!ret)
 			smb->charging_enabled = enable;
 	}
-	mutex_unlock(&smb->lock);
+
 	return ret;
 }
 
@@ -995,9 +983,9 @@ static int smb347_get_charging_status(struct smb347_charger *smb,
 	return status;
 }
 
-static int smb347_get_property(struct power_supply *psy,
-			       enum power_supply_property prop,
-			       union power_supply_propval *val)
+static int smb347_get_property_locked(struct power_supply *psy,
+				      enum power_supply_property prop,
+				      union power_supply_propval *val)
 {
 	struct smb347_charger *smb = power_supply_get_drvdata(psy);
 	int ret;
@@ -1064,6 +1052,21 @@ static int smb347_get_property(struct power_supply *psy,
 	return 0;
 }
 
+static int smb347_get_property(struct power_supply *psy,
+			       enum power_supply_property prop,
+			       union power_supply_propval *val)
+{
+	struct smb347_charger *smb = power_supply_get_drvdata(psy);
+	struct i2c_client *client = to_i2c_client(smb->dev);
+	int ret;
+
+	disable_irq(client->irq);
+	ret = smb347_get_property_locked(psy, prop, val);
+	enable_irq(client->irq);
+
+	return ret;
+}
+
 static enum power_supply_property smb347_properties[] = {
 	POWER_SUPPLY_PROP_STATUS,
 	POWER_SUPPLY_PROP_CHARGE_TYPE,
@@ -1273,7 +1276,6 @@ static int smb347_probe(struct i2c_client *client,
 
 	i2c_set_clientdata(client, smb);
 
-	mutex_init(&smb->lock);
 	smb->dev = &client->dev;
 	smb->id = id->driver_data;
 
-- 
2.26.0

[PATCH v2 9/9] ARM: dts: qcom: apq8064-nexus7: Add SMB345 charger node

[Dmitry Osipenko]

From: David Heidelberg <david@ixit.cz>

Add SMB345 charger node to Nexus 7 2013 DTS.
Proper charger configuration prevents battery from overcharging.

Original author: Vinay Simha BN <simhavcs@gmail.com>

Signed-off-by: David Heidelberg <david@ixit.cz>
---
 .../boot/dts/qcom-apq8064-asus-nexus7-flo.dts | 23 +++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts b/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts
index a701d4bac320..7a7784206dd8 100644
--- a/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts
+++ b/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts
@@ -3,6 +3,7 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
+#include <dt-bindings/power/summit,smb347-charger.h>
 / {
 	model = "Asus Nexus7(flo)";
 	compatible = "asus,nexus7-flo", "qcom,apq8064";
@@ -56,6 +57,11 @@ volume_down {
 		};
 	};
 
+	battery_cell: battery-cell {
+		compatible = "simple-battery";
+		constant-charge-current-max-microamp = <1800000>;
+	};
+
 	soc {
 		rpm@108000 {
 			regulators {
@@ -296,8 +302,25 @@ eeprom@52 {
 				bq27541@55 {
 					compatible = "ti,bq27541";
 					reg = <0x55>;
+					power-supplies = <&power_supply>;
+					monitored-battery = <&battery_cell>;
 				};
 
+				power_supply: charger@6a {
+					compatible = "summit,smb345";
+					reg = <0x6a>;
+
+					interrupt-parent = <&tlmm_pinmux>;
+					interrupts = <23 IRQ_TYPE_EDGE_BOTH>;
+
+					summit,chip-temperature-threshold-celsius = <110>;
+					summit,usb-current-limit-microamp = <500000>;
+					summit,enable-charge-control = <SMB3XX_CHG_ENABLE_SW>;
+					summit,enable-usb-charging;
+					summit,enable-otg-charging;
+
+					monitored-battery = <&battery_cell>;
+				};
 			};
 		};
 
-- 
2.26.0

Re: [PATCH v2 0/9] Summit SMB3xx driver & device-tree

[Dmitry Osipenko]

07.06.2020 17:41, Dmitry Osipenko пишет:
> We gathered existing patches, fixed and improved what we could and
> final result is an working charging driver with device-tree support
> for Nexus 7.
> 
> At this moment charging works with:
>  - Nexus 7 2012 (grouper and tilapia)
>  - Nexus 7 2013 (flo and deb)
>  - ... and there are more devices equipped with these chargers.

Hello Sebastian and everyone!

Sebastian, do you have any comments to this series? Will be nice to get
some progress :)

Re: [PATCH v2 1/9] dt-bindings: power: supply: Add device-tree binding for Summit SMB3xx

[Rob Herring]

On Sun, Jun 07, 2020 at 05:41:05PM +0300, Dmitry Osipenko wrote:
> From: David Heidelberg <david@ixit.cz>
> 
> Summit SMB3xx series is a Programmable Switching Li+ Battery Charger.
> This patch adds device-tree binding for Summit SMB345, SMB347 and SMB358
> chargers.
> 
> Signed-off-by: David Heidelberg <david@ixit.cz>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  .../power/supply/summit,smb347-charger.yaml   | 165 ++++++++++++++++++
>  .../dt-bindings/power/summit,smb347-charger.h |  19 ++
>  2 files changed, 184 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/supply/summit,smb347-charger.yaml
>  create mode 100644 include/dt-bindings/power/summit,smb347-charger.h
> 
> diff --git a/Documentation/devicetree/bindings/power/supply/summit,smb347-charger.yaml b/Documentation/devicetree/bindings/power/supply/summit,smb347-charger.yaml
> new file mode 100644
> index 000000000000..eea0a6398c95
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/summit,smb347-charger.yaml
> @@ -0,0 +1,165 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/power/supply/summit,smb347-charger.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Battery charger driver for SMB345, SMB347 and SMB358
> +
> +maintainers:
> +  - David Heidelberg <david@ixit.cz>
> +  - Dmitry Osipenko <digetx@gmail.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - summit,smb345
> +      - summit,smb347
> +      - summit,smb358
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  monitored-battery:
> +    description: phandle to the battery node
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +
> +  summit,enable-usb-charging:
> +    type: boolean
> +    description: Enable charging through USB.
> +
> +  summit,enable-otg-charging:
> +    type: boolean
> +    description: Provide power for USB OTG
> +
> +  summit,enable-mains-charging:
> +    type: boolean
> +    description: Enable charging through mains
> +
> +  summit,enable-charge-control:
> +    description: Enable charging control
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32
> +      - enum:
> +          - 0 # SMB3XX_CHG_ENABLE_SW SW (I2C interface)
> +          - 1 # SMB3XX_CHG_ENABLE_PIN_ACTIVE_LOW Pin control (Active Low)
> +          - 2 # SMB3XX_CHG_ENABLE_PIN_ACTIVE_HIGH Pin control (Active High)
> +
> +  summit,fast-voltage-threshold-microvolt:
> +    description: Voltage threshold to transit to fast charge mode (in uV)
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32

Anything with a standard unit suffix already has a type, so drop.

> +    minimum: 2400000
> +    maximum: 3000000
> +
> +  summit,mains-current-limit-microamp:
> +    description: Maximum input current from AC/DC input (in uA)
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  summit,usb-current-limit-microamp:
> +    description: Maximum input current from USB input (in uA)
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  summit,charge-current-compensation-microamp:
> +    description: Charge current compensation (in uA)
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  summit,chip-temperature-threshold-celsius:
> +    description: Chip temperature for thermal regulation in °C.
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [100, 110, 120, 130]
> +
> +  summit,soft-compensation-method:
> +    description: Soft temperature limit compensation method
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32
> +      - enum:
> +          - 0 # SMB3XX_SOFT_TEMP_COMPENSATE_NONE Compensation none
> +          - 1 # SMB3XX_SOFT_TEMP_COMPENSATE_CURRENT Current compensation
> +          - 2 # SMB3XX_SOFT_TEMP_COMPENSATE_VOLTAGE Voltage compensation
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - summit,smb345
> +            - summit,smb358
> +
> +    then:
> +      properties:
> +        summit,mains-current-limit-microamp:
> +          enum: [ 300000,  500000,  700000, 1000000,
> +                 1500000, 1800000, 2000000]
> +
> +        summit,usb-current-limit-microamp:
> +          enum: [ 300000,  500000,  700000, 1000000,
> +                 1500000, 1800000, 2000000]
> +
> +        summit,charge-current-compensation-microamp:
> +          enum: [200000, 450000, 600000, 900000]
> +
> +    else:
> +      properties:
> +        summit,mains-current-limit-microamp:
> +          enum: [ 300000,  500000,  700000,  900000, 1200000,
> +                 1500000, 1800000, 2000000, 2200000, 2500000]
> +
> +        summit,usb-current-limit-microamp:
> +          enum: [ 300000,  500000,  700000,  900000, 1200000,
> +                 1500000, 1800000, 2000000, 2200000, 2500000]
> +
> +        summit,charge-current-compensation-microamp:
> +          enum: [250000, 700000, 900000, 1200000]
> +
> +required:
> +  - compatible
> +  - reg
> +
> +anyOf:
> +  - required:
> +      - summit,enable-usb-charging
> +  - required:
> +      - summit,enable-otg-charging
> +  - required:
> +      - summit,enable-mains-charging
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/power/summit,smb347-charger.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        charger@7f {
> +            compatible = "summit,smb347";
> +            reg = <0x7f>;
> +            status = "okay";
> +
> +            summit,enable-charge-control = <SMB3XX_CHG_ENABLE_PIN_ACTIVE_HIGH>;
> +            summit,chip-temperature-threshold-celsius = <110>;
> +            summit,mains-current-limit-microamp = <2000000>;
> +            summit,usb-current-limit-microamp = <500000>;
> +            summit,enable-usb-charging;
> +            summit,enable-mains-charging;
> +
> +            monitored-battery = <&battery>;
> +        };
> +    };
> +
> +    battery: battery-cell {
> +        compatible = "simple-battery";
> +        constant-charge-current-max-microamp = <1800000>;
> +        temperature-min-alert-celsius = <5>;
> +        temperature-max-alert-celsius = <40>;
> +    };
> diff --git a/include/dt-bindings/power/summit,smb347-charger.h b/include/dt-bindings/power/summit,smb347-charger.h
> new file mode 100644
> index 000000000000..d918bf321a71
> --- /dev/null
> +++ b/include/dt-bindings/power/summit,smb347-charger.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: (GPL-2.0-or-later or MIT) */
> +/*
> + * Author: David Heidelberg <david@ixit.cz>
> + */
> +
> +#ifndef _DT_BINDINGS_SMB347_CHARGER_H
> +#define _DT_BINDINGS_SMB347_CHARGER_H
> +
> +/* Charging compensation method */
> +#define SMB3XX_SOFT_TEMP_COMPENSATE_NONE	0
> +#define SMB3XX_SOFT_TEMP_COMPENSATE_CURRENT	1
> +#define SMB3XX_SOFT_TEMP_COMPENSATE_VOLTAGE	2
> +
> +/* Charging enable control */
> +#define SMB3XX_CHG_ENABLE_SW			0
> +#define SMB3XX_CHG_ENABLE_PIN_ACTIVE_LOW	1
> +#define SMB3XX_CHG_ENABLE_PIN_ACTIVE_HIGH	2
> +
> +#endif
> -- 
> 2.26.0
> 

Re: [PATCH v2 2/9] dt-bindings: battery: Add temperature properties

[Rob Herring]

On Sun, Jun 07, 2020 at 05:41:06PM +0300, Dmitry Osipenko wrote:
> Document generic battery temperature properties.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  .../devicetree/bindings/power/supply/battery.txt       | 10 ++++++++++
>  1 file changed, 10 insertions(+)

This is close to being converted to schema:

https://lore.kernel.org/linux-pm/20200707212914.31540-1-r-rivera-matos@ti.com/
 
> diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt
> index 5e29595edd74..e0c35eff9d3f 100644
> --- a/Documentation/devicetree/bindings/power/supply/battery.txt
> +++ b/Documentation/devicetree/bindings/power/supply/battery.txt
> @@ -45,6 +45,16 @@ Optional Properties:
>     and corresponding battery internal resistance percent, which is used to look
>     up the resistance percent according to current temperature to get a accurate
>     batterty internal resistance in different temperatures.
> + - temperature-ambient-min-alert-celsius: Alert when ambient temperature of a
> +   battery is lower than threshold value.
> + - temperature-ambient-max-alert-celsius: Alert when ambient temperature of a
> +   battery is higher than threshold value.
> + - temperature-min-alert-celsius: Alert when battery temperature is lower
> +   than threshold value.
> + - temperature-max-alert-celsius: Alert when battery temperature is higher
> +   than threshold value.
> + - temperature-min-celsius: minimum temperature at which battery can operate
> + - temperature-max-celsius: maximum temperature at which battery can operate

Perhaps 'temperature' is redundant since we have units.

Perhaps do <min max> properties given specifying only min or max is 
probably not valid?

Rob

Re: [PATCH v2 1/9] dt-bindings: power: supply: Add device-tree binding for Summit SMB3xx

[Dmitry Osipenko]

14.07.2020 02:39, Rob Herring пишет:
> On Sun, Jun 07, 2020 at 05:41:05PM +0300, Dmitry Osipenko wrote:
>> From: David Heidelberg <david@ixit.cz>
>>
>> Summit SMB3xx series is a Programmable Switching Li+ Battery Charger.
>> This patch adds device-tree binding for Summit SMB345, SMB347 and SMB358
>> chargers.
>>
>> Signed-off-by: David Heidelberg <david@ixit.cz>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  .../power/supply/summit,smb347-charger.yaml   | 165 ++++++++++++++++++
>>  .../dt-bindings/power/summit,smb347-charger.h |  19 ++
>>  2 files changed, 184 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/power/supply/summit,smb347-charger.yaml
>>  create mode 100644 include/dt-bindings/power/summit,smb347-charger.h
>>
>> diff --git a/Documentation/devicetree/bindings/power/supply/summit,smb347-charger.yaml b/Documentation/devicetree/bindings/power/supply/summit,smb347-charger.yaml
>> new file mode 100644
>> index 000000000000..eea0a6398c95
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/supply/summit,smb347-charger.yaml
>> @@ -0,0 +1,165 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: "http://devicetree.org/schemas/power/supply/summit,smb347-charger.yaml#"
>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
>> +
>> +title: Battery charger driver for SMB345, SMB347 and SMB358
>> +
>> +maintainers:
>> +  - David Heidelberg <david@ixit.cz>
>> +  - Dmitry Osipenko <digetx@gmail.com>
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - summit,smb345
>> +      - summit,smb347
>> +      - summit,smb358
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  monitored-battery:
>> +    description: phandle to the battery node
>> +    $ref: /schemas/types.yaml#/definitions/phandle
>> +
>> +  summit,enable-usb-charging:
>> +    type: boolean
>> +    description: Enable charging through USB.
>> +
>> +  summit,enable-otg-charging:
>> +    type: boolean
>> +    description: Provide power for USB OTG
>> +
>> +  summit,enable-mains-charging:
>> +    type: boolean
>> +    description: Enable charging through mains
>> +
>> +  summit,enable-charge-control:
>> +    description: Enable charging control
>> +    allOf:
>> +      - $ref: /schemas/types.yaml#/definitions/uint32
>> +      - enum:
>> +          - 0 # SMB3XX_CHG_ENABLE_SW SW (I2C interface)
>> +          - 1 # SMB3XX_CHG_ENABLE_PIN_ACTIVE_LOW Pin control (Active Low)
>> +          - 2 # SMB3XX_CHG_ENABLE_PIN_ACTIVE_HIGH Pin control (Active High)
>> +
>> +  summit,fast-voltage-threshold-microvolt:
>> +    description: Voltage threshold to transit to fast charge mode (in uV)
>> +    allOf:
>> +      - $ref: /schemas/types.yaml#/definitions/uint32
> 
> Anything with a standard unit suffix already has a type, so drop.

I'll correct it in the next revision, thanks!

Re: [PATCH v2 2/9] dt-bindings: battery: Add temperature properties

[Dmitry Osipenko]

14.07.2020 02:46, Rob Herring пишет:
> On Sun, Jun 07, 2020 at 05:41:06PM +0300, Dmitry Osipenko wrote:
>> Document generic battery temperature properties.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  .../devicetree/bindings/power/supply/battery.txt       | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
> 
> This is close to being converted to schema:
> 
> https://lore.kernel.org/linux-pm/20200707212914.31540-1-r-rivera-matos@ti.com/

Thanks! I'll keep an eye on this patch.

>> diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt
>> index 5e29595edd74..e0c35eff9d3f 100644
>> --- a/Documentation/devicetree/bindings/power/supply/battery.txt
>> +++ b/Documentation/devicetree/bindings/power/supply/battery.txt
>> @@ -45,6 +45,16 @@ Optional Properties:
>>     and corresponding battery internal resistance percent, which is used to look
>>     up the resistance percent according to current temperature to get a accurate
>>     batterty internal resistance in different temperatures.
>> + - temperature-ambient-min-alert-celsius: Alert when ambient temperature of a
>> +   battery is lower than threshold value.
>> + - temperature-ambient-max-alert-celsius: Alert when ambient temperature of a
>> +   battery is higher than threshold value.
>> + - temperature-min-alert-celsius: Alert when battery temperature is lower
>> +   than threshold value.
>> + - temperature-max-alert-celsius: Alert when battery temperature is higher
>> +   than threshold value.
>> + - temperature-min-celsius: minimum temperature at which battery can operate
>> + - temperature-max-celsius: maximum temperature at which battery can operate
> 
> Perhaps 'temperature' is redundant since we have units.
> 
> Perhaps do <min max> properties given specifying only min or max is 
> probably not valid?

Thank you for the suggestions, I'll consider them for the v3!