* [PATCH v2 1/4] dt-bindings: power: supply: Add pm8916 VM-BMS
2023-07-31 17:06 [PATCH v2 0/4] Add pm8916 VM-BMS and LBC Nikita Travkin
@ 2023-07-31 17:06 ` Nikita Travkin
2023-08-11 18:06 ` Rob Herring
2023-07-31 17:06 ` [PATCH v2 2/4] dt-bindings: power: supply: Add pm8916 LBC Nikita Travkin
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Nikita Travkin @ 2023-07-31 17:06 UTC (permalink / raw)
To: Sebastian Reichel, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-pm, devicetree, linux-kernel, linux-arm-msm, Nikita Travkin
Qualcomm Voltage Mode BMS is a battery monitoring block in PM8916 PMIC.
Document it's DT binding.
Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---
v2: Describe all interrupts. (Conor)
---
.../bindings/power/supply/qcom,pm8916-bms-vm.yaml | 83 ++++++++++++++++++++++
1 file changed, 83 insertions(+)
diff --git a/Documentation/devicetree/bindings/power/supply/qcom,pm8916-bms-vm.yaml b/Documentation/devicetree/bindings/power/supply/qcom,pm8916-bms-vm.yaml
new file mode 100644
index 000000000000..ad764e69ab57
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/qcom,pm8916-bms-vm.yaml
@@ -0,0 +1,83 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/supply/qcom,pm8916-bms-vm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Voltage Mode BMS
+
+maintainers:
+ - Nikita Travkin <nikita@trvn.ru>
+
+description:
+ Voltage Mode BMS is a hardware block found in some Qualcomm PMICs
+ such as pm8916. This block performs battery voltage monitoring.
+
+allOf:
+ - $ref: power-supply.yaml#
+
+properties:
+ compatible:
+ const: qcom,pm8916-bms-vm
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ items:
+ - description: BMS FSM left S3 mode
+ - description: BMS FSM entered S2 mode
+ - description: OCV measured in S3 mode
+ - description: OCV below threshold
+ - description: FIFO update done
+ - description: BMS FSM switched state
+
+ interrupt-names:
+ items:
+ - const: cv_leave
+ - const: cv_enter
+ - const: ocv_good
+ - const: ocv_thr
+ - const: fifo
+ - const: state_chg
+
+ monitored-battery: true
+
+ power-supplies: true
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - interrupt-names
+ - monitored-battery
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ pmic {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ battery@4000 {
+ compatible = "qcom,pm8916-bms-vm";
+ reg = <0x4000>;
+ interrupts = <0x0 0x40 0 IRQ_TYPE_EDGE_RISING>,
+ <0x0 0x40 1 IRQ_TYPE_EDGE_RISING>,
+ <0x0 0x40 2 IRQ_TYPE_EDGE_RISING>,
+ <0x0 0x40 3 IRQ_TYPE_EDGE_RISING>,
+ <0x0 0x40 4 IRQ_TYPE_EDGE_RISING>,
+ <0x0 0x40 5 IRQ_TYPE_EDGE_RISING>;
+ interrupt-names = "cv_leave",
+ "cv_enter",
+ "ocv_good",
+ "ocv_thr",
+ "fifo",
+ "state_chg";
+
+ monitored-battery = <&battery>;
+ power-supplies = <&pm8916_charger>;
+ };
+ };
--
2.41.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v2 1/4] dt-bindings: power: supply: Add pm8916 VM-BMS
2023-07-31 17:06 ` [PATCH v2 1/4] dt-bindings: power: supply: Add pm8916 VM-BMS Nikita Travkin
@ 2023-08-11 18:06 ` Rob Herring
0 siblings, 0 replies; 11+ messages in thread
From: Rob Herring @ 2023-08-11 18:06 UTC (permalink / raw)
To: Nikita Travkin
Cc: Sebastian Reichel, linux-kernel, linux-arm-msm, Conor Dooley,
Rob Herring, linux-pm, devicetree, Krzysztof Kozlowski
On Mon, 31 Jul 2023 22:06:24 +0500, Nikita Travkin wrote:
> Qualcomm Voltage Mode BMS is a battery monitoring block in PM8916 PMIC.
> Document it's DT binding.
>
> Signed-off-by: Nikita Travkin <nikita@trvn.ru>
> ---
> v2: Describe all interrupts. (Conor)
> ---
> .../bindings/power/supply/qcom,pm8916-bms-vm.yaml | 83 ++++++++++++++++++++++
> 1 file changed, 83 insertions(+)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 2/4] dt-bindings: power: supply: Add pm8916 LBC
2023-07-31 17:06 [PATCH v2 0/4] Add pm8916 VM-BMS and LBC Nikita Travkin
2023-07-31 17:06 ` [PATCH v2 1/4] dt-bindings: power: supply: Add pm8916 VM-BMS Nikita Travkin
@ 2023-07-31 17:06 ` Nikita Travkin
2023-08-11 18:08 ` Rob Herring
2023-07-31 17:06 ` [PATCH v2 3/4] power: supply: Add pm8916 VM-BMS support Nikita Travkin
2023-07-31 17:06 ` [PATCH v2 4/4] power: supply: Add driver for pm8916 lbc Nikita Travkin
3 siblings, 1 reply; 11+ messages in thread
From: Nikita Travkin @ 2023-07-31 17:06 UTC (permalink / raw)
To: Sebastian Reichel, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-pm, devicetree, linux-kernel, linux-arm-msm, Nikita Travkin
Qualcomm Linear Battery Charger is a CC/CV charger block in PM8916 PMIC.
Document it's DT binding.
Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---
v2: List all interrupts (Conor)
---
.../bindings/power/supply/qcom,pm8916-lbc.yaml | 128 +++++++++++++++++++++
1 file changed, 128 insertions(+)
diff --git a/Documentation/devicetree/bindings/power/supply/qcom,pm8916-lbc.yaml b/Documentation/devicetree/bindings/power/supply/qcom,pm8916-lbc.yaml
new file mode 100644
index 000000000000..cdf14e5ed119
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/qcom,pm8916-lbc.yaml
@@ -0,0 +1,128 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/supply/qcom,pm8916-lbc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Linear Battery Charger
+
+maintainers:
+ - Nikita Travkin <nikita@trvn.ru>
+
+description:
+ Linear Battery Charger hardware block, found in some Qualcomm PMICs
+ such as pm8916. Implements a simple, autonomous CC/CV charger.
+
+allOf:
+ - $ref: power-supply.yaml#
+
+properties:
+ compatible:
+ const: qcom,pm8916-lbc
+
+ reg:
+ items:
+ - description: Charger
+ - description: Battery
+ - description: USB
+ - description: MISC
+
+ reg-names:
+ items:
+ - const: chgr
+ - const: bat_if
+ - const: usb
+ - const: misc
+
+ interrupts:
+ items:
+ - description: Battery detection
+ - description: Fast charging
+ - description: Charging failed
+ - description: Charging done
+ - description: Battery present
+ - description: Battery temperature OK
+ - description: USB coarse detection
+ - description: USB IN valid
+ - description: Charger gone
+ - description: Overtemperature
+
+ interrupt-names:
+ items:
+ - const: vbat_det
+ - const: fast_chg
+ - const: chg_fail
+ - const: chg_done
+ - const: bat_pres
+ - const: temp_ok
+ - const: coarse_det
+ - const: usb_vbus
+ - const: chg_gone
+ - const: overtemp
+
+ qcom,fast-charge-safe-voltage:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 4000000
+ maximum: 4775000
+ description:
+ Maximum safe battery voltage in uV; May be pre-set by bootloader,
+ in which case, setting this will harmlessly fail.
+
+ qcom,fast-charge-safe-current:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 90000
+ maximum: 1440000
+ description:
+ Maximum safe battery charge current in uA; May be pre-set by
+ bootloader, in which case setting this will harmlessly fail.
+
+ monitored-battery: true
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - interrupt-names
+ - qcom,fast-charge-safe-voltage
+ - qcom,fast-charge-safe-current
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ pmic {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ charger@1000 {
+ compatible = "qcom,pm8916-lbc";
+ reg = <0x1000>, <0x1200>, <0x1300>, <0x1600>;
+ reg-names = "chgr", "bat_if", "usb", "misc";
+
+ interrupts = <0x0 0x10 0 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x10 5 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x10 6 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x10 7 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x12 0 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x12 1 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x13 0 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x13 1 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x13 2 IRQ_TYPE_EDGE_BOTH>,
+ <0x0 0x13 4 IRQ_TYPE_EDGE_BOTH>;
+ interrupt-names = "vbat_det",
+ "fast_chg",
+ "chg_fail",
+ "chg_done",
+ "bat_pres",
+ "temp_ok",
+ "coarse_det",
+ "usb_vbus",
+ "chg_gone",
+ "overtemp";
+ monitored-battery = <&battery>;
+
+ qcom,fast-charge-safe-current = <900000>;
+ qcom,fast-charge-safe-voltage = <4300000>;
+ };
+ };
--
2.41.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v2 2/4] dt-bindings: power: supply: Add pm8916 LBC
2023-07-31 17:06 ` [PATCH v2 2/4] dt-bindings: power: supply: Add pm8916 LBC Nikita Travkin
@ 2023-08-11 18:08 ` Rob Herring
0 siblings, 0 replies; 11+ messages in thread
From: Rob Herring @ 2023-08-11 18:08 UTC (permalink / raw)
To: Nikita Travkin
Cc: linux-arm-msm, Krzysztof Kozlowski, Sebastian Reichel, devicetree,
linux-pm, Rob Herring, Conor Dooley, linux-kernel
On Mon, 31 Jul 2023 22:06:25 +0500, Nikita Travkin wrote:
> Qualcomm Linear Battery Charger is a CC/CV charger block in PM8916 PMIC.
> Document it's DT binding.
>
> Signed-off-by: Nikita Travkin <nikita@trvn.ru>
> ---
> v2: List all interrupts (Conor)
> ---
> .../bindings/power/supply/qcom,pm8916-lbc.yaml | 128 +++++++++++++++++++++
> 1 file changed, 128 insertions(+)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 3/4] power: supply: Add pm8916 VM-BMS support
2023-07-31 17:06 [PATCH v2 0/4] Add pm8916 VM-BMS and LBC Nikita Travkin
2023-07-31 17:06 ` [PATCH v2 1/4] dt-bindings: power: supply: Add pm8916 VM-BMS Nikita Travkin
2023-07-31 17:06 ` [PATCH v2 2/4] dt-bindings: power: supply: Add pm8916 LBC Nikita Travkin
@ 2023-07-31 17:06 ` Nikita Travkin
2023-09-14 14:35 ` Sebastian Reichel
2023-07-31 17:06 ` [PATCH v2 4/4] power: supply: Add driver for pm8916 lbc Nikita Travkin
3 siblings, 1 reply; 11+ messages in thread
From: Nikita Travkin @ 2023-07-31 17:06 UTC (permalink / raw)
To: Sebastian Reichel, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-pm, devicetree, linux-kernel, linux-arm-msm, Nikita Travkin
This driver adds basic support for VM-BMS found in pm8916.
VM-BMS is a very basic fuel-gauge hardware block that is, sadly,
incapable of any gauging. The hardware supports measuring OCV in
sleep mode, where the battery is not in use, or measuring average
voltage over time when the device is active.
This driver implements basic value readout from this block.
Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---
v2: Get irq by name
---
drivers/power/supply/Kconfig | 11 ++
drivers/power/supply/Makefile | 1 +
drivers/power/supply/pm8916_bms_vm.c | 296 +++++++++++++++++++++++++++++++++++
3 files changed, 308 insertions(+)
diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 663a1c423806..e93a5a4d03e2 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -629,6 +629,17 @@ config CHARGER_QCOM_SMBB
documentation for more detail. The base name for this driver is
'pm8941_charger'.
+config BATTERY_PM8916_BMS_VM
+ tristate "Qualcomm PM8916 BMS-VM support"
+ depends on MFD_SPMI_PMIC || COMPILE_TEST
+ help
+ Say Y to add support for Voltage Mode BMS block found in some
+ Qualcomm PMICs such as PM8916. This hardware block provides
+ battery voltage monitoring for the system.
+
+ To compile this driver as module, choose M here: the
+ module will be called pm8916_bms_vm.
+
config CHARGER_BQ2415X
tristate "TI BQ2415x battery charger driver"
depends on I2C
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index a8a9fa6de1e9..fdf7916f80ed 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -84,6 +84,7 @@ obj-$(CONFIG_CHARGER_MP2629) += mp2629_charger.o
obj-$(CONFIG_CHARGER_MT6360) += mt6360_charger.o
obj-$(CONFIG_CHARGER_MT6370) += mt6370-charger.o
obj-$(CONFIG_CHARGER_QCOM_SMBB) += qcom_smbb.o
+obj-$(CONFIG_BATTERY_PM8916_BMS_VM) += pm8916_bms_vm.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/supply/pm8916_bms_vm.c b/drivers/power/supply/pm8916_bms_vm.c
new file mode 100644
index 000000000000..6cf00bf1c466
--- /dev/null
+++ b/drivers/power/supply/pm8916_bms_vm.c
@@ -0,0 +1,296 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2023, Nikita Travkin <nikita@trvn.ru>
+ */
+
+#include <linux/errno.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+
+#define PM8916_PERPH_TYPE 0x04
+#define PM8916_BMS_VM_TYPE 0x020D
+
+#define PM8916_SEC_ACCESS 0xD0
+#define PM8916_SEC_MAGIC 0xA5
+
+#define PM8916_BMS_VM_STATUS1 0x08
+#define PM8916_BMS_VM_FSM_STATE(x) (((x) & 0b00111000) >> 3)
+#define PM8916_BMS_VM_FSM_STATE_S2 0x2
+
+#define PM8916_BMS_VM_MODE_CTL 0x40
+#define PM8916_BMS_VM_MODE_FORCE_S3 (BIT(0) | BIT(1))
+#define PM8916_BMS_VM_MODE_NORMAL (BIT(1) | BIT(3))
+
+#define PM8916_BMS_VM_EN_CTL 0x46
+#define PM8916_BMS_ENABLED BIT(7)
+
+#define PM8916_BMS_VM_FIFO_LENGTH_CTL 0x47
+#define PM8916_BMS_VM_S1_SAMPLE_INTERVAL_CTL 0x55
+#define PM8916_BMS_VM_S2_SAMPLE_INTERVAL_CTL 0x56
+#define PM8916_BMS_VM_S3_S7_OCV_DATA0 0x6A
+#define PM8916_BMS_VM_BMS_FIFO_REG_0_LSB 0xC0
+
+/* Using only 1 fifo is broken in hardware */
+#define PM8916_BMS_VM_FIFO_COUNT 2 /* 2 .. 8 */
+
+#define PM8916_BMS_VM_S1_SAMPLE_INTERVAL 10
+#define PM8916_BMS_VM_S2_SAMPLE_INTERVAL 10
+
+struct pm8916_bms_vm_battery {
+ struct device *dev;
+ struct power_supply *battery;
+ struct power_supply_battery_info *info;
+ struct regmap *regmap;
+ unsigned int reg;
+ unsigned int last_ocv;
+ unsigned int vbat_now;
+};
+
+static int pm8916_bms_vm_battery_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct pm8916_bms_vm_battery *bat = power_supply_get_drvdata(psy);
+ struct power_supply_battery_info *info = bat->info;
+ int supplied;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_STATUS:
+ supplied = power_supply_am_i_supplied(psy);
+
+ if (supplied < 0 && supplied != -ENODEV)
+ return supplied;
+ else if (supplied && supplied != -ENODEV)
+ val->intval = POWER_SUPPLY_STATUS_CHARGING;
+ else
+ val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+ return 0;
+
+ case POWER_SUPPLY_PROP_HEALTH:
+ if (bat->vbat_now < info->voltage_min_design_uv)
+ val->intval = POWER_SUPPLY_HEALTH_DEAD;
+ else if (bat->vbat_now > info->voltage_max_design_uv)
+ val->intval = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
+ else
+ val->intval = POWER_SUPPLY_HEALTH_GOOD;
+ return 0;
+
+ case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+ val->intval = bat->vbat_now;
+ return 0;
+
+ case POWER_SUPPLY_PROP_VOLTAGE_BOOT:
+ /* Returning last known ocv value here - it changes after suspend. */
+ val->intval = bat->last_ocv;
+ return 0;
+
+ default:
+ return -EINVAL;
+ }
+}
+
+static enum power_supply_property pm8916_bms_vm_battery_properties[] = {
+ POWER_SUPPLY_PROP_STATUS,
+ POWER_SUPPLY_PROP_VOLTAGE_NOW,
+ POWER_SUPPLY_PROP_VOLTAGE_BOOT,
+ POWER_SUPPLY_PROP_HEALTH,
+};
+
+static irqreturn_t pm8916_bms_vm_fifo_update_done_irq(int irq, void *data)
+{
+ struct pm8916_bms_vm_battery *bat = data;
+ u16 vbat_data[PM8916_BMS_VM_FIFO_COUNT];
+ int ret;
+
+ ret = regmap_bulk_read(bat->regmap, bat->reg + PM8916_BMS_VM_BMS_FIFO_REG_0_LSB,
+ &vbat_data, PM8916_BMS_VM_FIFO_COUNT * 2);
+ if (ret)
+ return IRQ_HANDLED;
+
+ /*
+ * The VM-BMS hardware only collects voltage data and the software
+ * has to process it to calculate the OCV and SoC. Hardware provides
+ * up to 8 averaged measurements for software to take in account.
+ *
+ * Just use the last measured value for now to report the current
+ * battery voltage.
+ */
+ bat->vbat_now = vbat_data[PM8916_BMS_VM_FIFO_COUNT - 1] * 300;
+
+ power_supply_changed(bat->battery);
+
+ return IRQ_HANDLED;
+}
+
+static const struct power_supply_desc pm8916_bms_vm_battery_psy_desc = {
+ .name = "pm8916-bms-vm",
+ .type = POWER_SUPPLY_TYPE_BATTERY,
+ .properties = pm8916_bms_vm_battery_properties,
+ .num_properties = ARRAY_SIZE(pm8916_bms_vm_battery_properties),
+ .get_property = pm8916_bms_vm_battery_get_property,
+};
+
+static int pm8916_bms_vm_battery_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct pm8916_bms_vm_battery *bat;
+ struct power_supply_config psy_cfg = {};
+ int ret, irq;
+ unsigned int tmp;
+
+ bat = devm_kzalloc(dev, sizeof(*bat), GFP_KERNEL);
+ if (!bat)
+ return -ENOMEM;
+
+ bat->dev = dev;
+
+ bat->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+ if (!bat->regmap)
+ return -ENODEV;
+
+ of_property_read_u32(dev->of_node, "reg", &bat->reg);
+ if (bat->reg < 0)
+ return -EINVAL;
+
+ irq = platform_get_irq_byname(pdev, "fifo");
+ if (irq < 0)
+ return irq;
+
+ ret = devm_request_threaded_irq(dev, irq, NULL, pm8916_bms_vm_fifo_update_done_irq,
+ IRQF_ONESHOT, "pm8916_vm_bms", bat);
+ if (ret)
+ return ret;
+
+ ret = regmap_bulk_read(bat->regmap, bat->reg + PM8916_PERPH_TYPE, &tmp, 2);
+ if (ret)
+ goto comm_error;
+
+ if (tmp != PM8916_BMS_VM_TYPE)
+ return dev_err_probe(dev, -ENODEV, "Device reported wrong type: 0x%X\n", tmp);
+
+ ret = regmap_write(bat->regmap, bat->reg + PM8916_BMS_VM_S1_SAMPLE_INTERVAL_CTL,
+ PM8916_BMS_VM_S1_SAMPLE_INTERVAL);
+ if (ret)
+ goto comm_error;
+ ret = regmap_write(bat->regmap, bat->reg + PM8916_BMS_VM_S2_SAMPLE_INTERVAL_CTL,
+ PM8916_BMS_VM_S2_SAMPLE_INTERVAL);
+ if (ret)
+ goto comm_error;
+ ret = regmap_write(bat->regmap, bat->reg + PM8916_BMS_VM_FIFO_LENGTH_CTL,
+ PM8916_BMS_VM_FIFO_COUNT << 4 | PM8916_BMS_VM_FIFO_COUNT);
+ if (ret)
+ goto comm_error;
+ ret = regmap_write(bat->regmap,
+ bat->reg + PM8916_BMS_VM_EN_CTL, PM8916_BMS_ENABLED);
+ if (ret)
+ goto comm_error;
+
+ ret = regmap_bulk_read(bat->regmap,
+ bat->reg + PM8916_BMS_VM_S3_S7_OCV_DATA0, &tmp, 2);
+ if (ret)
+ goto comm_error;
+
+ bat->last_ocv = tmp * 300;
+ bat->vbat_now = bat->last_ocv;
+
+ psy_cfg.drv_data = bat;
+ psy_cfg.of_node = dev->of_node;
+
+ bat->battery = devm_power_supply_register(dev, &pm8916_bms_vm_battery_psy_desc, &psy_cfg);
+ if (IS_ERR(bat->battery))
+ return dev_err_probe(dev, PTR_ERR(bat->battery), "Unable to register battery\n");
+
+ ret = power_supply_get_battery_info(bat->battery, &bat->info);
+ if (ret)
+ return dev_err_probe(dev, ret, "Unable to get battery info\n");
+
+ platform_set_drvdata(pdev, bat);
+
+ return 0;
+
+comm_error:
+ return dev_err_probe(dev, ret, "Unable to communicate with device\n");
+}
+
+static int pm8916_bms_vm_battery_suspend(struct platform_device *pdev, pm_message_t state)
+{
+ struct pm8916_bms_vm_battery *bat = platform_get_drvdata(pdev);
+ int ret;
+
+ /*
+ * Due to a hardware quirk the FSM doesn't switch states normally.
+ * Instead we unlock the debug registers and force S3 (Measure OCV/Sleep)
+ * mode every time we suspend.
+ */
+
+ ret = regmap_write(bat->regmap,
+ bat->reg + PM8916_SEC_ACCESS, PM8916_SEC_MAGIC);
+ if (ret)
+ goto error;
+ ret = regmap_write(bat->regmap,
+ bat->reg + PM8916_BMS_VM_MODE_CTL, PM8916_BMS_VM_MODE_FORCE_S3);
+ if (ret)
+ goto error;
+
+ return 0;
+
+error:
+ dev_err(bat->dev, "Failed to force S3 mode: %pe\n", ERR_PTR(ret));
+ return ret;
+}
+
+static int pm8916_bms_vm_battery_resume(struct platform_device *pdev)
+{
+ struct pm8916_bms_vm_battery *bat = platform_get_drvdata(pdev);
+ int ret;
+ unsigned int tmp;
+
+ ret = regmap_bulk_read(bat->regmap,
+ bat->reg + PM8916_BMS_VM_S3_S7_OCV_DATA0, &tmp, 2);
+
+ bat->last_ocv = tmp * 300;
+
+ ret = regmap_write(bat->regmap,
+ bat->reg + PM8916_SEC_ACCESS, PM8916_SEC_MAGIC);
+ if (ret)
+ goto error;
+ ret = regmap_write(bat->regmap,
+ bat->reg + PM8916_BMS_VM_MODE_CTL, PM8916_BMS_VM_MODE_NORMAL);
+ if (ret)
+ goto error;
+
+ return 0;
+
+error:
+ dev_err(bat->dev, "Failed to return normal mode: %pe\n", ERR_PTR(ret));
+ return ret;
+}
+
+static const struct of_device_id pm8916_bms_vm_battery_of_match[] = {
+ { .compatible = "qcom,pm8916-bms-vm", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, pm8916_bms_vm_battery_of_match);
+
+static struct platform_driver pm8916_bms_vm_battery_driver = {
+ .driver = {
+ .name = "pm8916-bms-vm",
+ .of_match_table = of_match_ptr(pm8916_bms_vm_battery_of_match),
+ },
+ .probe = pm8916_bms_vm_battery_probe,
+ .suspend = pm8916_bms_vm_battery_suspend,
+ .resume = pm8916_bms_vm_battery_resume,
+};
+module_platform_driver(pm8916_bms_vm_battery_driver);
+
+MODULE_DESCRIPTION("pm8916 BMS-VM driver");
+MODULE_AUTHOR("Nikita Travkin <nikita@trvn.ru>");
+MODULE_LICENSE("GPL");
--
2.41.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v2 3/4] power: supply: Add pm8916 VM-BMS support
2023-07-31 17:06 ` [PATCH v2 3/4] power: supply: Add pm8916 VM-BMS support Nikita Travkin
@ 2023-09-14 14:35 ` Sebastian Reichel
2023-09-14 14:58 ` Nikita Travkin
0 siblings, 1 reply; 11+ messages in thread
From: Sebastian Reichel @ 2023-09-14 14:35 UTC (permalink / raw)
To: Nikita Travkin
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-pm,
devicetree, linux-kernel, linux-arm-msm
[-- Attachment #1: Type: text/plain, Size: 12454 bytes --]
Hi,
On Mon, Jul 31, 2023 at 10:06:26PM +0500, Nikita Travkin wrote:
> This driver adds basic support for VM-BMS found in pm8916.
>
> VM-BMS is a very basic fuel-gauge hardware block that is, sadly,
> incapable of any gauging. The hardware supports measuring OCV in
> sleep mode, where the battery is not in use, or measuring average
> voltage over time when the device is active.
>
> This driver implements basic value readout from this block.
>
> Signed-off-by: Nikita Travkin <nikita@trvn.ru>
> ---
> v2: Get irq by name
> ---
Thanks for the patch. I have a few small change requests.
> drivers/power/supply/Kconfig | 11 ++
> drivers/power/supply/Makefile | 1 +
> drivers/power/supply/pm8916_bms_vm.c | 296 +++++++++++++++++++++++++++++++++++
> 3 files changed, 308 insertions(+)
>
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index 663a1c423806..e93a5a4d03e2 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -629,6 +629,17 @@ config CHARGER_QCOM_SMBB
> documentation for more detail. The base name for this driver is
> 'pm8941_charger'.
>
> +config BATTERY_PM8916_BMS_VM
> + tristate "Qualcomm PM8916 BMS-VM support"
> + depends on MFD_SPMI_PMIC || COMPILE_TEST
> + help
> + Say Y to add support for Voltage Mode BMS block found in some
> + Qualcomm PMICs such as PM8916. This hardware block provides
> + battery voltage monitoring for the system.
> +
> + To compile this driver as module, choose M here: the
> + module will be called pm8916_bms_vm.
> +
> config CHARGER_BQ2415X
> tristate "TI BQ2415x battery charger driver"
> depends on I2C
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index a8a9fa6de1e9..fdf7916f80ed 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -84,6 +84,7 @@ obj-$(CONFIG_CHARGER_MP2629) += mp2629_charger.o
> obj-$(CONFIG_CHARGER_MT6360) += mt6360_charger.o
> obj-$(CONFIG_CHARGER_MT6370) += mt6370-charger.o
> obj-$(CONFIG_CHARGER_QCOM_SMBB) += qcom_smbb.o
> +obj-$(CONFIG_BATTERY_PM8916_BMS_VM) += pm8916_bms_vm.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/supply/pm8916_bms_vm.c b/drivers/power/supply/pm8916_bms_vm.c
> new file mode 100644
> index 000000000000..6cf00bf1c466
> --- /dev/null
> +++ b/drivers/power/supply/pm8916_bms_vm.c
> @@ -0,0 +1,296 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2023, Nikita Travkin <nikita@trvn.ru>
> + */
> +
> +#include <linux/errno.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
You should be able to remove the of headers after my proposed
changes.
> +#include <linux/platform_device.h>
> +#include <linux/power_supply.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +
> +#define PM8916_PERPH_TYPE 0x04
> +#define PM8916_BMS_VM_TYPE 0x020D
> +
> +#define PM8916_SEC_ACCESS 0xD0
> +#define PM8916_SEC_MAGIC 0xA5
> +
> +#define PM8916_BMS_VM_STATUS1 0x08
> +#define PM8916_BMS_VM_FSM_STATE(x) (((x) & 0b00111000) >> 3)
> +#define PM8916_BMS_VM_FSM_STATE_S2 0x2
> +
> +#define PM8916_BMS_VM_MODE_CTL 0x40
> +#define PM8916_BMS_VM_MODE_FORCE_S3 (BIT(0) | BIT(1))
> +#define PM8916_BMS_VM_MODE_NORMAL (BIT(1) | BIT(3))
> +
> +#define PM8916_BMS_VM_EN_CTL 0x46
> +#define PM8916_BMS_ENABLED BIT(7)
> +
> +#define PM8916_BMS_VM_FIFO_LENGTH_CTL 0x47
> +#define PM8916_BMS_VM_S1_SAMPLE_INTERVAL_CTL 0x55
> +#define PM8916_BMS_VM_S2_SAMPLE_INTERVAL_CTL 0x56
> +#define PM8916_BMS_VM_S3_S7_OCV_DATA0 0x6A
> +#define PM8916_BMS_VM_BMS_FIFO_REG_0_LSB 0xC0
> +
> +/* Using only 1 fifo is broken in hardware */
> +#define PM8916_BMS_VM_FIFO_COUNT 2 /* 2 .. 8 */
> +
> +#define PM8916_BMS_VM_S1_SAMPLE_INTERVAL 10
> +#define PM8916_BMS_VM_S2_SAMPLE_INTERVAL 10
> +
> +struct pm8916_bms_vm_battery {
> + struct device *dev;
> + struct power_supply *battery;
> + struct power_supply_battery_info *info;
> + struct regmap *regmap;
> + unsigned int reg;
> + unsigned int last_ocv;
> + unsigned int vbat_now;
> +};
> +
> +static int pm8916_bms_vm_battery_get_property(struct power_supply *psy,
> + enum power_supply_property psp,
> + union power_supply_propval *val)
> +{
> + struct pm8916_bms_vm_battery *bat = power_supply_get_drvdata(psy);
> + struct power_supply_battery_info *info = bat->info;
> + int supplied;
> +
> + switch (psp) {
> + case POWER_SUPPLY_PROP_STATUS:
> + supplied = power_supply_am_i_supplied(psy);
> +
> + if (supplied < 0 && supplied != -ENODEV)
> + return supplied;
> + else if (supplied && supplied != -ENODEV)
> + val->intval = POWER_SUPPLY_STATUS_CHARGING;
> + else
> + val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
> + return 0;
> +
> + case POWER_SUPPLY_PROP_HEALTH:
> + if (bat->vbat_now < info->voltage_min_design_uv)
> + val->intval = POWER_SUPPLY_HEALTH_DEAD;
> + else if (bat->vbat_now > info->voltage_max_design_uv)
> + val->intval = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
> + else
> + val->intval = POWER_SUPPLY_HEALTH_GOOD;
> + return 0;
> +
> + case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> + val->intval = bat->vbat_now;
> + return 0;
> +
> + case POWER_SUPPLY_PROP_VOLTAGE_BOOT:
> + /* Returning last known ocv value here - it changes after suspend. */
> + val->intval = bat->last_ocv;
> + return 0;
Returning OCV from last suspend is not the same as VOLTAGE_BOOT. How
about exposing POWER_SUPPLY_PROP_VOLTAGE_OCV and returning -ENODATA
if the value is older than 180 seconds?
> +
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static enum power_supply_property pm8916_bms_vm_battery_properties[] = {
> + POWER_SUPPLY_PROP_STATUS,
> + POWER_SUPPLY_PROP_VOLTAGE_NOW,
> + POWER_SUPPLY_PROP_VOLTAGE_BOOT,
> + POWER_SUPPLY_PROP_HEALTH,
> +};
> +
> +static irqreturn_t pm8916_bms_vm_fifo_update_done_irq(int irq, void *data)
> +{
> + struct pm8916_bms_vm_battery *bat = data;
> + u16 vbat_data[PM8916_BMS_VM_FIFO_COUNT];
> + int ret;
> +
> + ret = regmap_bulk_read(bat->regmap, bat->reg + PM8916_BMS_VM_BMS_FIFO_REG_0_LSB,
> + &vbat_data, PM8916_BMS_VM_FIFO_COUNT * 2);
> + if (ret)
> + return IRQ_HANDLED;
> +
> + /*
> + * The VM-BMS hardware only collects voltage data and the software
> + * has to process it to calculate the OCV and SoC. Hardware provides
> + * up to 8 averaged measurements for software to take in account.
> + *
> + * Just use the last measured value for now to report the current
> + * battery voltage.
> + */
> + bat->vbat_now = vbat_data[PM8916_BMS_VM_FIFO_COUNT - 1] * 300;
> +
> + power_supply_changed(bat->battery);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static const struct power_supply_desc pm8916_bms_vm_battery_psy_desc = {
> + .name = "pm8916-bms-vm",
> + .type = POWER_SUPPLY_TYPE_BATTERY,
> + .properties = pm8916_bms_vm_battery_properties,
> + .num_properties = ARRAY_SIZE(pm8916_bms_vm_battery_properties),
> + .get_property = pm8916_bms_vm_battery_get_property,
> +};
> +
> +static int pm8916_bms_vm_battery_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct pm8916_bms_vm_battery *bat;
> + struct power_supply_config psy_cfg = {};
> + int ret, irq;
> + unsigned int tmp;
> +
> + bat = devm_kzalloc(dev, sizeof(*bat), GFP_KERNEL);
> + if (!bat)
> + return -ENOMEM;
> +
> + bat->dev = dev;
> +
> + bat->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> + if (!bat->regmap)
> + return -ENODEV;
> +
> + of_property_read_u32(dev->of_node, "reg", &bat->reg);
device_property_read_u32(...)
> + if (bat->reg < 0)
> + return -EINVAL;
> +
> + irq = platform_get_irq_byname(pdev, "fifo");
> + if (irq < 0)
> + return irq;
> +
> + ret = devm_request_threaded_irq(dev, irq, NULL, pm8916_bms_vm_fifo_update_done_irq,
> + IRQF_ONESHOT, "pm8916_vm_bms", bat);
> + if (ret)
> + return ret;
> +
> + ret = regmap_bulk_read(bat->regmap, bat->reg + PM8916_PERPH_TYPE, &tmp, 2);
> + if (ret)
> + goto comm_error;
> +
> + if (tmp != PM8916_BMS_VM_TYPE)
> + return dev_err_probe(dev, -ENODEV, "Device reported wrong type: 0x%X\n", tmp);
> +
> + ret = regmap_write(bat->regmap, bat->reg + PM8916_BMS_VM_S1_SAMPLE_INTERVAL_CTL,
> + PM8916_BMS_VM_S1_SAMPLE_INTERVAL);
> + if (ret)
> + goto comm_error;
> + ret = regmap_write(bat->regmap, bat->reg + PM8916_BMS_VM_S2_SAMPLE_INTERVAL_CTL,
> + PM8916_BMS_VM_S2_SAMPLE_INTERVAL);
> + if (ret)
> + goto comm_error;
> + ret = regmap_write(bat->regmap, bat->reg + PM8916_BMS_VM_FIFO_LENGTH_CTL,
> + PM8916_BMS_VM_FIFO_COUNT << 4 | PM8916_BMS_VM_FIFO_COUNT);
> + if (ret)
> + goto comm_error;
> + ret = regmap_write(bat->regmap,
> + bat->reg + PM8916_BMS_VM_EN_CTL, PM8916_BMS_ENABLED);
> + if (ret)
> + goto comm_error;
> +
> + ret = regmap_bulk_read(bat->regmap,
> + bat->reg + PM8916_BMS_VM_S3_S7_OCV_DATA0, &tmp, 2);
> + if (ret)
> + goto comm_error;
> +
> + bat->last_ocv = tmp * 300;
> + bat->vbat_now = bat->last_ocv;
> +
> + psy_cfg.drv_data = bat;
> + psy_cfg.of_node = dev->of_node;
> +
> + bat->battery = devm_power_supply_register(dev, &pm8916_bms_vm_battery_psy_desc, &psy_cfg);
> + if (IS_ERR(bat->battery))
> + return dev_err_probe(dev, PTR_ERR(bat->battery), "Unable to register battery\n");
> +
> + ret = power_supply_get_battery_info(bat->battery, &bat->info);
> + if (ret)
> + return dev_err_probe(dev, ret, "Unable to get battery info\n");
> +
> + platform_set_drvdata(pdev, bat);
> +
> + return 0;
> +
> +comm_error:
> + return dev_err_probe(dev, ret, "Unable to communicate with device\n");
> +}
> +
> +static int pm8916_bms_vm_battery_suspend(struct platform_device *pdev, pm_message_t state)
> +{
> + struct pm8916_bms_vm_battery *bat = platform_get_drvdata(pdev);
> + int ret;
> +
> + /*
> + * Due to a hardware quirk the FSM doesn't switch states normally.
> + * Instead we unlock the debug registers and force S3 (Measure OCV/Sleep)
> + * mode every time we suspend.
> + */
> +
> + ret = regmap_write(bat->regmap,
> + bat->reg + PM8916_SEC_ACCESS, PM8916_SEC_MAGIC);
> + if (ret)
> + goto error;
> + ret = regmap_write(bat->regmap,
> + bat->reg + PM8916_BMS_VM_MODE_CTL, PM8916_BMS_VM_MODE_FORCE_S3);
> + if (ret)
> + goto error;
> +
> + return 0;
> +
> +error:
> + dev_err(bat->dev, "Failed to force S3 mode: %pe\n", ERR_PTR(ret));
> + return ret;
> +}
> +
> +static int pm8916_bms_vm_battery_resume(struct platform_device *pdev)
> +{
> + struct pm8916_bms_vm_battery *bat = platform_get_drvdata(pdev);
> + int ret;
> + unsigned int tmp;
> +
> + ret = regmap_bulk_read(bat->regmap,
> + bat->reg + PM8916_BMS_VM_S3_S7_OCV_DATA0, &tmp, 2);
> +
> + bat->last_ocv = tmp * 300;
> +
> + ret = regmap_write(bat->regmap,
> + bat->reg + PM8916_SEC_ACCESS, PM8916_SEC_MAGIC);
> + if (ret)
> + goto error;
> + ret = regmap_write(bat->regmap,
> + bat->reg + PM8916_BMS_VM_MODE_CTL, PM8916_BMS_VM_MODE_NORMAL);
> + if (ret)
> + goto error;
> +
> + return 0;
> +
> +error:
> + dev_err(bat->dev, "Failed to return normal mode: %pe\n", ERR_PTR(ret));
> + return ret;
> +}
> +
> +static const struct of_device_id pm8916_bms_vm_battery_of_match[] = {
> + { .compatible = "qcom,pm8916-bms-vm", },
> + { },
{}
(i.e. remove space and trailing , for terminator entry)
> +};
> +MODULE_DEVICE_TABLE(of, pm8916_bms_vm_battery_of_match);
> +
> +static struct platform_driver pm8916_bms_vm_battery_driver = {
> + .driver = {
> + .name = "pm8916-bms-vm",
> + .of_match_table = of_match_ptr(pm8916_bms_vm_battery_of_match),
remove of_match_ptr().
> + },
> + .probe = pm8916_bms_vm_battery_probe,
> + .suspend = pm8916_bms_vm_battery_suspend,
> + .resume = pm8916_bms_vm_battery_resume,
> +};
> +module_platform_driver(pm8916_bms_vm_battery_driver);
> +
> +MODULE_DESCRIPTION("pm8916 BMS-VM driver");
> +MODULE_AUTHOR("Nikita Travkin <nikita@trvn.ru>");
> +MODULE_LICENSE("GPL");
Otherwise LGTM,
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2 3/4] power: supply: Add pm8916 VM-BMS support
2023-09-14 14:35 ` Sebastian Reichel
@ 2023-09-14 14:58 ` Nikita Travkin
0 siblings, 0 replies; 11+ messages in thread
From: Nikita Travkin @ 2023-09-14 14:58 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-pm,
devicetree, linux-kernel, linux-arm-msm
Sebastian Reichel писал(а) 14.09.2023 19:35:
> Hi,
>
> On Mon, Jul 31, 2023 at 10:06:26PM +0500, Nikita Travkin wrote:
>> This driver adds basic support for VM-BMS found in pm8916.
>>
>> VM-BMS is a very basic fuel-gauge hardware block that is, sadly,
>> incapable of any gauging. The hardware supports measuring OCV in
>> sleep mode, where the battery is not in use, or measuring average
>> voltage over time when the device is active.
>>
>> This driver implements basic value readout from this block.
>>
>> Signed-off-by: Nikita Travkin <nikita@trvn.ru>
>> ---
>> v2: Get irq by name
>> ---
>
> Thanks for the patch. I have a few small change requests.
>
>> drivers/power/supply/Kconfig | 11 ++
>> drivers/power/supply/Makefile | 1 +
>> drivers/power/supply/pm8916_bms_vm.c | 296 +++++++++++++++++++++++++++++++++++
>> 3 files changed, 308 insertions(+)
>>
>> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
>> index 663a1c423806..e93a5a4d03e2 100644
>> --- a/drivers/power/supply/Kconfig
>> +++ b/drivers/power/supply/Kconfig
>> @@ -629,6 +629,17 @@ config CHARGER_QCOM_SMBB
>> documentation for more detail. The base name for this driver is
>> 'pm8941_charger'.
>>
>> +config BATTERY_PM8916_BMS_VM
>> + tristate "Qualcomm PM8916 BMS-VM support"
>> + depends on MFD_SPMI_PMIC || COMPILE_TEST
>> + help
>> + Say Y to add support for Voltage Mode BMS block found in some
>> + Qualcomm PMICs such as PM8916. This hardware block provides
>> + battery voltage monitoring for the system.
>> +
>> + To compile this driver as module, choose M here: the
>> + module will be called pm8916_bms_vm.
>> +
>> config CHARGER_BQ2415X
>> tristate "TI BQ2415x battery charger driver"
>> depends on I2C
>> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
>> index a8a9fa6de1e9..fdf7916f80ed 100644
>> --- a/drivers/power/supply/Makefile
>> +++ b/drivers/power/supply/Makefile
>> @@ -84,6 +84,7 @@ obj-$(CONFIG_CHARGER_MP2629) += mp2629_charger.o
>> obj-$(CONFIG_CHARGER_MT6360) += mt6360_charger.o
>> obj-$(CONFIG_CHARGER_MT6370) += mt6370-charger.o
>> obj-$(CONFIG_CHARGER_QCOM_SMBB) += qcom_smbb.o
>> +obj-$(CONFIG_BATTERY_PM8916_BMS_VM) += pm8916_bms_vm.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/supply/pm8916_bms_vm.c b/drivers/power/supply/pm8916_bms_vm.c
>> new file mode 100644
>> index 000000000000..6cf00bf1c466
>> --- /dev/null
>> +++ b/drivers/power/supply/pm8916_bms_vm.c
>> @@ -0,0 +1,296 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2023, Nikita Travkin <nikita@trvn.ru>
>> + */
>> +
>> +#include <linux/errno.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>
> You should be able to remove the of headers after my proposed
> changes.
>
Will switch and drop this.
>> +#include <linux/platform_device.h>
>> +#include <linux/power_supply.h>
>> +#include <linux/property.h>
>> +#include <linux/regmap.h>
>> +#include <linux/slab.h>
>> +#include <linux/delay.h>
>> +#include <linux/interrupt.h>
>> +
>> +#define PM8916_PERPH_TYPE 0x04
>> +#define PM8916_BMS_VM_TYPE 0x020D
>> +
>> +#define PM8916_SEC_ACCESS 0xD0
>> +#define PM8916_SEC_MAGIC 0xA5
>> +
>> +#define PM8916_BMS_VM_STATUS1 0x08
>> +#define PM8916_BMS_VM_FSM_STATE(x) (((x) & 0b00111000) >> 3)
>> +#define PM8916_BMS_VM_FSM_STATE_S2 0x2
>> +
>> +#define PM8916_BMS_VM_MODE_CTL 0x40
>> +#define PM8916_BMS_VM_MODE_FORCE_S3 (BIT(0) | BIT(1))
>> +#define PM8916_BMS_VM_MODE_NORMAL (BIT(1) | BIT(3))
>> +
>> +#define PM8916_BMS_VM_EN_CTL 0x46
>> +#define PM8916_BMS_ENABLED BIT(7)
>> +
>> +#define PM8916_BMS_VM_FIFO_LENGTH_CTL 0x47
>> +#define PM8916_BMS_VM_S1_SAMPLE_INTERVAL_CTL 0x55
>> +#define PM8916_BMS_VM_S2_SAMPLE_INTERVAL_CTL 0x56
>> +#define PM8916_BMS_VM_S3_S7_OCV_DATA0 0x6A
>> +#define PM8916_BMS_VM_BMS_FIFO_REG_0_LSB 0xC0
>> +
>> +/* Using only 1 fifo is broken in hardware */
>> +#define PM8916_BMS_VM_FIFO_COUNT 2 /* 2 .. 8 */
>> +
>> +#define PM8916_BMS_VM_S1_SAMPLE_INTERVAL 10
>> +#define PM8916_BMS_VM_S2_SAMPLE_INTERVAL 10
>> +
>> +struct pm8916_bms_vm_battery {
>> + struct device *dev;
>> + struct power_supply *battery;
>> + struct power_supply_battery_info *info;
>> + struct regmap *regmap;
>> + unsigned int reg;
>> + unsigned int last_ocv;
>> + unsigned int vbat_now;
>> +};
>> +
>> +static int pm8916_bms_vm_battery_get_property(struct power_supply *psy,
>> + enum power_supply_property psp,
>> + union power_supply_propval *val)
>> +{
>> + struct pm8916_bms_vm_battery *bat = power_supply_get_drvdata(psy);
>> + struct power_supply_battery_info *info = bat->info;
>> + int supplied;
>> +
>> + switch (psp) {
>> + case POWER_SUPPLY_PROP_STATUS:
>> + supplied = power_supply_am_i_supplied(psy);
>> +
>> + if (supplied < 0 && supplied != -ENODEV)
>> + return supplied;
>> + else if (supplied && supplied != -ENODEV)
>> + val->intval = POWER_SUPPLY_STATUS_CHARGING;
>> + else
>> + val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
>> + return 0;
>> +
>> + case POWER_SUPPLY_PROP_HEALTH:
>> + if (bat->vbat_now < info->voltage_min_design_uv)
>> + val->intval = POWER_SUPPLY_HEALTH_DEAD;
>> + else if (bat->vbat_now > info->voltage_max_design_uv)
>> + val->intval = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
>> + else
>> + val->intval = POWER_SUPPLY_HEALTH_GOOD;
>> + return 0;
>> +
>> + case POWER_SUPPLY_PROP_VOLTAGE_NOW:
>> + val->intval = bat->vbat_now;
>> + return 0;
>> +
>> + case POWER_SUPPLY_PROP_VOLTAGE_BOOT:
>> + /* Returning last known ocv value here - it changes after suspend. */
>> + val->intval = bat->last_ocv;
>> + return 0;
>
> Returning OCV from last suspend is not the same as VOLTAGE_BOOT. How
> about exposing POWER_SUPPLY_PROP_VOLTAGE_OCV and returning -ENODATA
> if the value is older than 180 seconds?
>
Hm, indeed, I didn't think of this as an option... Will implement
that instead.
Thanks,
Nikita
>> +
>> + default:
>> + return -EINVAL;
>> + }
>> +}
>> +
>> +static enum power_supply_property pm8916_bms_vm_battery_properties[] = {
>> + POWER_SUPPLY_PROP_STATUS,
>> + POWER_SUPPLY_PROP_VOLTAGE_NOW,
>> + POWER_SUPPLY_PROP_VOLTAGE_BOOT,
>> + POWER_SUPPLY_PROP_HEALTH,
>> +};
>> +
>> +static irqreturn_t pm8916_bms_vm_fifo_update_done_irq(int irq, void *data)
>> +{
>> + struct pm8916_bms_vm_battery *bat = data;
>> + u16 vbat_data[PM8916_BMS_VM_FIFO_COUNT];
>> + int ret;
>> +
>> + ret = regmap_bulk_read(bat->regmap, bat->reg + PM8916_BMS_VM_BMS_FIFO_REG_0_LSB,
>> + &vbat_data, PM8916_BMS_VM_FIFO_COUNT * 2);
>> + if (ret)
>> + return IRQ_HANDLED;
>> +
>> + /*
>> + * The VM-BMS hardware only collects voltage data and the software
>> + * has to process it to calculate the OCV and SoC. Hardware provides
>> + * up to 8 averaged measurements for software to take in account.
>> + *
>> + * Just use the last measured value for now to report the current
>> + * battery voltage.
>> + */
>> + bat->vbat_now = vbat_data[PM8916_BMS_VM_FIFO_COUNT - 1] * 300;
>> +
>> + power_supply_changed(bat->battery);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static const struct power_supply_desc pm8916_bms_vm_battery_psy_desc = {
>> + .name = "pm8916-bms-vm",
>> + .type = POWER_SUPPLY_TYPE_BATTERY,
>> + .properties = pm8916_bms_vm_battery_properties,
>> + .num_properties = ARRAY_SIZE(pm8916_bms_vm_battery_properties),
>> + .get_property = pm8916_bms_vm_battery_get_property,
>> +};
>> +
>> +static int pm8916_bms_vm_battery_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct pm8916_bms_vm_battery *bat;
>> + struct power_supply_config psy_cfg = {};
>> + int ret, irq;
>> + unsigned int tmp;
>> +
>> + bat = devm_kzalloc(dev, sizeof(*bat), GFP_KERNEL);
>> + if (!bat)
>> + return -ENOMEM;
>> +
>> + bat->dev = dev;
>> +
>> + bat->regmap = dev_get_regmap(pdev->dev.parent, NULL);
>> + if (!bat->regmap)
>> + return -ENODEV;
>> +
>> + of_property_read_u32(dev->of_node, "reg", &bat->reg);
>
> device_property_read_u32(...)
>
>> + if (bat->reg < 0)
>> + return -EINVAL;
>> +
>> + irq = platform_get_irq_byname(pdev, "fifo");
>> + if (irq < 0)
>> + return irq;
>> +
>> + ret = devm_request_threaded_irq(dev, irq, NULL, pm8916_bms_vm_fifo_update_done_irq,
>> + IRQF_ONESHOT, "pm8916_vm_bms", bat);
>> + if (ret)
>> + return ret;
>> +
>> + ret = regmap_bulk_read(bat->regmap, bat->reg + PM8916_PERPH_TYPE, &tmp, 2);
>> + if (ret)
>> + goto comm_error;
>> +
>> + if (tmp != PM8916_BMS_VM_TYPE)
>> + return dev_err_probe(dev, -ENODEV, "Device reported wrong type: 0x%X\n", tmp);
>> +
>> + ret = regmap_write(bat->regmap, bat->reg + PM8916_BMS_VM_S1_SAMPLE_INTERVAL_CTL,
>> + PM8916_BMS_VM_S1_SAMPLE_INTERVAL);
>> + if (ret)
>> + goto comm_error;
>> + ret = regmap_write(bat->regmap, bat->reg + PM8916_BMS_VM_S2_SAMPLE_INTERVAL_CTL,
>> + PM8916_BMS_VM_S2_SAMPLE_INTERVAL);
>> + if (ret)
>> + goto comm_error;
>> + ret = regmap_write(bat->regmap, bat->reg + PM8916_BMS_VM_FIFO_LENGTH_CTL,
>> + PM8916_BMS_VM_FIFO_COUNT << 4 | PM8916_BMS_VM_FIFO_COUNT);
>> + if (ret)
>> + goto comm_error;
>> + ret = regmap_write(bat->regmap,
>> + bat->reg + PM8916_BMS_VM_EN_CTL, PM8916_BMS_ENABLED);
>> + if (ret)
>> + goto comm_error;
>> +
>> + ret = regmap_bulk_read(bat->regmap,
>> + bat->reg + PM8916_BMS_VM_S3_S7_OCV_DATA0, &tmp, 2);
>> + if (ret)
>> + goto comm_error;
>> +
>> + bat->last_ocv = tmp * 300;
>> + bat->vbat_now = bat->last_ocv;
>> +
>> + psy_cfg.drv_data = bat;
>> + psy_cfg.of_node = dev->of_node;
>> +
>> + bat->battery = devm_power_supply_register(dev, &pm8916_bms_vm_battery_psy_desc, &psy_cfg);
>> + if (IS_ERR(bat->battery))
>> + return dev_err_probe(dev, PTR_ERR(bat->battery), "Unable to register battery\n");
>> +
>> + ret = power_supply_get_battery_info(bat->battery, &bat->info);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "Unable to get battery info\n");
>> +
>> + platform_set_drvdata(pdev, bat);
>> +
>> + return 0;
>> +
>> +comm_error:
>> + return dev_err_probe(dev, ret, "Unable to communicate with device\n");
>> +}
>> +
>> +static int pm8916_bms_vm_battery_suspend(struct platform_device *pdev, pm_message_t state)
>> +{
>> + struct pm8916_bms_vm_battery *bat = platform_get_drvdata(pdev);
>> + int ret;
>> +
>> + /*
>> + * Due to a hardware quirk the FSM doesn't switch states normally.
>> + * Instead we unlock the debug registers and force S3 (Measure OCV/Sleep)
>> + * mode every time we suspend.
>> + */
>> +
>> + ret = regmap_write(bat->regmap,
>> + bat->reg + PM8916_SEC_ACCESS, PM8916_SEC_MAGIC);
>> + if (ret)
>> + goto error;
>> + ret = regmap_write(bat->regmap,
>> + bat->reg + PM8916_BMS_VM_MODE_CTL, PM8916_BMS_VM_MODE_FORCE_S3);
>> + if (ret)
>> + goto error;
>> +
>> + return 0;
>> +
>> +error:
>> + dev_err(bat->dev, "Failed to force S3 mode: %pe\n", ERR_PTR(ret));
>> + return ret;
>> +}
>> +
>> +static int pm8916_bms_vm_battery_resume(struct platform_device *pdev)
>> +{
>> + struct pm8916_bms_vm_battery *bat = platform_get_drvdata(pdev);
>> + int ret;
>> + unsigned int tmp;
>> +
>> + ret = regmap_bulk_read(bat->regmap,
>> + bat->reg + PM8916_BMS_VM_S3_S7_OCV_DATA0, &tmp, 2);
>> +
>> + bat->last_ocv = tmp * 300;
>> +
>> + ret = regmap_write(bat->regmap,
>> + bat->reg + PM8916_SEC_ACCESS, PM8916_SEC_MAGIC);
>> + if (ret)
>> + goto error;
>> + ret = regmap_write(bat->regmap,
>> + bat->reg + PM8916_BMS_VM_MODE_CTL, PM8916_BMS_VM_MODE_NORMAL);
>> + if (ret)
>> + goto error;
>> +
>> + return 0;
>> +
>> +error:
>> + dev_err(bat->dev, "Failed to return normal mode: %pe\n", ERR_PTR(ret));
>> + return ret;
>> +}
>> +
>> +static const struct of_device_id pm8916_bms_vm_battery_of_match[] = {
>> + { .compatible = "qcom,pm8916-bms-vm", },
>> + { },
>
> {}
>
> (i.e. remove space and trailing , for terminator entry)
>
>> +};
>> +MODULE_DEVICE_TABLE(of, pm8916_bms_vm_battery_of_match);
>> +
>> +static struct platform_driver pm8916_bms_vm_battery_driver = {
>> + .driver = {
>> + .name = "pm8916-bms-vm",
>> + .of_match_table = of_match_ptr(pm8916_bms_vm_battery_of_match),
>
> remove of_match_ptr().
>
>> + },
>> + .probe = pm8916_bms_vm_battery_probe,
>> + .suspend = pm8916_bms_vm_battery_suspend,
>> + .resume = pm8916_bms_vm_battery_resume,
>> +};
>> +module_platform_driver(pm8916_bms_vm_battery_driver);
>> +
>> +MODULE_DESCRIPTION("pm8916 BMS-VM driver");
>> +MODULE_AUTHOR("Nikita Travkin <nikita@trvn.ru>");
>> +MODULE_LICENSE("GPL");
>
> Otherwise LGTM,
>
> -- Sebastian
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 4/4] power: supply: Add driver for pm8916 lbc
2023-07-31 17:06 [PATCH v2 0/4] Add pm8916 VM-BMS and LBC Nikita Travkin
` (2 preceding siblings ...)
2023-07-31 17:06 ` [PATCH v2 3/4] power: supply: Add pm8916 VM-BMS support Nikita Travkin
@ 2023-07-31 17:06 ` Nikita Travkin
2023-09-14 14:25 ` Sebastian Reichel
3 siblings, 1 reply; 11+ messages in thread
From: Nikita Travkin @ 2023-07-31 17:06 UTC (permalink / raw)
To: Sebastian Reichel, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-pm, devicetree, linux-kernel, linux-arm-msm, Nikita Travkin
pm8916 LBC is a Linear Battery Charger hardware block in pm8916 PMIC.
This block implements simple CC/CV charging for Li-Po batteries.
The hardware has internal state machine to switch between modes and
works mostly autonomously, only needing the limits and targets to be
set to operate.
This driver allows setting limits and enabling the LBC block, monitoring
it's state.
Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---
v2: Fix missed warnings, get irq by name
---
drivers/power/supply/Kconfig | 11 ++
drivers/power/supply/Makefile | 1 +
drivers/power/supply/pm8916_lbc.c | 383 ++++++++++++++++++++++++++++++++++++++
3 files changed, 395 insertions(+)
diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index e93a5a4d03e2..a2ea249a57c6 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -640,6 +640,17 @@ config BATTERY_PM8916_BMS_VM
To compile this driver as module, choose M here: the
module will be called pm8916_bms_vm.
+config CHARGER_PM8916_LBC
+ tristate "Qualcomm PM8916 Linear Battery Charger support"
+ depends on MFD_SPMI_PMIC || COMPILE_TEST
+ help
+ Say Y here to add support for Linear Battery Charger block
+ found in some Qualcomm PMICs such as PM8916. This hardware
+ blokc provides simple CC/CV battery charger.
+
+ To compile this driver as module, choose M here: the
+ module will be called pm8916_lbc.
+
config CHARGER_BQ2415X
tristate "TI BQ2415x battery charger driver"
depends on I2C
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index fdf7916f80ed..e4bd9eb1261b 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -85,6 +85,7 @@ obj-$(CONFIG_CHARGER_MT6360) += mt6360_charger.o
obj-$(CONFIG_CHARGER_MT6370) += mt6370-charger.o
obj-$(CONFIG_CHARGER_QCOM_SMBB) += qcom_smbb.o
obj-$(CONFIG_BATTERY_PM8916_BMS_VM) += pm8916_bms_vm.o
+obj-$(CONFIG_CHARGER_PM8916_LBC) += pm8916_lbc.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/supply/pm8916_lbc.c b/drivers/power/supply/pm8916_lbc.c
new file mode 100644
index 000000000000..490cb7064dbf
--- /dev/null
+++ b/drivers/power/supply/pm8916_lbc.c
@@ -0,0 +1,383 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2023, Nikita Travkin <nikita@trvn.ru>
+ */
+
+#include <linux/errno.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/extcon-provider.h>
+
+/* Two bytes: type + subtype */
+#define PM8916_PERPH_TYPE 0x04
+#define PM8916_LBC_CHGR_TYPE 0x1502
+#define PM8916_LBC_BAT_IF_TYPE 0x1602
+#define PM8916_LBC_USB_TYPE 0x1702
+#define PM8916_LBC_MISC_TYPE 0x1802
+
+#define PM8916_LBC_CHGR_CHG_OPTION 0x08
+#define PM8916_LBC_CHGR_PMIC_CHARGER BIT(7)
+
+#define PM8916_LBC_CHGR_CHG_STATUS 0x09
+
+#define PM8916_INT_RT_STS 0x10
+
+#define PM8916_LBC_USB_USBIN_VALID BIT(1)
+
+#define PM8916_LBC_CHGR_VDD_MAX 0x40
+#define PM8916_LBC_CHGR_VDD_SAFE 0x41
+#define PM8916_LBC_CHGR_IBAT_MAX 0x44
+#define PM8916_LBC_CHGR_IBAT_SAFE 0x45
+
+#define PM8916_LBC_CHGR_TCHG_MAX_EN 0x60
+#define PM8916_LBC_CHGR_TCHG_MAX_ENABLED BIT(7)
+#define PM8916_LBC_CHGR_TCHG_MAX 0x61
+
+#define PM8916_LBC_CHGR_CHG_CTRL 0x49
+#define PM8916_LBC_CHGR_CHG_EN BIT(7)
+#define PM8916_LBC_CHGR_PSTG_EN BIT(5)
+
+#define PM8916_LBC_CHGR_MIN_CURRENT 90000
+#define PM8916_LBC_CHGR_MAX_CURRENT 1440000
+
+#define PM8916_LBC_CHGR_MIN_VOLTAGE 4000000
+#define PM8916_LBC_CHGR_MAX_VOLTAGE 4775000
+#define PM8916_LBC_CHGR_VOLTAGE_STEP 25000
+
+#define PM8916_LBC_CHGR_MIN_TIME 4
+#define PM8916_LBC_CHGR_MAX_TIME 256
+
+struct pm8916_lbc_charger {
+ struct device *dev;
+ struct extcon_dev *edev;
+ struct power_supply *charger;
+ struct power_supply_battery_info *info;
+ struct regmap *regmap;
+ unsigned int reg[4];
+ bool online;
+ unsigned int charge_voltage_max;
+ unsigned int charge_voltage_safe;
+ unsigned int charge_current_max;
+ unsigned int charge_current_safe;
+};
+
+static const unsigned int pm8916_lbc_charger_cable[] = {
+ EXTCON_USB,
+ EXTCON_NONE,
+};
+
+enum {
+ LBC_CHGR = 0,
+ LBC_BAT_IF,
+ LBC_USB,
+ LBC_MISC,
+};
+
+static int pm8916_lbc_charger_configure(struct pm8916_lbc_charger *chg)
+{
+ int ret = 0;
+ unsigned int tmp;
+
+ chg->charge_voltage_max = clamp_t(u32, chg->charge_voltage_max,
+ PM8916_LBC_CHGR_MIN_VOLTAGE, chg->charge_voltage_safe);
+
+ tmp = chg->charge_voltage_max - PM8916_LBC_CHGR_MIN_VOLTAGE;
+ tmp /= PM8916_LBC_CHGR_VOLTAGE_STEP;
+ chg->charge_voltage_max = PM8916_LBC_CHGR_MIN_VOLTAGE + tmp * PM8916_LBC_CHGR_VOLTAGE_STEP;
+
+ ret = regmap_write(chg->regmap, chg->reg[LBC_CHGR] + PM8916_LBC_CHGR_VDD_MAX, tmp);
+ if (ret)
+ goto error;
+
+ chg->charge_current_max = min(chg->charge_current_max, chg->charge_current_safe);
+
+ tmp = clamp_t(u32, chg->charge_current_max,
+ PM8916_LBC_CHGR_MIN_CURRENT, PM8916_LBC_CHGR_MAX_CURRENT);
+
+ tmp = chg->charge_current_max / PM8916_LBC_CHGR_MIN_CURRENT - 1;
+ chg->charge_current_max = (tmp + 1) * PM8916_LBC_CHGR_MIN_CURRENT;
+
+ ret = regmap_write(chg->regmap, chg->reg[LBC_CHGR] + PM8916_LBC_CHGR_IBAT_MAX, tmp);
+ if (ret)
+ goto error;
+
+ ret = regmap_write(chg->regmap, chg->reg[LBC_CHGR] + PM8916_LBC_CHGR_CHG_CTRL,
+ PM8916_LBC_CHGR_CHG_EN | PM8916_LBC_CHGR_PSTG_EN);
+ if (ret)
+ goto error;
+
+ return ret;
+
+error:
+ dev_err(chg->dev, "Failed to configure charging: %pe\n", ERR_PTR(ret));
+ return ret;
+}
+
+static int pm8916_lbc_charger_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct pm8916_lbc_charger *chg = power_supply_get_drvdata(psy);
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_ONLINE:
+ val->intval = chg->online;
+ return 0;
+
+ case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX:
+ val->intval = chg->charge_voltage_max;
+ return 0;
+
+ case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+ val->intval = chg->charge_current_max;
+ return 0;
+
+ default:
+ return -EINVAL;
+ };
+}
+
+static int pm8916_lbc_charger_set_property(struct power_supply *psy,
+ enum power_supply_property prop,
+ const union power_supply_propval *val)
+{
+ struct pm8916_lbc_charger *chg = power_supply_get_drvdata(psy);
+
+ switch (prop) {
+ case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+ chg->charge_current_max = val->intval;
+ return pm8916_lbc_charger_configure(chg);
+ default:
+ return -EINVAL;
+ }
+}
+
+static int pm8916_lbc_charger_property_is_writeable(struct power_supply *psy,
+ enum power_supply_property psp)
+{
+ switch (psp) {
+ case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static enum power_supply_property pm8916_lbc_charger_properties[] = {
+ POWER_SUPPLY_PROP_ONLINE,
+ POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
+ POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
+};
+
+static irqreturn_t pm8916_lbc_charger_state_changed_irq(int irq, void *data)
+{
+ struct pm8916_lbc_charger *chg = data;
+ unsigned int tmp;
+ int ret;
+
+ ret = regmap_read(chg->regmap, chg->reg[LBC_USB] + PM8916_INT_RT_STS, &tmp);
+ if (ret)
+ return IRQ_HANDLED;
+
+ chg->online = !!(tmp & PM8916_LBC_USB_USBIN_VALID);
+ extcon_set_state_sync(chg->edev, EXTCON_USB, chg->online);
+
+ power_supply_changed(chg->charger);
+
+ return IRQ_HANDLED;
+}
+
+static int pm8916_lbc_charger_probe_dt(struct pm8916_lbc_charger *chg)
+{
+ struct device *dev = chg->dev;
+ struct device_node *np = dev->of_node;
+ int ret = 0;
+ unsigned int tmp;
+
+ ret = of_property_read_u32(np, "qcom,fast-charge-safe-voltage", &chg->charge_voltage_safe);
+ if (ret)
+ return ret;
+ if (chg->charge_voltage_safe < PM8916_LBC_CHGR_MIN_VOLTAGE)
+ return -EINVAL;
+
+ chg->charge_voltage_safe = clamp_t(u32, chg->charge_voltage_safe,
+ PM8916_LBC_CHGR_MIN_VOLTAGE, PM8916_LBC_CHGR_MAX_VOLTAGE);
+
+ tmp = chg->charge_voltage_safe - PM8916_LBC_CHGR_MIN_VOLTAGE;
+ tmp /= PM8916_LBC_CHGR_VOLTAGE_STEP;
+ ret = regmap_write(chg->regmap, chg->reg[LBC_CHGR] + PM8916_LBC_CHGR_VDD_SAFE, tmp);
+ if (ret)
+ return ret;
+
+ ret = of_property_read_u32(np, "qcom,fast-charge-safe-current", &chg->charge_current_safe);
+ if (ret)
+ return ret;
+ if (chg->charge_current_safe < PM8916_LBC_CHGR_MIN_CURRENT)
+ return -EINVAL;
+
+ chg->charge_current_safe = clamp_t(u32, chg->charge_current_safe,
+ PM8916_LBC_CHGR_MIN_CURRENT, PM8916_LBC_CHGR_MAX_CURRENT);
+
+ chg->charge_current_max = chg->charge_current_safe;
+
+ tmp = chg->charge_current_safe / PM8916_LBC_CHGR_MIN_CURRENT - 1;
+ ret = regmap_write(chg->regmap, chg->reg[LBC_CHGR] + PM8916_LBC_CHGR_IBAT_SAFE, tmp);
+ if (ret)
+ return ret;
+
+ /* Disable charger timeout. */
+ ret = regmap_write(chg->regmap, chg->reg[LBC_CHGR] + PM8916_LBC_CHGR_TCHG_MAX_EN, 0x00);
+ if (ret)
+ return ret;
+
+ return ret;
+}
+
+static const struct power_supply_desc pm8916_lbc_charger_psy_desc = {
+ .name = "pm8916-lbc-chgr",
+ .type = POWER_SUPPLY_TYPE_USB,
+ .properties = pm8916_lbc_charger_properties,
+ .num_properties = ARRAY_SIZE(pm8916_lbc_charger_properties),
+ .get_property = pm8916_lbc_charger_get_property,
+ .set_property = pm8916_lbc_charger_set_property,
+ .property_is_writeable = pm8916_lbc_charger_property_is_writeable,
+};
+
+static int pm8916_lbc_charger_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct pm8916_lbc_charger *chg;
+ struct power_supply_config psy_cfg = {};
+ int ret, len, irq;
+ unsigned int tmp;
+
+ chg = devm_kzalloc(dev, sizeof(*chg), GFP_KERNEL);
+ if (!chg)
+ return -ENOMEM;
+
+ chg->dev = dev;
+
+ chg->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+ if (!chg->regmap)
+ return -ENODEV;
+
+ len = of_property_count_u32_elems(dev->of_node, "reg");
+ if (len < 0)
+ return len;
+ if (len != 4)
+ return dev_err_probe(dev, -EINVAL,
+ "Wrong amount of reg values: %d (4 expected)\n", len);
+
+ irq = platform_get_irq_byname(pdev, "usb_vbus");
+ if (irq < 0)
+ return irq;
+
+ ret = devm_request_threaded_irq(dev, irq, NULL, pm8916_lbc_charger_state_changed_irq,
+ IRQF_ONESHOT, "pm8916_lbc", chg);
+ if (ret)
+ return ret;
+
+ ret = of_property_read_u32_array(dev->of_node, "reg", chg->reg, len);
+ if (ret)
+ return ret;
+
+ ret = regmap_bulk_read(chg->regmap, chg->reg[LBC_CHGR] + PM8916_PERPH_TYPE, &tmp, 2);
+ if (ret)
+ goto comm_error;
+ if (tmp != PM8916_LBC_CHGR_TYPE)
+ goto type_error;
+
+ ret = regmap_bulk_read(chg->regmap, chg->reg[LBC_BAT_IF] + PM8916_PERPH_TYPE, &tmp, 2);
+ if (ret)
+ goto comm_error;
+ if (tmp != PM8916_LBC_BAT_IF_TYPE)
+ goto type_error;
+
+ ret = regmap_bulk_read(chg->regmap, chg->reg[LBC_USB] + PM8916_PERPH_TYPE, &tmp, 2);
+ if (ret)
+ goto comm_error;
+ if (tmp != PM8916_LBC_USB_TYPE)
+ goto type_error;
+
+ ret = regmap_bulk_read(chg->regmap, chg->reg[LBC_MISC] + PM8916_PERPH_TYPE, &tmp, 2);
+ if (ret)
+ goto comm_error;
+ if (tmp != PM8916_LBC_MISC_TYPE)
+ goto type_error;
+
+ ret = regmap_read(chg->regmap, chg->reg[LBC_CHGR] + PM8916_LBC_CHGR_CHG_OPTION, &tmp);
+ if (ret)
+ goto comm_error;
+ if (tmp != PM8916_LBC_CHGR_PMIC_CHARGER)
+ dev_err_probe(dev, -ENODEV, "The system is using an external charger\n");
+
+ ret = pm8916_lbc_charger_probe_dt(chg);
+ if (ret)
+ dev_err_probe(dev, ret, "Error while parsing device tree\n");
+
+ psy_cfg.drv_data = chg;
+ psy_cfg.of_node = dev->of_node;
+
+ chg->charger = devm_power_supply_register(dev, &pm8916_lbc_charger_psy_desc, &psy_cfg);
+ if (IS_ERR(chg->charger))
+ return dev_err_probe(dev, PTR_ERR(chg->charger), "Unable to register charger\n");
+
+ ret = power_supply_get_battery_info(chg->charger, &chg->info);
+ if (ret)
+ return dev_err_probe(dev, ret, "Unable to get battery info\n");
+
+ chg->edev = devm_extcon_dev_allocate(dev, pm8916_lbc_charger_cable);
+ if (IS_ERR(chg->edev))
+ return PTR_ERR(chg->edev);
+
+ ret = devm_extcon_dev_register(dev, chg->edev);
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "failed to register extcon device\n");
+
+ ret = regmap_read(chg->regmap, chg->reg[LBC_USB] + PM8916_INT_RT_STS, &tmp);
+ if (ret)
+ goto comm_error;
+
+ chg->online = !!(tmp & PM8916_LBC_USB_USBIN_VALID);
+ extcon_set_state_sync(chg->edev, EXTCON_USB, chg->online);
+
+ chg->charge_voltage_max = chg->info->voltage_max_design_uv;
+ ret = pm8916_lbc_charger_configure(chg);
+ if (ret)
+ return ret;
+
+ return 0;
+
+comm_error:
+ return dev_err_probe(dev, ret, "Unable to communicate with device\n");
+
+type_error:
+ return dev_err_probe(dev, -ENODEV, "Device reported wrong type: 0x%X\n", tmp);
+}
+
+static const struct of_device_id pm8916_lbc_charger_of_match[] = {
+ { .compatible = "qcom,pm8916-lbc", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, pm8916_lbc_charger_of_match);
+
+static struct platform_driver pm8916_lbc_charger_driver = {
+ .driver = {
+ .name = "pm8916-lbc",
+ .of_match_table = of_match_ptr(pm8916_lbc_charger_of_match),
+ },
+ .probe = pm8916_lbc_charger_probe,
+};
+module_platform_driver(pm8916_lbc_charger_driver);
+
+MODULE_DESCRIPTION("pm8916 LBC driver");
+MODULE_AUTHOR("Nikita Travkin <nikita@trvn.ru>");
+MODULE_LICENSE("GPL");
--
2.41.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v2 4/4] power: supply: Add driver for pm8916 lbc
2023-07-31 17:06 ` [PATCH v2 4/4] power: supply: Add driver for pm8916 lbc Nikita Travkin
@ 2023-09-14 14:25 ` Sebastian Reichel
2023-09-14 14:55 ` Nikita Travkin
0 siblings, 1 reply; 11+ messages in thread
From: Sebastian Reichel @ 2023-09-14 14:25 UTC (permalink / raw)
To: Nikita Travkin
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-pm,
devicetree, linux-kernel, linux-arm-msm
[-- Attachment #1: Type: text/plain, Size: 15473 bytes --]
Hi,
On Mon, Jul 31, 2023 at 10:06:27PM +0500, Nikita Travkin wrote:
> pm8916 LBC is a Linear Battery Charger hardware block in pm8916 PMIC.
>
> This block implements simple CC/CV charging for Li-Po batteries.
> The hardware has internal state machine to switch between modes and
> works mostly autonomously, only needing the limits and targets to be
> set to operate.
>
> This driver allows setting limits and enabling the LBC block, monitoring
> it's state.
>
> Signed-off-by: Nikita Travkin <nikita@trvn.ru>
> ---
> v2: Fix missed warnings, get irq by name
> ---
Looks mostly good, but I have a few small requests.
> drivers/power/supply/Kconfig | 11 ++
> drivers/power/supply/Makefile | 1 +
> drivers/power/supply/pm8916_lbc.c | 383 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 395 insertions(+)
>
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index e93a5a4d03e2..a2ea249a57c6 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -640,6 +640,17 @@ config BATTERY_PM8916_BMS_VM
> To compile this driver as module, choose M here: the
> module will be called pm8916_bms_vm.
>
> +config CHARGER_PM8916_LBC
> + tristate "Qualcomm PM8916 Linear Battery Charger support"
> + depends on MFD_SPMI_PMIC || COMPILE_TEST
> + help
> + Say Y here to add support for Linear Battery Charger block
> + found in some Qualcomm PMICs such as PM8916. This hardware
> + blokc provides simple CC/CV battery charger.
> +
> + To compile this driver as module, choose M here: the
> + module will be called pm8916_lbc.
> +
> config CHARGER_BQ2415X
> tristate "TI BQ2415x battery charger driver"
> depends on I2C
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index fdf7916f80ed..e4bd9eb1261b 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -85,6 +85,7 @@ obj-$(CONFIG_CHARGER_MT6360) += mt6360_charger.o
> obj-$(CONFIG_CHARGER_MT6370) += mt6370-charger.o
> obj-$(CONFIG_CHARGER_QCOM_SMBB) += qcom_smbb.o
> obj-$(CONFIG_BATTERY_PM8916_BMS_VM) += pm8916_bms_vm.o
> +obj-$(CONFIG_CHARGER_PM8916_LBC) += pm8916_lbc.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/supply/pm8916_lbc.c b/drivers/power/supply/pm8916_lbc.c
> new file mode 100644
> index 000000000000..490cb7064dbf
> --- /dev/null
> +++ b/drivers/power/supply/pm8916_lbc.c
> @@ -0,0 +1,383 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2023, Nikita Travkin <nikita@trvn.ru>
> + */
> +
> +#include <linux/errno.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
It should be fine to remove the of headers after my proposed
changes.
> +#include <linux/platform_device.h>
> +#include <linux/power_supply.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/extcon-provider.h>
> +
> +/* Two bytes: type + subtype */
> +#define PM8916_PERPH_TYPE 0x04
> +#define PM8916_LBC_CHGR_TYPE 0x1502
> +#define PM8916_LBC_BAT_IF_TYPE 0x1602
> +#define PM8916_LBC_USB_TYPE 0x1702
> +#define PM8916_LBC_MISC_TYPE 0x1802
> +
> +#define PM8916_LBC_CHGR_CHG_OPTION 0x08
> +#define PM8916_LBC_CHGR_PMIC_CHARGER BIT(7)
> +
> +#define PM8916_LBC_CHGR_CHG_STATUS 0x09
> +
> +#define PM8916_INT_RT_STS 0x10
> +
> +#define PM8916_LBC_USB_USBIN_VALID BIT(1)
> +
> +#define PM8916_LBC_CHGR_VDD_MAX 0x40
> +#define PM8916_LBC_CHGR_VDD_SAFE 0x41
> +#define PM8916_LBC_CHGR_IBAT_MAX 0x44
> +#define PM8916_LBC_CHGR_IBAT_SAFE 0x45
> +
> +#define PM8916_LBC_CHGR_TCHG_MAX_EN 0x60
> +#define PM8916_LBC_CHGR_TCHG_MAX_ENABLED BIT(7)
> +#define PM8916_LBC_CHGR_TCHG_MAX 0x61
> +
> +#define PM8916_LBC_CHGR_CHG_CTRL 0x49
> +#define PM8916_LBC_CHGR_CHG_EN BIT(7)
> +#define PM8916_LBC_CHGR_PSTG_EN BIT(5)
> +
> +#define PM8916_LBC_CHGR_MIN_CURRENT 90000
> +#define PM8916_LBC_CHGR_MAX_CURRENT 1440000
> +
> +#define PM8916_LBC_CHGR_MIN_VOLTAGE 4000000
> +#define PM8916_LBC_CHGR_MAX_VOLTAGE 4775000
> +#define PM8916_LBC_CHGR_VOLTAGE_STEP 25000
> +
> +#define PM8916_LBC_CHGR_MIN_TIME 4
> +#define PM8916_LBC_CHGR_MAX_TIME 256
> +
> +struct pm8916_lbc_charger {
> + struct device *dev;
> + struct extcon_dev *edev;
> + struct power_supply *charger;
> + struct power_supply_battery_info *info;
> + struct regmap *regmap;
> + unsigned int reg[4];
> + bool online;
> + unsigned int charge_voltage_max;
> + unsigned int charge_voltage_safe;
> + unsigned int charge_current_max;
> + unsigned int charge_current_safe;
> +};
> +
> +static const unsigned int pm8916_lbc_charger_cable[] = {
> + EXTCON_USB,
> + EXTCON_NONE,
> +};
> +
> +enum {
> + LBC_CHGR = 0,
> + LBC_BAT_IF,
> + LBC_USB,
> + LBC_MISC,
> +};
> +
> +static int pm8916_lbc_charger_configure(struct pm8916_lbc_charger *chg)
> +{
> + int ret = 0;
> + unsigned int tmp;
> +
> + chg->charge_voltage_max = clamp_t(u32, chg->charge_voltage_max,
> + PM8916_LBC_CHGR_MIN_VOLTAGE, chg->charge_voltage_safe);
> +
> + tmp = chg->charge_voltage_max - PM8916_LBC_CHGR_MIN_VOLTAGE;
> + tmp /= PM8916_LBC_CHGR_VOLTAGE_STEP;
> + chg->charge_voltage_max = PM8916_LBC_CHGR_MIN_VOLTAGE + tmp * PM8916_LBC_CHGR_VOLTAGE_STEP;
> +
> + ret = regmap_write(chg->regmap, chg->reg[LBC_CHGR] + PM8916_LBC_CHGR_VDD_MAX, tmp);
> + if (ret)
> + goto error;
> +
> + chg->charge_current_max = min(chg->charge_current_max, chg->charge_current_safe);
> +
> + tmp = clamp_t(u32, chg->charge_current_max,
> + PM8916_LBC_CHGR_MIN_CURRENT, PM8916_LBC_CHGR_MAX_CURRENT);
> +
> + tmp = chg->charge_current_max / PM8916_LBC_CHGR_MIN_CURRENT - 1;
> + chg->charge_current_max = (tmp + 1) * PM8916_LBC_CHGR_MIN_CURRENT;
> +
> + ret = regmap_write(chg->regmap, chg->reg[LBC_CHGR] + PM8916_LBC_CHGR_IBAT_MAX, tmp);
> + if (ret)
> + goto error;
> +
> + ret = regmap_write(chg->regmap, chg->reg[LBC_CHGR] + PM8916_LBC_CHGR_CHG_CTRL,
> + PM8916_LBC_CHGR_CHG_EN | PM8916_LBC_CHGR_PSTG_EN);
> + if (ret)
> + goto error;
> +
> + return ret;
> +
> +error:
> + dev_err(chg->dev, "Failed to configure charging: %pe\n", ERR_PTR(ret));
> + return ret;
> +}
> +
> +static int pm8916_lbc_charger_get_property(struct power_supply *psy,
> + enum power_supply_property psp,
> + union power_supply_propval *val)
> +{
> + struct pm8916_lbc_charger *chg = power_supply_get_drvdata(psy);
> +
> + switch (psp) {
> + case POWER_SUPPLY_PROP_ONLINE:
> + val->intval = chg->online;
> + return 0;
> +
> + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX:
> + val->intval = chg->charge_voltage_max;
> + return 0;
> +
> + case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
> + val->intval = chg->charge_current_max;
> + return 0;
> +
> + default:
> + return -EINVAL;
> + };
> +}
> +
> +static int pm8916_lbc_charger_set_property(struct power_supply *psy,
> + enum power_supply_property prop,
> + const union power_supply_propval *val)
> +{
> + struct pm8916_lbc_charger *chg = power_supply_get_drvdata(psy);
> +
> + switch (prop) {
> + case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
> + chg->charge_current_max = val->intval;
> + return pm8916_lbc_charger_configure(chg);
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int pm8916_lbc_charger_property_is_writeable(struct power_supply *psy,
> + enum power_supply_property psp)
> +{
> + switch (psp) {
> + case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +static enum power_supply_property pm8916_lbc_charger_properties[] = {
> + POWER_SUPPLY_PROP_ONLINE,
> + POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
> + POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
> +};
POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT is about the charger input,
e.g. 500mA limit for a USB based charger. The variable names you are
using suggests, that you want to expose
POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT instead.
> +static irqreturn_t pm8916_lbc_charger_state_changed_irq(int irq, void *data)
> +{
> + struct pm8916_lbc_charger *chg = data;
> + unsigned int tmp;
> + int ret;
> +
> + ret = regmap_read(chg->regmap, chg->reg[LBC_USB] + PM8916_INT_RT_STS, &tmp);
> + if (ret)
> + return IRQ_HANDLED;
> +
> + chg->online = !!(tmp & PM8916_LBC_USB_USBIN_VALID);
> + extcon_set_state_sync(chg->edev, EXTCON_USB, chg->online);
> +
> + power_supply_changed(chg->charger);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int pm8916_lbc_charger_probe_dt(struct pm8916_lbc_charger *chg)
> +{
> + struct device *dev = chg->dev;
> + struct device_node *np = dev->of_node;
> + int ret = 0;
> + unsigned int tmp;
> +
> + ret = of_property_read_u32(np, "qcom,fast-charge-safe-voltage", &chg->charge_voltage_safe);
device_property_read_u32(...)
> + if (ret)
> + return ret;
> + if (chg->charge_voltage_safe < PM8916_LBC_CHGR_MIN_VOLTAGE)
> + return -EINVAL;
> +
> + chg->charge_voltage_safe = clamp_t(u32, chg->charge_voltage_safe,
> + PM8916_LBC_CHGR_MIN_VOLTAGE, PM8916_LBC_CHGR_MAX_VOLTAGE);
> +
> + tmp = chg->charge_voltage_safe - PM8916_LBC_CHGR_MIN_VOLTAGE;
> + tmp /= PM8916_LBC_CHGR_VOLTAGE_STEP;
> + ret = regmap_write(chg->regmap, chg->reg[LBC_CHGR] + PM8916_LBC_CHGR_VDD_SAFE, tmp);
> + if (ret)
> + return ret;
> +
> + ret = of_property_read_u32(np, "qcom,fast-charge-safe-current", &chg->charge_current_safe);
device_property_read_u32(...)
> + if (ret)
> + return ret;
> + if (chg->charge_current_safe < PM8916_LBC_CHGR_MIN_CURRENT)
> + return -EINVAL;
> +
> + chg->charge_current_safe = clamp_t(u32, chg->charge_current_safe,
> + PM8916_LBC_CHGR_MIN_CURRENT, PM8916_LBC_CHGR_MAX_CURRENT);
> +
> + chg->charge_current_max = chg->charge_current_safe;
> +
> + tmp = chg->charge_current_safe / PM8916_LBC_CHGR_MIN_CURRENT - 1;
> + ret = regmap_write(chg->regmap, chg->reg[LBC_CHGR] + PM8916_LBC_CHGR_IBAT_SAFE, tmp);
> + if (ret)
> + return ret;
> +
> + /* Disable charger timeout. */
> + ret = regmap_write(chg->regmap, chg->reg[LBC_CHGR] + PM8916_LBC_CHGR_TCHG_MAX_EN, 0x00);
> + if (ret)
> + return ret;
> +
> + return ret;
> +}
> +
> +static const struct power_supply_desc pm8916_lbc_charger_psy_desc = {
> + .name = "pm8916-lbc-chgr",
> + .type = POWER_SUPPLY_TYPE_USB,
> + .properties = pm8916_lbc_charger_properties,
> + .num_properties = ARRAY_SIZE(pm8916_lbc_charger_properties),
> + .get_property = pm8916_lbc_charger_get_property,
> + .set_property = pm8916_lbc_charger_set_property,
> + .property_is_writeable = pm8916_lbc_charger_property_is_writeable,
> +};
> +
> +static int pm8916_lbc_charger_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct pm8916_lbc_charger *chg;
> + struct power_supply_config psy_cfg = {};
> + int ret, len, irq;
> + unsigned int tmp;
> +
> + chg = devm_kzalloc(dev, sizeof(*chg), GFP_KERNEL);
> + if (!chg)
> + return -ENOMEM;
> +
> + chg->dev = dev;
> +
> + chg->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> + if (!chg->regmap)
> + return -ENODEV;
> +
> + len = of_property_count_u32_elems(dev->of_node, "reg");
device_property_count_u32(...)
> + if (len < 0)
> + return len;
> + if (len != 4)
> + return dev_err_probe(dev, -EINVAL,
> + "Wrong amount of reg values: %d (4 expected)\n", len);
> +
> + irq = platform_get_irq_byname(pdev, "usb_vbus");
> + if (irq < 0)
> + return irq;
> +
> + ret = devm_request_threaded_irq(dev, irq, NULL, pm8916_lbc_charger_state_changed_irq,
> + IRQF_ONESHOT, "pm8916_lbc", chg);
> + if (ret)
> + return ret;
> +
> + ret = of_property_read_u32_array(dev->of_node, "reg", chg->reg, len);
device_property_read_u32_array(...)
> + if (ret)
> + return ret;
> +
> + ret = regmap_bulk_read(chg->regmap, chg->reg[LBC_CHGR] + PM8916_PERPH_TYPE, &tmp, 2);
> + if (ret)
> + goto comm_error;
> + if (tmp != PM8916_LBC_CHGR_TYPE)
> + goto type_error;
> +
> + ret = regmap_bulk_read(chg->regmap, chg->reg[LBC_BAT_IF] + PM8916_PERPH_TYPE, &tmp, 2);
> + if (ret)
> + goto comm_error;
> + if (tmp != PM8916_LBC_BAT_IF_TYPE)
> + goto type_error;
> +
> + ret = regmap_bulk_read(chg->regmap, chg->reg[LBC_USB] + PM8916_PERPH_TYPE, &tmp, 2);
> + if (ret)
> + goto comm_error;
> + if (tmp != PM8916_LBC_USB_TYPE)
> + goto type_error;
> +
> + ret = regmap_bulk_read(chg->regmap, chg->reg[LBC_MISC] + PM8916_PERPH_TYPE, &tmp, 2);
> + if (ret)
> + goto comm_error;
> + if (tmp != PM8916_LBC_MISC_TYPE)
> + goto type_error;
> +
> + ret = regmap_read(chg->regmap, chg->reg[LBC_CHGR] + PM8916_LBC_CHGR_CHG_OPTION, &tmp);
> + if (ret)
> + goto comm_error;
> + if (tmp != PM8916_LBC_CHGR_PMIC_CHARGER)
> + dev_err_probe(dev, -ENODEV, "The system is using an external charger\n");
> +
> + ret = pm8916_lbc_charger_probe_dt(chg);
> + if (ret)
> + dev_err_probe(dev, ret, "Error while parsing device tree\n");
> +
> + psy_cfg.drv_data = chg;
> + psy_cfg.of_node = dev->of_node;
> +
> + chg->charger = devm_power_supply_register(dev, &pm8916_lbc_charger_psy_desc, &psy_cfg);
> + if (IS_ERR(chg->charger))
> + return dev_err_probe(dev, PTR_ERR(chg->charger), "Unable to register charger\n");
> +
> + ret = power_supply_get_battery_info(chg->charger, &chg->info);
> + if (ret)
> + return dev_err_probe(dev, ret, "Unable to get battery info\n");
> +
> + chg->edev = devm_extcon_dev_allocate(dev, pm8916_lbc_charger_cable);
> + if (IS_ERR(chg->edev))
> + return PTR_ERR(chg->edev);
> +
> + ret = devm_extcon_dev_register(dev, chg->edev);
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "failed to register extcon device\n");
> +
> + ret = regmap_read(chg->regmap, chg->reg[LBC_USB] + PM8916_INT_RT_STS, &tmp);
> + if (ret)
> + goto comm_error;
> +
> + chg->online = !!(tmp & PM8916_LBC_USB_USBIN_VALID);
> + extcon_set_state_sync(chg->edev, EXTCON_USB, chg->online);
> +
> + chg->charge_voltage_max = chg->info->voltage_max_design_uv;
> + ret = pm8916_lbc_charger_configure(chg);
> + if (ret)
> + return ret;
> +
> + return 0;
> +
> +comm_error:
> + return dev_err_probe(dev, ret, "Unable to communicate with device\n");
> +
> +type_error:
> + return dev_err_probe(dev, -ENODEV, "Device reported wrong type: 0x%X\n", tmp);
> +}
> +
> +static const struct of_device_id pm8916_lbc_charger_of_match[] = {
> + { .compatible = "qcom,pm8916-lbc", },
> + { },
{},
(i.e. remove space and trailing, for the terminator entry)
> +};
> +MODULE_DEVICE_TABLE(of, pm8916_lbc_charger_of_match);
> +
> +static struct platform_driver pm8916_lbc_charger_driver = {
> + .driver = {
> + .name = "pm8916-lbc",
> + .of_match_table = of_match_ptr(pm8916_lbc_charger_of_match),
.of_match_table = pm8916_lbc_charger_of_match,
> + },
> + .probe = pm8916_lbc_charger_probe,
> +};
> +module_platform_driver(pm8916_lbc_charger_driver);
> +
> +MODULE_DESCRIPTION("pm8916 LBC driver");
> +MODULE_AUTHOR("Nikita Travkin <nikita@trvn.ru>");
> +MODULE_LICENSE("GPL");
Thanks and sorry for the slow review.
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2 4/4] power: supply: Add driver for pm8916 lbc
2023-09-14 14:25 ` Sebastian Reichel
@ 2023-09-14 14:55 ` Nikita Travkin
0 siblings, 0 replies; 11+ messages in thread
From: Nikita Travkin @ 2023-09-14 14:55 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-pm,
devicetree, linux-kernel, linux-arm-msm
Sebastian Reichel писал(а) 14.09.2023 19:25:
> Hi,
>
> On Mon, Jul 31, 2023 at 10:06:27PM +0500, Nikita Travkin wrote:
>> pm8916 LBC is a Linear Battery Charger hardware block in pm8916 PMIC.
>>
>> This block implements simple CC/CV charging for Li-Po batteries.
>> The hardware has internal state machine to switch between modes and
>> works mostly autonomously, only needing the limits and targets to be
>> set to operate.
>>
>> This driver allows setting limits and enabling the LBC block, monitoring
>> it's state.
>>
>> Signed-off-by: Nikita Travkin <nikita@trvn.ru>
>> ---
>> v2: Fix missed warnings, get irq by name
>> ---
>
> Looks mostly good, but I have a few small requests.
>
>> drivers/power/supply/Kconfig | 11 ++
>> drivers/power/supply/Makefile | 1 +
>> drivers/power/supply/pm8916_lbc.c | 383 ++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 395 insertions(+)
>>
>> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
>> index e93a5a4d03e2..a2ea249a57c6 100644
>> --- a/drivers/power/supply/Kconfig
>> +++ b/drivers/power/supply/Kconfig
>> @@ -640,6 +640,17 @@ config BATTERY_PM8916_BMS_VM
>> To compile this driver as module, choose M here: the
>> module will be called pm8916_bms_vm.
>>
>> +config CHARGER_PM8916_LBC
>> + tristate "Qualcomm PM8916 Linear Battery Charger support"
>> + depends on MFD_SPMI_PMIC || COMPILE_TEST
>> + help
>> + Say Y here to add support for Linear Battery Charger block
>> + found in some Qualcomm PMICs such as PM8916. This hardware
>> + blokc provides simple CC/CV battery charger.
>> +
>> + To compile this driver as module, choose M here: the
>> + module will be called pm8916_lbc.
>> +
>> config CHARGER_BQ2415X
>> tristate "TI BQ2415x battery charger driver"
>> depends on I2C
>> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
>> index fdf7916f80ed..e4bd9eb1261b 100644
>> --- a/drivers/power/supply/Makefile
>> +++ b/drivers/power/supply/Makefile
>> @@ -85,6 +85,7 @@ obj-$(CONFIG_CHARGER_MT6360) += mt6360_charger.o
>> obj-$(CONFIG_CHARGER_MT6370) += mt6370-charger.o
>> obj-$(CONFIG_CHARGER_QCOM_SMBB) += qcom_smbb.o
>> obj-$(CONFIG_BATTERY_PM8916_BMS_VM) += pm8916_bms_vm.o
>> +obj-$(CONFIG_CHARGER_PM8916_LBC) += pm8916_lbc.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/supply/pm8916_lbc.c b/drivers/power/supply/pm8916_lbc.c
>> new file mode 100644
>> index 000000000000..490cb7064dbf
>> --- /dev/null
>> +++ b/drivers/power/supply/pm8916_lbc.c
>> @@ -0,0 +1,383 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2023, Nikita Travkin <nikita@trvn.ru>
>> + */
>> +
>> +#include <linux/errno.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>
> It should be fine to remove the of headers after my proposed
> changes.
>
Will switch to the device_* and drop.
>> +#include <linux/platform_device.h>
>> +#include <linux/power_supply.h>
>> +#include <linux/property.h>
>> +#include <linux/regmap.h>
>> +#include <linux/slab.h>
>> +#include <linux/delay.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/extcon-provider.h>
>> +
>> +/* Two bytes: type + subtype */
>> +#define PM8916_PERPH_TYPE 0x04
>> +#define PM8916_LBC_CHGR_TYPE 0x1502
>> +#define PM8916_LBC_BAT_IF_TYPE 0x1602
>> +#define PM8916_LBC_USB_TYPE 0x1702
>> +#define PM8916_LBC_MISC_TYPE 0x1802
>> +
>> +#define PM8916_LBC_CHGR_CHG_OPTION 0x08
>> +#define PM8916_LBC_CHGR_PMIC_CHARGER BIT(7)
>> +
>> +#define PM8916_LBC_CHGR_CHG_STATUS 0x09
>> +
>> +#define PM8916_INT_RT_STS 0x10
>> +
>> +#define PM8916_LBC_USB_USBIN_VALID BIT(1)
>> +
>> +#define PM8916_LBC_CHGR_VDD_MAX 0x40
>> +#define PM8916_LBC_CHGR_VDD_SAFE 0x41
>> +#define PM8916_LBC_CHGR_IBAT_MAX 0x44
>> +#define PM8916_LBC_CHGR_IBAT_SAFE 0x45
>> +
>> +#define PM8916_LBC_CHGR_TCHG_MAX_EN 0x60
>> +#define PM8916_LBC_CHGR_TCHG_MAX_ENABLED BIT(7)
>> +#define PM8916_LBC_CHGR_TCHG_MAX 0x61
>> +
>> +#define PM8916_LBC_CHGR_CHG_CTRL 0x49
>> +#define PM8916_LBC_CHGR_CHG_EN BIT(7)
>> +#define PM8916_LBC_CHGR_PSTG_EN BIT(5)
>> +
>> +#define PM8916_LBC_CHGR_MIN_CURRENT 90000
>> +#define PM8916_LBC_CHGR_MAX_CURRENT 1440000
>> +
>> +#define PM8916_LBC_CHGR_MIN_VOLTAGE 4000000
>> +#define PM8916_LBC_CHGR_MAX_VOLTAGE 4775000
>> +#define PM8916_LBC_CHGR_VOLTAGE_STEP 25000
>> +
>> +#define PM8916_LBC_CHGR_MIN_TIME 4
>> +#define PM8916_LBC_CHGR_MAX_TIME 256
>> +
>> +struct pm8916_lbc_charger {
>> + struct device *dev;
>> + struct extcon_dev *edev;
>> + struct power_supply *charger;
>> + struct power_supply_battery_info *info;
>> + struct regmap *regmap;
>> + unsigned int reg[4];
>> + bool online;
>> + unsigned int charge_voltage_max;
>> + unsigned int charge_voltage_safe;
>> + unsigned int charge_current_max;
>> + unsigned int charge_current_safe;
>> +};
>> +
>> +static const unsigned int pm8916_lbc_charger_cable[] = {
>> + EXTCON_USB,
>> + EXTCON_NONE,
>> +};
>> +
>> +enum {
>> + LBC_CHGR = 0,
>> + LBC_BAT_IF,
>> + LBC_USB,
>> + LBC_MISC,
>> +};
>> +
>> +static int pm8916_lbc_charger_configure(struct pm8916_lbc_charger *chg)
>> +{
>> + int ret = 0;
>> + unsigned int tmp;
>> +
>> + chg->charge_voltage_max = clamp_t(u32, chg->charge_voltage_max,
>> + PM8916_LBC_CHGR_MIN_VOLTAGE, chg->charge_voltage_safe);
>> +
>> + tmp = chg->charge_voltage_max - PM8916_LBC_CHGR_MIN_VOLTAGE;
>> + tmp /= PM8916_LBC_CHGR_VOLTAGE_STEP;
>> + chg->charge_voltage_max = PM8916_LBC_CHGR_MIN_VOLTAGE + tmp * PM8916_LBC_CHGR_VOLTAGE_STEP;
>> +
>> + ret = regmap_write(chg->regmap, chg->reg[LBC_CHGR] + PM8916_LBC_CHGR_VDD_MAX, tmp);
>> + if (ret)
>> + goto error;
>> +
>> + chg->charge_current_max = min(chg->charge_current_max, chg->charge_current_safe);
>> +
>> + tmp = clamp_t(u32, chg->charge_current_max,
>> + PM8916_LBC_CHGR_MIN_CURRENT, PM8916_LBC_CHGR_MAX_CURRENT);
>> +
>> + tmp = chg->charge_current_max / PM8916_LBC_CHGR_MIN_CURRENT - 1;
>> + chg->charge_current_max = (tmp + 1) * PM8916_LBC_CHGR_MIN_CURRENT;
>> +
>> + ret = regmap_write(chg->regmap, chg->reg[LBC_CHGR] + PM8916_LBC_CHGR_IBAT_MAX, tmp);
>> + if (ret)
>> + goto error;
>> +
>> + ret = regmap_write(chg->regmap, chg->reg[LBC_CHGR] + PM8916_LBC_CHGR_CHG_CTRL,
>> + PM8916_LBC_CHGR_CHG_EN | PM8916_LBC_CHGR_PSTG_EN);
>> + if (ret)
>> + goto error;
>> +
>> + return ret;
>> +
>> +error:
>> + dev_err(chg->dev, "Failed to configure charging: %pe\n", ERR_PTR(ret));
>> + return ret;
>> +}
>> +
>> +static int pm8916_lbc_charger_get_property(struct power_supply *psy,
>> + enum power_supply_property psp,
>> + union power_supply_propval *val)
>> +{
>> + struct pm8916_lbc_charger *chg = power_supply_get_drvdata(psy);
>> +
>> + switch (psp) {
>> + case POWER_SUPPLY_PROP_ONLINE:
>> + val->intval = chg->online;
>> + return 0;
>> +
>> + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX:
>> + val->intval = chg->charge_voltage_max;
>> + return 0;
>> +
>> + case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
>> + val->intval = chg->charge_current_max;
>> + return 0;
>> +
>> + default:
>> + return -EINVAL;
>> + };
>> +}
>> +
>> +static int pm8916_lbc_charger_set_property(struct power_supply *psy,
>> + enum power_supply_property prop,
>> + const union power_supply_propval *val)
>> +{
>> + struct pm8916_lbc_charger *chg = power_supply_get_drvdata(psy);
>> +
>> + switch (prop) {
>> + case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
>> + chg->charge_current_max = val->intval;
>> + return pm8916_lbc_charger_configure(chg);
>> + default:
>> + return -EINVAL;
>> + }
>> +}
>> +
>> +static int pm8916_lbc_charger_property_is_writeable(struct power_supply *psy,
>> + enum power_supply_property psp)
>> +{
>> + switch (psp) {
>> + case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
>> + return true;
>> + default:
>> + return false;
>> + }
>> +}
>> +
>> +static enum power_supply_property pm8916_lbc_charger_properties[] = {
>> + POWER_SUPPLY_PROP_ONLINE,
>> + POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
>> + POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
>> +};
>
> POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT is about the charger input,
> e.g. 500mA limit for a USB based charger. The variable names you are
> using suggests, that you want to expose
> POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT instead.
>
Hm yes, I was not sure which one is more appropriate but CHARGE_CURRENT
seems more appropriate indeed. Will switch to that.
>> +static irqreturn_t pm8916_lbc_charger_state_changed_irq(int irq, void *data)
>> +{
>> + struct pm8916_lbc_charger *chg = data;
>> + unsigned int tmp;
>> + int ret;
>> +
>> + ret = regmap_read(chg->regmap, chg->reg[LBC_USB] + PM8916_INT_RT_STS, &tmp);
>> + if (ret)
>> + return IRQ_HANDLED;
>> +
>> + chg->online = !!(tmp & PM8916_LBC_USB_USBIN_VALID);
>> + extcon_set_state_sync(chg->edev, EXTCON_USB, chg->online);
>> +
>> + power_supply_changed(chg->charger);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static int pm8916_lbc_charger_probe_dt(struct pm8916_lbc_charger *chg)
>> +{
>> + struct device *dev = chg->dev;
>> + struct device_node *np = dev->of_node;
>> + int ret = 0;
>> + unsigned int tmp;
>> +
>> + ret = of_property_read_u32(np, "qcom,fast-charge-safe-voltage", &chg->charge_voltage_safe);
>
> device_property_read_u32(...)
>
>> + if (ret)
>> + return ret;
>> + if (chg->charge_voltage_safe < PM8916_LBC_CHGR_MIN_VOLTAGE)
>> + return -EINVAL;
>> +
>> + chg->charge_voltage_safe = clamp_t(u32, chg->charge_voltage_safe,
>> + PM8916_LBC_CHGR_MIN_VOLTAGE, PM8916_LBC_CHGR_MAX_VOLTAGE);
>> +
>> + tmp = chg->charge_voltage_safe - PM8916_LBC_CHGR_MIN_VOLTAGE;
>> + tmp /= PM8916_LBC_CHGR_VOLTAGE_STEP;
>> + ret = regmap_write(chg->regmap, chg->reg[LBC_CHGR] + PM8916_LBC_CHGR_VDD_SAFE, tmp);
>> + if (ret)
>> + return ret;
>> +
>> + ret = of_property_read_u32(np, "qcom,fast-charge-safe-current", &chg->charge_current_safe);
>
> device_property_read_u32(...)
>
>> + if (ret)
>> + return ret;
>> + if (chg->charge_current_safe < PM8916_LBC_CHGR_MIN_CURRENT)
>> + return -EINVAL;
>> +
>> + chg->charge_current_safe = clamp_t(u32, chg->charge_current_safe,
>> + PM8916_LBC_CHGR_MIN_CURRENT, PM8916_LBC_CHGR_MAX_CURRENT);
>> +
>> + chg->charge_current_max = chg->charge_current_safe;
>> +
>> + tmp = chg->charge_current_safe / PM8916_LBC_CHGR_MIN_CURRENT - 1;
>> + ret = regmap_write(chg->regmap, chg->reg[LBC_CHGR] + PM8916_LBC_CHGR_IBAT_SAFE, tmp);
>> + if (ret)
>> + return ret;
>> +
>> + /* Disable charger timeout. */
>> + ret = regmap_write(chg->regmap, chg->reg[LBC_CHGR] + PM8916_LBC_CHGR_TCHG_MAX_EN, 0x00);
>> + if (ret)
>> + return ret;
>> +
>> + return ret;
>> +}
>> +
>> +static const struct power_supply_desc pm8916_lbc_charger_psy_desc = {
>> + .name = "pm8916-lbc-chgr",
>> + .type = POWER_SUPPLY_TYPE_USB,
>> + .properties = pm8916_lbc_charger_properties,
>> + .num_properties = ARRAY_SIZE(pm8916_lbc_charger_properties),
>> + .get_property = pm8916_lbc_charger_get_property,
>> + .set_property = pm8916_lbc_charger_set_property,
>> + .property_is_writeable = pm8916_lbc_charger_property_is_writeable,
>> +};
>> +
>> +static int pm8916_lbc_charger_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct pm8916_lbc_charger *chg;
>> + struct power_supply_config psy_cfg = {};
>> + int ret, len, irq;
>> + unsigned int tmp;
>> +
>> + chg = devm_kzalloc(dev, sizeof(*chg), GFP_KERNEL);
>> + if (!chg)
>> + return -ENOMEM;
>> +
>> + chg->dev = dev;
>> +
>> + chg->regmap = dev_get_regmap(pdev->dev.parent, NULL);
>> + if (!chg->regmap)
>> + return -ENODEV;
>> +
>> + len = of_property_count_u32_elems(dev->of_node, "reg");
>
> device_property_count_u32(...)
>
>> + if (len < 0)
>> + return len;
>> + if (len != 4)
>> + return dev_err_probe(dev, -EINVAL,
>> + "Wrong amount of reg values: %d (4 expected)\n", len);
>> +
>> + irq = platform_get_irq_byname(pdev, "usb_vbus");
>> + if (irq < 0)
>> + return irq;
>> +
>> + ret = devm_request_threaded_irq(dev, irq, NULL, pm8916_lbc_charger_state_changed_irq,
>> + IRQF_ONESHOT, "pm8916_lbc", chg);
>> + if (ret)
>> + return ret;
>> +
>> + ret = of_property_read_u32_array(dev->of_node, "reg", chg->reg, len);
>
> device_property_read_u32_array(...)
>
>> + if (ret)
>> + return ret;
>> +
>> + ret = regmap_bulk_read(chg->regmap, chg->reg[LBC_CHGR] + PM8916_PERPH_TYPE, &tmp, 2);
>> + if (ret)
>> + goto comm_error;
>> + if (tmp != PM8916_LBC_CHGR_TYPE)
>> + goto type_error;
>> +
>> + ret = regmap_bulk_read(chg->regmap, chg->reg[LBC_BAT_IF] + PM8916_PERPH_TYPE, &tmp, 2);
>> + if (ret)
>> + goto comm_error;
>> + if (tmp != PM8916_LBC_BAT_IF_TYPE)
>> + goto type_error;
>> +
>> + ret = regmap_bulk_read(chg->regmap, chg->reg[LBC_USB] + PM8916_PERPH_TYPE, &tmp, 2);
>> + if (ret)
>> + goto comm_error;
>> + if (tmp != PM8916_LBC_USB_TYPE)
>> + goto type_error;
>> +
>> + ret = regmap_bulk_read(chg->regmap, chg->reg[LBC_MISC] + PM8916_PERPH_TYPE, &tmp, 2);
>> + if (ret)
>> + goto comm_error;
>> + if (tmp != PM8916_LBC_MISC_TYPE)
>> + goto type_error;
>> +
>> + ret = regmap_read(chg->regmap, chg->reg[LBC_CHGR] + PM8916_LBC_CHGR_CHG_OPTION, &tmp);
>> + if (ret)
>> + goto comm_error;
>> + if (tmp != PM8916_LBC_CHGR_PMIC_CHARGER)
>> + dev_err_probe(dev, -ENODEV, "The system is using an external charger\n");
>> +
>> + ret = pm8916_lbc_charger_probe_dt(chg);
>> + if (ret)
>> + dev_err_probe(dev, ret, "Error while parsing device tree\n");
>> +
>> + psy_cfg.drv_data = chg;
>> + psy_cfg.of_node = dev->of_node;
>> +
>> + chg->charger = devm_power_supply_register(dev, &pm8916_lbc_charger_psy_desc, &psy_cfg);
>> + if (IS_ERR(chg->charger))
>> + return dev_err_probe(dev, PTR_ERR(chg->charger), "Unable to register charger\n");
>> +
>> + ret = power_supply_get_battery_info(chg->charger, &chg->info);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "Unable to get battery info\n");
>> +
>> + chg->edev = devm_extcon_dev_allocate(dev, pm8916_lbc_charger_cable);
>> + if (IS_ERR(chg->edev))
>> + return PTR_ERR(chg->edev);
>> +
>> + ret = devm_extcon_dev_register(dev, chg->edev);
>> + if (ret < 0)
>> + return dev_err_probe(dev, ret, "failed to register extcon device\n");
>> +
>> + ret = regmap_read(chg->regmap, chg->reg[LBC_USB] + PM8916_INT_RT_STS, &tmp);
>> + if (ret)
>> + goto comm_error;
>> +
>> + chg->online = !!(tmp & PM8916_LBC_USB_USBIN_VALID);
>> + extcon_set_state_sync(chg->edev, EXTCON_USB, chg->online);
>> +
>> + chg->charge_voltage_max = chg->info->voltage_max_design_uv;
>> + ret = pm8916_lbc_charger_configure(chg);
>> + if (ret)
>> + return ret;
>> +
>> + return 0;
>> +
>> +comm_error:
>> + return dev_err_probe(dev, ret, "Unable to communicate with device\n");
>> +
>> +type_error:
>> + return dev_err_probe(dev, -ENODEV, "Device reported wrong type: 0x%X\n", tmp);
>> +}
>> +
>> +static const struct of_device_id pm8916_lbc_charger_of_match[] = {
>> + { .compatible = "qcom,pm8916-lbc", },
>> + { },
>
> {},
>
> (i.e. remove space and trailing, for the terminator entry)
>
Ack, will remove inner space and trailing comma.
>> +};
>> +MODULE_DEVICE_TABLE(of, pm8916_lbc_charger_of_match);
>> +
>> +static struct platform_driver pm8916_lbc_charger_driver = {
>> + .driver = {
>> + .name = "pm8916-lbc",
>> + .of_match_table = of_match_ptr(pm8916_lbc_charger_of_match),
>
> .of_match_table = pm8916_lbc_charger_of_match,
>
Ack
Thanks for the review!
Nikita
>> + },
>> + .probe = pm8916_lbc_charger_probe,
>> +};
>> +module_platform_driver(pm8916_lbc_charger_driver);
>> +
>> +MODULE_DESCRIPTION("pm8916 LBC driver");
>> +MODULE_AUTHOR("Nikita Travkin <nikita@trvn.ru>");
>> +MODULE_LICENSE("GPL");
>
> Thanks and sorry for the slow review.
>
> -- Sebastian
^ permalink raw reply [flat|nested] 11+ messages in thread