2015-06-18 21:14:02

by Bjorn Andersson

[permalink] [raw]
Subject: [PATCH 0/4] Qualcomm Switch-Mode Battery Charger and Boost

This series adds bindings and a device driver for the Qualcomm Switch-Mode
Battery Charger and Boost block found in pm8941.

Bjorn Andersson (4):
spmi: pmic-arb: add irq_get_irqchip_state implementation
dt-binding: power: Add Qualcomm SMBB binding
power: Add Qualcomm SMBB driver
ARM: dts: qcom-pm8941: Add charger node

.../devicetree/bindings/power_supply/qcom_smbb.txt | 137 +++
arch/arm/boot/dts/qcom-pm8941.dtsi | 21 +
drivers/power/Kconfig | 12 +
drivers/power/Makefile | 1 +
drivers/power/qcom_smbb.c | 952 +++++++++++++++++++++
drivers/spmi/spmi-pmic-arb.c | 17 +
6 files changed, 1140 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power_supply/qcom_smbb.txt
create mode 100644 drivers/power/qcom_smbb.c

--
1.8.2.2


2015-06-18 21:14:16

by Bjorn Andersson

[permalink] [raw]
Subject: [PATCH 1/4] spmi: pmic-arb: add irq_get_irqchip_state implementation

Signed-off-by: Courtney Cavin <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
---
drivers/spmi/spmi-pmic-arb.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
index d7119db49cfe..a4d8c043a710 100644
--- a/drivers/spmi/spmi-pmic-arb.c
+++ b/drivers/spmi/spmi-pmic-arb.c
@@ -575,6 +575,22 @@ static int qpnpint_irq_set_type(struct irq_data *d, unsigned int flow_type)
return 0;
}

+static int qpnpint_get_irqchip_state(struct irq_data *d,
+ enum irqchip_irq_state which,
+ bool *state)
+{
+ u8 irq = d->hwirq >> 8;
+ u8 status = 0;
+
+ if (which != IRQCHIP_STATE_LINE_LEVEL)
+ return -EINVAL;
+
+ qpnpint_spmi_read(d, QPNPINT_REG_RT_STS, &status, 1);
+ *state = !!(status & BIT(irq));
+
+ return 0;
+}
+
static struct irq_chip pmic_arb_irqchip = {
.name = "pmic_arb",
.irq_enable = qpnpint_irq_enable,
@@ -582,6 +598,7 @@ static struct irq_chip pmic_arb_irqchip = {
.irq_mask = qpnpint_irq_mask,
.irq_unmask = qpnpint_irq_unmask,
.irq_set_type = qpnpint_irq_set_type,
+ .irq_get_irqchip_state = qpnpint_get_irqchip_state,
.flags = IRQCHIP_MASK_ON_SUSPEND
| IRQCHIP_SKIP_SET_WAKE,
};
--
1.8.2.2

2015-06-18 21:14:38

by Bjorn Andersson

[permalink] [raw]
Subject: [PATCH 2/4] dt-binding: power: Add Qualcomm SMBB binding

Add the Qualcomm Switch-Mode Battery Charger and Boost device tree
binding.

Signed-off-by: Courtney Cavin <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
---
.../devicetree/bindings/power_supply/qcom_smbb.txt | 137 +++++++++++++++++++++
1 file changed, 137 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power_supply/qcom_smbb.txt

diff --git a/Documentation/devicetree/bindings/power_supply/qcom_smbb.txt b/Documentation/devicetree/bindings/power_supply/qcom_smbb.txt
new file mode 100644
index 000000000000..8a1d75978664
--- /dev/null
+++ b/Documentation/devicetree/bindings/power_supply/qcom_smbb.txt
@@ -0,0 +1,137 @@
+Qualcomm Switch-Mode Battery Charger and Boost
+
+PROPERTIES
+- compatible:
+ Usage: required
+ Value type: <stringlist>
+ Description: Must be one of:
+ - "qcom,pm8941-charger"
+
+- reg:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Description: Base address of registers for SMBB block
+
+- interrupts:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Description: The format of the specifier is defined by the binding document
+ describing the node's interrupt parent. Must contain one
+ specifier for each of the following interrupts, in order:
+ - charge done
+ - charge fast mode
+ - charge trickle mode
+ - battery temperature ok
+ - battery present
+ - charger disconnected
+ - USB-in valid
+ - DC-in valid
+
+- interrupt-names:
+ Usage: required
+ Value type: <stringlist>
+ Description: Must contain the following list, strictly ordered:
+ "chg-done",
+ "chg-fast",
+ "chg-trkl",
+ "bat-temp-ok",
+ "bat-present",
+ "chg-gone",
+ "usb-valid",
+ "dc-valid"
+
+- battery-charge-control-limit:
+ Usage: optional (default: 1A, or pre-configured value)
+ Value type: <u32>; uA; range [100mA : 3A]
+ Description: Maximum charge current; May be clamped to safety limits.
+
+- fast-charge-low-watermark:
+ Usage: optional (default: 3.2V, or pre-configured value)
+ Value type: <u32>; uV; range [2.1V : 3.6V]
+ Description: Battery voltage limit above which fast charging may operate;
+ Below this value linear or switch-mode auto-trickle-charging
+ will operate.
+
+- fast-charge-high-watermark:
+ Usage: optional (default: 4.2V, or pre-configured value)
+ Value type: <u32>; uV; range [3.24V : 5V]
+ Description: Battery voltage limit below which fast charging may operate;
+ The fast charger will attempt to charge the battery to this
+ voltage. May be clamped to safety limits.
+
+- fast-charge-safe-voltage:
+ Usage: optional (default: 4.2V, or pre-configured value)
+ Value type: <u32>; uV; range [3.24V : 5V]
+ Description: Maximum safe battery voltage; May be pre-set by bootloader, in
+ which case, setting this will harmlessly fail. The property
+ 'fast-charge-high-watermark' will be clamped by this value.
+
+- fast-charge-safe-current:
+ Usage: optional (default: 1A, or pre-configured value)
+ Value type: <u32>; uA; range [100mA : 3A]
+ Description: Maximum safe battery charge current; May pre-set by bootloader,
+ in which case, setting this will harmlessly fail. The property
+ 'battery-charge-control-limit' will be clamped by this value.
+
+- auto-recharge-low-watermark:
+ Usage: optional (default: 4.1V, or pre-configured value)
+ Value type: <u32>; uV; range [3.24V : 5V]
+ Description: Battery voltage limit below which auto-recharge functionality
+ will restart charging after end-of-charge; The high cutoff
+ limit for auto-recharge is 5% above this value.
+
+- minimum-input-voltage:
+ Usage: optional (default: 4.3V, or pre-configured value)
+ Value type: <u32>; uV; range [4.2V : 9.6V]
+ Description: Input voltage level above which charging may operate
+
+- usb-charge-control-limit:
+ Usage: optional (default: 100mA, or pre-configured value)
+ Value type: <u32>; uA; range [100mA : 2.5A]
+ Description: Default USB charge current
+
+- dc-charge-control-limit:
+ Usage: optional (default: 100mA, or pre-configured value)
+ Value type: <u32>; uA; range [100mA : 2.5A]
+ Description: Default DC charge current
+
+- disable-dc:
+ Usage: optional (default: false)
+ Value type: boolean: <u32> or <empty>
+ Description: Disable DC charger
+
+- jeita-extended-temp-range:
+ Usage: optional (default: false)
+ Value type: boolean: <u32> or <empty>
+ Description: Enable JEITA extended temperature range; This does *not*
+ adjust the maximum charge voltage or current in the extended
+ temperature range. It only allows charging when the battery
+ is in the extended temperature range. Voltage/current
+ regulation must be done externally to fully comply with
+ the JEITA safety guidelines if this flag is set.
+
+EXAMPLE
+charger@1000 {
+ compatible = "qcom,pm8941-charger";
+ reg = <0x1000 0x700>;
+ interrupts = <0x0 0x10 7 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x10 5 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x10 4 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x12 1 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x12 0 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x13 2 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x13 1 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x14 1 IRQ_TYPE_EDGE_BOTH>;
+ interrupt-names = "chg-done",
+ "chg-fast",
+ "chg-trkl",
+ "bat-temp-ok",
+ "bat-present",
+ "chg-gone",
+ "usb-valid",
+ "dc-valid";
+
+ battery-charge-control-limit = <1000000>;
+ usb-charge-control-limit = <500000>;
+ dc-charge-control-limit = <1000000>;
+};
--
1.8.2.2

2015-06-18 21:15:00

by Bjorn Andersson

[permalink] [raw]
Subject: [PATCH 3/4] power: Add Qualcomm SMBB driver

Add the Qualcomm Switch-Mode Battery Charger and Boost driver, found in
pm8941.

Signed-off-by: Courtney Cavin <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
---

The downstream (caf) driver for this hw block contains workarounds for a couple
of claimed hw issues. The most noteworthy is the software base charge-done
detection, which in a thread polls the end-of-charge status bit and upon seeing
the same value (done charging) several times in a row it disabled charging and
starts a timer to reenable charging at a later time. In our initial trials with
that driver we saw software race conditions that caused exactly these issues.

In this area there is however a possibly related issue, in that when the
battery is fully charged the hardware status bits indicates either charger gone
or fully charged - seemingly at random.

Driver is tested on 8974 and 8974ac, with battery as well as battery simulator
equipment.

drivers/power/Kconfig | 12 +
drivers/power/Makefile | 1 +
drivers/power/qcom_smbb.c | 952 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 965 insertions(+)
create mode 100644 drivers/power/qcom_smbb.c

diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 08beeed5485d..cfc236655661 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -379,6 +379,18 @@ config CHARGER_MAX8998
Say Y to enable support for the battery charger control sysfs and
platform data of MAX8998/LP3974 PMICs.

+config CHARGER_QCOM_SMBB
+ tristate "Qualcomm Switch-Mode Battery Charger and Boost"
+ depends on MFD_SPMI_PMIC || COMPILE_TEST
+ depends on OF
+ help
+ Say Y to include support for the Switch-Mode Battery Charger and
+ Boost (SMBB) hardware found in Qualcomm PM8941 PMICs. The charger
+ is an integrated, single-cell lithium-ion battery charger. DT
+ configuration is required for loading, see the devicetree
+ documentation for more detail. The base name for this driver is
+ 'pm8941_charger'.
+
config CHARGER_BQ2415X
tristate "TI BQ2415x battery charger driver"
depends on I2C
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index 5752ce818f51..48e459243820 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -57,6 +57,7 @@ obj-$(CONFIG_CHARGER_MAX14577) += max14577_charger.o
obj-$(CONFIG_CHARGER_MAX77693) += max77693_charger.o
obj-$(CONFIG_CHARGER_MAX8997) += max8997_charger.o
obj-$(CONFIG_CHARGER_MAX8998) += max8998_charger.o
+obj-$(CONFIG_CHARGER_QCOM_SMBB) += qcom_smbb.o
obj-$(CONFIG_CHARGER_BQ2415X) += bq2415x_charger.o
obj-$(CONFIG_CHARGER_BQ24190) += bq24190_charger.o
obj-$(CONFIG_CHARGER_BQ24257) += bq24257_charger.o
diff --git a/drivers/power/qcom_smbb.c b/drivers/power/qcom_smbb.c
new file mode 100644
index 000000000000..a092828cb9e8
--- /dev/null
+++ b/drivers/power/qcom_smbb.c
@@ -0,0 +1,952 @@
+/* Copyright (c) 2014, Sony Mobile Communications Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * This driver is for the multi-block Switch-Mode Battery Charger and Boost
+ * (SMBB) hardware, found in Qualcomm PM8941 PMICs. The charger is an
+ * integrated, single-cell lithium-ion battery charger.
+ *
+ * Sub-components:
+ * - Charger core
+ * - Buck
+ * - DC charge-path
+ * - USB charge-path
+ * - Battery interface
+ * - Boost (not implemented)
+ * - Misc
+ * - HF-Buck
+ */
+
+#include <linux/errno.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#define SMBB_CHG_VMAX 0x040
+#define SMBB_CHG_VSAFE 0x041
+#define SMBB_CHG_CFG 0x043
+#define SMBB_CHG_IMAX 0x044
+#define SMBB_CHG_ISAFE 0x045
+#define SMBB_CHG_VIN_MIN 0x047
+#define SMBB_CHG_CTRL 0x049
+#define CTRL_EN BIT(7)
+#define SMBB_CHG_VBAT_WEAK 0x052
+#define SMBB_CHG_IBAT_TERM_CHG 0x05b
+#define IBAT_TERM_CHG_IEOC BIT(7)
+#define IBAT_TERM_CHG_IEOC_BMS BIT(7)
+#define IBAT_TERM_CHG_IEOC_CHG 0
+#define SMBB_CHG_VBAT_DET 0x05d
+#define SMBB_CHG_TCHG_MAX_EN 0x060
+#define TCHG_MAX_EN BIT(7)
+#define SMBB_CHG_WDOG_TIME 0x062
+#define SMBB_CHG_WDOG_EN 0x065
+#define WDOG_EN BIT(7)
+
+#define SMBB_BUCK_REG_MODE 0x174
+#define BUCK_REG_MODE BIT(0)
+#define BUCK_REG_MODE_VBAT BIT(0)
+#define BUCK_REG_MODE_VSYS 0
+
+#define SMBB_BAT_PRES_STATUS 0x208
+#define PRES_STATUS_BAT_PRES BIT(7)
+#define SMBB_BAT_TEMP_STATUS 0x209
+#define TEMP_STATUS_OK BIT(7)
+#define TEMP_STATUS_HOT BIT(6)
+#define SMBB_BAT_BTC_CTRL 0x249
+#define BTC_CTRL_COMP_EN BIT(7)
+#define BTC_CTRL_COLD_EXT BIT(1)
+#define BTC_CTRL_HOT_EXT_N BIT(0)
+
+#define SMBB_USB_IMAX 0x344
+#define SMBB_USB_ENUM_TIMER_STOP 0x34e
+#define ENUM_TIMER_STOP BIT(0)
+#define SMBB_USB_SEC_ACCESS 0x3d0
+#define SEC_ACCESS_MAGIC 0xa5
+#define SMBB_USB_REV_BST 0x3ed
+#define REV_BST_CHG_GONE BIT(7)
+
+#define SMBB_DC_IMAX 0x444
+
+#define SMBB_MISC_REV2 0x601
+#define SMBB_MISC_BOOT_DONE 0x642
+#define BOOT_DONE BIT(7)
+
+#define STATUS_USBIN_VALID BIT(0) /* USB connection is valid */
+#define STATUS_DCIN_VALID BIT(1) /* DC connection is valid */
+#define STATUS_BAT_HOT BIT(2) /* Battery temp 1=Hot, 0=Cold */
+#define STATUS_BAT_OK BIT(3) /* Battery temp OK */
+#define STATUS_BAT_PRESENT BIT(4) /* Battery is present */
+#define STATUS_CHG_DONE BIT(5) /* Charge cycle is complete */
+#define STATUS_CHG_TRKL BIT(6) /* Trickle charging */
+#define STATUS_CHG_FAST BIT(7) /* Fast charging */
+#define STATUS_CHG_GONE BIT(8) /* No charger is connected */
+
+enum smbb_attr {
+ ATTR_BAT_ISAFE,
+ ATTR_BAT_IMAX,
+ ATTR_USBIN_IMAX,
+ ATTR_DCIN_IMAX,
+ ATTR_BAT_VSAFE,
+ ATTR_BAT_VMAX,
+ ATTR_BAT_VMIN,
+ ATTR_CHG_VDET,
+ ATTR_VIN_MIN,
+ _ATTR_CNT,
+};
+
+struct smbb_charger {
+ unsigned int revision;
+ unsigned int addr;
+ struct device *dev;
+
+ bool dc_disabled;
+ bool jeita_ext_temp;
+ unsigned long status;
+ struct mutex statlock;
+
+ unsigned int attr[_ATTR_CNT];
+
+ struct power_supply *usb_psy;
+ struct power_supply *dc_psy;
+ struct power_supply *bat_psy;
+ struct regmap *regmap;
+};
+
+static int smbb_vbat_weak_fn(unsigned int index)
+{
+ return 2100000 + index * 100000;
+}
+
+static int smbb_vin_fn(unsigned int index)
+{
+ if (index > 42)
+ return 5600000 + (index - 43) * 200000;
+ return 3400000 + index * 50000;
+}
+
+static int smbb_vmax_fn(unsigned int index)
+{
+ return 3240000 + index * 10000;
+}
+
+static int smbb_vbat_det_fn(unsigned int index)
+{
+ return 3240000 + index * 20000;
+}
+
+static int smbb_imax_fn(unsigned int index)
+{
+ if (index < 2)
+ return 100000 + index * 50000;
+ return index * 100000;
+}
+
+static int smbb_bat_imax_fn(unsigned int index)
+{
+ return index * 50000;
+}
+
+static unsigned int smbb_hw_lookup(unsigned int val, int (*fn)(unsigned int))
+{
+ unsigned int widx;
+ unsigned int sel;
+
+ for (widx = sel = 0; (*fn)(widx) <= val; ++widx)
+ sel = widx;
+
+ return sel;
+}
+
+static const struct smbb_charger_attr {
+ const char *name;
+ unsigned int reg;
+ unsigned int safe_reg;
+ unsigned int max;
+ unsigned int min;
+ unsigned int fail_ok;
+ int (*hw_fn)(unsigned int);
+} smbb_charger_attrs[] = {
+ [ATTR_BAT_ISAFE] = {
+ .name = "fast-charge-safe-current",
+ .reg = SMBB_CHG_ISAFE,
+ .max = 3000000,
+ .min = 200000,
+ .hw_fn = smbb_bat_imax_fn,
+ .fail_ok = 1,
+ },
+ [ATTR_BAT_IMAX] = {
+ .name = "battery-charge-control-limit",
+ .reg = SMBB_CHG_IMAX,
+ .safe_reg = SMBB_CHG_ISAFE,
+ .max = 3000000,
+ .min = 200000,
+ .hw_fn = smbb_bat_imax_fn,
+ },
+ [ATTR_USBIN_IMAX] = {
+ .name = "usb-charge-control-limit",
+ .reg = SMBB_USB_IMAX,
+ .max = 2500000,
+ .min = 100000,
+ .hw_fn = smbb_imax_fn,
+ },
+ [ATTR_DCIN_IMAX] = {
+ .name = "dc-charge-control-limit",
+ .reg = SMBB_DC_IMAX,
+ .max = 2500000,
+ .min = 100000,
+ .hw_fn = smbb_imax_fn,
+ },
+ [ATTR_BAT_VSAFE] = {
+ .name = "fast-charge-safe-voltage",
+ .reg = SMBB_CHG_VSAFE,
+ .max = 5000000,
+ .min = 3240000,
+ .hw_fn = smbb_vmax_fn,
+ .fail_ok = 1,
+ },
+ [ATTR_BAT_VMAX] = {
+ .name = "fast-charge-high-watermark",
+ .reg = SMBB_CHG_VMAX,
+ .safe_reg = SMBB_CHG_VSAFE,
+ .max = 5000000,
+ .min = 3240000,
+ .hw_fn = smbb_vmax_fn,
+ },
+ [ATTR_BAT_VMIN] = {
+ .name = "fast-charge-low-watermark",
+ .reg = SMBB_CHG_VBAT_WEAK,
+ .max = 3600000,
+ .min = 2100000,
+ .hw_fn = smbb_vbat_weak_fn,
+ },
+ [ATTR_CHG_VDET] = {
+ .name = "auto-recharge-low-watermark",
+ .reg = SMBB_CHG_VBAT_DET,
+ .max = 5000000,
+ .min = 3240000,
+ .hw_fn = smbb_vbat_det_fn,
+ },
+ [ATTR_VIN_MIN] = {
+ .name = "minimum-input-voltage",
+ .reg = SMBB_CHG_VIN_MIN,
+ .max = 9600000,
+ .min = 4200000,
+ .hw_fn = smbb_vin_fn,
+ },
+};
+
+static int smbb_charger_attr_write(struct smbb_charger *chg,
+ enum smbb_attr which, unsigned int val)
+{
+ const struct smbb_charger_attr *prop;
+ unsigned int wval;
+ unsigned int out;
+ int rc;
+
+ prop = &smbb_charger_attrs[which];
+
+ if (val > prop->max || val < prop->min) {
+ dev_err(chg->dev, "value out of range for %s [%u:%u]\n",
+ prop->name, prop->min, prop->max);
+ return -EINVAL;
+ }
+
+ if (prop->safe_reg) {
+ rc = regmap_read(chg->regmap,
+ chg->addr + prop->safe_reg, &wval);
+ if (rc) {
+ dev_err(chg->dev,
+ "unable to read safe value for '%s'\n",
+ prop->name);
+ return rc;
+ }
+
+ wval = prop->hw_fn(wval);
+
+ if (val > wval) {
+ dev_warn(chg->dev,
+ "%s above safe value, clamping at %u\n",
+ prop->name, wval);
+ val = wval;
+ }
+ }
+
+ wval = smbb_hw_lookup(val, prop->hw_fn);
+
+ rc = regmap_write(chg->regmap, chg->addr + prop->reg, wval);
+ if (rc) {
+ dev_err(chg->dev, "unable to update %s", prop->name);
+ return rc;
+ }
+ out = prop->hw_fn(wval);
+ if (out != val) {
+ dev_warn(chg->dev,
+ "%s inaccurate, rounded to %u\n",
+ prop->name, out);
+ }
+
+ dev_dbg(chg->dev, "%s <= %d\n", prop->name, out);
+
+ chg->attr[which] = out;
+
+ return 0;
+}
+
+static int smbb_charger_attr_read(struct smbb_charger *chg,
+ enum smbb_attr which)
+{
+ const struct smbb_charger_attr *prop;
+ unsigned int val;
+ int rc;
+
+ prop = &smbb_charger_attrs[which];
+
+ rc = regmap_read(chg->regmap, chg->addr + prop->reg, &val);
+ if (rc) {
+ dev_err(chg->dev, "failed to read %s\n", prop->name);
+ return rc;
+ }
+ val = prop->hw_fn(val);
+ dev_dbg(chg->dev, "%s => %d\n", prop->name, val);
+
+ chg->attr[which] = val;
+
+ return 0;
+}
+
+static int smbb_charger_attr_parse(struct smbb_charger *chg,
+ enum smbb_attr which)
+{
+ const struct smbb_charger_attr *prop;
+ unsigned int val;
+ int rc;
+
+ prop = &smbb_charger_attrs[which];
+
+ rc = of_property_read_u32(chg->dev->of_node, prop->name, &val);
+ if (rc == 0) {
+ rc = smbb_charger_attr_write(chg, which, val);
+ if (!rc || !prop->fail_ok)
+ return rc;
+ }
+ return smbb_charger_attr_read(chg, which);
+}
+
+static void smbb_set_line_flag(struct smbb_charger *chg, int irq, int flag)
+{
+ bool state;
+ int ret;
+
+ ret = irq_get_irqchip_state(irq, IRQCHIP_STATE_LINE_LEVEL, &state);
+ if (state < 0) {
+ dev_err(chg->dev, "failed to read irq line\n");
+ return;
+ }
+
+ mutex_lock(&chg->statlock);
+ if (state)
+ chg->status |= flag;
+ else
+ chg->status &= ~flag;
+ mutex_unlock(&chg->statlock);
+
+ dev_dbg(chg->dev, "status = %03lx\n", chg->status);
+}
+
+static irqreturn_t smbb_usb_valid_handler(int irq, void *_data)
+{
+ struct smbb_charger *chg = _data;
+
+ smbb_set_line_flag(chg, irq, STATUS_USBIN_VALID);
+ power_supply_changed(chg->usb_psy);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t smbb_dc_valid_handler(int irq, void *_data)
+{
+ struct smbb_charger *chg = _data;
+
+ smbb_set_line_flag(chg, irq, STATUS_DCIN_VALID);
+ if (!chg->dc_disabled)
+ power_supply_changed(chg->dc_psy);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t smbb_bat_temp_handler(int irq, void *_data)
+{
+ struct smbb_charger *chg = _data;
+ unsigned int val;
+ int rc;
+
+ rc = regmap_read(chg->regmap, chg->addr + SMBB_BAT_TEMP_STATUS, &val);
+ if (rc)
+ return IRQ_HANDLED;
+
+ mutex_lock(&chg->statlock);
+ if (val & TEMP_STATUS_OK) {
+ chg->status |= STATUS_BAT_OK;
+ } else {
+ chg->status &= ~STATUS_BAT_OK;
+ if (val & TEMP_STATUS_HOT)
+ chg->status |= STATUS_BAT_HOT;
+ }
+ mutex_unlock(&chg->statlock);
+
+ power_supply_changed(chg->bat_psy);
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t smbb_bat_present_handler(int irq, void *_data)
+{
+ struct smbb_charger *chg = _data;
+
+ smbb_set_line_flag(chg, irq, STATUS_BAT_PRESENT);
+ power_supply_changed(chg->bat_psy);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t smbb_chg_done_handler(int irq, void *_data)
+{
+ struct smbb_charger *chg = _data;
+
+ smbb_set_line_flag(chg, irq, STATUS_CHG_DONE);
+ power_supply_changed(chg->bat_psy);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t smbb_chg_gone_handler(int irq, void *_data)
+{
+ struct smbb_charger *chg = _data;
+
+ smbb_set_line_flag(chg, irq, STATUS_CHG_GONE);
+ power_supply_changed(chg->bat_psy);
+ power_supply_changed(chg->usb_psy);
+ if (!chg->dc_disabled)
+ power_supply_changed(chg->dc_psy);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t smbb_chg_fast_handler(int irq, void *_data)
+{
+ struct smbb_charger *chg = _data;
+
+ smbb_set_line_flag(chg, irq, STATUS_CHG_FAST);
+ power_supply_changed(chg->bat_psy);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t smbb_chg_trkl_handler(int irq, void *_data)
+{
+ struct smbb_charger *chg = _data;
+
+ smbb_set_line_flag(chg, irq, STATUS_CHG_TRKL);
+ power_supply_changed(chg->bat_psy);
+
+ return IRQ_HANDLED;
+}
+
+static const struct smbb_irq {
+ const char *name;
+ irqreturn_t (*handler)(int, void *);
+} smbb_charger_irqs[] = {
+ { "chg-done", smbb_chg_done_handler },
+ { "chg-fast", smbb_chg_fast_handler },
+ { "chg-trkl", smbb_chg_trkl_handler },
+ { "bat-temp-ok", smbb_bat_temp_handler },
+ { "bat-present", smbb_bat_present_handler },
+ { "chg-gone", smbb_chg_gone_handler },
+ { "usb-valid", smbb_usb_valid_handler },
+ { "dc-valid", smbb_dc_valid_handler },
+};
+
+static int smbb_usbin_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct smbb_charger *chg = power_supply_get_drvdata(psy);
+ int rc = 0;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_ONLINE:
+ mutex_lock(&chg->statlock);
+ val->intval = !(chg->status & STATUS_CHG_GONE) &&
+ (chg->status & STATUS_USBIN_VALID);
+ mutex_unlock(&chg->statlock);
+ break;
+ case POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT:
+ val->intval = chg->attr[ATTR_USBIN_IMAX];
+ break;
+ case POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT_MAX:
+ val->intval = 2500000;
+ break;
+ default:
+ rc = -EINVAL;
+ break;
+ }
+
+ return rc;
+}
+
+static int smbb_usbin_set_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ const union power_supply_propval *val)
+{
+ struct smbb_charger *chg = power_supply_get_drvdata(psy);
+ int rc;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT:
+ rc = smbb_charger_attr_write(chg, ATTR_USBIN_IMAX,
+ val->intval);
+ break;
+ default:
+ rc = -EINVAL;
+ break;
+ }
+
+ return rc;
+}
+
+static int smbb_dcin_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct smbb_charger *chg = power_supply_get_drvdata(psy);
+ int rc = 0;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_ONLINE:
+ mutex_lock(&chg->statlock);
+ val->intval = !(chg->status & STATUS_CHG_GONE) &&
+ (chg->status & STATUS_DCIN_VALID);
+ mutex_unlock(&chg->statlock);
+ break;
+ case POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT:
+ val->intval = chg->attr[ATTR_DCIN_IMAX];
+ break;
+ case POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT_MAX:
+ val->intval = 2500000;
+ break;
+ default:
+ rc = -EINVAL;
+ break;
+ }
+
+ return rc;
+}
+
+static int smbb_dcin_set_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ const union power_supply_propval *val)
+{
+ struct smbb_charger *chg = power_supply_get_drvdata(psy);
+ int rc;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT:
+ rc = smbb_charger_attr_write(chg, ATTR_DCIN_IMAX,
+ val->intval);
+ break;
+ default:
+ rc = -EINVAL;
+ break;
+ }
+
+ return rc;
+}
+
+static int smbb_charger_writable_property(struct power_supply *psy,
+ enum power_supply_property psp)
+{
+ return psp == POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT;
+}
+
+static int smbb_battery_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct smbb_charger *chg = power_supply_get_drvdata(psy);
+ unsigned long status;
+ int rc = 0;
+
+ mutex_lock(&chg->statlock);
+ status = chg->status;
+ mutex_unlock(&chg->statlock);
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_STATUS:
+ if (status & STATUS_CHG_GONE)
+ val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+ else if (!(status & (STATUS_DCIN_VALID | STATUS_USBIN_VALID)))
+ val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+ else if (status & STATUS_CHG_DONE)
+ val->intval = POWER_SUPPLY_STATUS_FULL;
+ else if (!(status & STATUS_BAT_OK))
+ val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+ else if (status & (STATUS_CHG_FAST | STATUS_CHG_TRKL))
+ val->intval = POWER_SUPPLY_STATUS_CHARGING;
+ else /* everything is ok for charging, but we are not... */
+ val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+ break;
+ case POWER_SUPPLY_PROP_HEALTH:
+ if (status & STATUS_BAT_OK)
+ val->intval = POWER_SUPPLY_HEALTH_GOOD;
+ else if (status & STATUS_BAT_HOT)
+ val->intval = POWER_SUPPLY_HEALTH_OVERHEAT;
+ else
+ val->intval = POWER_SUPPLY_HEALTH_COLD;
+ break;
+ case POWER_SUPPLY_PROP_CHARGE_TYPE:
+ if (status & STATUS_CHG_FAST)
+ val->intval = POWER_SUPPLY_CHARGE_TYPE_FAST;
+ else if (status & STATUS_CHG_TRKL)
+ val->intval = POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
+ else
+ val->intval = POWER_SUPPLY_CHARGE_TYPE_NONE;
+ break;
+ case POWER_SUPPLY_PROP_PRESENT:
+ val->intval = !!(status & STATUS_BAT_PRESENT);
+ break;
+ case POWER_SUPPLY_PROP_CURRENT_MAX:
+ val->intval = chg->attr[ATTR_BAT_IMAX];
+ break;
+ case POWER_SUPPLY_PROP_VOLTAGE_MAX:
+ val->intval = chg->attr[ATTR_BAT_VMAX];
+ break;
+ case POWER_SUPPLY_PROP_TECHNOLOGY:
+ /* this charger is a single-cell lithium-ion battery charger
+ * only. If you hook up some other technology, there will be
+ * fireworks.
+ */
+ val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
+ break;
+ case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
+ val->intval = 3000000; /* single-cell li-ion low end */
+ break;
+ default:
+ rc = -EINVAL;
+ break;
+ }
+
+ return rc;
+}
+
+static int smbb_battery_set_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ const union power_supply_propval *val)
+{
+ struct smbb_charger *chg = power_supply_get_drvdata(psy);
+ int rc;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_CURRENT_MAX:
+ rc = smbb_charger_attr_write(chg, ATTR_BAT_IMAX, val->intval);
+ break;
+ case POWER_SUPPLY_PROP_VOLTAGE_MAX:
+ rc = smbb_charger_attr_write(chg, ATTR_BAT_VMAX, val->intval);
+ break;
+ default:
+ rc = -EINVAL;
+ break;
+ }
+
+ return rc;
+}
+
+static int smbb_battery_writable_property(struct power_supply *psy,
+ enum power_supply_property psp)
+{
+ switch (psp) {
+ case POWER_SUPPLY_PROP_CURRENT_MAX:
+ case POWER_SUPPLY_PROP_VOLTAGE_MAX:
+ return 1;
+ default:
+ return 0;
+ }
+}
+
+static enum power_supply_property smbb_charger_properties[] = {
+ POWER_SUPPLY_PROP_ONLINE,
+ POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT,
+ POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT_MAX,
+};
+
+static enum power_supply_property smbb_battery_properties[] = {
+ POWER_SUPPLY_PROP_STATUS,
+ POWER_SUPPLY_PROP_HEALTH,
+ POWER_SUPPLY_PROP_PRESENT,
+ POWER_SUPPLY_PROP_CHARGE_TYPE,
+ POWER_SUPPLY_PROP_CURRENT_MAX,
+ POWER_SUPPLY_PROP_VOLTAGE_MAX,
+ POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
+ POWER_SUPPLY_PROP_TECHNOLOGY,
+};
+
+static const struct reg_off_mask_default {
+ unsigned int offset;
+ unsigned int mask;
+ unsigned int value;
+ unsigned int rev_mask;
+} smbb_charger_setup[] = {
+ /* The bootloader is supposed to set this... make sure anyway. */
+ { SMBB_MISC_BOOT_DONE, BOOT_DONE, BOOT_DONE },
+
+ /* Disable software timer */
+ { SMBB_CHG_TCHG_MAX_EN, TCHG_MAX_EN, 0 },
+
+ /* Clear and disable watchdog */
+ { SMBB_CHG_WDOG_TIME, 0xff, 160 },
+ { SMBB_CHG_WDOG_EN, WDOG_EN, 0 },
+
+ /* Use charger based EoC detection */
+ { SMBB_CHG_IBAT_TERM_CHG, IBAT_TERM_CHG_IEOC, IBAT_TERM_CHG_IEOC_CHG },
+
+ /* Disable GSM PA load adjustment.
+ * The PA signal is incorrectly connected on v2.
+ */
+ { SMBB_CHG_CFG, 0xff, 0x00, BIT(3) },
+
+ /* Use VBAT (not VSYS) to compensate for IR drop during fast charging */
+ { SMBB_BUCK_REG_MODE, BUCK_REG_MODE, BUCK_REG_MODE_VBAT },
+
+ /* Enable battery temperature comparators */
+ { SMBB_BAT_BTC_CTRL, BTC_CTRL_COMP_EN, BTC_CTRL_COMP_EN },
+
+ /* Stop USB enumeration timer */
+ { SMBB_USB_ENUM_TIMER_STOP, ENUM_TIMER_STOP, ENUM_TIMER_STOP },
+
+#if 0 /* FIXME supposedly only to disable hardware ARB termination */
+ { SMBB_USB_SEC_ACCESS, SEC_ACCESS_MAGIC },
+ { SMBB_USB_REV_BST, 0xff, REV_BST_CHG_GONE },
+#endif
+
+ /* Stop USB enumeration timer, again */
+ { SMBB_USB_ENUM_TIMER_STOP, ENUM_TIMER_STOP, ENUM_TIMER_STOP },
+
+ /* Enable charging */
+ { SMBB_CHG_CTRL, CTRL_EN, CTRL_EN },
+};
+
+static char *smbb_bif[] = { "smbb-bif" };
+
+static const struct power_supply_desc bat_psy_desc = {
+ .name = "smbb-bif",
+ .type = POWER_SUPPLY_TYPE_BATTERY,
+ .properties = smbb_battery_properties,
+ .num_properties = ARRAY_SIZE(smbb_battery_properties),
+ .get_property = smbb_battery_get_property,
+ .set_property = smbb_battery_set_property,
+ .property_is_writeable = smbb_battery_writable_property,
+};
+
+static const struct power_supply_desc usb_psy_desc = {
+ .name = "smbb-usbin",
+ .type = POWER_SUPPLY_TYPE_USB,
+ .properties = smbb_charger_properties,
+ .num_properties = ARRAY_SIZE(smbb_charger_properties),
+ .get_property = smbb_usbin_get_property,
+ .set_property = smbb_usbin_set_property,
+ .property_is_writeable = smbb_charger_writable_property,
+};
+
+static const struct power_supply_desc dc_psy_desc = {
+ .name = "smbb-dcin",
+ .type = POWER_SUPPLY_TYPE_MAINS,
+ .properties = smbb_charger_properties,
+ .num_properties = ARRAY_SIZE(smbb_charger_properties),
+ .get_property = smbb_dcin_get_property,
+ .set_property = smbb_dcin_set_property,
+ .property_is_writeable = smbb_charger_writable_property,
+};
+
+static int smbb_charger_probe(struct platform_device *pdev)
+{
+ struct power_supply_config bat_cfg = {};
+ struct power_supply_config usb_cfg = {};
+ struct power_supply_config dc_cfg = {};
+ struct smbb_charger *chg;
+ int rc, i;
+
+ chg = devm_kzalloc(&pdev->dev, sizeof(*chg), GFP_KERNEL);
+ if (!chg)
+ return -ENOMEM;
+
+ chg->dev = &pdev->dev;
+ mutex_init(&chg->statlock);
+
+ chg->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+ if (!chg->regmap) {
+ dev_err(&pdev->dev, "failed to locate regmap\n");
+ return -ENODEV;
+ }
+
+ rc = of_property_read_u32(pdev->dev.of_node, "reg", &chg->addr);
+ if (rc) {
+ dev_err(&pdev->dev, "missing or invalid 'reg' property\n");
+ return rc;
+ }
+
+ rc = regmap_read(chg->regmap, chg->addr + SMBB_MISC_REV2, &chg->revision);
+ if (rc) {
+ dev_err(&pdev->dev, "unable to read revision\n");
+ return rc;
+ }
+
+ chg->revision += 1;
+ if (chg->revision != 2 && chg->revision != 3) {
+ dev_err(&pdev->dev, "v1 hardware not supported\n");
+ return -ENODEV;
+ }
+ dev_info(&pdev->dev, "Initializing SMBB rev %u", chg->revision);
+
+ chg->dc_disabled = of_property_read_bool(pdev->dev.of_node, "disable-dc");
+
+ for (i = 0; i < _ATTR_CNT; ++i) {
+ rc = smbb_charger_attr_parse(chg, i);
+ if (rc) {
+ dev_err(&pdev->dev, "failed to parse/apply settings\n");
+ return rc;
+ }
+ }
+
+ bat_cfg.drv_data = chg;
+ bat_cfg.of_node = pdev->dev.of_node;
+ chg->bat_psy = devm_power_supply_register(&pdev->dev,
+ &bat_psy_desc,
+ &bat_cfg);
+ if (IS_ERR(chg->bat_psy)) {
+ dev_err(&pdev->dev, "failed to register battery\n");
+ return PTR_ERR(chg->bat_psy);
+ }
+
+ usb_cfg.drv_data = chg;
+ usb_cfg.supplied_to = smbb_bif;
+ usb_cfg.num_supplicants = ARRAY_SIZE(smbb_bif);
+ chg->usb_psy = devm_power_supply_register(&pdev->dev,
+ &usb_psy_desc,
+ &usb_cfg);
+ if (IS_ERR(chg->usb_psy)) {
+ dev_err(&pdev->dev, "failed to register USB power supply\n");
+ return PTR_ERR(chg->usb_psy);
+ }
+
+ if (!chg->dc_disabled) {
+ dc_cfg.drv_data = chg;
+ dc_cfg.supplied_to = smbb_bif;
+ dc_cfg.num_supplicants = ARRAY_SIZE(smbb_bif);
+ chg->dc_psy = devm_power_supply_register(&pdev->dev,
+ &dc_psy_desc,
+ &dc_cfg);
+ if (IS_ERR(chg->dc_psy)) {
+ dev_err(&pdev->dev, "failed to register DC power supply\n");
+ return PTR_ERR(chg->dc_psy);
+ }
+ }
+
+ for (i = 0; i < ARRAY_SIZE(smbb_charger_irqs); ++i) {
+ int irq;
+
+ irq = platform_get_irq_byname(pdev, smbb_charger_irqs[i].name);
+ if (irq < 0) {
+ dev_err(&pdev->dev, "failed to get irq '%s'\n",
+ smbb_charger_irqs[i].name);
+ return irq;
+ }
+
+ smbb_charger_irqs[i].handler(irq, chg);
+
+ rc = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+ smbb_charger_irqs[i].handler, IRQF_ONESHOT,
+ smbb_charger_irqs[i].name, chg);
+ if (rc) {
+ dev_err(&pdev->dev, "failed to request irq '%s'\n",
+ smbb_charger_irqs[i].name);
+ return rc;
+ }
+ }
+
+ chg->jeita_ext_temp = of_property_read_bool(pdev->dev.of_node,
+ "jeita-extended-temp-range");
+
+ /* Set temperature range to [35%:70%] or [25%:80%] accordingly */
+ rc = regmap_update_bits(chg->regmap, chg->addr + SMBB_BAT_BTC_CTRL,
+ BTC_CTRL_COLD_EXT | BTC_CTRL_HOT_EXT_N,
+ chg->jeita_ext_temp ?
+ BTC_CTRL_COLD_EXT :
+ BTC_CTRL_HOT_EXT_N);
+ if (rc) {
+ dev_err(&pdev->dev,
+ "unable to set %s temperature range\n",
+ chg->jeita_ext_temp ? "JEITA extended" : "normal");
+ return rc;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(smbb_charger_setup); ++i) {
+ const struct reg_off_mask_default *r = &smbb_charger_setup[i];
+
+ if (r->rev_mask & BIT(chg->revision))
+ continue;
+
+ rc = regmap_update_bits(chg->regmap, chg->addr + r->offset,
+ r->mask, r->value);
+ if (rc) {
+ dev_err(&pdev->dev,
+ "unable to initializing charging, bailing\n");
+ return rc;
+ }
+ }
+
+ platform_set_drvdata(pdev, chg);
+
+ return 0;
+}
+
+static int smbb_charger_remove(struct platform_device *pdev)
+{
+ struct smbb_charger *chg;
+
+ chg = platform_get_drvdata(pdev);
+
+ regmap_update_bits(chg->regmap, chg->addr + SMBB_CHG_CTRL, CTRL_EN, 0);
+
+ return 0;
+}
+
+static const struct of_device_id smbb_charger_id_table[] = {
+ { .compatible = "qcom,pm8941-charger" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, smbb_charger_id_table);
+
+static struct platform_driver smbb_charger_driver = {
+ .probe = smbb_charger_probe,
+ .remove = smbb_charger_remove,
+ .driver = {
+ .name = "qcom-smbb",
+ .of_match_table = smbb_charger_id_table,
+ },
+};
+module_platform_driver(smbb_charger_driver);
+
+MODULE_ALIAS("platform:qcom_smbb");
+MODULE_DESCRIPTION("Qualcomm Switch-Mode Battery Charger and Boost driver");
+MODULE_LICENSE("GPL v2");
--
1.8.2.2

2015-06-18 21:14:25

by Bjorn Andersson

[permalink] [raw]
Subject: [PATCH 4/4] ARM: dts: qcom-pm8941: Add charger node

Signed-off-by: Bjorn Andersson <[email protected]>
---
arch/arm/boot/dts/qcom-pm8941.dtsi | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-pm8941.dtsi b/arch/arm/boot/dts/qcom-pm8941.dtsi
index aa774e685018..6063c7ddda0f 100644
--- a/arch/arm/boot/dts/qcom-pm8941.dtsi
+++ b/arch/arm/boot/dts/qcom-pm8941.dtsi
@@ -26,6 +26,27 @@
bias-pull-up;
};

+ charger@1000 {
+ compatible = "qcom,pm8941-charger";
+ reg = <0x1000 0x700>;
+ interrupts = <0x0 0x10 7 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x10 5 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x10 4 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x12 1 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x12 0 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x13 2 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x13 1 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x14 1 IRQ_TYPE_EDGE_BOTH>;
+ interrupt-names = "chg-done",
+ "chg-fast",
+ "chg-trkl",
+ "bat-temp-ok",
+ "bat-present",
+ "chg-gone",
+ "usb-valid",
+ "dc-valid";
+ };
+
pm8941_gpios: gpios@c000 {
compatible = "qcom,pm8941-gpio";
reg = <0xc000 0x2400>;
--
1.8.2.2

2015-06-19 17:01:43

by Paul Bolle

[permalink] [raw]
Subject: Re: [PATCH 3/4] power: Add Qualcomm SMBB driver

On Thu, 2015-06-18 at 14:13 -0700, Bjorn Andersson wrote:
> --- /dev/null
> +++ b/drivers/power/qcom_smbb.c

> +MODULE_ALIAS("platform:qcom_smbb");

(The day before yesterday and yesterday I had a, well, lively
conversation regarding this macro. The interesting bits start at
https://lkml.org/lkml/2015/6/17/383 .

But in a converstaion today things were rather silent. See
https://lkml.org/lkml/2015/6/19/68 and the reply, of sorts, in
https://lkml.org/lkml/2015/6/19/117. Let's see what happens here.)

As I understand it, this alias is only useful if there's a corresponding
struct platform_device, somewhere. Ie, this alias implies a
platform_device that will fire of a "MODALIAS=platform:qcom_smbb" uevent
when it's created. That would be a platform_device using a "qcom_smbb"
name.

If that's correct, then I think this MODULE_ALIAS macro isn't needed
here, as I couldn't find a platform_device using that name. (But perhaps
a patch that adds it is pending, somewhere.)

Thanks,


Paul Bolle

2015-06-22 05:06:26

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH 3/4] power: Add Qualcomm SMBB driver

On Fri, Jun 19, 2015 at 10:01 AM, Paul Bolle <[email protected]> wrote:
> On Thu, 2015-06-18 at 14:13 -0700, Bjorn Andersson wrote:
>> --- /dev/null
>> +++ b/drivers/power/qcom_smbb.c
>
>> +MODULE_ALIAS("platform:qcom_smbb");
>
> (The day before yesterday and yesterday I had a, well, lively
> conversation regarding this macro. The interesting bits start at
> https://lkml.org/lkml/2015/6/17/383 .
>
> But in a converstaion today things were rather silent. See
> https://lkml.org/lkml/2015/6/19/68 and the reply, of sorts, in
> https://lkml.org/lkml/2015/6/19/117. Let's see what happens here.)
>
> As I understand it, this alias is only useful if there's a corresponding
> struct platform_device, somewhere. Ie, this alias implies a
> platform_device that will fire of a "MODALIAS=platform:qcom_smbb" uevent
> when it's created. That would be a platform_device using a "qcom_smbb"
> name.
>

As far as I could see the uevents, including MODALIAS, is sent when
the device is added or removed; but the content comes from the device
tree alias, not the MODULE_ALIAS.

The MODULE_ALIAS seems to add an alias to the kernel module
information, which can be used to find this driver during modprobe; in
addition to the actual name of the module.

> If that's correct, then I think this MODULE_ALIAS macro isn't needed
> here, as I couldn't find a platform_device using that name. (But perhaps
> a patch that adds it is pending, somewhere.)
>

I don't see any reason for keeping the MODULE_ALIAS, so I think it
should be removed for v2.

Regards,
Bjorn

2015-07-25 15:42:08

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCH 3/4] power: Add Qualcomm SMBB driver

Hi,

On Thu, Jun 18, 2015 at 02:13:44PM -0700, Bjorn Andersson wrote:
> Add the Qualcomm Switch-Mode Battery Charger and Boost driver, found in
> pm8941.

The driver's sourcecode looks fine to me. I'm not convinced by all
those new DT properties, though. I think "watermark" should be
replaced with "threshold" and "control" with "current" for all
properties. Additionally some comments. Note, that I only used the
driver's sourcecode as reference, since the DT binding document
was neither send to me, nor to linux-pm mailinglist.

* battery-charge-control-limit

It's unclear, what this property is used for. Is the limit only
for "normal" charging or also for fast charging?

* fast-charge-low-watermark
* fast-charge-high-watermark

Add a unit to this property. Maybe "fast-charge-start-voltage"
and "fast-charge-stop-voltage"?

* fast-charge-safe-voltage
* fast-charge-safe-current

These properties are fine to me. I wonder if they should be named
fast-charge-max-*, though.

* auto-recharge-low-watermark

I think the "low" can be dropped. Instead a -voltage
should be appended, since it could also be a percentage.

* minimum-input-voltage

Add a vendor prefix to this property.

* usb-charge-control-limit

I suggest to remove this from DT. If no USB detection is
implemented, the default should be 100mA according to USB
standard.

* dc-charge-control-limit

Please add a vendor prefix and I think "dc-current-limit"
is a more fitting name.

* disable-dc

Please add a vendor prefix.

* jeita-extended-temp-range

Looks ok to me.

-- Sebastian

smbb_charger_attr_parse


Attachments:
(No filename) (1.55 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2015-07-26 01:04:21

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH 3/4] power: Add Qualcomm SMBB driver

On Sat 25 Jul 08:42 PDT 2015, Sebastian Reichel wrote:

> Hi,
>
> On Thu, Jun 18, 2015 at 02:13:44PM -0700, Bjorn Andersson wrote:
> > Add the Qualcomm Switch-Mode Battery Charger and Boost driver, found in
> > pm8941.
>
> The driver's sourcecode looks fine to me.

Thanks.

> I'm not convinced by all those new DT properties, though. I think
> "watermark" should be replaced with "threshold" and "control" with
> "current" for all properties. Additionally some comments.

I think both of these comes from the documentation, but I agree with
your suggestion.

> Note, that I only used the driver's sourcecode as reference, since the
> DT binding document was neither send to me, nor to linux-pm
> mailinglist.
>

Sorry about that, I will make sure to double check my recipients in the
future.

> * battery-charge-control-limit
>
> It's unclear, what this property is used for. Is the limit only
> for "normal" charging or also for fast charging?
>

This is described as the current limit during fast charging. However,
"fast charging" is the normal state.

I think the most consistent (regards documentation and other properties)
would be:

qcom,fast-charge-current-limit

> * fast-charge-low-watermark
> * fast-charge-high-watermark
>
> Add a unit to this property. Maybe "fast-charge-start-voltage"
> and "fast-charge-stop-voltage"?
>

Will update to:

qcom,fast-charge-{low,high}-threshold-voltage

> * fast-charge-safe-voltage
> * fast-charge-safe-current
>
> These properties are fine to me. I wonder if they should be named
> fast-charge-max-*, though.
>

The safe naming is in accordance with the hw documentation, so I think
we should keep those.

> * auto-recharge-low-watermark
>
> I think the "low" can be dropped. Instead a -voltage
> should be appended, since it could also be a percentage.
>

qcom,auto-charge-threshold-voltage

> * minimum-input-voltage
>
> Add a vendor prefix to this property.
>

Shouldn't they all have a vendor prefix?

> * usb-charge-control-limit
>
> I suggest to remove this from DT. If no USB detection is
> implemented, the default should be 100mA according to USB
> standard.
>

Right, this have been convenient during testing as no-one actually does
implement the USB current limit propagation. But that should be
corrected and then you're right that this should only default to 100mA.

I'll drop it.

> * dc-charge-control-limit
>
> Please add a vendor prefix and I think "dc-current-limit"
> is a more fitting name.
>

Sounds good.

> * disable-dc
>
> Please add a vendor prefix.
>

Ok

> * jeita-extended-temp-range
>
> Looks ok to me.

Thanks for the review, I'll update the patches accordingly and will send
out v2 (and make sure you get the dt binding document as well).

Regards,
Bjorn

2015-07-27 05:34:41

by Andy Gross

[permalink] [raw]
Subject: Re: [PATCH 1/4] spmi: pmic-arb: add irq_get_irqchip_state implementation

On Thu, Jun 18, 2015 at 02:13:42PM -0700, Bjorn Andersson wrote:
> Signed-off-by: Courtney Cavin <[email protected]>
> Signed-off-by: Bjorn Andersson <[email protected]>
> ---

Looks good.

Greg, can you pick this up?

Reviewed-by: Andy Gross <[email protected]>

--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2015-07-27 14:06:32

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCH 3/4] power: Add Qualcomm SMBB driver

Hi,

On Sat, Jul 25, 2015 at 06:04:14PM -0700, Bjorn Andersson wrote:
> On Sat 25 Jul 08:42 PDT 2015, Sebastian Reichel wrote:
> > * battery-charge-control-limit
> >
> > It's unclear, what this property is used for. Is the limit only
> > for "normal" charging or also for fast charging?
> >
>
> This is described as the current limit during fast charging. However,
> "fast charging" is the normal state.
>
> I think the most consistent (regards documentation and other properties)
> would be:
>
> qcom,fast-charge-current-limit

So what's the difference to "fast-charge-safe-current"?

> > * minimum-input-voltage
> >
> > Add a vendor prefix to this property.
> >
>
> Shouldn't they all have a vendor prefix?

Some of the properties are quite generic and used on multiple
chips, so we may create a power_supply/battery-fuel-gauge.txt
and power_supply/battery-charger.txt with generic bindings.

I'm fine with just adding vendor prefixed properties for all
instances, though.

> Thanks for the review, I'll update the patches accordingly and
> will send out v2 (and make sure you get the dt binding document
> as well).

OK, thanks.

-- Sebastian


Attachments:
(No filename) (1.13 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2015-07-30 17:17:13

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH 3/4] power: Add Qualcomm SMBB driver

On Mon 27 Jul 07:06 PDT 2015, Sebastian Reichel wrote:

> Hi,
>
> On Sat, Jul 25, 2015 at 06:04:14PM -0700, Bjorn Andersson wrote:
> > On Sat 25 Jul 08:42 PDT 2015, Sebastian Reichel wrote:
> > > * battery-charge-control-limit
> > >
> > > It's unclear, what this property is used for. Is the limit only
> > > for "normal" charging or also for fast charging?
> > >
> >
> > This is described as the current limit during fast charging. However,
> > "fast charging" is the normal state.
> >
> > I think the most consistent (regards documentation and other properties)
> > would be:
> >
> > qcom,fast-charge-current-limit

I spoke with Courtney about this and he pointed out that it really is
the limit of the current flowing into the battery, hence his original
naming.

>
> So what's the difference to "fast-charge-safe-current"?
>

The "safe" values are write-once values that should be set once during
boot to protect the hardware. The fast-charge-current-limit can be
modified in runtime by e.g. userspace or a thermal mitigation solution -
but will never be allowed to go above the safe limit.

Regards,
Bjorn