* [PATCH v7 1/6] dt-bindings: mfd: add pf1550
2025-06-12 14:55 [PATCH v7 0/6] add support for pf1550 PMIC MFD-based drivers Samuel Kayode via B4 Relay
@ 2025-06-12 14:55 ` Samuel Kayode via B4 Relay
2025-06-12 14:55 ` [PATCH v7 2/6] mfd: pf1550: add core mfd driver Samuel Kayode via B4 Relay
` (4 subsequent siblings)
5 siblings, 0 replies; 17+ messages in thread
From: Samuel Kayode via B4 Relay @ 2025-06-12 14:55 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Liam Girdwood, Mark Brown, Dmitry Torokhov, Sebastian Reichel,
Frank Li
Cc: imx, devicetree, linux-kernel, linux-input, linux-pm, Abel Vesa,
Abel Vesa, Robin Gong, Robin Gong, Enric Balletbo i Serra,
Samuel Kayode, Abel Vesa, Krzysztof Kozlowski
From: Samuel Kayode <samuel.kayode@savoirfairelinux.com>
Add a DT binding document for pf1550 PMIC. This describes the core mfd
device along with its children: regulators, charger and onkey.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Samuel Kayode <samuel.kayode@savoirfairelinux.com>
---
v5:
- Address Krzystof's feedback:
- Drop monitored battery ref already included in power supply schema
- Move `additionalProperties` close to `type` for regulator
- Drop unneccessary |
- Change `additionalProperties` to `unevaluatedProperties` for the
PMIC
v4:
- Address Krzystof's feedback:
- Filename changed to nxp,pf1550.yaml
- Replace Freescale with NXP
- Define include before battery-cell
- Drop operating-range-celsius in example since
nxp,thermal-regulation-celsisus already exists
- Not sure if there is similar binding to thermal-regulation...
for regulating temperature on thermal-zones? @Sebastian and @Krzysztof
v3:
- Address Krzysztof's feedback:
- Fold charger and onkey objects
- Drop compatible for sub-devices: onkey, charger and regulator.
- Drop constant voltage property already included in
monitored-battery
- Fix whitespace warnings
- Fix license
v2:
- Add yamls for the PMIC and the sub-devices
---
.../devicetree/bindings/mfd/nxp,pf1550.yaml | 137 +++++++++++++++++++++
1 file changed, 137 insertions(+)
diff --git a/Documentation/devicetree/bindings/mfd/nxp,pf1550.yaml b/Documentation/devicetree/bindings/mfd/nxp,pf1550.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..a0b412c3468a3d8972e5c129387c5417933c78e3
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/nxp,pf1550.yaml
@@ -0,0 +1,137 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/nxp,pf1550.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP PF1550 Power Management IC
+
+maintainers:
+ - Samuel Kayode <samuel.kayode@savoirfairelinux.com>
+
+description:
+ PF1550 PMIC provides battery charging and power supply for low power IoT and
+ wearable applications. This device consists of an i2c controlled MFD that
+ includes regulators, battery charging and an onkey/power button.
+
+$ref: /schemas/power/supply/power-supply.yaml
+
+properties:
+ compatible:
+ const: nxp,pf1550
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ wakeup-source: true
+
+ regulators:
+ type: object
+ additionalProperties: false
+
+ patternProperties:
+ "^(ldo[1-3]|sw[1-3]|vrefddr)$":
+ type: object
+ $ref: /schemas/regulator/regulator.yaml
+ description:
+ regulator configuration for ldo1-3, buck converters(sw1-3)
+ and DDR termination reference voltage (vrefddr)
+ unevaluatedProperties: false
+
+ monitored-battery:
+ description: |
+ A phandle to a monitored battery node that contains a valid value
+ for:
+ constant-charge-voltage-max-microvolt.
+
+ nxp,thermal-regulation-celsius:
+ description:
+ Temperature threshold for thermal regulation of charger in celsius.
+ enum: [ 60, 75, 90, 105 ]
+
+ nxp,min-system-microvolt:
+ description:
+ System specific lower limit voltage.
+ enum: [ 3500000, 3700000, 4300000 ]
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/input/linux-event-codes.h>
+
+ battery: battery-cell {
+ compatible = "simple-battery";
+ constant-charge-voltage-max-microvolt = <4400000>;
+ };
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pmic@8 {
+ compatible = "nxp,pf1550";
+ reg = <0x8>;
+
+ interrupt-parent = <&gpio1>;
+ interrupts = <2 IRQ_TYPE_LEVEL_LOW>;
+ wakeup-source;
+ monitored-battery = <&battery>;
+ nxp,min-system-microvolt = <4300000>;
+ nxp,thermal-regulation-celsius = <75>;
+
+ regulators {
+ sw1_reg: sw1 {
+ regulator-name = "sw1";
+ regulator-min-microvolt = <600000>;
+ regulator-max-microvolt = <1387500>;
+ regulator-always-on;
+ regulator-ramp-delay = <6250>;
+ };
+
+ sw2_reg: sw2 {
+ regulator-name = "sw2";
+ regulator-min-microvolt = <600000>;
+ regulator-max-microvolt = <1387500>;
+ regulator-always-on;
+ };
+
+ sw3_reg: sw3 {
+ regulator-name = "sw3";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-always-on;
+ };
+
+ vldo1_reg: ldo1 {
+ regulator-name = "ldo1";
+ regulator-min-microvolt = <750000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-always-on;
+ };
+
+ vldo2_reg: ldo2 {
+ regulator-name = "ldo2";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-always-on;
+ };
+
+ vldo3_reg: ldo3 {
+ regulator-name = "ldo3";
+ regulator-min-microvolt = <750000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-always-on;
+ };
+ };
+ };
+ };
--
2.49.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v7 2/6] mfd: pf1550: add core mfd driver
2025-06-12 14:55 [PATCH v7 0/6] add support for pf1550 PMIC MFD-based drivers Samuel Kayode via B4 Relay
2025-06-12 14:55 ` [PATCH v7 1/6] dt-bindings: mfd: add pf1550 Samuel Kayode via B4 Relay
@ 2025-06-12 14:55 ` Samuel Kayode via B4 Relay
2025-06-19 13:03 ` Lee Jones
2025-06-12 14:55 ` [PATCH v7 3/6] regulator: pf1550: add support for regulator Samuel Kayode via B4 Relay
` (3 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Samuel Kayode via B4 Relay @ 2025-06-12 14:55 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Liam Girdwood, Mark Brown, Dmitry Torokhov, Sebastian Reichel,
Frank Li
Cc: imx, devicetree, linux-kernel, linux-input, linux-pm, Abel Vesa,
Abel Vesa, Robin Gong, Robin Gong, Enric Balletbo i Serra,
Samuel Kayode, Abel Vesa, Frank Li
From: Samuel Kayode <samuel.kayode@savoirfairelinux.com>
Add the core mfd driver for pf1550 PMIC. There are 3 subdevices for
which the drivers will be added in subsequent patches.
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Samuel Kayode <samuel.kayode@savoirfairelinux.com>
---
v7:
- Address Frank's feedback:
- Ensure reverse christmas tree order for local variable definitions
- Drop unnecessary driver data definition in id table
v6:
- Address Frank's feedback:
- Ensure lowercase when defining register addresses
- Use GENMASK macro for masking
- Hardcode IRQ flags in pf1550_add_child_device
- Add dvs_enb variable for SW2 regulator
- Drop chip type variable
v5:
- Use top level interrupt to manage interrupts for the sub-drivers as
recommended by Mark Brown. The regmap_irq_sub_irq_map would have been used
if not for the irregular charger irq address. For all children, the mask
register is directly after the irq register (i.e., 0x08, 0x09) except
for the charger: 0x80, 0x82. Meaning .mask_base would be applicable
for all but the charger
- Fix bad offset for temperature interrupts of regulator
v4:
- Use struct resource to define irq so platform_get_irq can be used in
children as suggested by Dmitry
- Let mfd_add_devices create the mappings for the interrupts
- ack_base and init_ack_masked defined for charger and regulator irq
chips
- No need to define driver_data in table id
v3:
- Address Dmitry's feedback:
- Place Table IDs next to each other
- Drop of_match_ptr
- Replace dev_err with dev_err_probe in probe method
- Drop useless log in probe
- Map all irqs instead of doing it in the sub-devices as recommended by
Dmitry.
v2:
- Address feedback from Enric Balletbo Serra
---
drivers/mfd/Kconfig | 14 ++
drivers/mfd/Makefile | 2 +
drivers/mfd/pf1550.c | 339 +++++++++++++++++++++++++++++++++++++++++++++
include/linux/mfd/pf1550.h | 254 +++++++++++++++++++++++++++++++++
4 files changed, 609 insertions(+)
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 96992af22565205716d72db0494c7bf2567b045e..de3fc9c5e88b5c2a2c7325e2ceeb8f9c3ca057de 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -558,6 +558,20 @@ config MFD_MX25_TSADC
i.MX25 processors. They consist of a conversion queue for general
purpose ADC and a queue for Touchscreens.
+config MFD_PF1550
+ tristate "NXP PF1550 PMIC Support"
+ depends on I2C=y && OF
+ select MFD_CORE
+ select REGMAP_I2C
+ select REGMAP_IRQ
+ help
+ Say yes here to add support for NXP PF1550.
+ This is a companion Power Management IC with regulators, onkey,
+ and charger control on chip.
+ This driver provides common support for accessing the device;
+ additional drivers must be enabled in order to use the functionality
+ of the device.
+
config MFD_HI6421_PMIC
tristate "HiSilicon Hi6421 PMU/Codec IC"
depends on OF
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 5e5cc279af6036a6b3ea1f1f0feeddf45b85f15c..7391d1b81d1ee499507b4ac24ff00eb2e344d60b 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -120,6 +120,8 @@ obj-$(CONFIG_MFD_MC13XXX) += mc13xxx-core.o
obj-$(CONFIG_MFD_MC13XXX_SPI) += mc13xxx-spi.o
obj-$(CONFIG_MFD_MC13XXX_I2C) += mc13xxx-i2c.o
+obj-$(CONFIG_MFD_PF1550) += pf1550.o
+
obj-$(CONFIG_MFD_CORE) += mfd-core.o
ocelot-soc-objs := ocelot-core.o ocelot-spi.o
diff --git a/drivers/mfd/pf1550.c b/drivers/mfd/pf1550.c
new file mode 100644
index 0000000000000000000000000000000000000000..f42c35b6f3c7ce63d615545e85fb2a143fccb488
--- /dev/null
+++ b/drivers/mfd/pf1550.c
@@ -0,0 +1,339 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// pf1550.c - mfd core driver for the PF1550
+//
+// Copyright (C) 2016 Freescale Semiconductor, Inc.
+// Robin Gong <yibin.gong@freescale.com>
+//
+// Portions Copyright (c) 2025 Savoir-faire Linux Inc.
+// Samuel Kayode <samuel.kayode@savoirfairelinux.com>
+//
+// This driver is based on max77693.c
+//
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/pf1550.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+
+static const struct regmap_config pf1550_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = PF1550_PMIC_REG_END,
+};
+
+static const struct regmap_irq pf1550_irqs[] = {
+ REGMAP_IRQ_REG(PF1550_IRQ_CHG, 0, IRQ_CHG),
+ REGMAP_IRQ_REG(PF1550_IRQ_REGULATOR, 0, IRQ_REGULATOR),
+ REGMAP_IRQ_REG(PF1550_IRQ_ONKEY, 0, IRQ_ONKEY),
+};
+
+static const struct regmap_irq_chip pf1550_irq_chip = {
+ .name = "pf1550",
+ .status_base = PF1550_PMIC_REG_INT_CATEGORY,
+ .init_ack_masked = 1,
+ .num_regs = 1,
+ .irqs = pf1550_irqs,
+ .num_irqs = ARRAY_SIZE(pf1550_irqs),
+};
+
+static const struct regmap_irq pf1550_regulator_irqs[] = {
+ REGMAP_IRQ_REG(PF1550_PMIC_IRQ_SW1_LS, 0, PMIC_IRQ_SW1_LS),
+ REGMAP_IRQ_REG(PF1550_PMIC_IRQ_SW2_LS, 0, PMIC_IRQ_SW2_LS),
+ REGMAP_IRQ_REG(PF1550_PMIC_IRQ_SW3_LS, 0, PMIC_IRQ_SW3_LS),
+ REGMAP_IRQ_REG(PF1550_PMIC_IRQ_SW1_HS, 3, PMIC_IRQ_SW1_HS),
+ REGMAP_IRQ_REG(PF1550_PMIC_IRQ_SW2_HS, 3, PMIC_IRQ_SW2_HS),
+ REGMAP_IRQ_REG(PF1550_PMIC_IRQ_SW3_HS, 3, PMIC_IRQ_SW3_HS),
+ REGMAP_IRQ_REG(PF1550_PMIC_IRQ_LDO1_FAULT, 16, PMIC_IRQ_LDO1_FAULT),
+ REGMAP_IRQ_REG(PF1550_PMIC_IRQ_LDO2_FAULT, 16, PMIC_IRQ_LDO2_FAULT),
+ REGMAP_IRQ_REG(PF1550_PMIC_IRQ_LDO3_FAULT, 16, PMIC_IRQ_LDO3_FAULT),
+ REGMAP_IRQ_REG(PF1550_PMIC_IRQ_TEMP_110, 24, PMIC_IRQ_TEMP_110),
+ REGMAP_IRQ_REG(PF1550_PMIC_IRQ_TEMP_125, 24, PMIC_IRQ_TEMP_125),
+};
+
+static const struct regmap_irq_chip pf1550_regulator_irq_chip = {
+ .name = "pf1550-regulator",
+ .status_base = PF1550_PMIC_REG_SW_INT_STAT0,
+ .ack_base = PF1550_PMIC_REG_SW_INT_STAT0,
+ .mask_base = PF1550_PMIC_REG_SW_INT_MASK0,
+ .use_ack = 1,
+ .init_ack_masked = 1,
+ .num_regs = 25,
+ .irqs = pf1550_regulator_irqs,
+ .num_irqs = ARRAY_SIZE(pf1550_regulator_irqs),
+};
+
+static const struct resource regulator_resources[] = {
+ DEFINE_RES_IRQ(PF1550_PMIC_IRQ_SW1_LS),
+ DEFINE_RES_IRQ(PF1550_PMIC_IRQ_SW2_LS),
+ DEFINE_RES_IRQ(PF1550_PMIC_IRQ_SW3_LS),
+ DEFINE_RES_IRQ(PF1550_PMIC_IRQ_SW1_HS),
+ DEFINE_RES_IRQ(PF1550_PMIC_IRQ_SW2_HS),
+ DEFINE_RES_IRQ(PF1550_PMIC_IRQ_SW3_HS),
+ DEFINE_RES_IRQ(PF1550_PMIC_IRQ_LDO1_FAULT),
+ DEFINE_RES_IRQ(PF1550_PMIC_IRQ_LDO2_FAULT),
+ DEFINE_RES_IRQ(PF1550_PMIC_IRQ_LDO3_FAULT),
+ DEFINE_RES_IRQ(PF1550_PMIC_IRQ_TEMP_110),
+ DEFINE_RES_IRQ(PF1550_PMIC_IRQ_TEMP_125),
+};
+
+static const struct regmap_irq pf1550_onkey_irqs[] = {
+ REGMAP_IRQ_REG(PF1550_ONKEY_IRQ_PUSHI, 0, ONKEY_IRQ_PUSHI),
+ REGMAP_IRQ_REG(PF1550_ONKEY_IRQ_1SI, 0, ONKEY_IRQ_1SI),
+ REGMAP_IRQ_REG(PF1550_ONKEY_IRQ_2SI, 0, ONKEY_IRQ_2SI),
+ REGMAP_IRQ_REG(PF1550_ONKEY_IRQ_3SI, 0, ONKEY_IRQ_3SI),
+ REGMAP_IRQ_REG(PF1550_ONKEY_IRQ_4SI, 0, ONKEY_IRQ_4SI),
+ REGMAP_IRQ_REG(PF1550_ONKEY_IRQ_8SI, 0, ONKEY_IRQ_8SI),
+};
+
+static const struct regmap_irq_chip pf1550_onkey_irq_chip = {
+ .name = "pf1550-onkey",
+ .status_base = PF1550_PMIC_REG_ONKEY_INT_STAT0,
+ .ack_base = PF1550_PMIC_REG_ONKEY_INT_STAT0,
+ .mask_base = PF1550_PMIC_REG_ONKEY_INT_MASK0,
+ .use_ack = 1,
+ .init_ack_masked = 1,
+ .num_regs = 1,
+ .irqs = pf1550_onkey_irqs,
+ .num_irqs = ARRAY_SIZE(pf1550_onkey_irqs),
+};
+
+static const struct resource onkey_resources[] = {
+ DEFINE_RES_IRQ(PF1550_ONKEY_IRQ_PUSHI),
+ DEFINE_RES_IRQ(PF1550_ONKEY_IRQ_1SI),
+ DEFINE_RES_IRQ(PF1550_ONKEY_IRQ_2SI),
+ DEFINE_RES_IRQ(PF1550_ONKEY_IRQ_3SI),
+ DEFINE_RES_IRQ(PF1550_ONKEY_IRQ_4SI),
+ DEFINE_RES_IRQ(PF1550_ONKEY_IRQ_8SI),
+};
+
+static const struct regmap_irq pf1550_charger_irqs[] = {
+ REGMAP_IRQ_REG(PF1550_CHARG_IRQ_BAT2SOCI, 0, CHARG_IRQ_BAT2SOCI),
+ REGMAP_IRQ_REG(PF1550_CHARG_IRQ_BATI, 0, CHARG_IRQ_BATI),
+ REGMAP_IRQ_REG(PF1550_CHARG_IRQ_CHGI, 0, CHARG_IRQ_CHGI),
+ REGMAP_IRQ_REG(PF1550_CHARG_IRQ_VBUSI, 0, CHARG_IRQ_VBUSI),
+ REGMAP_IRQ_REG(PF1550_CHARG_IRQ_THMI, 0, CHARG_IRQ_THMI),
+};
+
+static const struct regmap_irq_chip pf1550_charger_irq_chip = {
+ .name = "pf1550-charger",
+ .status_base = PF1550_CHARG_REG_CHG_INT,
+ .ack_base = PF1550_CHARG_REG_CHG_INT,
+ .mask_base = PF1550_CHARG_REG_CHG_INT_MASK,
+ .use_ack = 1,
+ .init_ack_masked = 1,
+ .num_regs = 1,
+ .irqs = pf1550_charger_irqs,
+ .num_irqs = ARRAY_SIZE(pf1550_charger_irqs),
+};
+
+static const struct resource charger_resources[] = {
+ DEFINE_RES_IRQ(PF1550_CHARG_IRQ_BAT2SOCI),
+ DEFINE_RES_IRQ(PF1550_CHARG_IRQ_BATI),
+ DEFINE_RES_IRQ(PF1550_CHARG_IRQ_CHGI),
+ DEFINE_RES_IRQ(PF1550_CHARG_IRQ_VBUSI),
+ DEFINE_RES_IRQ(PF1550_CHARG_IRQ_THMI),
+};
+
+static const struct mfd_cell pf1550_regulator_cell = {
+ .name = "pf1550-regulator",
+ .num_resources = ARRAY_SIZE(regulator_resources),
+ .resources = regulator_resources,
+};
+
+static const struct mfd_cell pf1550_onkey_cell = {
+ .name = "pf1550-onkey",
+ .num_resources = ARRAY_SIZE(onkey_resources),
+ .resources = onkey_resources,
+};
+
+static const struct mfd_cell pf1550_charger_cell = {
+ .name = "pf1550-charger",
+ .num_resources = ARRAY_SIZE(charger_resources),
+ .resources = charger_resources,
+};
+
+static int pf1550_read_otp(const struct pf1550_dev *pf1550, unsigned int index,
+ unsigned int *val)
+{
+ int ret = 0;
+
+ ret = regmap_write(pf1550->regmap, PF1550_PMIC_REG_KEY, 0x15);
+ if (ret)
+ goto read_err;
+ ret = regmap_write(pf1550->regmap, PF1550_CHARG_REG_CHGR_KEY2, 0x50);
+ if (ret)
+ goto read_err;
+ ret = regmap_write(pf1550->regmap, PF1550_TEST_REG_KEY3, 0xab);
+ if (ret)
+ goto read_err;
+ ret = regmap_write(pf1550->regmap, PF1550_TEST_REG_FMRADDR, index);
+ if (ret)
+ goto read_err;
+ ret = regmap_read(pf1550->regmap, PF1550_TEST_REG_FMRDATA, val);
+ if (ret)
+ goto read_err;
+
+ return 0;
+
+read_err:
+ dev_err_probe(pf1550->dev, ret, "read otp reg %x found!\n", index);
+ return ret;
+}
+
+static int pf1550_add_child_device(struct pf1550_dev *pmic,
+ const struct mfd_cell *cell,
+ struct regmap_irq_chip_data *pdata,
+ int pirq,
+ const struct regmap_irq_chip *chip,
+ struct regmap_irq_chip_data **data)
+{
+ struct device *dev = pmic->dev;
+ struct irq_domain *domain;
+ int irq, ret;
+
+ irq = regmap_irq_get_virq(pdata, pirq);
+ if (irq < 0)
+ return dev_err_probe(dev, irq,
+ "Failed to get parent vIRQ(%d) for chip %s\n",
+ pirq, chip->name);
+
+ ret = devm_regmap_add_irq_chip(dev, pmic->regmap, irq,
+ IRQF_ONESHOT | IRQF_SHARED |
+ IRQF_TRIGGER_FALLING, 0, chip, data);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Failed to add %s IRQ chip\n",
+ chip->name);
+
+ domain = regmap_irq_get_domain(*data);
+
+ return devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE, cell, 1,
+ NULL, 0, domain);
+}
+
+static int pf1550_i2c_probe(struct i2c_client *i2c)
+{
+ const struct mfd_cell *regulator = &pf1550_regulator_cell;
+ const struct mfd_cell *charger = &pf1550_charger_cell;
+ const struct mfd_cell *onkey = &pf1550_onkey_cell;
+ unsigned int reg_data = 0, otp_data = 0;
+ struct pf1550_dev *pf1550;
+ int ret = 0;
+
+ pf1550 = devm_kzalloc(&i2c->dev, sizeof(*pf1550), GFP_KERNEL);
+ if (!pf1550)
+ return -ENOMEM;
+
+ i2c_set_clientdata(i2c, pf1550);
+ pf1550->dev = &i2c->dev;
+ pf1550->i2c = i2c;
+ pf1550->irq = i2c->irq;
+ pf1550->dvs_enb = false;
+
+ pf1550->regmap = devm_regmap_init_i2c(i2c, &pf1550_regmap_config);
+ if (IS_ERR(pf1550->regmap))
+ return dev_err_probe(pf1550->dev, PTR_ERR(pf1550->regmap),
+ "failed to allocate register map\n");
+
+ ret = regmap_read(pf1550->regmap, PF1550_PMIC_REG_DEVICE_ID, ®_data);
+ if (ret < 0 || reg_data != PF1550_DEVICE_ID)
+ return dev_err_probe(pf1550->dev, ret ?: -EINVAL,
+ "device not found!\n");
+
+ /* regulator DVS */
+ ret = pf1550_read_otp(pf1550, 0x1f, &otp_data);
+ if (ret)
+ return ret;
+
+ if (otp_data & BIT(3))
+ pf1550->dvs_enb = true;
+
+ /* add top level interrupts */
+ ret = devm_regmap_add_irq_chip(pf1550->dev, pf1550->regmap, pf1550->irq,
+ IRQF_ONESHOT | IRQF_SHARED |
+ IRQF_TRIGGER_FALLING,
+ 0, &pf1550_irq_chip,
+ &pf1550->irq_data);
+ if (ret)
+ return ret;
+
+ ret = pf1550_add_child_device(pf1550, regulator, pf1550->irq_data,
+ PF1550_IRQ_REGULATOR,
+ &pf1550_regulator_irq_chip,
+ &pf1550->irq_data_regulator);
+ if (ret)
+ return ret;
+
+ ret = pf1550_add_child_device(pf1550, onkey, pf1550->irq_data,
+ PF1550_IRQ_ONKEY,
+ &pf1550_onkey_irq_chip,
+ &pf1550->irq_data_onkey);
+ if (ret)
+ return ret;
+
+ ret = pf1550_add_child_device(pf1550, charger, pf1550->irq_data,
+ PF1550_IRQ_CHG,
+ &pf1550_charger_irq_chip,
+ &pf1550->irq_data_charger);
+ return ret;
+}
+
+static int pf1550_suspend(struct device *dev)
+{
+ struct i2c_client *i2c = container_of(dev, struct i2c_client, dev);
+ struct pf1550_dev *pf1550 = i2c_get_clientdata(i2c);
+
+ if (device_may_wakeup(dev)) {
+ enable_irq_wake(pf1550->irq);
+ disable_irq(pf1550->irq);
+ }
+
+ return 0;
+}
+
+static int pf1550_resume(struct device *dev)
+{
+ struct i2c_client *i2c = container_of(dev, struct i2c_client, dev);
+ struct pf1550_dev *pf1550 = i2c_get_clientdata(i2c);
+
+ if (device_may_wakeup(dev)) {
+ disable_irq_wake(pf1550->irq);
+ enable_irq(pf1550->irq);
+ }
+
+ return 0;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(pf1550_pm, pf1550_suspend, pf1550_resume);
+
+static const struct i2c_device_id pf1550_i2c_id[] = {
+ { "pf1550" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(i2c, pf1550_i2c_id);
+
+static const struct of_device_id pf1550_dt_match[] = {
+ { .compatible = "nxp,pf1550" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, pf1550_dt_match);
+
+static struct i2c_driver pf1550_i2c_driver = {
+ .driver = {
+ .name = "pf1550",
+ .pm = pm_sleep_ptr(&pf1550_pm),
+ .of_match_table = pf1550_dt_match,
+ },
+ .probe = pf1550_i2c_probe,
+ .id_table = pf1550_i2c_id,
+};
+module_i2c_driver(pf1550_i2c_driver);
+
+MODULE_DESCRIPTION("NXP PF1550 multi-function core driver");
+MODULE_AUTHOR("Robin Gong <yibin.gong@freescale.com>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/pf1550.h b/include/linux/mfd/pf1550.h
new file mode 100644
index 0000000000000000000000000000000000000000..64ff475215cced21a5ebc24f799f48315a51d260
--- /dev/null
+++ b/include/linux/mfd/pf1550.h
@@ -0,0 +1,254 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * pf1550.h - mfd head file for PF1550
+ *
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Robin Gong <yibin.gong@freescale.com>
+ *
+ * Portions Copyright (c) 2025 Savoir-faire Linux Inc.
+ * Samuel Kayode <samuel.kayode@savoirfairelinux.com>
+ */
+
+#ifndef __LINUX_MFD_PF1550_H
+#define __LINUX_MFD_PF1550_H
+
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+
+enum pf1550_pmic_reg {
+ /* PMIC regulator part */
+ PF1550_PMIC_REG_DEVICE_ID = 0x00,
+ PF1550_PMIC_REG_OTP_FLAVOR = 0x01,
+ PF1550_PMIC_REG_SILICON_REV = 0x02,
+
+ PF1550_PMIC_REG_INT_CATEGORY = 0x06,
+ PF1550_PMIC_REG_SW_INT_STAT0 = 0x08,
+ PF1550_PMIC_REG_SW_INT_MASK0 = 0x09,
+ PF1550_PMIC_REG_SW_INT_SENSE0 = 0x0a,
+ PF1550_PMIC_REG_SW_INT_STAT1 = 0x0b,
+ PF1550_PMIC_REG_SW_INT_MASK1 = 0x0c,
+ PF1550_PMIC_REG_SW_INT_SENSE1 = 0x0d,
+ PF1550_PMIC_REG_SW_INT_STAT2 = 0x0e,
+ PF1550_PMIC_REG_SW_INT_MASK2 = 0x0f,
+ PF1550_PMIC_REG_SW_INT_SENSE2 = 0x10,
+ PF1550_PMIC_REG_LDO_INT_STAT0 = 0x18,
+ PF1550_PMIC_REG_LDO_INT_MASK0 = 0x19,
+ PF1550_PMIC_REG_LDO_INT_SENSE0 = 0x1a,
+ PF1550_PMIC_REG_TEMP_INT_STAT0 = 0x20,
+ PF1550_PMIC_REG_TEMP_INT_MASK0 = 0x21,
+ PF1550_PMIC_REG_TEMP_INT_SENSE0 = 0x22,
+ PF1550_PMIC_REG_ONKEY_INT_STAT0 = 0x24,
+ PF1550_PMIC_REG_ONKEY_INT_MASK0 = 0x25,
+ PF1550_PMIC_REG_ONKEY_INT_SENSE0 = 0x26,
+ PF1550_PMIC_REG_MISC_INT_STAT0 = 0x28,
+ PF1550_PMIC_REG_MISC_INT_MASK0 = 0x29,
+ PF1550_PMIC_REG_MISC_INT_SENSE0 = 0x2a,
+
+ PF1550_PMIC_REG_COINCELL_CONTROL = 0x30,
+
+ PF1550_PMIC_REG_SW1_VOLT = 0x32,
+ PF1550_PMIC_REG_SW1_STBY_VOLT = 0x33,
+ PF1550_PMIC_REG_SW1_SLP_VOLT = 0x34,
+ PF1550_PMIC_REG_SW1_CTRL = 0x35,
+ PF1550_PMIC_REG_SW1_CTRL1 = 0x36,
+ PF1550_PMIC_REG_SW2_VOLT = 0x38,
+ PF1550_PMIC_REG_SW2_STBY_VOLT = 0x39,
+ PF1550_PMIC_REG_SW2_SLP_VOLT = 0x3a,
+ PF1550_PMIC_REG_SW2_CTRL = 0x3b,
+ PF1550_PMIC_REG_SW2_CTRL1 = 0x3c,
+ PF1550_PMIC_REG_SW3_VOLT = 0x3e,
+ PF1550_PMIC_REG_SW3_STBY_VOLT = 0x3f,
+ PF1550_PMIC_REG_SW3_SLP_VOLT = 0x40,
+ PF1550_PMIC_REG_SW3_CTRL = 0x41,
+ PF1550_PMIC_REG_SW3_CTRL1 = 0x42,
+ PF1550_PMIC_REG_VSNVS_CTRL = 0x48,
+ PF1550_PMIC_REG_VREFDDR_CTRL = 0x4a,
+ PF1550_PMIC_REG_LDO1_VOLT = 0x4c,
+ PF1550_PMIC_REG_LDO1_CTRL = 0x4d,
+ PF1550_PMIC_REG_LDO2_VOLT = 0x4f,
+ PF1550_PMIC_REG_LDO2_CTRL = 0x50,
+ PF1550_PMIC_REG_LDO3_VOLT = 0x52,
+ PF1550_PMIC_REG_LDO3_CTRL = 0x53,
+ PF1550_PMIC_REG_PWRCTRL0 = 0x58,
+ PF1550_PMIC_REG_PWRCTRL1 = 0x59,
+ PF1550_PMIC_REG_PWRCTRL2 = 0x5a,
+ PF1550_PMIC_REG_PWRCTRL3 = 0x5b,
+ PF1550_PMIC_REG_SW1_PWRDN_SEQ = 0x5f,
+ PF1550_PMIC_REG_SW2_PWRDN_SEQ = 0x60,
+ PF1550_PMIC_REG_SW3_PWRDN_SEQ = 0x61,
+ PF1550_PMIC_REG_LDO1_PWRDN_SEQ = 0x62,
+ PF1550_PMIC_REG_LDO2_PWRDN_SEQ = 0x63,
+ PF1550_PMIC_REG_LDO3_PWRDN_SEQ = 0x64,
+ PF1550_PMIC_REG_VREFDDR_PWRDN_SEQ = 0x65,
+
+ PF1550_PMIC_REG_STATE_INFO = 0x67,
+ PF1550_PMIC_REG_I2C_ADDR = 0x68,
+ PF1550_PMIC_REG_IO_DRV0 = 0x69,
+ PF1550_PMIC_REG_IO_DRV1 = 0x6a,
+ PF1550_PMIC_REG_RC_16MHZ = 0x6b,
+ PF1550_PMIC_REG_KEY = 0x6f,
+
+ /* charger part */
+ PF1550_CHARG_REG_CHG_INT = 0x80,
+ PF1550_CHARG_REG_CHG_INT_MASK = 0x82,
+ PF1550_CHARG_REG_CHG_INT_OK = 0x84,
+ PF1550_CHARG_REG_VBUS_SNS = 0x86,
+ PF1550_CHARG_REG_CHG_SNS = 0x87,
+ PF1550_CHARG_REG_BATT_SNS = 0x88,
+ PF1550_CHARG_REG_CHG_OPER = 0x89,
+ PF1550_CHARG_REG_CHG_TMR = 0x8a,
+ PF1550_CHARG_REG_CHG_EOC_CNFG = 0x8d,
+ PF1550_CHARG_REG_CHG_CURR_CNFG = 0x8e,
+ PF1550_CHARG_REG_BATT_REG = 0x8f,
+ PF1550_CHARG_REG_BATFET_CNFG = 0x91,
+ PF1550_CHARG_REG_THM_REG_CNFG = 0x92,
+ PF1550_CHARG_REG_VBUS_INLIM_CNFG = 0x94,
+ PF1550_CHARG_REG_VBUS_LIN_DPM = 0x95,
+ PF1550_CHARG_REG_USB_PHY_LDO_CNFG = 0x96,
+ PF1550_CHARG_REG_DBNC_DELAY_TIME = 0x98,
+ PF1550_CHARG_REG_CHG_INT_CNFG = 0x99,
+ PF1550_CHARG_REG_THM_ADJ_SETTING = 0x9a,
+ PF1550_CHARG_REG_VBUS2SYS_CNFG = 0x9b,
+ PF1550_CHARG_REG_LED_PWM = 0x9c,
+ PF1550_CHARG_REG_FAULT_BATFET_CNFG = 0x9d,
+ PF1550_CHARG_REG_LED_CNFG = 0x9e,
+ PF1550_CHARG_REG_CHGR_KEY2 = 0x9f,
+
+ PF1550_TEST_REG_FMRADDR = 0xc4,
+ PF1550_TEST_REG_FMRDATA = 0xc5,
+ PF1550_TEST_REG_KEY3 = 0xdf,
+
+ PF1550_PMIC_REG_END = 0xff,
+};
+
+#define PF1550_DEVICE_ID 0x7c
+
+#define PF1550_CHG_TURNON 0x2
+
+#define PF1550_CHG_PRECHARGE 0
+#define PF1550_CHG_CONSTANT_CURRENT 1
+#define PF1550_CHG_CONSTANT_VOL 2
+#define PF1550_CHG_EOC 3
+#define PF1550_CHG_DONE 4
+#define PF1550_CHG_TIMER_FAULT 6
+#define PF1550_CHG_SUSPEND 7
+#define PF1550_CHG_OFF_INV 8
+#define PF1550_CHG_BAT_OVER 9
+#define PF1550_CHG_OFF_TEMP 10
+#define PF1550_CHG_LINEAR_ONLY 12
+#define PF1550_CHG_SNS_MASK 0xf
+#define PF1550_CHG_INT_MASK 0x51
+
+#define PF1550_BAT_NO_VBUS 0
+#define PF1550_BAT_LOW_THAN_PRECHARG 1
+#define PF1550_BAT_CHARG_FAIL 2
+#define PF1550_BAT_HIGH_THAN_PRECHARG 4
+#define PF1550_BAT_OVER_VOL 5
+#define PF1550_BAT_NO_DETECT 6
+#define PF1550_BAT_SNS_MASK 0x7
+
+#define PF1550_VBUS_UVLO BIT(2)
+#define PF1550_VBUS_IN2SYS BIT(3)
+#define PF1550_VBUS_OVLO BIT(4)
+#define PF1550_VBUS_VALID BIT(5)
+
+#define PF1550_CHARG_REG_BATT_REG_CHGCV_MASK 0x3f
+#define PF1550_CHARG_REG_BATT_REG_VMINSYS_SHIFT 6
+#define PF1550_CHARG_REG_BATT_REG_VMINSYS_MASK GENMASK(7, 6)
+#define PF1550_CHARG_REG_THM_REG_CNFG_REGTEMP_SHIFT 2
+#define PF1550_CHARG_REG_THM_REG_CNFG_REGTEMP_MASK GENMASK(3, 2)
+
+/* top level interrupt masks */
+#define IRQ_REGULATOR (BIT(1) | BIT(2) | BIT(3) | BIT(4) | BIT(6))
+#define IRQ_ONKEY BIT(5)
+#define IRQ_CHG BIT(0)
+
+/* regulator interrupt masks */
+#define PMIC_IRQ_SW1_LS BIT(0)
+#define PMIC_IRQ_SW2_LS BIT(1)
+#define PMIC_IRQ_SW3_LS BIT(2)
+#define PMIC_IRQ_SW1_HS BIT(0)
+#define PMIC_IRQ_SW2_HS BIT(1)
+#define PMIC_IRQ_SW3_HS BIT(2)
+#define PMIC_IRQ_LDO1_FAULT BIT(0)
+#define PMIC_IRQ_LDO2_FAULT BIT(1)
+#define PMIC_IRQ_LDO3_FAULT BIT(2)
+#define PMIC_IRQ_TEMP_110 BIT(0)
+#define PMIC_IRQ_TEMP_125 BIT(1)
+
+/* onkey interrupt masks */
+#define ONKEY_IRQ_PUSHI BIT(0)
+#define ONKEY_IRQ_1SI BIT(1)
+#define ONKEY_IRQ_2SI BIT(2)
+#define ONKEY_IRQ_3SI BIT(3)
+#define ONKEY_IRQ_4SI BIT(4)
+#define ONKEY_IRQ_8SI BIT(5)
+
+/* charger interrupt masks */
+#define CHARG_IRQ_BAT2SOCI BIT(1)
+#define CHARG_IRQ_BATI BIT(2)
+#define CHARG_IRQ_CHGI BIT(3)
+#define CHARG_IRQ_VBUSI BIT(5)
+#define CHARG_IRQ_DPMI BIT(6)
+#define CHARG_IRQ_THMI BIT(7)
+
+enum pf1550_irq {
+ PF1550_IRQ_CHG,
+ PF1550_IRQ_REGULATOR,
+ PF1550_IRQ_ONKEY,
+};
+
+enum pf1550_pmic_irq {
+ PF1550_PMIC_IRQ_SW1_LS,
+ PF1550_PMIC_IRQ_SW2_LS,
+ PF1550_PMIC_IRQ_SW3_LS,
+ PF1550_PMIC_IRQ_SW1_HS,
+ PF1550_PMIC_IRQ_SW2_HS,
+ PF1550_PMIC_IRQ_SW3_HS,
+ PF1550_PMIC_IRQ_LDO1_FAULT,
+ PF1550_PMIC_IRQ_LDO2_FAULT,
+ PF1550_PMIC_IRQ_LDO3_FAULT,
+ PF1550_PMIC_IRQ_TEMP_110,
+ PF1550_PMIC_IRQ_TEMP_125,
+};
+
+enum pf1550_onkey_irq {
+ PF1550_ONKEY_IRQ_PUSHI,
+ PF1550_ONKEY_IRQ_1SI,
+ PF1550_ONKEY_IRQ_2SI,
+ PF1550_ONKEY_IRQ_3SI,
+ PF1550_ONKEY_IRQ_4SI,
+ PF1550_ONKEY_IRQ_8SI,
+};
+
+enum pf1550_charg_irq {
+ PF1550_CHARG_IRQ_BAT2SOCI,
+ PF1550_CHARG_IRQ_BATI,
+ PF1550_CHARG_IRQ_CHGI,
+ PF1550_CHARG_IRQ_VBUSI,
+ PF1550_CHARG_IRQ_THMI,
+};
+
+enum pf1550_regulators {
+ PF1550_SW1,
+ PF1550_SW2,
+ PF1550_SW3,
+ PF1550_VREFDDR,
+ PF1550_LDO1,
+ PF1550_LDO2,
+ PF1550_LDO3,
+};
+
+struct pf1550_dev {
+ bool dvs_enb;
+ struct device *dev;
+ struct i2c_client *i2c;
+ struct regmap *regmap;
+ struct regmap_irq_chip_data *irq_data_regulator;
+ struct regmap_irq_chip_data *irq_data_onkey;
+ struct regmap_irq_chip_data *irq_data_charger;
+ struct regmap_irq_chip_data *irq_data;
+ int irq;
+};
+
+#endif /* __LINUX_MFD_PF1550_H */
--
2.49.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v7 2/6] mfd: pf1550: add core mfd driver
2025-06-12 14:55 ` [PATCH v7 2/6] mfd: pf1550: add core mfd driver Samuel Kayode via B4 Relay
@ 2025-06-19 13:03 ` Lee Jones
2025-06-20 18:43 ` Samuel Kayode
0 siblings, 1 reply; 17+ messages in thread
From: Lee Jones @ 2025-06-19 13:03 UTC (permalink / raw)
To: samuel.kayode
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Liam Girdwood,
Mark Brown, Dmitry Torokhov, Sebastian Reichel, Frank Li, imx,
devicetree, linux-kernel, linux-input, linux-pm, Abel Vesa,
Abel Vesa, Robin Gong, Robin Gong, Enric Balletbo i Serra
Remove mention of MFD from the subject.
Consider renaming for something like "core driver" or use whatever this
driver really is. MFD isn't a real thing - we made it up.
On Thu, 12 Jun 2025, Samuel Kayode via B4 Relay wrote:
> From: Samuel Kayode <samuel.kayode@savoirfairelinux.com>
>
> Add the core mfd driver for pf1550 PMIC. There are 3 subdevices for
Same here.
> which the drivers will be added in subsequent patches.
>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Samuel Kayode <samuel.kayode@savoirfairelinux.com>
> ---
> v7:
> - Address Frank's feedback:
> - Ensure reverse christmas tree order for local variable definitions
> - Drop unnecessary driver data definition in id table
> v6:
> - Address Frank's feedback:
> - Ensure lowercase when defining register addresses
> - Use GENMASK macro for masking
> - Hardcode IRQ flags in pf1550_add_child_device
> - Add dvs_enb variable for SW2 regulator
> - Drop chip type variable
> v5:
> - Use top level interrupt to manage interrupts for the sub-drivers as
> recommended by Mark Brown. The regmap_irq_sub_irq_map would have been used
> if not for the irregular charger irq address. For all children, the mask
> register is directly after the irq register (i.e., 0x08, 0x09) except
> for the charger: 0x80, 0x82. Meaning .mask_base would be applicable
> for all but the charger
> - Fix bad offset for temperature interrupts of regulator
> v4:
> - Use struct resource to define irq so platform_get_irq can be used in
> children as suggested by Dmitry
> - Let mfd_add_devices create the mappings for the interrupts
> - ack_base and init_ack_masked defined for charger and regulator irq
> chips
> - No need to define driver_data in table id
> v3:
> - Address Dmitry's feedback:
> - Place Table IDs next to each other
> - Drop of_match_ptr
> - Replace dev_err with dev_err_probe in probe method
> - Drop useless log in probe
> - Map all irqs instead of doing it in the sub-devices as recommended by
> Dmitry.
> v2:
> - Address feedback from Enric Balletbo Serra
> ---
> drivers/mfd/Kconfig | 14 ++
> drivers/mfd/Makefile | 2 +
> drivers/mfd/pf1550.c | 339 +++++++++++++++++++++++++++++++++++++++++++++
> include/linux/mfd/pf1550.h | 254 +++++++++++++++++++++++++++++++++
> 4 files changed, 609 insertions(+)
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 96992af22565205716d72db0494c7bf2567b045e..de3fc9c5e88b5c2a2c7325e2ceeb8f9c3ca057de 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -558,6 +558,20 @@ config MFD_MX25_TSADC
> i.MX25 processors. They consist of a conversion queue for general
> purpose ADC and a queue for Touchscreens.
>
> +config MFD_PF1550
> + tristate "NXP PF1550 PMIC Support"
> + depends on I2C=y && OF
> + select MFD_CORE
> + select REGMAP_I2C
> + select REGMAP_IRQ
> + help
> + Say yes here to add support for NXP PF1550.
No need for the early line break. It harms readability.
> + This is a companion Power Management IC with regulators, onkey,
> + and charger control on chip.
As above.
> + This driver provides common support for accessing the device;
> + additional drivers must be enabled in order to use the functionality
> + of the device.
What is the module called?
> +
> config MFD_HI6421_PMIC
> tristate "HiSilicon Hi6421 PMU/Codec IC"
> depends on OF
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 5e5cc279af6036a6b3ea1f1f0feeddf45b85f15c..7391d1b81d1ee499507b4ac24ff00eb2e344d60b 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -120,6 +120,8 @@ obj-$(CONFIG_MFD_MC13XXX) += mc13xxx-core.o
> obj-$(CONFIG_MFD_MC13XXX_SPI) += mc13xxx-spi.o
> obj-$(CONFIG_MFD_MC13XXX_I2C) += mc13xxx-i2c.o
>
> +obj-$(CONFIG_MFD_PF1550) += pf1550.o
> +
> obj-$(CONFIG_MFD_CORE) += mfd-core.o
>
> ocelot-soc-objs := ocelot-core.o ocelot-spi.o
> diff --git a/drivers/mfd/pf1550.c b/drivers/mfd/pf1550.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..f42c35b6f3c7ce63d615545e85fb2a143fccb488
> --- /dev/null
> +++ b/drivers/mfd/pf1550.c
> @@ -0,0 +1,339 @@
> +// SPDX-License-Identifier: GPL-2.0
This is the only line that should use C++ style comments.
> +//
> +// pf1550.c - mfd core driver for the PF1550
Remove filenames - they have a tendency to rot.
It's not an MFD anything.
> +//
> +// Copyright (C) 2016 Freescale Semiconductor, Inc.
> +// Robin Gong <yibin.gong@freescale.com>
> +//
> +// Portions Copyright (c) 2025 Savoir-faire Linux Inc.
> +// Samuel Kayode <samuel.kayode@savoirfairelinux.com>
> +//
> +// This driver is based on max77693.c
Drop this line.
> +//
Drop empty comments.
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/pf1550.h>
> +#include <linux/of.h>
> +#include <linux/regmap.h>
> +
Alphabetical.
> +static const struct regmap_config pf1550_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .max_register = PF1550_PMIC_REG_END,
> +};
> +
> +static const struct regmap_irq pf1550_irqs[] = {
> + REGMAP_IRQ_REG(PF1550_IRQ_CHG, 0, IRQ_CHG),
> + REGMAP_IRQ_REG(PF1550_IRQ_REGULATOR, 0, IRQ_REGULATOR),
> + REGMAP_IRQ_REG(PF1550_IRQ_ONKEY, 0, IRQ_ONKEY),
> +};
Not sure this or subsequent tabbing helps in any way.
> +
> +static const struct regmap_irq_chip pf1550_irq_chip = {
> + .name = "pf1550",
> + .status_base = PF1550_PMIC_REG_INT_CATEGORY,
> + .init_ack_masked = 1,
> + .num_regs = 1,
> + .irqs = pf1550_irqs,
> + .num_irqs = ARRAY_SIZE(pf1550_irqs),
> +};
> +
> +static const struct regmap_irq pf1550_regulator_irqs[] = {
> + REGMAP_IRQ_REG(PF1550_PMIC_IRQ_SW1_LS, 0, PMIC_IRQ_SW1_LS),
> + REGMAP_IRQ_REG(PF1550_PMIC_IRQ_SW2_LS, 0, PMIC_IRQ_SW2_LS),
> + REGMAP_IRQ_REG(PF1550_PMIC_IRQ_SW3_LS, 0, PMIC_IRQ_SW3_LS),
> + REGMAP_IRQ_REG(PF1550_PMIC_IRQ_SW1_HS, 3, PMIC_IRQ_SW1_HS),
> + REGMAP_IRQ_REG(PF1550_PMIC_IRQ_SW2_HS, 3, PMIC_IRQ_SW2_HS),
> + REGMAP_IRQ_REG(PF1550_PMIC_IRQ_SW3_HS, 3, PMIC_IRQ_SW3_HS),
> + REGMAP_IRQ_REG(PF1550_PMIC_IRQ_LDO1_FAULT, 16, PMIC_IRQ_LDO1_FAULT),
> + REGMAP_IRQ_REG(PF1550_PMIC_IRQ_LDO2_FAULT, 16, PMIC_IRQ_LDO2_FAULT),
> + REGMAP_IRQ_REG(PF1550_PMIC_IRQ_LDO3_FAULT, 16, PMIC_IRQ_LDO3_FAULT),
> + REGMAP_IRQ_REG(PF1550_PMIC_IRQ_TEMP_110, 24, PMIC_IRQ_TEMP_110),
> + REGMAP_IRQ_REG(PF1550_PMIC_IRQ_TEMP_125, 24, PMIC_IRQ_TEMP_125),
> +};
> +
> +static const struct regmap_irq_chip pf1550_regulator_irq_chip = {
> + .name = "pf1550-regulator",
> + .status_base = PF1550_PMIC_REG_SW_INT_STAT0,
> + .ack_base = PF1550_PMIC_REG_SW_INT_STAT0,
> + .mask_base = PF1550_PMIC_REG_SW_INT_MASK0,
> + .use_ack = 1,
> + .init_ack_masked = 1,
> + .num_regs = 25,
> + .irqs = pf1550_regulator_irqs,
> + .num_irqs = ARRAY_SIZE(pf1550_regulator_irqs),
> +};
> +
> +static const struct resource regulator_resources[] = {
> + DEFINE_RES_IRQ(PF1550_PMIC_IRQ_SW1_LS),
> + DEFINE_RES_IRQ(PF1550_PMIC_IRQ_SW2_LS),
> + DEFINE_RES_IRQ(PF1550_PMIC_IRQ_SW3_LS),
> + DEFINE_RES_IRQ(PF1550_PMIC_IRQ_SW1_HS),
> + DEFINE_RES_IRQ(PF1550_PMIC_IRQ_SW2_HS),
> + DEFINE_RES_IRQ(PF1550_PMIC_IRQ_SW3_HS),
> + DEFINE_RES_IRQ(PF1550_PMIC_IRQ_LDO1_FAULT),
> + DEFINE_RES_IRQ(PF1550_PMIC_IRQ_LDO2_FAULT),
> + DEFINE_RES_IRQ(PF1550_PMIC_IRQ_LDO3_FAULT),
> + DEFINE_RES_IRQ(PF1550_PMIC_IRQ_TEMP_110),
> + DEFINE_RES_IRQ(PF1550_PMIC_IRQ_TEMP_125),
> +};
> +
> +static const struct regmap_irq pf1550_onkey_irqs[] = {
> + REGMAP_IRQ_REG(PF1550_ONKEY_IRQ_PUSHI, 0, ONKEY_IRQ_PUSHI),
> + REGMAP_IRQ_REG(PF1550_ONKEY_IRQ_1SI, 0, ONKEY_IRQ_1SI),
> + REGMAP_IRQ_REG(PF1550_ONKEY_IRQ_2SI, 0, ONKEY_IRQ_2SI),
> + REGMAP_IRQ_REG(PF1550_ONKEY_IRQ_3SI, 0, ONKEY_IRQ_3SI),
> + REGMAP_IRQ_REG(PF1550_ONKEY_IRQ_4SI, 0, ONKEY_IRQ_4SI),
> + REGMAP_IRQ_REG(PF1550_ONKEY_IRQ_8SI, 0, ONKEY_IRQ_8SI),
> +};
> +
> +static const struct regmap_irq_chip pf1550_onkey_irq_chip = {
> + .name = "pf1550-onkey",
> + .status_base = PF1550_PMIC_REG_ONKEY_INT_STAT0,
> + .ack_base = PF1550_PMIC_REG_ONKEY_INT_STAT0,
> + .mask_base = PF1550_PMIC_REG_ONKEY_INT_MASK0,
> + .use_ack = 1,
> + .init_ack_masked = 1,
> + .num_regs = 1,
> + .irqs = pf1550_onkey_irqs,
> + .num_irqs = ARRAY_SIZE(pf1550_onkey_irqs),
> +};
> +
> +static const struct resource onkey_resources[] = {
> + DEFINE_RES_IRQ(PF1550_ONKEY_IRQ_PUSHI),
> + DEFINE_RES_IRQ(PF1550_ONKEY_IRQ_1SI),
> + DEFINE_RES_IRQ(PF1550_ONKEY_IRQ_2SI),
> + DEFINE_RES_IRQ(PF1550_ONKEY_IRQ_3SI),
> + DEFINE_RES_IRQ(PF1550_ONKEY_IRQ_4SI),
> + DEFINE_RES_IRQ(PF1550_ONKEY_IRQ_8SI),
> +};
> +
> +static const struct regmap_irq pf1550_charger_irqs[] = {
> + REGMAP_IRQ_REG(PF1550_CHARG_IRQ_BAT2SOCI, 0, CHARG_IRQ_BAT2SOCI),
> + REGMAP_IRQ_REG(PF1550_CHARG_IRQ_BATI, 0, CHARG_IRQ_BATI),
> + REGMAP_IRQ_REG(PF1550_CHARG_IRQ_CHGI, 0, CHARG_IRQ_CHGI),
> + REGMAP_IRQ_REG(PF1550_CHARG_IRQ_VBUSI, 0, CHARG_IRQ_VBUSI),
> + REGMAP_IRQ_REG(PF1550_CHARG_IRQ_THMI, 0, CHARG_IRQ_THMI),
> +};
> +
> +static const struct regmap_irq_chip pf1550_charger_irq_chip = {
> + .name = "pf1550-charger",
> + .status_base = PF1550_CHARG_REG_CHG_INT,
> + .ack_base = PF1550_CHARG_REG_CHG_INT,
> + .mask_base = PF1550_CHARG_REG_CHG_INT_MASK,
> + .use_ack = 1,
> + .init_ack_masked = 1,
> + .num_regs = 1,
> + .irqs = pf1550_charger_irqs,
> + .num_irqs = ARRAY_SIZE(pf1550_charger_irqs),
> +};
> +
> +static const struct resource charger_resources[] = {
> + DEFINE_RES_IRQ(PF1550_CHARG_IRQ_BAT2SOCI),
> + DEFINE_RES_IRQ(PF1550_CHARG_IRQ_BATI),
> + DEFINE_RES_IRQ(PF1550_CHARG_IRQ_CHGI),
> + DEFINE_RES_IRQ(PF1550_CHARG_IRQ_VBUSI),
> + DEFINE_RES_IRQ(PF1550_CHARG_IRQ_THMI),
> +};
> +
> +static const struct mfd_cell pf1550_regulator_cell = {
> + .name = "pf1550-regulator",
> + .num_resources = ARRAY_SIZE(regulator_resources),
> + .resources = regulator_resources,
> +};
> +
> +static const struct mfd_cell pf1550_onkey_cell = {
> + .name = "pf1550-onkey",
> + .num_resources = ARRAY_SIZE(onkey_resources),
> + .resources = onkey_resources,
> +};
> +
> +static const struct mfd_cell pf1550_charger_cell = {
> + .name = "pf1550-charger",
> + .num_resources = ARRAY_SIZE(charger_resources),
> + .resources = charger_resources,
> +};
> +
> +static int pf1550_read_otp(const struct pf1550_dev *pf1550, unsigned int index,
What does OTP mean?
Why do you have to write to 4 registers first?
This should all be made clear in some way or another.
> + unsigned int *val)
> +{
> + int ret = 0;
> +
> + ret = regmap_write(pf1550->regmap, PF1550_PMIC_REG_KEY, 0x15);
No magic numbers. These should all be defined.
> + if (ret)
> + goto read_err;
> + ret = regmap_write(pf1550->regmap, PF1550_CHARG_REG_CHGR_KEY2, 0x50);
> + if (ret)
> + goto read_err;
> + ret = regmap_write(pf1550->regmap, PF1550_TEST_REG_KEY3, 0xab);
> + if (ret)
> + goto read_err;
> + ret = regmap_write(pf1550->regmap, PF1550_TEST_REG_FMRADDR, index);
> + if (ret)
> + goto read_err;
> + ret = regmap_read(pf1550->regmap, PF1550_TEST_REG_FMRDATA, val);
> + if (ret)
> + goto read_err;
> +
> + return 0;
> +
> +read_err:
> + dev_err_probe(pf1550->dev, ret, "read otp reg %x found!\n", index);
This doesn't look like a fail message?
Besides, it should be on the 'return' line, or else what's the point?
> + return ret;
> +}
> +
> +static int pf1550_add_child_device(struct pf1550_dev *pmic,
> + const struct mfd_cell *cell,
> + struct regmap_irq_chip_data *pdata,
This is not pdata.
> + int pirq,
> + const struct regmap_irq_chip *chip,
> + struct regmap_irq_chip_data **data)
> +{
> + struct device *dev = pmic->dev;
> + struct irq_domain *domain;
> + int irq, ret;
> +
> + irq = regmap_irq_get_virq(pdata, pirq);
> + if (irq < 0)
> + return dev_err_probe(dev, irq,
> + "Failed to get parent vIRQ(%d) for chip %s\n",
> + pirq, chip->name);
> +
> + ret = devm_regmap_add_irq_chip(dev, pmic->regmap, irq,
> + IRQF_ONESHOT | IRQF_SHARED |
> + IRQF_TRIGGER_FALLING, 0, chip, data);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Failed to add %s IRQ chip\n",
> + chip->name);
> +
> + domain = regmap_irq_get_domain(*data);
> +
> + return devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE, cell, 1,
> + NULL, 0, domain);
Why can't all 3 devices be registered in one call?
> +}
To be honest, the premise around this function is a bit of a mess.
Please move all of this into .probe().
> +static int pf1550_i2c_probe(struct i2c_client *i2c)
> +{
> + const struct mfd_cell *regulator = &pf1550_regulator_cell;
> + const struct mfd_cell *charger = &pf1550_charger_cell;
> + const struct mfd_cell *onkey = &pf1550_onkey_cell;
> + unsigned int reg_data = 0, otp_data = 0;
> + struct pf1550_dev *pf1550;
> + int ret = 0;
> +
> + pf1550 = devm_kzalloc(&i2c->dev, sizeof(*pf1550), GFP_KERNEL);
> + if (!pf1550)
> + return -ENOMEM;
> +
> + i2c_set_clientdata(i2c, pf1550);
> + pf1550->dev = &i2c->dev;
> + pf1550->i2c = i2c;
What are you storing i2c for?
Either store dev and irq OR i2c. You don't need all three.
> + pf1550->irq = i2c->irq;
> + pf1550->dvs_enb = false;
This is already false.
> + pf1550->regmap = devm_regmap_init_i2c(i2c, &pf1550_regmap_config);
> + if (IS_ERR(pf1550->regmap))
> + return dev_err_probe(pf1550->dev, PTR_ERR(pf1550->regmap),
> + "failed to allocate register map\n");
> +
> + ret = regmap_read(pf1550->regmap, PF1550_PMIC_REG_DEVICE_ID, ®_data);
> + if (ret < 0 || reg_data != PF1550_DEVICE_ID)
> + return dev_err_probe(pf1550->dev, ret ?: -EINVAL,
> + "device not found!\n");
Are you sure? What if the wrong device was found?
Also, no device should return -ENODEV.
> +
> + /* regulator DVS */
All comments should start with an uppercase char.
> + ret = pf1550_read_otp(pf1550, 0x1f, &otp_data);
No magic numbers. Define them please.
> + if (ret)
> + return ret;
> +
> + if (otp_data & BIT(3))
> + pf1550->dvs_enb = true;
Gibberish. More defines and/or comments please.
> + /* add top level interrupts */
> + ret = devm_regmap_add_irq_chip(pf1550->dev, pf1550->regmap, pf1550->irq,
> + IRQF_ONESHOT | IRQF_SHARED |
> + IRQF_TRIGGER_FALLING,
> + 0, &pf1550_irq_chip,
> + &pf1550->irq_data);
> + if (ret)
> + return ret;
> +
> + ret = pf1550_add_child_device(pf1550, regulator, pf1550->irq_data,
> + PF1550_IRQ_REGULATOR,
> + &pf1550_regulator_irq_chip,
> + &pf1550->irq_data_regulator);
> + if (ret)
> + return ret;
> +
> + ret = pf1550_add_child_device(pf1550, onkey, pf1550->irq_data,
> + PF1550_IRQ_ONKEY,
> + &pf1550_onkey_irq_chip,
> + &pf1550->irq_data_onkey);
> + if (ret)
> + return ret;
> +
> + ret = pf1550_add_child_device(pf1550, charger, pf1550->irq_data,
> + PF1550_IRQ_CHG,
> + &pf1550_charger_irq_chip,
> + &pf1550->irq_data_charger);
> + return ret;
> +}
> +
> +static int pf1550_suspend(struct device *dev)
> +{
> + struct i2c_client *i2c = container_of(dev, struct i2c_client, dev);
> + struct pf1550_dev *pf1550 = i2c_get_clientdata(i2c);
You can swap all of this for:
struct pf1550_dev *pf1550 = dev_get_drvdata(dev).
> +
> + if (device_may_wakeup(dev)) {
> + enable_irq_wake(pf1550->irq);
> + disable_irq(pf1550->irq);
> + }
> +
> + return 0;
> +}
> +
> +static int pf1550_resume(struct device *dev)
> +{
> + struct i2c_client *i2c = container_of(dev, struct i2c_client, dev);
> + struct pf1550_dev *pf1550 = i2c_get_clientdata(i2c);
As above.
> +
> + if (device_may_wakeup(dev)) {
> + disable_irq_wake(pf1550->irq);
> + enable_irq(pf1550->irq);
I would normally expect these to be around the opposite way to the ones
in .suspend().
> + }
> +
> + return 0;
> +}
> +
Remove this line.
> +static DEFINE_SIMPLE_DEV_PM_OPS(pf1550_pm, pf1550_suspend, pf1550_resume);
> +
> +static const struct i2c_device_id pf1550_i2c_id[] = {
> + { "pf1550" },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(i2c, pf1550_i2c_id);
> +
> +static const struct of_device_id pf1550_dt_match[] = {
> + { .compatible = "nxp,pf1550" },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, pf1550_dt_match);
> +
> +static struct i2c_driver pf1550_i2c_driver = {
> + .driver = {
> + .name = "pf1550",
> + .pm = pm_sleep_ptr(&pf1550_pm),
> + .of_match_table = pf1550_dt_match,
> + },
> + .probe = pf1550_i2c_probe,
> + .id_table = pf1550_i2c_id,
> +};
> +module_i2c_driver(pf1550_i2c_driver);
> +
> +MODULE_DESCRIPTION("NXP PF1550 multi-function core driver");
Reword please.
> +MODULE_AUTHOR("Robin Gong <yibin.gong@freescale.com>");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/pf1550.h b/include/linux/mfd/pf1550.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..64ff475215cced21a5ebc24f799f48315a51d260
> --- /dev/null
> +++ b/include/linux/mfd/pf1550.h
> @@ -0,0 +1,254 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * pf1550.h - mfd head file for PF1550
As above.
> + *
> + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> + * Robin Gong <yibin.gong@freescale.com>
> + *
> + * Portions Copyright (c) 2025 Savoir-faire Linux Inc.
> + * Samuel Kayode <samuel.kayode@savoirfairelinux.com>
> + */
> +
> +#ifndef __LINUX_MFD_PF1550_H
> +#define __LINUX_MFD_PF1550_H
> +
> +#include <linux/i2c.h>
> +#include <linux/regmap.h>
> +
> +enum pf1550_pmic_reg {
> + /* PMIC regulator part */
> + PF1550_PMIC_REG_DEVICE_ID = 0x00,
> + PF1550_PMIC_REG_OTP_FLAVOR = 0x01,
> + PF1550_PMIC_REG_SILICON_REV = 0x02,
> +
> + PF1550_PMIC_REG_INT_CATEGORY = 0x06,
> + PF1550_PMIC_REG_SW_INT_STAT0 = 0x08,
> + PF1550_PMIC_REG_SW_INT_MASK0 = 0x09,
> + PF1550_PMIC_REG_SW_INT_SENSE0 = 0x0a,
> + PF1550_PMIC_REG_SW_INT_STAT1 = 0x0b,
> + PF1550_PMIC_REG_SW_INT_MASK1 = 0x0c,
> + PF1550_PMIC_REG_SW_INT_SENSE1 = 0x0d,
> + PF1550_PMIC_REG_SW_INT_STAT2 = 0x0e,
> + PF1550_PMIC_REG_SW_INT_MASK2 = 0x0f,
> + PF1550_PMIC_REG_SW_INT_SENSE2 = 0x10,
> + PF1550_PMIC_REG_LDO_INT_STAT0 = 0x18,
> + PF1550_PMIC_REG_LDO_INT_MASK0 = 0x19,
> + PF1550_PMIC_REG_LDO_INT_SENSE0 = 0x1a,
> + PF1550_PMIC_REG_TEMP_INT_STAT0 = 0x20,
> + PF1550_PMIC_REG_TEMP_INT_MASK0 = 0x21,
> + PF1550_PMIC_REG_TEMP_INT_SENSE0 = 0x22,
> + PF1550_PMIC_REG_ONKEY_INT_STAT0 = 0x24,
> + PF1550_PMIC_REG_ONKEY_INT_MASK0 = 0x25,
> + PF1550_PMIC_REG_ONKEY_INT_SENSE0 = 0x26,
> + PF1550_PMIC_REG_MISC_INT_STAT0 = 0x28,
> + PF1550_PMIC_REG_MISC_INT_MASK0 = 0x29,
> + PF1550_PMIC_REG_MISC_INT_SENSE0 = 0x2a,
> +
> + PF1550_PMIC_REG_COINCELL_CONTROL = 0x30,
> +
> + PF1550_PMIC_REG_SW1_VOLT = 0x32,
> + PF1550_PMIC_REG_SW1_STBY_VOLT = 0x33,
> + PF1550_PMIC_REG_SW1_SLP_VOLT = 0x34,
> + PF1550_PMIC_REG_SW1_CTRL = 0x35,
> + PF1550_PMIC_REG_SW1_CTRL1 = 0x36,
> + PF1550_PMIC_REG_SW2_VOLT = 0x38,
> + PF1550_PMIC_REG_SW2_STBY_VOLT = 0x39,
> + PF1550_PMIC_REG_SW2_SLP_VOLT = 0x3a,
> + PF1550_PMIC_REG_SW2_CTRL = 0x3b,
> + PF1550_PMIC_REG_SW2_CTRL1 = 0x3c,
> + PF1550_PMIC_REG_SW3_VOLT = 0x3e,
> + PF1550_PMIC_REG_SW3_STBY_VOLT = 0x3f,
> + PF1550_PMIC_REG_SW3_SLP_VOLT = 0x40,
> + PF1550_PMIC_REG_SW3_CTRL = 0x41,
> + PF1550_PMIC_REG_SW3_CTRL1 = 0x42,
> + PF1550_PMIC_REG_VSNVS_CTRL = 0x48,
> + PF1550_PMIC_REG_VREFDDR_CTRL = 0x4a,
> + PF1550_PMIC_REG_LDO1_VOLT = 0x4c,
> + PF1550_PMIC_REG_LDO1_CTRL = 0x4d,
> + PF1550_PMIC_REG_LDO2_VOLT = 0x4f,
> + PF1550_PMIC_REG_LDO2_CTRL = 0x50,
> + PF1550_PMIC_REG_LDO3_VOLT = 0x52,
> + PF1550_PMIC_REG_LDO3_CTRL = 0x53,
> + PF1550_PMIC_REG_PWRCTRL0 = 0x58,
> + PF1550_PMIC_REG_PWRCTRL1 = 0x59,
> + PF1550_PMIC_REG_PWRCTRL2 = 0x5a,
> + PF1550_PMIC_REG_PWRCTRL3 = 0x5b,
> + PF1550_PMIC_REG_SW1_PWRDN_SEQ = 0x5f,
> + PF1550_PMIC_REG_SW2_PWRDN_SEQ = 0x60,
> + PF1550_PMIC_REG_SW3_PWRDN_SEQ = 0x61,
> + PF1550_PMIC_REG_LDO1_PWRDN_SEQ = 0x62,
> + PF1550_PMIC_REG_LDO2_PWRDN_SEQ = 0x63,
> + PF1550_PMIC_REG_LDO3_PWRDN_SEQ = 0x64,
> + PF1550_PMIC_REG_VREFDDR_PWRDN_SEQ = 0x65,
> +
> + PF1550_PMIC_REG_STATE_INFO = 0x67,
> + PF1550_PMIC_REG_I2C_ADDR = 0x68,
> + PF1550_PMIC_REG_IO_DRV0 = 0x69,
> + PF1550_PMIC_REG_IO_DRV1 = 0x6a,
> + PF1550_PMIC_REG_RC_16MHZ = 0x6b,
> + PF1550_PMIC_REG_KEY = 0x6f,
> +
> + /* charger part */
> + PF1550_CHARG_REG_CHG_INT = 0x80,
> + PF1550_CHARG_REG_CHG_INT_MASK = 0x82,
> + PF1550_CHARG_REG_CHG_INT_OK = 0x84,
> + PF1550_CHARG_REG_VBUS_SNS = 0x86,
> + PF1550_CHARG_REG_CHG_SNS = 0x87,
> + PF1550_CHARG_REG_BATT_SNS = 0x88,
> + PF1550_CHARG_REG_CHG_OPER = 0x89,
> + PF1550_CHARG_REG_CHG_TMR = 0x8a,
> + PF1550_CHARG_REG_CHG_EOC_CNFG = 0x8d,
> + PF1550_CHARG_REG_CHG_CURR_CNFG = 0x8e,
> + PF1550_CHARG_REG_BATT_REG = 0x8f,
> + PF1550_CHARG_REG_BATFET_CNFG = 0x91,
> + PF1550_CHARG_REG_THM_REG_CNFG = 0x92,
> + PF1550_CHARG_REG_VBUS_INLIM_CNFG = 0x94,
> + PF1550_CHARG_REG_VBUS_LIN_DPM = 0x95,
> + PF1550_CHARG_REG_USB_PHY_LDO_CNFG = 0x96,
> + PF1550_CHARG_REG_DBNC_DELAY_TIME = 0x98,
> + PF1550_CHARG_REG_CHG_INT_CNFG = 0x99,
> + PF1550_CHARG_REG_THM_ADJ_SETTING = 0x9a,
> + PF1550_CHARG_REG_VBUS2SYS_CNFG = 0x9b,
> + PF1550_CHARG_REG_LED_PWM = 0x9c,
> + PF1550_CHARG_REG_FAULT_BATFET_CNFG = 0x9d,
> + PF1550_CHARG_REG_LED_CNFG = 0x9e,
> + PF1550_CHARG_REG_CHGR_KEY2 = 0x9f,
> +
> + PF1550_TEST_REG_FMRADDR = 0xc4,
> + PF1550_TEST_REG_FMRDATA = 0xc5,
> + PF1550_TEST_REG_KEY3 = 0xdf,
> +
> + PF1550_PMIC_REG_END = 0xff,
> +};
> +
> +#define PF1550_DEVICE_ID 0x7c
> +
> +#define PF1550_CHG_TURNON 0x2
> +
> +#define PF1550_CHG_PRECHARGE 0
> +#define PF1550_CHG_CONSTANT_CURRENT 1
> +#define PF1550_CHG_CONSTANT_VOL 2
> +#define PF1550_CHG_EOC 3
> +#define PF1550_CHG_DONE 4
> +#define PF1550_CHG_TIMER_FAULT 6
> +#define PF1550_CHG_SUSPEND 7
> +#define PF1550_CHG_OFF_INV 8
> +#define PF1550_CHG_BAT_OVER 9
> +#define PF1550_CHG_OFF_TEMP 10
> +#define PF1550_CHG_LINEAR_ONLY 12
> +#define PF1550_CHG_SNS_MASK 0xf
> +#define PF1550_CHG_INT_MASK 0x51
> +
> +#define PF1550_BAT_NO_VBUS 0
> +#define PF1550_BAT_LOW_THAN_PRECHARG 1
> +#define PF1550_BAT_CHARG_FAIL 2
> +#define PF1550_BAT_HIGH_THAN_PRECHARG 4
> +#define PF1550_BAT_OVER_VOL 5
> +#define PF1550_BAT_NO_DETECT 6
> +#define PF1550_BAT_SNS_MASK 0x7
> +
> +#define PF1550_VBUS_UVLO BIT(2)
> +#define PF1550_VBUS_IN2SYS BIT(3)
> +#define PF1550_VBUS_OVLO BIT(4)
> +#define PF1550_VBUS_VALID BIT(5)
> +
> +#define PF1550_CHARG_REG_BATT_REG_CHGCV_MASK 0x3f
> +#define PF1550_CHARG_REG_BATT_REG_VMINSYS_SHIFT 6
> +#define PF1550_CHARG_REG_BATT_REG_VMINSYS_MASK GENMASK(7, 6)
> +#define PF1550_CHARG_REG_THM_REG_CNFG_REGTEMP_SHIFT 2
> +#define PF1550_CHARG_REG_THM_REG_CNFG_REGTEMP_MASK GENMASK(3, 2)
> +
> +/* top level interrupt masks */
> +#define IRQ_REGULATOR (BIT(1) | BIT(2) | BIT(3) | BIT(4) | BIT(6))
> +#define IRQ_ONKEY BIT(5)
> +#define IRQ_CHG BIT(0)
> +
> +/* regulator interrupt masks */
> +#define PMIC_IRQ_SW1_LS BIT(0)
> +#define PMIC_IRQ_SW2_LS BIT(1)
> +#define PMIC_IRQ_SW3_LS BIT(2)
> +#define PMIC_IRQ_SW1_HS BIT(0)
> +#define PMIC_IRQ_SW2_HS BIT(1)
> +#define PMIC_IRQ_SW3_HS BIT(2)
> +#define PMIC_IRQ_LDO1_FAULT BIT(0)
> +#define PMIC_IRQ_LDO2_FAULT BIT(1)
> +#define PMIC_IRQ_LDO3_FAULT BIT(2)
> +#define PMIC_IRQ_TEMP_110 BIT(0)
> +#define PMIC_IRQ_TEMP_125 BIT(1)
> +
> +/* onkey interrupt masks */
> +#define ONKEY_IRQ_PUSHI BIT(0)
> +#define ONKEY_IRQ_1SI BIT(1)
> +#define ONKEY_IRQ_2SI BIT(2)
> +#define ONKEY_IRQ_3SI BIT(3)
> +#define ONKEY_IRQ_4SI BIT(4)
> +#define ONKEY_IRQ_8SI BIT(5)
> +
> +/* charger interrupt masks */
> +#define CHARG_IRQ_BAT2SOCI BIT(1)
> +#define CHARG_IRQ_BATI BIT(2)
> +#define CHARG_IRQ_CHGI BIT(3)
> +#define CHARG_IRQ_VBUSI BIT(5)
> +#define CHARG_IRQ_DPMI BIT(6)
> +#define CHARG_IRQ_THMI BIT(7)
> +
> +enum pf1550_irq {
> + PF1550_IRQ_CHG,
> + PF1550_IRQ_REGULATOR,
> + PF1550_IRQ_ONKEY,
> +};
> +
> +enum pf1550_pmic_irq {
> + PF1550_PMIC_IRQ_SW1_LS,
> + PF1550_PMIC_IRQ_SW2_LS,
> + PF1550_PMIC_IRQ_SW3_LS,
> + PF1550_PMIC_IRQ_SW1_HS,
> + PF1550_PMIC_IRQ_SW2_HS,
> + PF1550_PMIC_IRQ_SW3_HS,
> + PF1550_PMIC_IRQ_LDO1_FAULT,
> + PF1550_PMIC_IRQ_LDO2_FAULT,
> + PF1550_PMIC_IRQ_LDO3_FAULT,
> + PF1550_PMIC_IRQ_TEMP_110,
> + PF1550_PMIC_IRQ_TEMP_125,
> +};
> +
> +enum pf1550_onkey_irq {
> + PF1550_ONKEY_IRQ_PUSHI,
> + PF1550_ONKEY_IRQ_1SI,
> + PF1550_ONKEY_IRQ_2SI,
> + PF1550_ONKEY_IRQ_3SI,
> + PF1550_ONKEY_IRQ_4SI,
> + PF1550_ONKEY_IRQ_8SI,
> +};
> +
> +enum pf1550_charg_irq {
> + PF1550_CHARG_IRQ_BAT2SOCI,
> + PF1550_CHARG_IRQ_BATI,
> + PF1550_CHARG_IRQ_CHGI,
> + PF1550_CHARG_IRQ_VBUSI,
> + PF1550_CHARG_IRQ_THMI,
> +};
> +
> +enum pf1550_regulators {
> + PF1550_SW1,
> + PF1550_SW2,
> + PF1550_SW3,
> + PF1550_VREFDDR,
> + PF1550_LDO1,
> + PF1550_LDO2,
> + PF1550_LDO3,
> +};
> +
> +struct pf1550_dev {
pf1550_ddata
> + bool dvs_enb;
> + struct device *dev;
> + struct i2c_client *i2c;
One or the other. You don't need both.
> + struct regmap *regmap;
> + struct regmap_irq_chip_data *irq_data_regulator;
> + struct regmap_irq_chip_data *irq_data_onkey;
> + struct regmap_irq_chip_data *irq_data_charger;
> + struct regmap_irq_chip_data *irq_data;
> + int irq;
> +};
> +
> +#endif /* __LINUX_MFD_PF1550_H */
>
> --
> 2.49.0
>
>
--
Lee Jones [李琼斯]
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v7 2/6] mfd: pf1550: add core mfd driver
2025-06-19 13:03 ` Lee Jones
@ 2025-06-20 18:43 ` Samuel Kayode
2025-06-24 16:02 ` Lee Jones
0 siblings, 1 reply; 17+ messages in thread
From: Samuel Kayode @ 2025-06-20 18:43 UTC (permalink / raw)
To: Lee Jones
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Liam Girdwood,
Mark Brown, Dmitry Torokhov, Sebastian Reichel, Frank Li, imx,
devicetree, linux-kernel, linux-input, linux-pm, Abel Vesa,
Abel Vesa, Robin Gong, Robin Gong, Enric Balletbo i Serra
Hi Lee,
Thanks a lot for the review.
On Thu, Jun 19, 2025 at 02:03:37PM +0100, Lee Jones wrote:
> > +static int pf1550_read_otp(const struct pf1550_dev *pf1550, unsigned int index,
>
> What does OTP mean?
>
It's a One-Time Programmable memory with configuration for the pf1550. I will
expand on this in the commit description of the next version.
> Why do you have to write to 4 registers first?
>
The pf1550 was designed such that the registers of the accompanying OTP is
accessed indirectly. Valid keys have to be written to specific pf1550
registers. After writing the keys, the address of the OTP register to be read
is then written to PF1550_TEST_REG_FMRADDR and its corresponding value read from
PF1550_TEST_REG_FMRDATA.
> This should all be made clear in some way or another.
>
I'll be adding comments on this in the next version.
> > + unsigned int *val)
> > +{
> > + int ret = 0;
> > +
> > + ret = regmap_write(pf1550->regmap, PF1550_PMIC_REG_KEY, 0x15);
>
> No magic numbers. These should all be defined.
Will do.
>
> > + if (ret)
> > + goto read_err;
> > + ret = regmap_write(pf1550->regmap, PF1550_CHARG_REG_CHGR_KEY2, 0x50);
> > + if (ret)
> > + goto read_err;
> > + ret = regmap_write(pf1550->regmap, PF1550_TEST_REG_KEY3, 0xab);
> > + if (ret)
> > + goto read_err;
> > + ret = regmap_write(pf1550->regmap, PF1550_TEST_REG_FMRADDR, index);
> > + if (ret)
> > + goto read_err;
> > + ret = regmap_read(pf1550->regmap, PF1550_TEST_REG_FMRDATA, val);
> > + if (ret)
> > + goto read_err;
> > +
> > + return 0;
> > +
> > +read_err:
> > + dev_err_probe(pf1550->dev, ret, "read otp reg %x found!\n", index);
...
> > +static int pf1550_add_child_device(struct pf1550_dev *pmic,
> > + const struct mfd_cell *cell,
> > + struct regmap_irq_chip_data *pdata,
>
> This is not pdata.
>
> > + int pirq,
> > + const struct regmap_irq_chip *chip,
> > + struct regmap_irq_chip_data **data)
> > +{
> > + struct device *dev = pmic->dev;
> > + struct irq_domain *domain;
> > + int irq, ret;
> > +
> > + irq = regmap_irq_get_virq(pdata, pirq);
> > + if (irq < 0)
> > + return dev_err_probe(dev, irq,
> > + "Failed to get parent vIRQ(%d) for chip %s\n",
> > + pirq, chip->name);
> > +
> > + ret = devm_regmap_add_irq_chip(dev, pmic->regmap, irq,
> > + IRQF_ONESHOT | IRQF_SHARED |
> > + IRQF_TRIGGER_FALLING, 0, chip, data);
> > + if (ret)
> > + return dev_err_probe(dev, ret,
> > + "Failed to add %s IRQ chip\n",
> > + chip->name);
> > +
> > + domain = regmap_irq_get_domain(*data);
> > +
> > + return devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE, cell, 1,
> > + NULL, 0, domain);
>
> Why can't all 3 devices be registered in one call?
>
The 3 devices use different regmap_irq_chip s. I have to register them
separately cause they have different irq domains but perhaps there is a better
way to handle this?
> > +}
>
> To be honest, the premise around this function is a bit of a mess.
>
> Please move all of this into .probe().
Will do.
>
> > +static int pf1550_i2c_probe(struct i2c_client *i2c)
> > +{
> > + const struct mfd_cell *regulator = &pf1550_regulator_cell;
> > + const struct mfd_cell *charger = &pf1550_charger_cell;
> > + const struct mfd_cell *onkey = &pf1550_onkey_cell;
> > + unsigned int reg_data = 0, otp_data = 0;
> > + struct pf1550_dev *pf1550;
> > + int ret = 0;
> > +
> > + pf1550 = devm_kzalloc(&i2c->dev, sizeof(*pf1550), GFP_KERNEL);
> > + if (!pf1550)
> > + return -ENOMEM;
> > +
> > + i2c_set_clientdata(i2c, pf1550);
> > + pf1550->dev = &i2c->dev;
> > + pf1550->i2c = i2c;
>
> What are you storing i2c for?
>
It doesn't need to be stored.
> Either store dev and irq OR i2c. You don't need all three.
>
Will do.
> > + ret = regmap_read(pf1550->regmap, PF1550_PMIC_REG_DEVICE_ID, ®_data);
> > + if (ret < 0 || reg_data != PF1550_DEVICE_ID)
> > + return dev_err_probe(pf1550->dev, ret ?: -EINVAL,
> > + "device not found!\n");
>
> Are you sure? What if the wrong device was found?
>
I can change the error log here to "Invalid device ID: ..."?
>
...
> > + /* add top level interrupts */
> > + ret = devm_regmap_add_irq_chip(pf1550->dev, pf1550->regmap, pf1550->irq,
> > + IRQF_ONESHOT | IRQF_SHARED |
> > + IRQF_TRIGGER_FALLING,
> > + 0, &pf1550_irq_chip,
> > + &pf1550->irq_data);
> > + if (ret)
> > + return ret;
> > +
> > + ret = pf1550_add_child_device(pf1550, regulator, pf1550->irq_data,
> > + PF1550_IRQ_REGULATOR,
> > + &pf1550_regulator_irq_chip,
> > + &pf1550->irq_data_regulator);
> > + if (ret)
> > + return ret;
> > +
> > + ret = pf1550_add_child_device(pf1550, onkey, pf1550->irq_data,
> > + PF1550_IRQ_ONKEY,
> > + &pf1550_onkey_irq_chip,
> > + &pf1550->irq_data_onkey);
> > + if (ret)
> > + return ret;
> > +
> > + ret = pf1550_add_child_device(pf1550, charger, pf1550->irq_data,
> > + PF1550_IRQ_CHG,
> > + &pf1550_charger_irq_chip,
> > + &pf1550->irq_data_charger);
> > + return ret;
> > +}
> > +
> > +static int pf1550_suspend(struct device *dev)
> > +{
> > + struct i2c_client *i2c = container_of(dev, struct i2c_client, dev);
> > + struct pf1550_dev *pf1550 = i2c_get_clientdata(i2c);
>
> You can swap all of this for:
>
> struct pf1550_dev *pf1550 = dev_get_drvdata(dev).
>
Will do.
> > +
> > + if (device_may_wakeup(dev)) {
> > + enable_irq_wake(pf1550->irq);
> > + disable_irq(pf1550->irq);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int pf1550_resume(struct device *dev)
> > +{
> > + struct i2c_client *i2c = container_of(dev, struct i2c_client, dev);
> > + struct pf1550_dev *pf1550 = i2c_get_clientdata(i2c);
>
> As above.
>
> > +
> > + if (device_may_wakeup(dev)) {
> > + disable_irq_wake(pf1550->irq);
> > + enable_irq(pf1550->irq);
>
> I would normally expect these to be around the opposite way to the ones
> in .suspend().
Do you mean enable_irq_wake and disable_irq in .resume() and the opposite for
.suspend()?
>
> > + }
> > +
> > + return 0;
> > +}
> > +
Thanks,
Sam
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v7 2/6] mfd: pf1550: add core mfd driver
2025-06-20 18:43 ` Samuel Kayode
@ 2025-06-24 16:02 ` Lee Jones
0 siblings, 0 replies; 17+ messages in thread
From: Lee Jones @ 2025-06-24 16:02 UTC (permalink / raw)
To: Samuel Kayode
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Liam Girdwood,
Mark Brown, Dmitry Torokhov, Sebastian Reichel, Frank Li, imx,
devicetree, linux-kernel, linux-input, linux-pm, Abel Vesa,
Abel Vesa, Robin Gong, Robin Gong, Enric Balletbo i Serra
On Fri, 20 Jun 2025, Samuel Kayode wrote:
> Hi Lee,
>
> Thanks a lot for the review.
>
> On Thu, Jun 19, 2025 at 02:03:37PM +0100, Lee Jones wrote:
> > > +static int pf1550_read_otp(const struct pf1550_dev *pf1550, unsigned int index,
> >
> > What does OTP mean?
> >
> It's a One-Time Programmable memory with configuration for the pf1550. I will
> expand on this in the commit description of the next version.
Place it in a comment please.
> > Why do you have to write to 4 registers first?
> >
> The pf1550 was designed such that the registers of the accompanying OTP is
> accessed indirectly. Valid keys have to be written to specific pf1550
> registers. After writing the keys, the address of the OTP register to be read
> is then written to PF1550_TEST_REG_FMRADDR and its corresponding value read from
> PF1550_TEST_REG_FMRDATA.
In a comment please. If I wondered, so with others.
> > This should all be made clear in some way or another.
> >
> I'll be adding comments on this in the next version.
Great!
> > > + unsigned int *val)
> > > +{
> > > + int ret = 0;
> > > +
> > > + ret = regmap_write(pf1550->regmap, PF1550_PMIC_REG_KEY, 0x15);
> >
> > No magic numbers. These should all be defined.
> Will do.
> >
> > > + if (ret)
> > > + goto read_err;
> > > + ret = regmap_write(pf1550->regmap, PF1550_CHARG_REG_CHGR_KEY2, 0x50);
> > > + if (ret)
> > > + goto read_err;
> > > + ret = regmap_write(pf1550->regmap, PF1550_TEST_REG_KEY3, 0xab);
> > > + if (ret)
> > > + goto read_err;
> > > + ret = regmap_write(pf1550->regmap, PF1550_TEST_REG_FMRADDR, index);
> > > + if (ret)
> > > + goto read_err;
> > > + ret = regmap_read(pf1550->regmap, PF1550_TEST_REG_FMRDATA, val);
> > > + if (ret)
> > > + goto read_err;
> > > +
> > > + return 0;
> > > +
> > > +read_err:
> > > + dev_err_probe(pf1550->dev, ret, "read otp reg %x found!\n", index);
> ...
> > > +static int pf1550_add_child_device(struct pf1550_dev *pmic,
> > > + const struct mfd_cell *cell,
> > > + struct regmap_irq_chip_data *pdata,
> >
> > This is not pdata.
> >
> > > + int pirq,
> > > + const struct regmap_irq_chip *chip,
> > > + struct regmap_irq_chip_data **data)
> > > +{
> > > + struct device *dev = pmic->dev;
> > > + struct irq_domain *domain;
> > > + int irq, ret;
> > > +
> > > + irq = regmap_irq_get_virq(pdata, pirq);
> > > + if (irq < 0)
> > > + return dev_err_probe(dev, irq,
> > > + "Failed to get parent vIRQ(%d) for chip %s\n",
> > > + pirq, chip->name);
> > > +
> > > + ret = devm_regmap_add_irq_chip(dev, pmic->regmap, irq,
> > > + IRQF_ONESHOT | IRQF_SHARED |
> > > + IRQF_TRIGGER_FALLING, 0, chip, data);
> > > + if (ret)
> > > + return dev_err_probe(dev, ret,
> > > + "Failed to add %s IRQ chip\n",
> > > + chip->name);
> > > +
> > > + domain = regmap_irq_get_domain(*data);
> > > +
> > > + return devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE, cell, 1,
> > > + NULL, 0, domain);
> >
> > Why can't all 3 devices be registered in one call?
> >
> The 3 devices use different regmap_irq_chip s. I have to register them
> separately cause they have different irq domains but perhaps there is a better
> way to handle this?
That's okay, just do 3 calls.
Must neater than what we have here.
> > > +}
> >
> > To be honest, the premise around this function is a bit of a mess.
> >
> > Please move all of this into .probe().
> Will do.
> >
> > > +static int pf1550_i2c_probe(struct i2c_client *i2c)
> > > +{
> > > + const struct mfd_cell *regulator = &pf1550_regulator_cell;
> > > + const struct mfd_cell *charger = &pf1550_charger_cell;
> > > + const struct mfd_cell *onkey = &pf1550_onkey_cell;
> > > + unsigned int reg_data = 0, otp_data = 0;
> > > + struct pf1550_dev *pf1550;
> > > + int ret = 0;
> > > +
> > > + pf1550 = devm_kzalloc(&i2c->dev, sizeof(*pf1550), GFP_KERNEL);
> > > + if (!pf1550)
> > > + return -ENOMEM;
> > > +
> > > + i2c_set_clientdata(i2c, pf1550);
> > > + pf1550->dev = &i2c->dev;
> > > + pf1550->i2c = i2c;
> >
> > What are you storing i2c for?
> >
> It doesn't need to be stored.
> > Either store dev and irq OR i2c. You don't need all three.
> >
> Will do.
> > > + ret = regmap_read(pf1550->regmap, PF1550_PMIC_REG_DEVICE_ID, ®_data);
> > > + if (ret < 0 || reg_data != PF1550_DEVICE_ID)
> > > + return dev_err_probe(pf1550->dev, ret ?: -EINVAL,
> > > + "device not found!\n");
> >
> > Are you sure? What if the wrong device was found?
> >
> I can change the error log here to "Invalid device ID: ..."?
Right. Invalid or unsupported, etc.
> >
> ...
> > > + /* add top level interrupts */
> > > + ret = devm_regmap_add_irq_chip(pf1550->dev, pf1550->regmap, pf1550->irq,
> > > + IRQF_ONESHOT | IRQF_SHARED |
> > > + IRQF_TRIGGER_FALLING,
> > > + 0, &pf1550_irq_chip,
> > > + &pf1550->irq_data);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + ret = pf1550_add_child_device(pf1550, regulator, pf1550->irq_data,
> > > + PF1550_IRQ_REGULATOR,
> > > + &pf1550_regulator_irq_chip,
> > > + &pf1550->irq_data_regulator);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + ret = pf1550_add_child_device(pf1550, onkey, pf1550->irq_data,
> > > + PF1550_IRQ_ONKEY,
> > > + &pf1550_onkey_irq_chip,
> > > + &pf1550->irq_data_onkey);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + ret = pf1550_add_child_device(pf1550, charger, pf1550->irq_data,
> > > + PF1550_IRQ_CHG,
> > > + &pf1550_charger_irq_chip,
> > > + &pf1550->irq_data_charger);
> > > + return ret;
> > > +}
> > > +
> > > +static int pf1550_suspend(struct device *dev)
> > > +{
> > > + struct i2c_client *i2c = container_of(dev, struct i2c_client, dev);
> > > + struct pf1550_dev *pf1550 = i2c_get_clientdata(i2c);
> >
> > You can swap all of this for:
> >
> > struct pf1550_dev *pf1550 = dev_get_drvdata(dev).
> >
> Will do.
> > > +
> > > + if (device_may_wakeup(dev)) {
> > > + enable_irq_wake(pf1550->irq);
> > > + disable_irq(pf1550->irq);
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int pf1550_resume(struct device *dev)
> > > +{
> > > + struct i2c_client *i2c = container_of(dev, struct i2c_client, dev);
> > > + struct pf1550_dev *pf1550 = i2c_get_clientdata(i2c);
> >
> > As above.
> >
> > > +
> > > + if (device_may_wakeup(dev)) {
> > > + disable_irq_wake(pf1550->irq);
> > > + enable_irq(pf1550->irq);
> >
> > I would normally expect these to be around the opposite way to the ones
> > in .suspend().
> Do you mean enable_irq_wake and disable_irq in .resume() and the opposite for
> .suspend()?
Yes. Or whatever fits best.
Maybe the h/w doesn't work that way, I just found it odd.
--
Lee Jones [李琼斯]
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v7 3/6] regulator: pf1550: add support for regulator
2025-06-12 14:55 [PATCH v7 0/6] add support for pf1550 PMIC MFD-based drivers Samuel Kayode via B4 Relay
2025-06-12 14:55 ` [PATCH v7 1/6] dt-bindings: mfd: add pf1550 Samuel Kayode via B4 Relay
2025-06-12 14:55 ` [PATCH v7 2/6] mfd: pf1550: add core mfd driver Samuel Kayode via B4 Relay
@ 2025-06-12 14:55 ` Samuel Kayode via B4 Relay
2025-06-12 14:55 ` [PATCH v7 4/6] input: pf1550: add onkey support Samuel Kayode via B4 Relay
` (2 subsequent siblings)
5 siblings, 0 replies; 17+ messages in thread
From: Samuel Kayode via B4 Relay @ 2025-06-12 14:55 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Liam Girdwood, Mark Brown, Dmitry Torokhov, Sebastian Reichel,
Frank Li
Cc: imx, devicetree, linux-kernel, linux-input, linux-pm, Abel Vesa,
Abel Vesa, Robin Gong, Robin Gong, Enric Balletbo i Serra,
Samuel Kayode, Abel Vesa, Frank Li
From: Samuel Kayode <samuel.kayode@savoirfairelinux.com>
Add regulator support for the pf1550 PMIC.
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Samuel Kayode <samuel.kayode@savoirfairelinux.com>
---
v7:
- Use reverese christmas tree style
- Drop unecessary 0 in id table's driver data
v6:
- Use dvs_enb variable in pf1550_dev as suggested by Frank Li
v5:
- Address Mark's feedback:
- Add comments to clarify difference in interrupts
- Issue warn event for _LS(low side) interrupt
- Validate maximum ramp_delay
v4:
- Address Mark's feedback:
- Use C++ comments for SPDX license
- Add portions copyright to reflect my update
- Validate ramp_delay
- Report overcurrent and temperature events
- Use platform_get_irq
v3:
- Drop duplicate include
- Drop unnecessary includes
- Accept lower case regulator names from devicetree
- Use virqs mapped in core MFD driver
v2:
- Add driver for regulator
---
drivers/regulator/Kconfig | 9 +
drivers/regulator/Makefile | 1 +
drivers/regulator/pf1550-regulator.c | 362 +++++++++++++++++++++++++++++++++++
3 files changed, 372 insertions(+)
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 6d8988387da4599633ca9bde2698b9711e34a245..de455887f9aeeada5546e44b8dc9d7ed041618a6 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -1049,6 +1049,15 @@ config REGULATOR_PV88090
Say y here to support the voltage regulators and convertors
on PV88090
+config REGULATOR_PF1550
+ tristate "NXP PF1550 regulator"
+ depends on MFD_PF1550
+ help
+ Say y here to select this option to enable the regulators on
+ the PF1550 PMICs.
+ This driver controls the PF1550 regulators via I2C bus.
+ The regulators include three bucks and three ldos.
+
config REGULATOR_PWM
tristate "PWM voltage regulator"
depends on PWM
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index c0bc7a0f4e67098c50ac3cf887ae95f46b2eac44..891174b511fc0653bac662c71659498122e8441f 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -125,6 +125,7 @@ obj-$(CONFIG_REGULATOR_QCOM_USB_VBUS) += qcom_usb_vbus-regulator.o
obj-$(CONFIG_REGULATOR_PALMAS) += palmas-regulator.o
obj-$(CONFIG_REGULATOR_PCA9450) += pca9450-regulator.o
obj-$(CONFIG_REGULATOR_PF9453) += pf9453-regulator.o
+obj-$(CONFIG_REGULATOR_PF1550) += pf1550-regulator.o
obj-$(CONFIG_REGULATOR_PF8X00) += pf8x00-regulator.o
obj-$(CONFIG_REGULATOR_PFUZE100) += pfuze100-regulator.o
obj-$(CONFIG_REGULATOR_PV88060) += pv88060-regulator.o
diff --git a/drivers/regulator/pf1550-regulator.c b/drivers/regulator/pf1550-regulator.c
new file mode 100644
index 0000000000000000000000000000000000000000..d5591a47aeeea1c50fac098e56006b7ba95fe46e
--- /dev/null
+++ b/drivers/regulator/pf1550-regulator.c
@@ -0,0 +1,362 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// pf1550.c - regulator driver for the PF1550
+//
+// Copyright (C) 2016 Freescale Semiconductor, Inc.
+// Robin Gong <yibin.gong@freescale.com>
+//
+// Portions Copyright (c) 2025 Savoir-faire Linux Inc.
+// Samuel Kayode <samuel.kayode@savoirfairelinux.com>
+//
+// This driver is based on pfuze100-regulator.c
+//
+
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/pf1550.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+
+#define PF1550_REGULATOR_IRQ_NR 11
+#define PF1550_MAX_REGULATOR 7
+
+struct pf1550_desc {
+ struct regulator_desc desc;
+ unsigned char stby_reg;
+ unsigned char stby_mask;
+};
+
+struct pf1550_regulator_info {
+ struct device *dev;
+ const struct pf1550_dev *pf1550;
+ struct pf1550_desc regulator_descs[PF1550_MAX_REGULATOR];
+ struct regulator_dev *rdevs[PF1550_MAX_REGULATOR];
+};
+
+static const int pf1550_sw12_volts[] = {
+ 1100000, 1200000, 1350000, 1500000, 1800000, 2500000, 3000000, 3300000,
+};
+
+static const int pf1550_ldo13_volts[] = {
+ 750000, 800000, 850000, 900000, 950000, 1000000, 1050000, 1100000,
+ 1150000, 1200000, 1250000, 1300000, 1350000, 1400000, 1450000, 1500000,
+ 1800000, 1900000, 2000000, 2100000, 2200000, 2300000, 2400000, 2500000,
+ 2600000, 2700000, 2800000, 2900000, 3000000, 3100000, 3200000, 3300000,
+};
+
+static int pf1550_set_ramp_delay(struct regulator_dev *rdev, int ramp_delay)
+{
+ int id = rdev_get_id(rdev);
+ unsigned int ramp_bits = 0;
+ int ret;
+
+ if (id > PF1550_VREFDDR)
+ return -EACCES;
+
+ if (ramp_delay < 0 || ramp_delay > 6250)
+ return -EINVAL;
+
+ ramp_delay = 6250 / ramp_delay;
+ ramp_bits = ramp_delay >> 1;
+
+ ret = regmap_update_bits(rdev->regmap, rdev->desc->vsel_reg + 4, 0x10,
+ ramp_bits << 4);
+ if (ret < 0)
+ dev_err(&rdev->dev, "ramp failed, err %d\n", ret);
+
+ return ret;
+}
+
+static const struct regulator_ops pf1550_sw1_ops = {
+ .list_voltage = regulator_list_voltage_table,
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+ .set_voltage_time_sel = regulator_set_voltage_time_sel,
+ .set_ramp_delay = pf1550_set_ramp_delay,
+};
+
+static const struct regulator_ops pf1550_sw2_ops = {
+ .list_voltage = regulator_list_voltage_linear,
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+ .set_voltage_time_sel = regulator_set_voltage_time_sel,
+ .set_ramp_delay = pf1550_set_ramp_delay,
+};
+
+static const struct regulator_ops pf1550_ldo1_ops = {
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .is_enabled = regulator_is_enabled_regmap,
+ .list_voltage = regulator_list_voltage_table,
+ .map_voltage = regulator_map_voltage_ascend,
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+};
+
+static const struct regulator_ops pf1550_ldo2_ops = {
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .is_enabled = regulator_is_enabled_regmap,
+ .list_voltage = regulator_list_voltage_linear,
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+};
+
+static const struct regulator_ops pf1550_fixed_ops = {
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .is_enabled = regulator_is_enabled_regmap,
+ .list_voltage = regulator_list_voltage_linear,
+};
+
+#define PF_VREF(_chip, match, _name, voltage) { \
+ .desc = { \
+ .name = #_name, \
+ .of_match = of_match_ptr(match), \
+ .regulators_node = of_match_ptr("regulators"), \
+ .n_voltages = 1, \
+ .ops = &pf1550_fixed_ops, \
+ .type = REGULATOR_VOLTAGE, \
+ .id = _chip ## _ ## _name, \
+ .owner = THIS_MODULE, \
+ .min_uV = (voltage), \
+ .enable_reg = _chip ## _PMIC_REG_ ## _name ## _CTRL, \
+ .enable_mask = 0x1, \
+ }, \
+ .stby_reg = _chip ## _PMIC_REG_ ## _name ## _CTRL, \
+ .stby_mask = 0x2, \
+}
+
+#define PF_SW1(_chip, match, _name, mask, voltages) { \
+ .desc = { \
+ .name = #_name, \
+ .of_match = of_match_ptr(match), \
+ .regulators_node = of_match_ptr("regulators"), \
+ .n_voltages = ARRAY_SIZE(voltages), \
+ .ops = &pf1550_sw1_ops, \
+ .type = REGULATOR_VOLTAGE, \
+ .id = _chip ## _ ## _name, \
+ .owner = THIS_MODULE, \
+ .volt_table = voltages, \
+ .vsel_reg = _chip ## _PMIC_REG_ ## _name ## _VOLT, \
+ .vsel_mask = (mask), \
+ }, \
+ .stby_reg = _chip ## _PMIC_REG_ ## _name ## _STBY_VOLT, \
+ .stby_mask = (mask), \
+}
+
+#define PF_SW3(_chip, match, _name, min, max, mask, step) { \
+ .desc = { \
+ .name = #_name, \
+ .of_match = of_match_ptr(match), \
+ .regulators_node = of_match_ptr("regulators"), \
+ .n_voltages = ((max) - (min)) / (step) + 1, \
+ .ops = &pf1550_sw2_ops, \
+ .type = REGULATOR_VOLTAGE, \
+ .id = _chip ## _ ## _name, \
+ .owner = THIS_MODULE, \
+ .min_uV = (min), \
+ .uV_step = (step), \
+ .vsel_reg = _chip ## _PMIC_REG_ ## _name ## _VOLT, \
+ .vsel_mask = (mask), \
+ }, \
+ .stby_reg = _chip ## _PMIC_REG_ ## _name ## _STBY_VOLT, \
+ .stby_mask = (mask), \
+}
+
+#define PF_LDO1(_chip, match, _name, mask, voltages) { \
+ .desc = { \
+ .name = #_name, \
+ .of_match = of_match_ptr(match), \
+ .regulators_node = of_match_ptr("regulators"), \
+ .n_voltages = ARRAY_SIZE(voltages), \
+ .ops = &pf1550_ldo1_ops, \
+ .type = REGULATOR_VOLTAGE, \
+ .id = _chip ## _ ## _name, \
+ .owner = THIS_MODULE, \
+ .volt_table = voltages, \
+ .vsel_reg = _chip ## _PMIC_REG_ ## _name ## _VOLT, \
+ .vsel_mask = (mask), \
+ .enable_reg = _chip ## _PMIC_REG_ ## _name ## _CTRL, \
+ .enable_mask = 0x1, \
+ }, \
+ .stby_reg = _chip ## _PMIC_REG_ ## _name ## _CTRL, \
+ .stby_mask = 0x2, \
+}
+
+#define PF_LDO2(_chip, match, _name, mask, min, max, step) { \
+ .desc = { \
+ .name = #_name, \
+ .of_match = of_match_ptr(match), \
+ .regulators_node = of_match_ptr("regulators"), \
+ .n_voltages = ((max) - (min)) / (step) + 1, \
+ .ops = &pf1550_ldo2_ops, \
+ .type = REGULATOR_VOLTAGE, \
+ .id = _chip ## _ ## _name, \
+ .owner = THIS_MODULE, \
+ .min_uV = (min), \
+ .uV_step = (step), \
+ .vsel_reg = _chip ## _PMIC_REG_ ## _name ## _VOLT, \
+ .vsel_mask = (mask), \
+ .enable_reg = _chip ## _PMIC_REG_ ## _name ## _CTRL, \
+ .enable_mask = 0x1, \
+ }, \
+ .stby_reg = _chip ## _PMIC_REG_ ## _name ## _CTRL, \
+ .stby_mask = 0x2, \
+}
+
+static struct pf1550_desc pf1550_regulators[] = {
+ PF_SW3(PF1550, "sw1", SW1, 600000, 1387500, 0x3f, 12500),
+ PF_SW3(PF1550, "sw2", SW2, 600000, 1387500, 0x3f, 12500),
+ PF_SW3(PF1550, "sw3", SW3, 1800000, 3300000, 0xf, 100000),
+ PF_VREF(PF1550, "vrefddr", VREFDDR, 1200000),
+ PF_LDO1(PF1550, "ldo1", LDO1, 0x1f, pf1550_ldo13_volts),
+ PF_LDO2(PF1550, "ldo2", LDO2, 0xf, 1800000, 3300000, 100000),
+ PF_LDO1(PF1550, "ldo3", LDO3, 0x1f, pf1550_ldo13_volts),
+};
+
+static irqreturn_t pf1550_regulator_irq_handler(int irq, void *data)
+{
+ struct pf1550_regulator_info *info = data;
+ struct device *dev = info->dev;
+ struct platform_device *pdev = to_platform_device(dev);
+ int i, irq_type = -1;
+ unsigned int event;
+
+ for (i = 0; i < PF1550_REGULATOR_IRQ_NR; i++)
+ if (irq == platform_get_irq(pdev, i))
+ irq_type = i;
+
+ switch (irq_type) {
+ /* The _LS interrupts indicate over-current event. The _HS interrupts
+ * which are more accurate and can detect catastrophic faults, issue
+ * an error event. The current limit FAULT interrupt is similar to the
+ * _HS'
+ */
+ case PF1550_PMIC_IRQ_SW1_LS:
+ case PF1550_PMIC_IRQ_SW2_LS:
+ case PF1550_PMIC_IRQ_SW3_LS:
+ event = REGULATOR_EVENT_OVER_CURRENT_WARN;
+ for (i = 0; i < PF1550_MAX_REGULATOR; i++)
+ if (!strcmp(rdev_get_name(info->rdevs[i]), "SW3"))
+ regulator_notifier_call_chain(info->rdevs[i],
+ event, NULL);
+ break;
+ case PF1550_PMIC_IRQ_SW1_HS:
+ case PF1550_PMIC_IRQ_SW2_HS:
+ case PF1550_PMIC_IRQ_SW3_HS:
+ event = REGULATOR_EVENT_OVER_CURRENT;
+ for (i = 0; i < PF1550_MAX_REGULATOR; i++)
+ if (!strcmp(rdev_get_name(info->rdevs[i]), "SW3"))
+ regulator_notifier_call_chain(info->rdevs[i],
+ event, NULL);
+ break;
+ case PF1550_PMIC_IRQ_LDO1_FAULT:
+ case PF1550_PMIC_IRQ_LDO2_FAULT:
+ case PF1550_PMIC_IRQ_LDO3_FAULT:
+ event = REGULATOR_EVENT_OVER_CURRENT;
+ for (i = 0; i < PF1550_MAX_REGULATOR; i++)
+ if (!strcmp(rdev_get_name(info->rdevs[i]), "LDO3"))
+ regulator_notifier_call_chain(info->rdevs[i],
+ event, NULL);
+ break;
+ case PF1550_PMIC_IRQ_TEMP_110:
+ case PF1550_PMIC_IRQ_TEMP_125:
+ event = REGULATOR_EVENT_OVER_TEMP;
+ for (i = 0; i < PF1550_MAX_REGULATOR; i++)
+ regulator_notifier_call_chain(info->rdevs[i],
+ event, NULL);
+ break;
+ default:
+ dev_err(dev, "regulator interrupt: irq %d occurred\n",
+ irq_type);
+ }
+
+ return IRQ_HANDLED;
+}
+
+static int pf1550_regulator_probe(struct platform_device *pdev)
+{
+ const struct pf1550_dev *pf1550 = dev_get_drvdata(pdev->dev.parent);
+ struct regulator_config config = { };
+ struct pf1550_regulator_info *info;
+ int i, irq = -1, ret = 0;
+
+ info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
+ if (!info)
+ return -ENOMEM;
+
+ config.regmap = dev_get_regmap(pf1550->dev, NULL);
+ if (!config.regmap)
+ return dev_err_probe(&pdev->dev, -ENODEV,
+ "failed to get parent regmap\n");
+
+ config.dev = pf1550->dev;
+ config.regmap = pf1550->regmap;
+ info->dev = &pdev->dev;
+ info->pf1550 = pf1550;
+
+ memcpy(info->regulator_descs, pf1550_regulators,
+ sizeof(info->regulator_descs));
+
+ for (i = 0; i < ARRAY_SIZE(pf1550_regulators); i++) {
+ struct regulator_desc *desc;
+
+ desc = &info->regulator_descs[i].desc;
+
+ if (desc->id == PF1550_SW2 && pf1550->dvs_enb) {
+ /* OTP_SW2_DVS_ENB == 1? */
+ desc->volt_table = pf1550_sw12_volts;
+ desc->n_voltages = ARRAY_SIZE(pf1550_sw12_volts);
+ desc->ops = &pf1550_sw1_ops;
+ }
+
+ info->rdevs[i] = devm_regulator_register(&pdev->dev, desc,
+ &config);
+ if (IS_ERR(info->rdevs[i]))
+ return dev_err_probe(&pdev->dev,
+ PTR_ERR(info->rdevs[i]),
+ "failed to initialize regulator-%d\n",
+ i);
+ }
+
+ platform_set_drvdata(pdev, info);
+
+ for (i = 0; i < PF1550_REGULATOR_IRQ_NR; i++) {
+ irq = platform_get_irq(pdev, i);
+ if (irq < 0)
+ return irq;
+
+ ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+ pf1550_regulator_irq_handler,
+ IRQF_NO_SUSPEND,
+ "pf1550-regulator", info);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "failed: irq request (IRQ: %d)\n",
+ i);
+ }
+
+ return 0;
+}
+
+static const struct platform_device_id pf1550_regulator_id[] = {
+ { "pf1550-regulator", },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, pf1550_regulator_id);
+
+static struct platform_driver pf1550_regulator_driver = {
+ .driver = {
+ .name = "pf1550-regulator",
+ },
+ .probe = pf1550_regulator_probe,
+ .id_table = pf1550_regulator_id,
+};
+module_platform_driver(pf1550_regulator_driver);
+
+MODULE_DESCRIPTION("NXP PF1550 regulator driver");
+MODULE_AUTHOR("Robin Gong <yibin.gong@freescale.com>");
+MODULE_LICENSE("GPL");
--
2.49.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v7 4/6] input: pf1550: add onkey support
2025-06-12 14:55 [PATCH v7 0/6] add support for pf1550 PMIC MFD-based drivers Samuel Kayode via B4 Relay
` (2 preceding siblings ...)
2025-06-12 14:55 ` [PATCH v7 3/6] regulator: pf1550: add support for regulator Samuel Kayode via B4 Relay
@ 2025-06-12 14:55 ` Samuel Kayode via B4 Relay
2025-06-17 22:23 ` Dmitry Torokhov
2025-06-12 14:55 ` [PATCH v7 5/6] power: supply: pf1550: add battery charger support Samuel Kayode via B4 Relay
2025-06-12 14:55 ` [PATCH v7 6/6] MAINTAINERS: add an entry for pf1550 mfd driver Samuel Kayode via B4 Relay
5 siblings, 1 reply; 17+ messages in thread
From: Samuel Kayode via B4 Relay @ 2025-06-12 14:55 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Liam Girdwood, Mark Brown, Dmitry Torokhov, Sebastian Reichel,
Frank Li
Cc: imx, devicetree, linux-kernel, linux-input, linux-pm, Abel Vesa,
Abel Vesa, Robin Gong, Robin Gong, Enric Balletbo i Serra,
Samuel Kayode, Abel Vesa, Frank Li
From: Samuel Kayode <samuel.kayode@savoirfairelinux.com>
Add support for the onkey of the pf1550 PMIC.
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Samuel Kayode <samuel.kayode@savoirfairelinux.com>
---
v7:
- Use reverese christmas tree style
- Drop unecessary 0 in id table's driver data
v4:
- Address Dmitry's feedback
- Drop irq variable in onkey_drv_data
- Drop keycode variable in onkey_drv_data
- Define wakeup as type bool
- Use platform_get_irq
- Use type const for struct pf1550_dev in onkey_drv_data
- Replace (error < 0) with (error) in if statement when applicable
- No need to define driver_data in table id
- Define driver.pm with pm_sleep_ptr
v3:
- Address Dmitry's feedback
- Drop compatible string
- Remove dependency on OF
- Use generic device properties
- Drop unnecessary includes
- Drop unnecessary initializations in probe
- Always use the KEY_POWER property for onkey->keycode
- Do mapping of irqs in MFD driver
- Define onkey->input before interrupts are active
- Drop unnecessary input_free_device since devm
- Manage onkey irqs instead of the main interrupt line.
- Fix integer overflow when unmasking onkey irqs in onkey_resume.
v2:
- Add driver for onkey
---
drivers/input/misc/Kconfig | 11 +++
drivers/input/misc/Makefile | 1 +
drivers/input/misc/pf1550-onkey.c | 183 ++++++++++++++++++++++++++++++++++++++
3 files changed, 195 insertions(+)
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index f5496ca0c0d2bfcb7968503ccd1844ff43bbc1c0..47b3c43ff0550f14d61990997976366436411adc 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -179,6 +179,17 @@ config INPUT_PCSPKR
To compile this driver as a module, choose M here: the
module will be called pcspkr.
+config INPUT_PF1550_ONKEY
+ tristate "NXP PF1550 Onkey support"
+ depends on MFD_PF1550
+ help
+ Say Y here if you want support for PF1550 PMIC. Onkey can trigger
+ release and 1s(push hold), 2s, 3s, 4s, 8s interrupt for long press
+ detect.
+
+ To compile this driver as a module, choose M here. The module will be
+ called pf1550-onkey.
+
config INPUT_PM8941_PWRKEY
tristate "Qualcomm PM8941 power key support"
depends on MFD_SPMI_PMIC
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 6d91804d0a6f761a094e6c380f878f74c3054d63..c652337de464c1eeaf1515d0bc84d10de0cb3a74 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -62,6 +62,7 @@ obj-$(CONFIG_INPUT_PCAP) += pcap_keys.o
obj-$(CONFIG_INPUT_PCF50633_PMU) += pcf50633-input.o
obj-$(CONFIG_INPUT_PCF8574) += pcf8574_keypad.o
obj-$(CONFIG_INPUT_PCSPKR) += pcspkr.o
+obj-$(CONFIG_INPUT_PF1550_ONKEY) += pf1550-onkey.o
obj-$(CONFIG_INPUT_PM8941_PWRKEY) += pm8941-pwrkey.o
obj-$(CONFIG_INPUT_PM8XXX_VIBRATOR) += pm8xxx-vibrator.o
obj-$(CONFIG_INPUT_PMIC8XXX_PWRKEY) += pmic8xxx-pwrkey.o
diff --git a/drivers/input/misc/pf1550-onkey.c b/drivers/input/misc/pf1550-onkey.c
new file mode 100644
index 0000000000000000000000000000000000000000..a93346f5de9ed9c7e64c3a5f559d66380dc35472
--- /dev/null
+++ b/drivers/input/misc/pf1550-onkey.c
@@ -0,0 +1,183 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Driver for the PF1550 ON_KEY
+// Copyright (C) 2016 Freescale Semiconductor, Inc. All Rights Reserved.
+//
+// Portions Copyright (c) 2025 Savoir-faire Linux Inc.
+// Samuel Kayode <samuel.kayode@savoirfairelinux.com>
+//
+
+#include <linux/err.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mfd/pf1550.h>
+#include <linux/platform_device.h>
+
+#define PF1550_ONKEY_IRQ_NR 6
+
+struct onkey_drv_data {
+ struct device *dev;
+ const struct pf1550_dev *pf1550;
+ bool wakeup;
+ struct input_dev *input;
+};
+
+static irqreturn_t pf1550_onkey_irq_handler(int irq, void *data)
+{
+ struct onkey_drv_data *onkey = data;
+ struct platform_device *pdev = to_platform_device(onkey->dev);
+ int i, state, irq_type = -1;
+
+ for (i = 0; i < PF1550_ONKEY_IRQ_NR; i++)
+ if (irq == platform_get_irq(pdev, i))
+ irq_type = i;
+
+ switch (irq_type) {
+ case PF1550_ONKEY_IRQ_PUSHI:
+ state = 0;
+ break;
+ case PF1550_ONKEY_IRQ_1SI:
+ case PF1550_ONKEY_IRQ_2SI:
+ case PF1550_ONKEY_IRQ_3SI:
+ case PF1550_ONKEY_IRQ_4SI:
+ case PF1550_ONKEY_IRQ_8SI:
+ state = 1;
+ break;
+ default:
+ dev_err(onkey->dev, "onkey interrupt: irq %d occurred\n",
+ irq_type);
+ return IRQ_HANDLED;
+ }
+
+ input_event(onkey->input, EV_KEY, KEY_POWER, state);
+ input_sync(onkey->input);
+
+ return IRQ_HANDLED;
+}
+
+static int pf1550_onkey_probe(struct platform_device *pdev)
+{
+ struct onkey_drv_data *onkey;
+ struct input_dev *input;
+ int i, irq, error;
+
+ onkey = devm_kzalloc(&pdev->dev, sizeof(*onkey), GFP_KERNEL);
+ if (!onkey)
+ return -ENOMEM;
+
+ onkey->dev = &pdev->dev;
+
+ onkey->pf1550 = dev_get_drvdata(pdev->dev.parent);
+ if (!onkey->pf1550->regmap)
+ return dev_err_probe(&pdev->dev, -ENODEV,
+ "failed to get regmap\n");
+
+ onkey->wakeup = device_property_read_bool(pdev->dev.parent,
+ "wakeup-source");
+
+ input = devm_input_allocate_device(&pdev->dev);
+ if (!input)
+ return dev_err_probe(&pdev->dev, -ENOMEM,
+ "failed to allocate the input device\n");
+
+ input->name = pdev->name;
+ input->phys = "pf1550-onkey/input0";
+ input->id.bustype = BUS_HOST;
+
+ input_set_capability(input, EV_KEY, KEY_POWER);
+
+ onkey->input = input;
+ platform_set_drvdata(pdev, onkey);
+
+ for (i = 0; i < PF1550_ONKEY_IRQ_NR; i++) {
+ irq = platform_get_irq(pdev, i);
+ if (irq < 0)
+ return irq;
+
+ error = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+ pf1550_onkey_irq_handler,
+ IRQF_NO_SUSPEND,
+ "pf1550-onkey", onkey);
+ if (error)
+ return dev_err_probe(&pdev->dev, error,
+ "failed: irq request (IRQ: %d)\n",
+ i);
+ }
+
+ error = input_register_device(input);
+ if (error)
+ return dev_err_probe(&pdev->dev, error,
+ "failed to register input device\n");
+
+ device_init_wakeup(&pdev->dev, onkey->wakeup);
+
+ return 0;
+}
+
+static int pf1550_onkey_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct onkey_drv_data *onkey = platform_get_drvdata(pdev);
+ int i, irq;
+
+ if (!device_may_wakeup(&pdev->dev))
+ regmap_write(onkey->pf1550->regmap,
+ PF1550_PMIC_REG_ONKEY_INT_MASK0,
+ ONKEY_IRQ_PUSHI | ONKEY_IRQ_1SI | ONKEY_IRQ_2SI |
+ ONKEY_IRQ_3SI | ONKEY_IRQ_4SI | ONKEY_IRQ_8SI);
+ else
+ for (i = 0; i < PF1550_ONKEY_IRQ_NR; i++) {
+ irq = platform_get_irq(pdev, i);
+ if (irq > 0)
+ enable_irq_wake(irq);
+ }
+
+ return 0;
+}
+
+static int pf1550_onkey_resume(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct onkey_drv_data *onkey = platform_get_drvdata(pdev);
+ int i, irq;
+
+ if (!device_may_wakeup(&pdev->dev))
+ regmap_write(onkey->pf1550->regmap,
+ PF1550_PMIC_REG_ONKEY_INT_MASK0,
+ ~((u8)(ONKEY_IRQ_PUSHI | ONKEY_IRQ_1SI |
+ ONKEY_IRQ_2SI | ONKEY_IRQ_3SI | ONKEY_IRQ_4SI |
+ ONKEY_IRQ_8SI)));
+ else
+ for (i = 0; i < PF1550_ONKEY_IRQ_NR; i++) {
+ irq = platform_get_irq(pdev, i);
+ if (irq > 0)
+ disable_irq_wake(irq);
+ }
+
+ return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(pf1550_onkey_pm_ops, pf1550_onkey_suspend,
+ pf1550_onkey_resume);
+
+static const struct platform_device_id pf1550_onkey_id[] = {
+ { "pf1550-onkey", },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, pf1550_onkey_id);
+
+static struct platform_driver pf1550_onkey_driver = {
+ .driver = {
+ .name = "pf1550-onkey",
+ .pm = pm_sleep_ptr(&pf1550_onkey_pm_ops),
+ },
+ .probe = pf1550_onkey_probe,
+ .id_table = pf1550_onkey_id,
+};
+module_platform_driver(pf1550_onkey_driver);
+
+MODULE_AUTHOR("Freescale Semiconductor");
+MODULE_DESCRIPTION("PF1550 onkey Driver");
+MODULE_LICENSE("GPL");
--
2.49.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v7 4/6] input: pf1550: add onkey support
2025-06-12 14:55 ` [PATCH v7 4/6] input: pf1550: add onkey support Samuel Kayode via B4 Relay
@ 2025-06-17 22:23 ` Dmitry Torokhov
0 siblings, 0 replies; 17+ messages in thread
From: Dmitry Torokhov @ 2025-06-17 22:23 UTC (permalink / raw)
To: samuel.kayode
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Liam Girdwood, Mark Brown, Sebastian Reichel, Frank Li, imx,
devicetree, linux-kernel, linux-input, linux-pm, Abel Vesa,
Abel Vesa, Robin Gong, Robin Gong, Enric Balletbo i Serra
On Thu, Jun 12, 2025 at 10:55:46AM -0400, Samuel Kayode via B4 Relay wrote:
> From: Samuel Kayode <samuel.kayode@savoirfairelinux.com>
>
> Add support for the onkey of the pf1550 PMIC.
>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Samuel Kayode <samuel.kayode@savoirfairelinux.com>
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Thank you for making the changes.
--
Dmitry
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v7 5/6] power: supply: pf1550: add battery charger support
2025-06-12 14:55 [PATCH v7 0/6] add support for pf1550 PMIC MFD-based drivers Samuel Kayode via B4 Relay
` (3 preceding siblings ...)
2025-06-12 14:55 ` [PATCH v7 4/6] input: pf1550: add onkey support Samuel Kayode via B4 Relay
@ 2025-06-12 14:55 ` Samuel Kayode via B4 Relay
2025-06-12 16:02 ` Frank Li
2025-06-22 0:43 ` Sebastian Reichel
2025-06-12 14:55 ` [PATCH v7 6/6] MAINTAINERS: add an entry for pf1550 mfd driver Samuel Kayode via B4 Relay
5 siblings, 2 replies; 17+ messages in thread
From: Samuel Kayode via B4 Relay @ 2025-06-12 14:55 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Liam Girdwood, Mark Brown, Dmitry Torokhov, Sebastian Reichel,
Frank Li
Cc: imx, devicetree, linux-kernel, linux-input, linux-pm, Abel Vesa,
Abel Vesa, Robin Gong, Robin Gong, Enric Balletbo i Serra,
Samuel Kayode, Abel Vesa
From: Samuel Kayode <samuel.kayode@savoirfairelinux.com>
Add support for the battery charger for pf1550 PMIC.
Signed-off-by: Samuel Kayode <samuel.kayode@savoirfairelinux.com>
---
v7:
- Use reverse christmas tree order
- Drop unecessary 0 in id table's driver data field
- Store virqs to avoid reinvoking platform_get_irq in the interrupt
service routine
- Drop manufacturer and model global variables
v6:
- Drop lock entirely
- Reverse christmas tree order for variables defined in probe as
suggested by Frank
- return pf1550_reg_init
v5:
- Drop lock for battery and charger delayed_work
- More conservative locking in vbus delayed_work
- Apply lock when setting power supply type during register initialization
v4:
- Finish handling of some interrupts in threaded irq handler
- Use platform_get_irq
v3:
- Use struct power_supply_get_battery_info to get constant charge
voltage if specified
- Use virqs mapped in MFD driver
v2:
- Address feedback from Enric Balletbo Serra
---
drivers/power/supply/Kconfig | 11 +
drivers/power/supply/Makefile | 1 +
drivers/power/supply/pf1550-charger.c | 632 ++++++++++++++++++++++++++++++++++
3 files changed, 644 insertions(+)
diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 79ddb006e2dad6bf96b71ed570a37c006b5f9433..6d0c872edac1f45da314632e671af1aeda4c87b8 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -471,6 +471,17 @@ config CHARGER_88PM860X
help
Say Y here to enable charger for Marvell 88PM860x chip.
+config CHARGER_PF1550
+ tristate "NXP PF1550 battery charger driver"
+ depends on MFD_PF1550
+ help
+ Say Y to enable support for the NXP PF1550 battery charger.
+ The device is a single cell Li-Ion/Li-Polymer battery charger for
+ portable application.
+
+ This driver can also be built as a module. If so, the module will be
+ called pf1550-charger.
+
config BATTERY_RX51
tristate "Nokia RX-51 (N900) battery driver"
depends on TWL4030_MADC
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index 4f5f8e3507f80da02812f0d08c2d81ddff0a272f..7f68380099c59dab71b73120150612a23e16a745 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -64,6 +64,7 @@ obj-$(CONFIG_CHARGER_RT9467) += rt9467-charger.o
obj-$(CONFIG_CHARGER_RT9471) += rt9471.o
obj-$(CONFIG_BATTERY_TWL4030_MADC) += twl4030_madc_battery.o
obj-$(CONFIG_CHARGER_88PM860X) += 88pm860x_charger.o
+obj-$(CONFIG_CHARGER_PF1550) += pf1550-charger.o
obj-$(CONFIG_BATTERY_RX51) += rx51_battery.o
obj-$(CONFIG_AB8500_BM) += ab8500_bmdata.o ab8500_charger.o ab8500_fg.o ab8500_btemp.o ab8500_chargalg.o
obj-$(CONFIG_CHARGER_CPCAP) += cpcap-charger.o
diff --git a/drivers/power/supply/pf1550-charger.c b/drivers/power/supply/pf1550-charger.c
new file mode 100644
index 0000000000000000000000000000000000000000..1693fb2afdd444e088827edc476fa271fb0937e0
--- /dev/null
+++ b/drivers/power/supply/pf1550-charger.c
@@ -0,0 +1,632 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// pf1550_charger.c - charger driver for the PF1550
+//
+// Copyright (C) 2016 Freescale Semiconductor, Inc.
+// Robin Gong <yibin.gong@freescale.com>
+//
+// Portions Copyright (c) 2025 Savoir-faire Linux Inc.
+// Samuel Kayode <samuel.kayode@savoirfairelinux.com>
+//
+
+#include <linux/interrupt.h>
+#include <linux/mfd/pf1550.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+
+#define PF1550_CHARGER_NAME "pf1550-charger"
+#define PF1550_DEFAULT_CONSTANT_VOLT 4200000
+#define PF1550_DEFAULT_MIN_SYSTEM_VOLT 3500000
+#define PF1550_DEFAULT_THERMAL_TEMP 75
+#define PF1550_CHARGER_IRQ_NR 5
+
+struct pf1550_charger {
+ struct device *dev;
+ const struct pf1550_dev *pf1550;
+ struct power_supply *charger;
+ struct power_supply_desc psy_desc;
+ struct delayed_work vbus_sense_work;
+ struct delayed_work chg_sense_work;
+ struct delayed_work bat_sense_work;
+ int virqs[PF1550_CHARGER_IRQ_NR];
+
+ u32 constant_volt;
+ u32 min_system_volt;
+ u32 thermal_regulation_temp;
+};
+
+static int pf1550_get_charger_state(struct regmap *regmap, int *val)
+{
+ unsigned int data;
+ int ret;
+
+ ret = regmap_read(regmap, PF1550_CHARG_REG_CHG_SNS, &data);
+ if (ret < 0)
+ return ret;
+
+ data &= PF1550_CHG_SNS_MASK;
+
+ switch (data) {
+ case PF1550_CHG_PRECHARGE:
+ case PF1550_CHG_CONSTANT_CURRENT:
+ *val = POWER_SUPPLY_STATUS_CHARGING;
+ break;
+ case PF1550_CHG_CONSTANT_VOL:
+ *val = POWER_SUPPLY_STATUS_CHARGING;
+ break;
+ case PF1550_CHG_EOC:
+ *val = POWER_SUPPLY_STATUS_CHARGING;
+ break;
+ case PF1550_CHG_DONE:
+ *val = POWER_SUPPLY_STATUS_FULL;
+ break;
+ case PF1550_CHG_TIMER_FAULT:
+ case PF1550_CHG_SUSPEND:
+ *val = POWER_SUPPLY_STATUS_NOT_CHARGING;
+ break;
+ case PF1550_CHG_OFF_INV:
+ case PF1550_CHG_OFF_TEMP:
+ case PF1550_CHG_LINEAR_ONLY:
+ *val = POWER_SUPPLY_STATUS_DISCHARGING;
+ break;
+ default:
+ *val = POWER_SUPPLY_STATUS_UNKNOWN;
+ }
+
+ return 0;
+}
+
+static int pf1550_get_charge_type(struct regmap *regmap, int *val)
+{
+ unsigned int data;
+ int ret;
+
+ ret = regmap_read(regmap, PF1550_CHARG_REG_CHG_SNS, &data);
+ if (ret < 0)
+ return ret;
+
+ data &= PF1550_CHG_SNS_MASK;
+
+ switch (data) {
+ case PF1550_CHG_SNS_MASK:
+ *val = POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
+ break;
+ case PF1550_CHG_CONSTANT_CURRENT:
+ case PF1550_CHG_CONSTANT_VOL:
+ case PF1550_CHG_EOC:
+ *val = POWER_SUPPLY_CHARGE_TYPE_FAST;
+ break;
+ case PF1550_CHG_DONE:
+ case PF1550_CHG_TIMER_FAULT:
+ case PF1550_CHG_SUSPEND:
+ case PF1550_CHG_OFF_INV:
+ case PF1550_CHG_BAT_OVER:
+ case PF1550_CHG_OFF_TEMP:
+ case PF1550_CHG_LINEAR_ONLY:
+ *val = POWER_SUPPLY_CHARGE_TYPE_NONE;
+ break;
+ default:
+ *val = POWER_SUPPLY_CHARGE_TYPE_UNKNOWN;
+ }
+
+ return 0;
+}
+
+/*
+ * Supported health statuses:
+ * - POWER_SUPPLY_HEALTH_DEAD
+ * - POWER_SUPPLY_HEALTH_GOOD
+ * - POWER_SUPPLY_HEALTH_OVERVOLTAGE
+ * - POWER_SUPPLY_HEALTH_UNKNOWN
+ */
+static int pf1550_get_battery_health(struct regmap *regmap, int *val)
+{
+ unsigned int data;
+ int ret;
+
+ ret = regmap_read(regmap, PF1550_CHARG_REG_BATT_SNS, &data);
+ if (ret < 0)
+ return ret;
+
+ data &= PF1550_BAT_SNS_MASK;
+
+ switch (data) {
+ case PF1550_BAT_NO_DETECT:
+ *val = POWER_SUPPLY_HEALTH_DEAD;
+ break;
+ case PF1550_BAT_NO_VBUS:
+ case PF1550_BAT_LOW_THAN_PRECHARG:
+ case PF1550_BAT_CHARG_FAIL:
+ case PF1550_BAT_HIGH_THAN_PRECHARG:
+ *val = POWER_SUPPLY_HEALTH_GOOD;
+ break;
+ case PF1550_BAT_OVER_VOL:
+ *val = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
+ break;
+ default:
+ *val = POWER_SUPPLY_HEALTH_UNKNOWN;
+ break;
+ }
+
+ return 0;
+}
+
+static int pf1550_get_present(struct regmap *regmap, int *val)
+{
+ unsigned int data;
+ int ret;
+
+ ret = regmap_read(regmap, PF1550_CHARG_REG_BATT_SNS, &data);
+ if (ret < 0)
+ return ret;
+
+ data &= PF1550_BAT_SNS_MASK;
+ *val = (data == PF1550_BAT_NO_DETECT) ? 0 : 1;
+
+ return 0;
+}
+
+static int pf1550_get_online(struct regmap *regmap, int *val)
+{
+ unsigned int data;
+ int ret;
+
+ ret = regmap_read(regmap, PF1550_CHARG_REG_VBUS_SNS, &data);
+ if (ret < 0)
+ return ret;
+
+ *val = (data & PF1550_VBUS_VALID) ? 1 : 0;
+
+ return 0;
+}
+
+static void pf1550_chg_bat_work(struct work_struct *work)
+{
+ struct pf1550_charger *chg = container_of(to_delayed_work(work),
+ struct pf1550_charger,
+ bat_sense_work);
+ unsigned int data;
+
+ if (!chg->charger)
+ return;
+
+ if (regmap_read(chg->pf1550->regmap, PF1550_CHARG_REG_BATT_SNS, &data)) {
+ dev_err(chg->dev, "Read BATT_SNS error.\n");
+ return;
+ }
+
+ switch (data & PF1550_BAT_SNS_MASK) {
+ case PF1550_BAT_NO_VBUS:
+ dev_dbg(chg->dev, "No valid VBUS input.\n");
+ break;
+ case PF1550_BAT_LOW_THAN_PRECHARG:
+ dev_dbg(chg->dev, "VBAT < VPRECHG.LB.\n");
+ break;
+ case PF1550_BAT_CHARG_FAIL:
+ dev_dbg(chg->dev, "Battery charging failed.\n");
+ break;
+ case PF1550_BAT_HIGH_THAN_PRECHARG:
+ dev_dbg(chg->dev, "VBAT > VPRECHG.LB.\n");
+ break;
+ case PF1550_BAT_OVER_VOL:
+ dev_dbg(chg->dev, "VBAT > VBATOV.\n");
+ break;
+ case PF1550_BAT_NO_DETECT:
+ dev_dbg(chg->dev, "Battery not detected.\n");
+ break;
+ default:
+ dev_err(chg->dev, "Unknown value read:%x\n",
+ data & PF1550_CHG_SNS_MASK);
+ }
+}
+
+static void pf1550_chg_chg_work(struct work_struct *work)
+{
+ struct pf1550_charger *chg = container_of(to_delayed_work(work),
+ struct pf1550_charger,
+ chg_sense_work);
+ unsigned int data;
+
+ if (!chg->charger)
+ return;
+
+ if (regmap_read(chg->pf1550->regmap, PF1550_CHARG_REG_CHG_SNS, &data)) {
+ dev_err(chg->dev, "Read CHG_SNS error.\n");
+ return;
+ }
+
+ switch (data & PF1550_CHG_SNS_MASK) {
+ case PF1550_CHG_PRECHARGE:
+ dev_dbg(chg->dev, "In pre-charger mode.\n");
+ break;
+ case PF1550_CHG_CONSTANT_CURRENT:
+ dev_dbg(chg->dev, "In fast-charge constant current mode.\n");
+ break;
+ case PF1550_CHG_CONSTANT_VOL:
+ dev_dbg(chg->dev, "In fast-charge constant voltage mode.\n");
+ break;
+ case PF1550_CHG_EOC:
+ dev_dbg(chg->dev, "In EOC mode.\n");
+ break;
+ case PF1550_CHG_DONE:
+ dev_dbg(chg->dev, "In DONE mode.\n");
+ break;
+ case PF1550_CHG_TIMER_FAULT:
+ dev_info(chg->dev, "In timer fault mode.\n");
+ break;
+ case PF1550_CHG_SUSPEND:
+ dev_info(chg->dev, "In thermistor suspend mode.\n");
+ break;
+ case PF1550_CHG_OFF_INV:
+ dev_info(chg->dev, "Input invalid, charger off.\n");
+ break;
+ case PF1550_CHG_BAT_OVER:
+ dev_info(chg->dev, "Battery over-voltage.\n");
+ break;
+ case PF1550_CHG_OFF_TEMP:
+ dev_info(chg->dev, "Temp high, charger off.\n");
+ break;
+ case PF1550_CHG_LINEAR_ONLY:
+ dev_dbg(chg->dev, "In Linear mode, not charging.\n");
+ break;
+ default:
+ dev_err(chg->dev, "Unknown value read:%x\n",
+ data & PF1550_CHG_SNS_MASK);
+ }
+}
+
+static void pf1550_chg_vbus_work(struct work_struct *work)
+{
+ struct pf1550_charger *chg = container_of(to_delayed_work(work),
+ struct pf1550_charger,
+ vbus_sense_work);
+ bool psy_changed = false;
+ unsigned int data;
+
+ if (!chg->charger)
+ return;
+
+ if (regmap_read(chg->pf1550->regmap, PF1550_CHARG_REG_VBUS_SNS, &data)) {
+ dev_err(chg->dev, "Read VBUS_SNS error.\n");
+ return;
+ }
+
+ if (data & PF1550_VBUS_UVLO) {
+ chg->psy_desc.type = POWER_SUPPLY_TYPE_BATTERY;
+ psy_changed = true;
+ dev_dbg(chg->dev, "VBUS detached.\n");
+ }
+ if (data & PF1550_VBUS_IN2SYS)
+ dev_dbg(chg->dev, "VBUS_IN2SYS_SNS.\n");
+ if (data & PF1550_VBUS_OVLO)
+ dev_dbg(chg->dev, "VBUS_OVLO_SNS.\n");
+ if (data & PF1550_VBUS_VALID) {
+ chg->psy_desc.type = POWER_SUPPLY_TYPE_MAINS;
+ psy_changed = true;
+ dev_dbg(chg->dev, "VBUS attached.\n");
+ }
+
+ if (psy_changed)
+ power_supply_changed(chg->charger);
+}
+
+static irqreturn_t pf1550_charger_irq_handler(int irq, void *data)
+{
+ struct pf1550_charger *chg = data;
+ struct device *dev = chg->dev;
+ int i, irq_type = -1;
+
+ for (i = 0; i < PF1550_CHARGER_IRQ_NR; i++)
+ if (irq == chg->virqs[i])
+ irq_type = i;
+
+ switch (irq_type) {
+ case PF1550_CHARG_IRQ_BAT2SOCI:
+ dev_info(dev, "BAT to SYS Overcurrent interrupt.\n");
+ break;
+ case PF1550_CHARG_IRQ_BATI:
+ schedule_delayed_work(&chg->bat_sense_work,
+ msecs_to_jiffies(10));
+ break;
+ case PF1550_CHARG_IRQ_CHGI:
+ schedule_delayed_work(&chg->chg_sense_work,
+ msecs_to_jiffies(10));
+ break;
+ case PF1550_CHARG_IRQ_VBUSI:
+ schedule_delayed_work(&chg->vbus_sense_work,
+ msecs_to_jiffies(10));
+ break;
+ case PF1550_CHARG_IRQ_THMI:
+ dev_info(dev, "Thermal interrupt.\n");
+ break;
+ default:
+ dev_err(dev, "unknown interrupt occurred.\n");
+ }
+
+ return IRQ_HANDLED;
+}
+
+static enum power_supply_property pf1550_charger_props[] = {
+ POWER_SUPPLY_PROP_STATUS,
+ POWER_SUPPLY_PROP_CHARGE_TYPE,
+ POWER_SUPPLY_PROP_HEALTH,
+ POWER_SUPPLY_PROP_PRESENT,
+ POWER_SUPPLY_PROP_ONLINE,
+ POWER_SUPPLY_PROP_MODEL_NAME,
+ POWER_SUPPLY_PROP_MANUFACTURER,
+};
+
+static int pf1550_charger_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct pf1550_charger *chg = power_supply_get_drvdata(psy);
+ struct regmap *regmap = chg->pf1550->regmap;
+ int ret = 0;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_STATUS:
+ ret = pf1550_get_charger_state(regmap, &val->intval);
+ break;
+ case POWER_SUPPLY_PROP_CHARGE_TYPE:
+ ret = pf1550_get_charge_type(regmap, &val->intval);
+ break;
+ case POWER_SUPPLY_PROP_HEALTH:
+ ret = pf1550_get_battery_health(regmap, &val->intval);
+ break;
+ case POWER_SUPPLY_PROP_PRESENT:
+ ret = pf1550_get_present(regmap, &val->intval);
+ break;
+ case POWER_SUPPLY_PROP_ONLINE:
+ ret = pf1550_get_online(regmap, &val->intval);
+ break;
+ case POWER_SUPPLY_PROP_MODEL_NAME:
+ val->strval = "PF1550";
+ break;
+ case POWER_SUPPLY_PROP_MANUFACTURER:
+ val->strval = "NXP";
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return ret;
+}
+
+static int pf1550_set_constant_volt(struct pf1550_charger *chg,
+ unsigned int uvolt)
+{
+ unsigned int data;
+
+ if (uvolt >= 3500000 && uvolt <= 4440000)
+ data = 8 + (uvolt - 3500000) / 20000;
+ else
+ return dev_err_probe(chg->dev, -EINVAL,
+ "Wrong value for constant voltage\n");
+
+ dev_dbg(chg->dev, "Charging constant voltage: %u (0x%x)\n", uvolt,
+ data);
+
+ return regmap_update_bits(chg->pf1550->regmap,
+ PF1550_CHARG_REG_BATT_REG,
+ PF1550_CHARG_REG_BATT_REG_CHGCV_MASK, data);
+}
+
+static int pf1550_set_min_system_volt(struct pf1550_charger *chg,
+ unsigned int uvolt)
+{
+ unsigned int data;
+
+ switch (uvolt) {
+ case 3500000:
+ data = 0x0;
+ break;
+ case 3700000:
+ data = 0x1;
+ break;
+ case 4300000:
+ data = 0x2;
+ break;
+ default:
+ return dev_err_probe(chg->dev, -EINVAL,
+ "Wrong value for minimum system voltage\n");
+ }
+
+ data <<= PF1550_CHARG_REG_BATT_REG_VMINSYS_SHIFT;
+
+ dev_dbg(chg->dev, "Minimum system regulation voltage: %u (0x%x)\n",
+ uvolt, data);
+
+ return regmap_update_bits(chg->pf1550->regmap,
+ PF1550_CHARG_REG_BATT_REG,
+ PF1550_CHARG_REG_BATT_REG_VMINSYS_MASK, data);
+}
+
+static int pf1550_set_thermal_regulation_temp(struct pf1550_charger *chg,
+ unsigned int cells)
+{
+ unsigned int data;
+
+ switch (cells) {
+ case 60:
+ data = 0x0;
+ break;
+ case 75:
+ data = 0x1;
+ break;
+ case 90:
+ data = 0x2;
+ break;
+ case 105:
+ data = 0x3;
+ break;
+ default:
+ return dev_err_probe(chg->dev, -EINVAL,
+ "Wrong value for thermal temperature\n");
+ }
+
+ data <<= PF1550_CHARG_REG_THM_REG_CNFG_REGTEMP_SHIFT;
+
+ dev_dbg(chg->dev, "Thermal regulation loop temperature: %u (0x%x)\n",
+ cells, data);
+
+ return regmap_update_bits(chg->pf1550->regmap,
+ PF1550_CHARG_REG_THM_REG_CNFG,
+ PF1550_CHARG_REG_THM_REG_CNFG_REGTEMP_MASK,
+ data);
+}
+
+/*
+ * Sets charger registers to proper and safe default values.
+ */
+static int pf1550_reg_init(struct pf1550_charger *chg)
+{
+ struct device *dev = chg->dev;
+ unsigned int data;
+ int ret;
+
+ /* Unmask charger interrupt, mask DPMI and reserved bit */
+ ret = regmap_write(chg->pf1550->regmap, PF1550_CHARG_REG_CHG_INT_MASK,
+ PF1550_CHG_INT_MASK);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Error unmask charger interrupt\n");
+
+ ret = regmap_read(chg->pf1550->regmap, PF1550_CHARG_REG_VBUS_SNS,
+ &data);
+ if (ret)
+ return dev_err_probe(dev, ret, "Read charg vbus_sns error\n");
+
+ if (data & PF1550_VBUS_VALID)
+ chg->psy_desc.type = POWER_SUPPLY_TYPE_MAINS;
+
+ ret = pf1550_set_constant_volt(chg, chg->constant_volt);
+ if (ret)
+ return ret;
+
+ ret = pf1550_set_min_system_volt(chg, chg->min_system_volt);
+ if (ret)
+ return ret;
+
+ ret = pf1550_set_thermal_regulation_temp(chg,
+ chg->thermal_regulation_temp);
+ if (ret)
+ return ret;
+
+ /* Turn on charger */
+ ret = regmap_write(chg->pf1550->regmap, PF1550_CHARG_REG_CHG_OPER,
+ PF1550_CHG_TURNON);
+ if (ret)
+ return dev_err_probe(dev, ret, "Error turn on charger\n");
+
+ return 0;
+}
+
+static void pf1550_dt_parse_dev_info(struct pf1550_charger *chg)
+{
+ struct power_supply_battery_info *info;
+ struct device *dev = chg->dev;
+
+ if (device_property_read_u32(dev->parent, "nxp,min-system-microvolt",
+ &chg->min_system_volt))
+ chg->min_system_volt = PF1550_DEFAULT_MIN_SYSTEM_VOLT;
+
+ if (device_property_read_u32(dev->parent,
+ "nxp,thermal-regulation-celsius",
+ &chg->thermal_regulation_temp))
+ chg->thermal_regulation_temp = PF1550_DEFAULT_THERMAL_TEMP;
+
+ if (power_supply_get_battery_info(chg->charger, &info))
+ chg->constant_volt = PF1550_DEFAULT_CONSTANT_VOLT;
+ else
+ chg->constant_volt = info->constant_charge_voltage_max_uv;
+}
+
+static int pf1550_charger_probe(struct platform_device *pdev)
+{
+ const struct pf1550_dev *pf1550 = dev_get_drvdata(pdev->dev.parent);
+ struct power_supply_config psy_cfg = {};
+ struct pf1550_charger *chg;
+ int i, irq, ret;
+
+ chg = devm_kzalloc(&pdev->dev, sizeof(*chg), GFP_KERNEL);
+ if (!chg)
+ return -ENOMEM;
+
+ chg->dev = &pdev->dev;
+ chg->pf1550 = pf1550;
+
+ if (!chg->pf1550->regmap)
+ return dev_err_probe(&pdev->dev, -ENODEV,
+ "failed to get regmap\n");
+
+ platform_set_drvdata(pdev, chg);
+
+ INIT_DELAYED_WORK(&chg->vbus_sense_work, pf1550_chg_vbus_work);
+ INIT_DELAYED_WORK(&chg->chg_sense_work, pf1550_chg_chg_work);
+ INIT_DELAYED_WORK(&chg->bat_sense_work, pf1550_chg_bat_work);
+
+ for (i = 0; i < PF1550_CHARGER_IRQ_NR; i++) {
+ irq = platform_get_irq(pdev, i);
+ if (irq < 0)
+ return irq;
+
+ chg->virqs[i] = irq;
+
+ ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+ pf1550_charger_irq_handler,
+ IRQF_NO_SUSPEND,
+ "pf1550-charger", chg);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "failed irq request\n");
+ }
+
+ psy_cfg.drv_data = chg;
+
+ chg->psy_desc.name = PF1550_CHARGER_NAME;
+ chg->psy_desc.type = POWER_SUPPLY_TYPE_BATTERY;
+ chg->psy_desc.get_property = pf1550_charger_get_property;
+ chg->psy_desc.properties = pf1550_charger_props;
+ chg->psy_desc.num_properties = ARRAY_SIZE(pf1550_charger_props);
+
+ chg->charger = devm_power_supply_register(&pdev->dev, &chg->psy_desc,
+ &psy_cfg);
+ if (IS_ERR(chg->charger))
+ return dev_err_probe(&pdev->dev, PTR_ERR(chg->charger),
+ "failed: power supply register\n");
+
+ pf1550_dt_parse_dev_info(chg);
+
+ return pf1550_reg_init(chg);
+}
+
+static void pf1550_charger_remove(struct platform_device *pdev)
+{
+ struct pf1550_charger *chg = platform_get_drvdata(pdev);
+
+ cancel_delayed_work_sync(&chg->bat_sense_work);
+ cancel_delayed_work_sync(&chg->chg_sense_work);
+ cancel_delayed_work_sync(&chg->vbus_sense_work);
+}
+
+static const struct platform_device_id pf1550_charger_id[] = {
+ { "pf1550-charger", },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, pf1550_charger_id);
+
+static struct platform_driver pf1550_charger_driver = {
+ .driver = {
+ .name = "pf1550-charger",
+ },
+ .probe = pf1550_charger_probe,
+ .remove = pf1550_charger_remove,
+ .id_table = pf1550_charger_id,
+};
+module_platform_driver(pf1550_charger_driver);
+
+MODULE_AUTHOR("Robin Gong <yibin.gong@freescale.com>");
+MODULE_DESCRIPTION("PF1550 charger driver");
+MODULE_LICENSE("GPL");
--
2.49.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v7 5/6] power: supply: pf1550: add battery charger support
2025-06-12 14:55 ` [PATCH v7 5/6] power: supply: pf1550: add battery charger support Samuel Kayode via B4 Relay
@ 2025-06-12 16:02 ` Frank Li
2025-06-22 0:43 ` Sebastian Reichel
1 sibling, 0 replies; 17+ messages in thread
From: Frank Li @ 2025-06-12 16:02 UTC (permalink / raw)
To: samuel.kayode
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Liam Girdwood, Mark Brown, Dmitry Torokhov, Sebastian Reichel,
imx, devicetree, linux-kernel, linux-input, linux-pm, Abel Vesa,
Abel Vesa, Robin Gong, Robin Gong, Enric Balletbo i Serra
On Thu, Jun 12, 2025 at 10:55:47AM -0400, Samuel Kayode via B4 Relay wrote:
> From: Samuel Kayode <samuel.kayode@savoirfairelinux.com>
>
> Add support for the battery charger for pf1550 PMIC.
>
> Signed-off-by: Samuel Kayode <samuel.kayode@savoirfairelinux.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> ---
> v7:
> - Use reverse christmas tree order
> - Drop unecessary 0 in id table's driver data field
> - Store virqs to avoid reinvoking platform_get_irq in the interrupt
> service routine
> - Drop manufacturer and model global variables
> v6:
> - Drop lock entirely
> - Reverse christmas tree order for variables defined in probe as
> suggested by Frank
> - return pf1550_reg_init
> v5:
> - Drop lock for battery and charger delayed_work
> - More conservative locking in vbus delayed_work
> - Apply lock when setting power supply type during register initialization
> v4:
> - Finish handling of some interrupts in threaded irq handler
> - Use platform_get_irq
> v3:
> - Use struct power_supply_get_battery_info to get constant charge
> voltage if specified
> - Use virqs mapped in MFD driver
> v2:
> - Address feedback from Enric Balletbo Serra
> ---
> drivers/power/supply/Kconfig | 11 +
> drivers/power/supply/Makefile | 1 +
> drivers/power/supply/pf1550-charger.c | 632 ++++++++++++++++++++++++++++++++++
> 3 files changed, 644 insertions(+)
>
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index 79ddb006e2dad6bf96b71ed570a37c006b5f9433..6d0c872edac1f45da314632e671af1aeda4c87b8 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -471,6 +471,17 @@ config CHARGER_88PM860X
> help
> Say Y here to enable charger for Marvell 88PM860x chip.
>
> +config CHARGER_PF1550
> + tristate "NXP PF1550 battery charger driver"
> + depends on MFD_PF1550
> + help
> + Say Y to enable support for the NXP PF1550 battery charger.
> + The device is a single cell Li-Ion/Li-Polymer battery charger for
> + portable application.
> +
> + This driver can also be built as a module. If so, the module will be
> + called pf1550-charger.
> +
> config BATTERY_RX51
> tristate "Nokia RX-51 (N900) battery driver"
> depends on TWL4030_MADC
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index 4f5f8e3507f80da02812f0d08c2d81ddff0a272f..7f68380099c59dab71b73120150612a23e16a745 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -64,6 +64,7 @@ obj-$(CONFIG_CHARGER_RT9467) += rt9467-charger.o
> obj-$(CONFIG_CHARGER_RT9471) += rt9471.o
> obj-$(CONFIG_BATTERY_TWL4030_MADC) += twl4030_madc_battery.o
> obj-$(CONFIG_CHARGER_88PM860X) += 88pm860x_charger.o
> +obj-$(CONFIG_CHARGER_PF1550) += pf1550-charger.o
> obj-$(CONFIG_BATTERY_RX51) += rx51_battery.o
> obj-$(CONFIG_AB8500_BM) += ab8500_bmdata.o ab8500_charger.o ab8500_fg.o ab8500_btemp.o ab8500_chargalg.o
> obj-$(CONFIG_CHARGER_CPCAP) += cpcap-charger.o
> diff --git a/drivers/power/supply/pf1550-charger.c b/drivers/power/supply/pf1550-charger.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..1693fb2afdd444e088827edc476fa271fb0937e0
> --- /dev/null
> +++ b/drivers/power/supply/pf1550-charger.c
> @@ -0,0 +1,632 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// pf1550_charger.c - charger driver for the PF1550
> +//
> +// Copyright (C) 2016 Freescale Semiconductor, Inc.
> +// Robin Gong <yibin.gong@freescale.com>
> +//
> +// Portions Copyright (c) 2025 Savoir-faire Linux Inc.
> +// Samuel Kayode <samuel.kayode@savoirfairelinux.com>
> +//
> +
> +#include <linux/interrupt.h>
> +#include <linux/mfd/pf1550.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/power_supply.h>
> +
> +#define PF1550_CHARGER_NAME "pf1550-charger"
> +#define PF1550_DEFAULT_CONSTANT_VOLT 4200000
> +#define PF1550_DEFAULT_MIN_SYSTEM_VOLT 3500000
> +#define PF1550_DEFAULT_THERMAL_TEMP 75
> +#define PF1550_CHARGER_IRQ_NR 5
> +
> +struct pf1550_charger {
> + struct device *dev;
> + const struct pf1550_dev *pf1550;
> + struct power_supply *charger;
> + struct power_supply_desc psy_desc;
> + struct delayed_work vbus_sense_work;
> + struct delayed_work chg_sense_work;
> + struct delayed_work bat_sense_work;
> + int virqs[PF1550_CHARGER_IRQ_NR];
> +
> + u32 constant_volt;
> + u32 min_system_volt;
> + u32 thermal_regulation_temp;
> +};
> +
> +static int pf1550_get_charger_state(struct regmap *regmap, int *val)
> +{
> + unsigned int data;
> + int ret;
> +
> + ret = regmap_read(regmap, PF1550_CHARG_REG_CHG_SNS, &data);
> + if (ret < 0)
> + return ret;
> +
> + data &= PF1550_CHG_SNS_MASK;
> +
> + switch (data) {
> + case PF1550_CHG_PRECHARGE:
> + case PF1550_CHG_CONSTANT_CURRENT:
> + *val = POWER_SUPPLY_STATUS_CHARGING;
> + break;
> + case PF1550_CHG_CONSTANT_VOL:
> + *val = POWER_SUPPLY_STATUS_CHARGING;
> + break;
> + case PF1550_CHG_EOC:
> + *val = POWER_SUPPLY_STATUS_CHARGING;
> + break;
> + case PF1550_CHG_DONE:
> + *val = POWER_SUPPLY_STATUS_FULL;
> + break;
> + case PF1550_CHG_TIMER_FAULT:
> + case PF1550_CHG_SUSPEND:
> + *val = POWER_SUPPLY_STATUS_NOT_CHARGING;
> + break;
> + case PF1550_CHG_OFF_INV:
> + case PF1550_CHG_OFF_TEMP:
> + case PF1550_CHG_LINEAR_ONLY:
> + *val = POWER_SUPPLY_STATUS_DISCHARGING;
> + break;
> + default:
> + *val = POWER_SUPPLY_STATUS_UNKNOWN;
> + }
> +
> + return 0;
> +}
> +
> +static int pf1550_get_charge_type(struct regmap *regmap, int *val)
> +{
> + unsigned int data;
> + int ret;
> +
> + ret = regmap_read(regmap, PF1550_CHARG_REG_CHG_SNS, &data);
> + if (ret < 0)
> + return ret;
> +
> + data &= PF1550_CHG_SNS_MASK;
> +
> + switch (data) {
> + case PF1550_CHG_SNS_MASK:
> + *val = POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
> + break;
> + case PF1550_CHG_CONSTANT_CURRENT:
> + case PF1550_CHG_CONSTANT_VOL:
> + case PF1550_CHG_EOC:
> + *val = POWER_SUPPLY_CHARGE_TYPE_FAST;
> + break;
> + case PF1550_CHG_DONE:
> + case PF1550_CHG_TIMER_FAULT:
> + case PF1550_CHG_SUSPEND:
> + case PF1550_CHG_OFF_INV:
> + case PF1550_CHG_BAT_OVER:
> + case PF1550_CHG_OFF_TEMP:
> + case PF1550_CHG_LINEAR_ONLY:
> + *val = POWER_SUPPLY_CHARGE_TYPE_NONE;
> + break;
> + default:
> + *val = POWER_SUPPLY_CHARGE_TYPE_UNKNOWN;
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * Supported health statuses:
> + * - POWER_SUPPLY_HEALTH_DEAD
> + * - POWER_SUPPLY_HEALTH_GOOD
> + * - POWER_SUPPLY_HEALTH_OVERVOLTAGE
> + * - POWER_SUPPLY_HEALTH_UNKNOWN
> + */
> +static int pf1550_get_battery_health(struct regmap *regmap, int *val)
> +{
> + unsigned int data;
> + int ret;
> +
> + ret = regmap_read(regmap, PF1550_CHARG_REG_BATT_SNS, &data);
> + if (ret < 0)
> + return ret;
> +
> + data &= PF1550_BAT_SNS_MASK;
> +
> + switch (data) {
> + case PF1550_BAT_NO_DETECT:
> + *val = POWER_SUPPLY_HEALTH_DEAD;
> + break;
> + case PF1550_BAT_NO_VBUS:
> + case PF1550_BAT_LOW_THAN_PRECHARG:
> + case PF1550_BAT_CHARG_FAIL:
> + case PF1550_BAT_HIGH_THAN_PRECHARG:
> + *val = POWER_SUPPLY_HEALTH_GOOD;
> + break;
> + case PF1550_BAT_OVER_VOL:
> + *val = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
> + break;
> + default:
> + *val = POWER_SUPPLY_HEALTH_UNKNOWN;
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int pf1550_get_present(struct regmap *regmap, int *val)
> +{
> + unsigned int data;
> + int ret;
> +
> + ret = regmap_read(regmap, PF1550_CHARG_REG_BATT_SNS, &data);
> + if (ret < 0)
> + return ret;
> +
> + data &= PF1550_BAT_SNS_MASK;
> + *val = (data == PF1550_BAT_NO_DETECT) ? 0 : 1;
> +
> + return 0;
> +}
> +
> +static int pf1550_get_online(struct regmap *regmap, int *val)
> +{
> + unsigned int data;
> + int ret;
> +
> + ret = regmap_read(regmap, PF1550_CHARG_REG_VBUS_SNS, &data);
> + if (ret < 0)
> + return ret;
> +
> + *val = (data & PF1550_VBUS_VALID) ? 1 : 0;
> +
> + return 0;
> +}
> +
> +static void pf1550_chg_bat_work(struct work_struct *work)
> +{
> + struct pf1550_charger *chg = container_of(to_delayed_work(work),
> + struct pf1550_charger,
> + bat_sense_work);
> + unsigned int data;
> +
> + if (!chg->charger)
> + return;
> +
> + if (regmap_read(chg->pf1550->regmap, PF1550_CHARG_REG_BATT_SNS, &data)) {
> + dev_err(chg->dev, "Read BATT_SNS error.\n");
> + return;
> + }
> +
> + switch (data & PF1550_BAT_SNS_MASK) {
> + case PF1550_BAT_NO_VBUS:
> + dev_dbg(chg->dev, "No valid VBUS input.\n");
> + break;
> + case PF1550_BAT_LOW_THAN_PRECHARG:
> + dev_dbg(chg->dev, "VBAT < VPRECHG.LB.\n");
> + break;
> + case PF1550_BAT_CHARG_FAIL:
> + dev_dbg(chg->dev, "Battery charging failed.\n");
> + break;
> + case PF1550_BAT_HIGH_THAN_PRECHARG:
> + dev_dbg(chg->dev, "VBAT > VPRECHG.LB.\n");
> + break;
> + case PF1550_BAT_OVER_VOL:
> + dev_dbg(chg->dev, "VBAT > VBATOV.\n");
> + break;
> + case PF1550_BAT_NO_DETECT:
> + dev_dbg(chg->dev, "Battery not detected.\n");
> + break;
> + default:
> + dev_err(chg->dev, "Unknown value read:%x\n",
> + data & PF1550_CHG_SNS_MASK);
> + }
> +}
> +
> +static void pf1550_chg_chg_work(struct work_struct *work)
> +{
> + struct pf1550_charger *chg = container_of(to_delayed_work(work),
> + struct pf1550_charger,
> + chg_sense_work);
> + unsigned int data;
> +
> + if (!chg->charger)
> + return;
> +
> + if (regmap_read(chg->pf1550->regmap, PF1550_CHARG_REG_CHG_SNS, &data)) {
> + dev_err(chg->dev, "Read CHG_SNS error.\n");
> + return;
> + }
> +
> + switch (data & PF1550_CHG_SNS_MASK) {
> + case PF1550_CHG_PRECHARGE:
> + dev_dbg(chg->dev, "In pre-charger mode.\n");
> + break;
> + case PF1550_CHG_CONSTANT_CURRENT:
> + dev_dbg(chg->dev, "In fast-charge constant current mode.\n");
> + break;
> + case PF1550_CHG_CONSTANT_VOL:
> + dev_dbg(chg->dev, "In fast-charge constant voltage mode.\n");
> + break;
> + case PF1550_CHG_EOC:
> + dev_dbg(chg->dev, "In EOC mode.\n");
> + break;
> + case PF1550_CHG_DONE:
> + dev_dbg(chg->dev, "In DONE mode.\n");
> + break;
> + case PF1550_CHG_TIMER_FAULT:
> + dev_info(chg->dev, "In timer fault mode.\n");
> + break;
> + case PF1550_CHG_SUSPEND:
> + dev_info(chg->dev, "In thermistor suspend mode.\n");
> + break;
> + case PF1550_CHG_OFF_INV:
> + dev_info(chg->dev, "Input invalid, charger off.\n");
> + break;
> + case PF1550_CHG_BAT_OVER:
> + dev_info(chg->dev, "Battery over-voltage.\n");
> + break;
> + case PF1550_CHG_OFF_TEMP:
> + dev_info(chg->dev, "Temp high, charger off.\n");
> + break;
> + case PF1550_CHG_LINEAR_ONLY:
> + dev_dbg(chg->dev, "In Linear mode, not charging.\n");
> + break;
> + default:
> + dev_err(chg->dev, "Unknown value read:%x\n",
> + data & PF1550_CHG_SNS_MASK);
> + }
> +}
> +
> +static void pf1550_chg_vbus_work(struct work_struct *work)
> +{
> + struct pf1550_charger *chg = container_of(to_delayed_work(work),
> + struct pf1550_charger,
> + vbus_sense_work);
> + bool psy_changed = false;
> + unsigned int data;
> +
> + if (!chg->charger)
> + return;
> +
> + if (regmap_read(chg->pf1550->regmap, PF1550_CHARG_REG_VBUS_SNS, &data)) {
> + dev_err(chg->dev, "Read VBUS_SNS error.\n");
> + return;
> + }
> +
> + if (data & PF1550_VBUS_UVLO) {
> + chg->psy_desc.type = POWER_SUPPLY_TYPE_BATTERY;
> + psy_changed = true;
> + dev_dbg(chg->dev, "VBUS detached.\n");
> + }
> + if (data & PF1550_VBUS_IN2SYS)
> + dev_dbg(chg->dev, "VBUS_IN2SYS_SNS.\n");
> + if (data & PF1550_VBUS_OVLO)
> + dev_dbg(chg->dev, "VBUS_OVLO_SNS.\n");
> + if (data & PF1550_VBUS_VALID) {
> + chg->psy_desc.type = POWER_SUPPLY_TYPE_MAINS;
> + psy_changed = true;
> + dev_dbg(chg->dev, "VBUS attached.\n");
> + }
> +
> + if (psy_changed)
> + power_supply_changed(chg->charger);
> +}
> +
> +static irqreturn_t pf1550_charger_irq_handler(int irq, void *data)
> +{
> + struct pf1550_charger *chg = data;
> + struct device *dev = chg->dev;
> + int i, irq_type = -1;
> +
> + for (i = 0; i < PF1550_CHARGER_IRQ_NR; i++)
> + if (irq == chg->virqs[i])
> + irq_type = i;
> +
> + switch (irq_type) {
> + case PF1550_CHARG_IRQ_BAT2SOCI:
> + dev_info(dev, "BAT to SYS Overcurrent interrupt.\n");
> + break;
> + case PF1550_CHARG_IRQ_BATI:
> + schedule_delayed_work(&chg->bat_sense_work,
> + msecs_to_jiffies(10));
> + break;
> + case PF1550_CHARG_IRQ_CHGI:
> + schedule_delayed_work(&chg->chg_sense_work,
> + msecs_to_jiffies(10));
> + break;
> + case PF1550_CHARG_IRQ_VBUSI:
> + schedule_delayed_work(&chg->vbus_sense_work,
> + msecs_to_jiffies(10));
> + break;
> + case PF1550_CHARG_IRQ_THMI:
> + dev_info(dev, "Thermal interrupt.\n");
> + break;
> + default:
> + dev_err(dev, "unknown interrupt occurred.\n");
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static enum power_supply_property pf1550_charger_props[] = {
> + POWER_SUPPLY_PROP_STATUS,
> + POWER_SUPPLY_PROP_CHARGE_TYPE,
> + POWER_SUPPLY_PROP_HEALTH,
> + POWER_SUPPLY_PROP_PRESENT,
> + POWER_SUPPLY_PROP_ONLINE,
> + POWER_SUPPLY_PROP_MODEL_NAME,
> + POWER_SUPPLY_PROP_MANUFACTURER,
> +};
> +
> +static int pf1550_charger_get_property(struct power_supply *psy,
> + enum power_supply_property psp,
> + union power_supply_propval *val)
> +{
> + struct pf1550_charger *chg = power_supply_get_drvdata(psy);
> + struct regmap *regmap = chg->pf1550->regmap;
> + int ret = 0;
> +
> + switch (psp) {
> + case POWER_SUPPLY_PROP_STATUS:
> + ret = pf1550_get_charger_state(regmap, &val->intval);
> + break;
> + case POWER_SUPPLY_PROP_CHARGE_TYPE:
> + ret = pf1550_get_charge_type(regmap, &val->intval);
> + break;
> + case POWER_SUPPLY_PROP_HEALTH:
> + ret = pf1550_get_battery_health(regmap, &val->intval);
> + break;
> + case POWER_SUPPLY_PROP_PRESENT:
> + ret = pf1550_get_present(regmap, &val->intval);
> + break;
> + case POWER_SUPPLY_PROP_ONLINE:
> + ret = pf1550_get_online(regmap, &val->intval);
> + break;
> + case POWER_SUPPLY_PROP_MODEL_NAME:
> + val->strval = "PF1550";
> + break;
> + case POWER_SUPPLY_PROP_MANUFACTURER:
> + val->strval = "NXP";
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return ret;
> +}
> +
> +static int pf1550_set_constant_volt(struct pf1550_charger *chg,
> + unsigned int uvolt)
> +{
> + unsigned int data;
> +
> + if (uvolt >= 3500000 && uvolt <= 4440000)
> + data = 8 + (uvolt - 3500000) / 20000;
> + else
> + return dev_err_probe(chg->dev, -EINVAL,
> + "Wrong value for constant voltage\n");
> +
> + dev_dbg(chg->dev, "Charging constant voltage: %u (0x%x)\n", uvolt,
> + data);
> +
> + return regmap_update_bits(chg->pf1550->regmap,
> + PF1550_CHARG_REG_BATT_REG,
> + PF1550_CHARG_REG_BATT_REG_CHGCV_MASK, data);
> +}
> +
> +static int pf1550_set_min_system_volt(struct pf1550_charger *chg,
> + unsigned int uvolt)
> +{
> + unsigned int data;
> +
> + switch (uvolt) {
> + case 3500000:
> + data = 0x0;
> + break;
> + case 3700000:
> + data = 0x1;
> + break;
> + case 4300000:
> + data = 0x2;
> + break;
> + default:
> + return dev_err_probe(chg->dev, -EINVAL,
> + "Wrong value for minimum system voltage\n");
> + }
> +
> + data <<= PF1550_CHARG_REG_BATT_REG_VMINSYS_SHIFT;
> +
> + dev_dbg(chg->dev, "Minimum system regulation voltage: %u (0x%x)\n",
> + uvolt, data);
> +
> + return regmap_update_bits(chg->pf1550->regmap,
> + PF1550_CHARG_REG_BATT_REG,
> + PF1550_CHARG_REG_BATT_REG_VMINSYS_MASK, data);
> +}
> +
> +static int pf1550_set_thermal_regulation_temp(struct pf1550_charger *chg,
> + unsigned int cells)
> +{
> + unsigned int data;
> +
> + switch (cells) {
> + case 60:
> + data = 0x0;
> + break;
> + case 75:
> + data = 0x1;
> + break;
> + case 90:
> + data = 0x2;
> + break;
> + case 105:
> + data = 0x3;
> + break;
> + default:
> + return dev_err_probe(chg->dev, -EINVAL,
> + "Wrong value for thermal temperature\n");
> + }
> +
> + data <<= PF1550_CHARG_REG_THM_REG_CNFG_REGTEMP_SHIFT;
> +
> + dev_dbg(chg->dev, "Thermal regulation loop temperature: %u (0x%x)\n",
> + cells, data);
> +
> + return regmap_update_bits(chg->pf1550->regmap,
> + PF1550_CHARG_REG_THM_REG_CNFG,
> + PF1550_CHARG_REG_THM_REG_CNFG_REGTEMP_MASK,
> + data);
> +}
> +
> +/*
> + * Sets charger registers to proper and safe default values.
> + */
> +static int pf1550_reg_init(struct pf1550_charger *chg)
> +{
> + struct device *dev = chg->dev;
> + unsigned int data;
> + int ret;
> +
> + /* Unmask charger interrupt, mask DPMI and reserved bit */
> + ret = regmap_write(chg->pf1550->regmap, PF1550_CHARG_REG_CHG_INT_MASK,
> + PF1550_CHG_INT_MASK);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Error unmask charger interrupt\n");
> +
> + ret = regmap_read(chg->pf1550->regmap, PF1550_CHARG_REG_VBUS_SNS,
> + &data);
> + if (ret)
> + return dev_err_probe(dev, ret, "Read charg vbus_sns error\n");
> +
> + if (data & PF1550_VBUS_VALID)
> + chg->psy_desc.type = POWER_SUPPLY_TYPE_MAINS;
> +
> + ret = pf1550_set_constant_volt(chg, chg->constant_volt);
> + if (ret)
> + return ret;
> +
> + ret = pf1550_set_min_system_volt(chg, chg->min_system_volt);
> + if (ret)
> + return ret;
> +
> + ret = pf1550_set_thermal_regulation_temp(chg,
> + chg->thermal_regulation_temp);
> + if (ret)
> + return ret;
> +
> + /* Turn on charger */
> + ret = regmap_write(chg->pf1550->regmap, PF1550_CHARG_REG_CHG_OPER,
> + PF1550_CHG_TURNON);
> + if (ret)
> + return dev_err_probe(dev, ret, "Error turn on charger\n");
> +
> + return 0;
> +}
> +
> +static void pf1550_dt_parse_dev_info(struct pf1550_charger *chg)
> +{
> + struct power_supply_battery_info *info;
> + struct device *dev = chg->dev;
> +
> + if (device_property_read_u32(dev->parent, "nxp,min-system-microvolt",
> + &chg->min_system_volt))
> + chg->min_system_volt = PF1550_DEFAULT_MIN_SYSTEM_VOLT;
> +
> + if (device_property_read_u32(dev->parent,
> + "nxp,thermal-regulation-celsius",
> + &chg->thermal_regulation_temp))
> + chg->thermal_regulation_temp = PF1550_DEFAULT_THERMAL_TEMP;
> +
> + if (power_supply_get_battery_info(chg->charger, &info))
> + chg->constant_volt = PF1550_DEFAULT_CONSTANT_VOLT;
> + else
> + chg->constant_volt = info->constant_charge_voltage_max_uv;
> +}
> +
> +static int pf1550_charger_probe(struct platform_device *pdev)
> +{
> + const struct pf1550_dev *pf1550 = dev_get_drvdata(pdev->dev.parent);
> + struct power_supply_config psy_cfg = {};
> + struct pf1550_charger *chg;
> + int i, irq, ret;
> +
> + chg = devm_kzalloc(&pdev->dev, sizeof(*chg), GFP_KERNEL);
> + if (!chg)
> + return -ENOMEM;
> +
> + chg->dev = &pdev->dev;
> + chg->pf1550 = pf1550;
> +
> + if (!chg->pf1550->regmap)
> + return dev_err_probe(&pdev->dev, -ENODEV,
> + "failed to get regmap\n");
> +
> + platform_set_drvdata(pdev, chg);
> +
> + INIT_DELAYED_WORK(&chg->vbus_sense_work, pf1550_chg_vbus_work);
> + INIT_DELAYED_WORK(&chg->chg_sense_work, pf1550_chg_chg_work);
> + INIT_DELAYED_WORK(&chg->bat_sense_work, pf1550_chg_bat_work);
> +
> + for (i = 0; i < PF1550_CHARGER_IRQ_NR; i++) {
> + irq = platform_get_irq(pdev, i);
> + if (irq < 0)
> + return irq;
> +
> + chg->virqs[i] = irq;
> +
> + ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
> + pf1550_charger_irq_handler,
> + IRQF_NO_SUSPEND,
> + "pf1550-charger", chg);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "failed irq request\n");
> + }
> +
> + psy_cfg.drv_data = chg;
> +
> + chg->psy_desc.name = PF1550_CHARGER_NAME;
> + chg->psy_desc.type = POWER_SUPPLY_TYPE_BATTERY;
> + chg->psy_desc.get_property = pf1550_charger_get_property;
> + chg->psy_desc.properties = pf1550_charger_props;
> + chg->psy_desc.num_properties = ARRAY_SIZE(pf1550_charger_props);
> +
> + chg->charger = devm_power_supply_register(&pdev->dev, &chg->psy_desc,
> + &psy_cfg);
> + if (IS_ERR(chg->charger))
> + return dev_err_probe(&pdev->dev, PTR_ERR(chg->charger),
> + "failed: power supply register\n");
> +
> + pf1550_dt_parse_dev_info(chg);
> +
> + return pf1550_reg_init(chg);
> +}
> +
> +static void pf1550_charger_remove(struct platform_device *pdev)
> +{
> + struct pf1550_charger *chg = platform_get_drvdata(pdev);
> +
> + cancel_delayed_work_sync(&chg->bat_sense_work);
> + cancel_delayed_work_sync(&chg->chg_sense_work);
> + cancel_delayed_work_sync(&chg->vbus_sense_work);
> +}
> +
> +static const struct platform_device_id pf1550_charger_id[] = {
> + { "pf1550-charger", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(platform, pf1550_charger_id);
> +
> +static struct platform_driver pf1550_charger_driver = {
> + .driver = {
> + .name = "pf1550-charger",
> + },
> + .probe = pf1550_charger_probe,
> + .remove = pf1550_charger_remove,
> + .id_table = pf1550_charger_id,
> +};
> +module_platform_driver(pf1550_charger_driver);
> +
> +MODULE_AUTHOR("Robin Gong <yibin.gong@freescale.com>");
> +MODULE_DESCRIPTION("PF1550 charger driver");
> +MODULE_LICENSE("GPL");
>
> --
> 2.49.0
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v7 5/6] power: supply: pf1550: add battery charger support
2025-06-12 14:55 ` [PATCH v7 5/6] power: supply: pf1550: add battery charger support Samuel Kayode via B4 Relay
2025-06-12 16:02 ` Frank Li
@ 2025-06-22 0:43 ` Sebastian Reichel
2025-06-25 14:19 ` Samuel Kayode
1 sibling, 1 reply; 17+ messages in thread
From: Sebastian Reichel @ 2025-06-22 0:43 UTC (permalink / raw)
To: samuel.kayode
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Liam Girdwood, Mark Brown, Dmitry Torokhov, Frank Li, imx,
devicetree, linux-kernel, linux-input, linux-pm, Abel Vesa,
Abel Vesa, Robin Gong, Robin Gong, Enric Balletbo i Serra
[-- Attachment #1: Type: text/plain, Size: 22690 bytes --]
Hi,
On Thu, Jun 12, 2025 at 10:55:47AM -0400, Samuel Kayode via B4 Relay wrote:
> From: Samuel Kayode <samuel.kayode@savoirfairelinux.com>
>
> Add support for the battery charger for pf1550 PMIC.
>
> Signed-off-by: Samuel Kayode <samuel.kayode@savoirfairelinux.com>
> ---
> v7:
> - Use reverse christmas tree order
> - Drop unecessary 0 in id table's driver data field
> - Store virqs to avoid reinvoking platform_get_irq in the interrupt
> service routine
> - Drop manufacturer and model global variables
> v6:
> - Drop lock entirely
> - Reverse christmas tree order for variables defined in probe as
> suggested by Frank
> - return pf1550_reg_init
> v5:
> - Drop lock for battery and charger delayed_work
> - More conservative locking in vbus delayed_work
> - Apply lock when setting power supply type during register initialization
> v4:
> - Finish handling of some interrupts in threaded irq handler
> - Use platform_get_irq
> v3:
> - Use struct power_supply_get_battery_info to get constant charge
> voltage if specified
> - Use virqs mapped in MFD driver
> v2:
> - Address feedback from Enric Balletbo Serra
> ---
> drivers/power/supply/Kconfig | 11 +
> drivers/power/supply/Makefile | 1 +
> drivers/power/supply/pf1550-charger.c | 632 ++++++++++++++++++++++++++++++++++
> 3 files changed, 644 insertions(+)
>
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index 79ddb006e2dad6bf96b71ed570a37c006b5f9433..6d0c872edac1f45da314632e671af1aeda4c87b8 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -471,6 +471,17 @@ config CHARGER_88PM860X
> help
> Say Y here to enable charger for Marvell 88PM860x chip.
>
> +config CHARGER_PF1550
> + tristate "NXP PF1550 battery charger driver"
> + depends on MFD_PF1550
> + help
> + Say Y to enable support for the NXP PF1550 battery charger.
> + The device is a single cell Li-Ion/Li-Polymer battery charger for
> + portable application.
> +
> + This driver can also be built as a module. If so, the module will be
> + called pf1550-charger.
> +
> config BATTERY_RX51
> tristate "Nokia RX-51 (N900) battery driver"
> depends on TWL4030_MADC
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index 4f5f8e3507f80da02812f0d08c2d81ddff0a272f..7f68380099c59dab71b73120150612a23e16a745 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -64,6 +64,7 @@ obj-$(CONFIG_CHARGER_RT9467) += rt9467-charger.o
> obj-$(CONFIG_CHARGER_RT9471) += rt9471.o
> obj-$(CONFIG_BATTERY_TWL4030_MADC) += twl4030_madc_battery.o
> obj-$(CONFIG_CHARGER_88PM860X) += 88pm860x_charger.o
> +obj-$(CONFIG_CHARGER_PF1550) += pf1550-charger.o
> obj-$(CONFIG_BATTERY_RX51) += rx51_battery.o
> obj-$(CONFIG_AB8500_BM) += ab8500_bmdata.o ab8500_charger.o ab8500_fg.o ab8500_btemp.o ab8500_chargalg.o
> obj-$(CONFIG_CHARGER_CPCAP) += cpcap-charger.o
> diff --git a/drivers/power/supply/pf1550-charger.c b/drivers/power/supply/pf1550-charger.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..1693fb2afdd444e088827edc476fa271fb0937e0
> --- /dev/null
> +++ b/drivers/power/supply/pf1550-charger.c
> @@ -0,0 +1,632 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// pf1550_charger.c - charger driver for the PF1550
> +//
> +// Copyright (C) 2016 Freescale Semiconductor, Inc.
> +// Robin Gong <yibin.gong@freescale.com>
> +//
> +// Portions Copyright (c) 2025 Savoir-faire Linux Inc.
> +// Samuel Kayode <samuel.kayode@savoirfairelinux.com>
> +//
> +
> +#include <linux/interrupt.h>
> +#include <linux/mfd/pf1550.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/power_supply.h>
> +
> +#define PF1550_CHARGER_NAME "pf1550-charger"
There is only one place where this is being used, so drop the define
and place it directly.
> +#define PF1550_DEFAULT_CONSTANT_VOLT 4200000
> +#define PF1550_DEFAULT_MIN_SYSTEM_VOLT 3500000
> +#define PF1550_DEFAULT_THERMAL_TEMP 75
> +#define PF1550_CHARGER_IRQ_NR 5
> +
> +struct pf1550_charger {
> + struct device *dev;
> + const struct pf1550_dev *pf1550;
> + struct power_supply *charger;
> + struct power_supply_desc psy_desc;
> + struct delayed_work vbus_sense_work;
> + struct delayed_work chg_sense_work;
> + struct delayed_work bat_sense_work;
> + int virqs[PF1550_CHARGER_IRQ_NR];
> +
> + u32 constant_volt;
> + u32 min_system_volt;
> + u32 thermal_regulation_temp;
> +};
> +
> +static int pf1550_get_charger_state(struct regmap *regmap, int *val)
> +{
> + unsigned int data;
> + int ret;
> +
> + ret = regmap_read(regmap, PF1550_CHARG_REG_CHG_SNS, &data);
> + if (ret < 0)
> + return ret;
> +
> + data &= PF1550_CHG_SNS_MASK;
> +
> + switch (data) {
> + case PF1550_CHG_PRECHARGE:
> + case PF1550_CHG_CONSTANT_CURRENT:
> + *val = POWER_SUPPLY_STATUS_CHARGING;
> + break;
> + case PF1550_CHG_CONSTANT_VOL:
> + *val = POWER_SUPPLY_STATUS_CHARGING;
> + break;
> + case PF1550_CHG_EOC:
> + *val = POWER_SUPPLY_STATUS_CHARGING;
case PF1550_CHG_PRECHARGE:
case PF1550_CHG_CONSTANT_CURRENT:
case PF1550_CHG_CONSTANT_VOL:
case PF1550_CHG_EOC:
*val = POWER_SUPPLY_STATUS_CHARGING;
break;
> + case PF1550_CHG_DONE:
> + *val = POWER_SUPPLY_STATUS_FULL;
> + break;
> + case PF1550_CHG_TIMER_FAULT:
> + case PF1550_CHG_SUSPEND:
> + *val = POWER_SUPPLY_STATUS_NOT_CHARGING;
> + break;
> + case PF1550_CHG_OFF_INV:
> + case PF1550_CHG_OFF_TEMP:
> + case PF1550_CHG_LINEAR_ONLY:
> + *val = POWER_SUPPLY_STATUS_DISCHARGING;
> + break;
> + default:
> + *val = POWER_SUPPLY_STATUS_UNKNOWN;
> + }
> +
> + return 0;
> +}
> +
> +static int pf1550_get_charge_type(struct regmap *regmap, int *val)
> +{
> + unsigned int data;
> + int ret;
> +
> + ret = regmap_read(regmap, PF1550_CHARG_REG_CHG_SNS, &data);
> + if (ret < 0)
> + return ret;
> +
> + data &= PF1550_CHG_SNS_MASK;
> +
> + switch (data) {
> + case PF1550_CHG_SNS_MASK:
> + *val = POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
> + break;
> + case PF1550_CHG_CONSTANT_CURRENT:
> + case PF1550_CHG_CONSTANT_VOL:
> + case PF1550_CHG_EOC:
> + *val = POWER_SUPPLY_CHARGE_TYPE_FAST;
> + break;
> + case PF1550_CHG_DONE:
> + case PF1550_CHG_TIMER_FAULT:
> + case PF1550_CHG_SUSPEND:
> + case PF1550_CHG_OFF_INV:
> + case PF1550_CHG_BAT_OVER:
> + case PF1550_CHG_OFF_TEMP:
> + case PF1550_CHG_LINEAR_ONLY:
> + *val = POWER_SUPPLY_CHARGE_TYPE_NONE;
> + break;
> + default:
> + *val = POWER_SUPPLY_CHARGE_TYPE_UNKNOWN;
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * Supported health statuses:
> + * - POWER_SUPPLY_HEALTH_DEAD
> + * - POWER_SUPPLY_HEALTH_GOOD
> + * - POWER_SUPPLY_HEALTH_OVERVOLTAGE
> + * - POWER_SUPPLY_HEALTH_UNKNOWN
> + */
> +static int pf1550_get_battery_health(struct regmap *regmap, int *val)
> +{
> + unsigned int data;
> + int ret;
> +
> + ret = regmap_read(regmap, PF1550_CHARG_REG_BATT_SNS, &data);
> + if (ret < 0)
> + return ret;
> +
> + data &= PF1550_BAT_SNS_MASK;
> +
> + switch (data) {
> + case PF1550_BAT_NO_DETECT:
> + *val = POWER_SUPPLY_HEALTH_DEAD;
POWER_SUPPLY_HEALTH_NO_BATTERY ?
> + break;
> + case PF1550_BAT_NO_VBUS:
> + case PF1550_BAT_LOW_THAN_PRECHARG:
> + case PF1550_BAT_CHARG_FAIL:
> + case PF1550_BAT_HIGH_THAN_PRECHARG:
> + *val = POWER_SUPPLY_HEALTH_GOOD;
> + break;
> + case PF1550_BAT_OVER_VOL:
> + *val = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
> + break;
> + default:
> + *val = POWER_SUPPLY_HEALTH_UNKNOWN;
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int pf1550_get_present(struct regmap *regmap, int *val)
> +{
> + unsigned int data;
> + int ret;
> +
> + ret = regmap_read(regmap, PF1550_CHARG_REG_BATT_SNS, &data);
> + if (ret < 0)
> + return ret;
> +
> + data &= PF1550_BAT_SNS_MASK;
> + *val = (data == PF1550_BAT_NO_DETECT) ? 0 : 1;
> +
> + return 0;
> +}
You can drop this function + property. It's meant for a battery type
device.
> +static int pf1550_get_online(struct regmap *regmap, int *val)
> +{
> + unsigned int data;
> + int ret;
> +
> + ret = regmap_read(regmap, PF1550_CHARG_REG_VBUS_SNS, &data);
> + if (ret < 0)
> + return ret;
> +
> + *val = (data & PF1550_VBUS_VALID) ? 1 : 0;
> +
> + return 0;
> +}
> +
> +static void pf1550_chg_bat_work(struct work_struct *work)
> +{
> + struct pf1550_charger *chg = container_of(to_delayed_work(work),
> + struct pf1550_charger,
> + bat_sense_work);
> + unsigned int data;
> +
> + if (!chg->charger)
> + return;
This can't be called without a chg->charger being set.
> + if (regmap_read(chg->pf1550->regmap, PF1550_CHARG_REG_BATT_SNS, &data)) {
> + dev_err(chg->dev, "Read BATT_SNS error.\n");
> + return;
> + }
> +
> + switch (data & PF1550_BAT_SNS_MASK) {
> + case PF1550_BAT_NO_VBUS:
> + dev_dbg(chg->dev, "No valid VBUS input.\n");
> + break;
> + case PF1550_BAT_LOW_THAN_PRECHARG:
> + dev_dbg(chg->dev, "VBAT < VPRECHG.LB.\n");
> + break;
> + case PF1550_BAT_CHARG_FAIL:
> + dev_dbg(chg->dev, "Battery charging failed.\n");
> + break;
> + case PF1550_BAT_HIGH_THAN_PRECHARG:
> + dev_dbg(chg->dev, "VBAT > VPRECHG.LB.\n");
> + break;
> + case PF1550_BAT_OVER_VOL:
> + dev_dbg(chg->dev, "VBAT > VBATOV.\n");
> + break;
> + case PF1550_BAT_NO_DETECT:
> + dev_dbg(chg->dev, "Battery not detected.\n");
> + break;
> + default:
> + dev_err(chg->dev, "Unknown value read:%x\n",
> + data & PF1550_CHG_SNS_MASK);
> + }
So the whole handler is just for debug purposes?
> +}
> +
> +static void pf1550_chg_chg_work(struct work_struct *work)
> +{
> + struct pf1550_charger *chg = container_of(to_delayed_work(work),
> + struct pf1550_charger,
> + chg_sense_work);
> + unsigned int data;
> +
> + if (!chg->charger)
> + return;
same as pf1550_chg_bat_work.
> + if (regmap_read(chg->pf1550->regmap, PF1550_CHARG_REG_CHG_SNS, &data)) {
> + dev_err(chg->dev, "Read CHG_SNS error.\n");
> + return;
> + }
> +
> + switch (data & PF1550_CHG_SNS_MASK) {
> + case PF1550_CHG_PRECHARGE:
> + dev_dbg(chg->dev, "In pre-charger mode.\n");
> + break;
> + case PF1550_CHG_CONSTANT_CURRENT:
> + dev_dbg(chg->dev, "In fast-charge constant current mode.\n");
> + break;
> + case PF1550_CHG_CONSTANT_VOL:
> + dev_dbg(chg->dev, "In fast-charge constant voltage mode.\n");
> + break;
> + case PF1550_CHG_EOC:
> + dev_dbg(chg->dev, "In EOC mode.\n");
> + break;
> + case PF1550_CHG_DONE:
> + dev_dbg(chg->dev, "In DONE mode.\n");
> + break;
> + case PF1550_CHG_TIMER_FAULT:
> + dev_info(chg->dev, "In timer fault mode.\n");
> + break;
> + case PF1550_CHG_SUSPEND:
> + dev_info(chg->dev, "In thermistor suspend mode.\n");
> + break;
> + case PF1550_CHG_OFF_INV:
> + dev_info(chg->dev, "Input invalid, charger off.\n");
> + break;
> + case PF1550_CHG_BAT_OVER:
> + dev_info(chg->dev, "Battery over-voltage.\n");
dev_warn possibly?
> + break;
> + case PF1550_CHG_OFF_TEMP:
> + dev_info(chg->dev, "Temp high, charger off.\n");
> + break;
> + case PF1550_CHG_LINEAR_ONLY:
> + dev_dbg(chg->dev, "In Linear mode, not charging.\n");
> + break;
> + default:
> + dev_err(chg->dev, "Unknown value read:%x\n",
> + data & PF1550_CHG_SNS_MASK);
> + }
also all just for debug and othewise the IRQ is not used?
> +}
> +
> +static void pf1550_chg_vbus_work(struct work_struct *work)
> +{
> + struct pf1550_charger *chg = container_of(to_delayed_work(work),
> + struct pf1550_charger,
> + vbus_sense_work);
> + bool psy_changed = false;
> + unsigned int data;
> +
> + if (!chg->charger)
> + return;
same as pf1550_chg_bat_work.
> + if (regmap_read(chg->pf1550->regmap, PF1550_CHARG_REG_VBUS_SNS, &data)) {
> + dev_err(chg->dev, "Read VBUS_SNS error.\n");
> + return;
> + }
> +
> + if (data & PF1550_VBUS_UVLO) {
> + chg->psy_desc.type = POWER_SUPPLY_TYPE_BATTERY;
> + psy_changed = true;
> + dev_dbg(chg->dev, "VBUS detached.\n");
> + }
> + if (data & PF1550_VBUS_IN2SYS)
> + dev_dbg(chg->dev, "VBUS_IN2SYS_SNS.\n");
> + if (data & PF1550_VBUS_OVLO)
> + dev_dbg(chg->dev, "VBUS_OVLO_SNS.\n");
> + if (data & PF1550_VBUS_VALID) {
> + chg->psy_desc.type = POWER_SUPPLY_TYPE_MAINS;
> + psy_changed = true;
> + dev_dbg(chg->dev, "VBUS attached.\n");
> + }
Changing the power_supply type dynamically is a big no go. FWIW I'm
not sure how you came up with this weird type change in the first
place. As far as I can see this is a normal charger and should just
set POWER_SUPPLY_TYPE_MAINS in the probe routine and not touch it
otherwise.
> +
> + if (psy_changed)
> + power_supply_changed(chg->charger);
> +}
> +
> +static irqreturn_t pf1550_charger_irq_handler(int irq, void *data)
> +{
> + struct pf1550_charger *chg = data;
> + struct device *dev = chg->dev;
> + int i, irq_type = -1;
> +
> + for (i = 0; i < PF1550_CHARGER_IRQ_NR; i++)
> + if (irq == chg->virqs[i])
> + irq_type = i;
> +
> + switch (irq_type) {
> + case PF1550_CHARG_IRQ_BAT2SOCI:
> + dev_info(dev, "BAT to SYS Overcurrent interrupt.\n");
> + break;
> + case PF1550_CHARG_IRQ_BATI:
> + schedule_delayed_work(&chg->bat_sense_work,
> + msecs_to_jiffies(10));
> + break;
> + case PF1550_CHARG_IRQ_CHGI:
> + schedule_delayed_work(&chg->chg_sense_work,
> + msecs_to_jiffies(10));
> + break;
> + case PF1550_CHARG_IRQ_VBUSI:
> + schedule_delayed_work(&chg->vbus_sense_work,
> + msecs_to_jiffies(10));
> + break;
> + case PF1550_CHARG_IRQ_THMI:
> + dev_info(dev, "Thermal interrupt.\n");
> + break;
> + default:
> + dev_err(dev, "unknown interrupt occurred.\n");
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static enum power_supply_property pf1550_charger_props[] = {
> + POWER_SUPPLY_PROP_STATUS,
> + POWER_SUPPLY_PROP_CHARGE_TYPE,
> + POWER_SUPPLY_PROP_HEALTH,
> + POWER_SUPPLY_PROP_PRESENT,
> + POWER_SUPPLY_PROP_ONLINE,
> + POWER_SUPPLY_PROP_MODEL_NAME,
> + POWER_SUPPLY_PROP_MANUFACTURER,
> +};
> +
> +static int pf1550_charger_get_property(struct power_supply *psy,
> + enum power_supply_property psp,
> + union power_supply_propval *val)
> +{
> + struct pf1550_charger *chg = power_supply_get_drvdata(psy);
> + struct regmap *regmap = chg->pf1550->regmap;
> + int ret = 0;
> +
> + switch (psp) {
> + case POWER_SUPPLY_PROP_STATUS:
> + ret = pf1550_get_charger_state(regmap, &val->intval);
> + break;
> + case POWER_SUPPLY_PROP_CHARGE_TYPE:
> + ret = pf1550_get_charge_type(regmap, &val->intval);
> + break;
> + case POWER_SUPPLY_PROP_HEALTH:
> + ret = pf1550_get_battery_health(regmap, &val->intval);
> + break;
> + case POWER_SUPPLY_PROP_PRESENT:
> + ret = pf1550_get_present(regmap, &val->intval);
> + break;
> + case POWER_SUPPLY_PROP_ONLINE:
> + ret = pf1550_get_online(regmap, &val->intval);
> + break;
> + case POWER_SUPPLY_PROP_MODEL_NAME:
> + val->strval = "PF1550";
> + break;
> + case POWER_SUPPLY_PROP_MANUFACTURER:
> + val->strval = "NXP";
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return ret;
> +}
> +
> +static int pf1550_set_constant_volt(struct pf1550_charger *chg,
> + unsigned int uvolt)
> +{
> + unsigned int data;
> +
> + if (uvolt >= 3500000 && uvolt <= 4440000)
> + data = 8 + (uvolt - 3500000) / 20000;
> + else
> + return dev_err_probe(chg->dev, -EINVAL,
> + "Wrong value for constant voltage\n");
> +
> + dev_dbg(chg->dev, "Charging constant voltage: %u (0x%x)\n", uvolt,
> + data);
> +
> + return regmap_update_bits(chg->pf1550->regmap,
> + PF1550_CHARG_REG_BATT_REG,
> + PF1550_CHARG_REG_BATT_REG_CHGCV_MASK, data);
> +}
> +
> +static int pf1550_set_min_system_volt(struct pf1550_charger *chg,
> + unsigned int uvolt)
> +{
> + unsigned int data;
> +
> + switch (uvolt) {
> + case 3500000:
> + data = 0x0;
> + break;
> + case 3700000:
> + data = 0x1;
> + break;
> + case 4300000:
> + data = 0x2;
> + break;
> + default:
> + return dev_err_probe(chg->dev, -EINVAL,
> + "Wrong value for minimum system voltage\n");
> + }
> +
> + data <<= PF1550_CHARG_REG_BATT_REG_VMINSYS_SHIFT;
> +
> + dev_dbg(chg->dev, "Minimum system regulation voltage: %u (0x%x)\n",
> + uvolt, data);
> +
> + return regmap_update_bits(chg->pf1550->regmap,
> + PF1550_CHARG_REG_BATT_REG,
> + PF1550_CHARG_REG_BATT_REG_VMINSYS_MASK, data);
> +}
> +
> +static int pf1550_set_thermal_regulation_temp(struct pf1550_charger *chg,
> + unsigned int cells)
> +{
> + unsigned int data;
> +
> + switch (cells) {
> + case 60:
> + data = 0x0;
> + break;
> + case 75:
> + data = 0x1;
> + break;
> + case 90:
> + data = 0x2;
> + break;
> + case 105:
> + data = 0x3;
> + break;
> + default:
> + return dev_err_probe(chg->dev, -EINVAL,
> + "Wrong value for thermal temperature\n");
> + }
> +
> + data <<= PF1550_CHARG_REG_THM_REG_CNFG_REGTEMP_SHIFT;
> +
> + dev_dbg(chg->dev, "Thermal regulation loop temperature: %u (0x%x)\n",
> + cells, data);
> +
> + return regmap_update_bits(chg->pf1550->regmap,
> + PF1550_CHARG_REG_THM_REG_CNFG,
> + PF1550_CHARG_REG_THM_REG_CNFG_REGTEMP_MASK,
> + data);
> +}
> +
> +/*
> + * Sets charger registers to proper and safe default values.
> + */
> +static int pf1550_reg_init(struct pf1550_charger *chg)
> +{
> + struct device *dev = chg->dev;
> + unsigned int data;
> + int ret;
> +
> + /* Unmask charger interrupt, mask DPMI and reserved bit */
> + ret = regmap_write(chg->pf1550->regmap, PF1550_CHARG_REG_CHG_INT_MASK,
> + PF1550_CHG_INT_MASK);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Error unmask charger interrupt\n");
> +
> + ret = regmap_read(chg->pf1550->regmap, PF1550_CHARG_REG_VBUS_SNS,
> + &data);
> + if (ret)
> + return dev_err_probe(dev, ret, "Read charg vbus_sns error\n");
> +
> + if (data & PF1550_VBUS_VALID)
> + chg->psy_desc.type = POWER_SUPPLY_TYPE_MAINS;
> +
> + ret = pf1550_set_constant_volt(chg, chg->constant_volt);
> + if (ret)
> + return ret;
> +
> + ret = pf1550_set_min_system_volt(chg, chg->min_system_volt);
> + if (ret)
> + return ret;
> +
> + ret = pf1550_set_thermal_regulation_temp(chg,
> + chg->thermal_regulation_temp);
> + if (ret)
> + return ret;
> +
> + /* Turn on charger */
> + ret = regmap_write(chg->pf1550->regmap, PF1550_CHARG_REG_CHG_OPER,
> + PF1550_CHG_TURNON);
> + if (ret)
> + return dev_err_probe(dev, ret, "Error turn on charger\n");
> +
> + return 0;
> +}
> +
> +static void pf1550_dt_parse_dev_info(struct pf1550_charger *chg)
> +{
> + struct power_supply_battery_info *info;
> + struct device *dev = chg->dev;
> +
> + if (device_property_read_u32(dev->parent, "nxp,min-system-microvolt",
> + &chg->min_system_volt))
> + chg->min_system_volt = PF1550_DEFAULT_MIN_SYSTEM_VOLT;
> +
> + if (device_property_read_u32(dev->parent,
> + "nxp,thermal-regulation-celsius",
> + &chg->thermal_regulation_temp))
> + chg->thermal_regulation_temp = PF1550_DEFAULT_THERMAL_TEMP;
> +
> + if (power_supply_get_battery_info(chg->charger, &info))
> + chg->constant_volt = PF1550_DEFAULT_CONSTANT_VOLT;
> + else
> + chg->constant_volt = info->constant_charge_voltage_max_uv;
> +}
> +
> +static int pf1550_charger_probe(struct platform_device *pdev)
> +{
> + const struct pf1550_dev *pf1550 = dev_get_drvdata(pdev->dev.parent);
> + struct power_supply_config psy_cfg = {};
> + struct pf1550_charger *chg;
> + int i, irq, ret;
> +
> + chg = devm_kzalloc(&pdev->dev, sizeof(*chg), GFP_KERNEL);
> + if (!chg)
> + return -ENOMEM;
> +
> + chg->dev = &pdev->dev;
> + chg->pf1550 = pf1550;
> +
> + if (!chg->pf1550->regmap)
> + return dev_err_probe(&pdev->dev, -ENODEV,
> + "failed to get regmap\n");
> +
> + platform_set_drvdata(pdev, chg);
> +
> + INIT_DELAYED_WORK(&chg->vbus_sense_work, pf1550_chg_vbus_work);
> + INIT_DELAYED_WORK(&chg->chg_sense_work, pf1550_chg_chg_work);
> + INIT_DELAYED_WORK(&chg->bat_sense_work, pf1550_chg_bat_work);
use devm_delayed_work_autocancel(). It's not just a cleanup, but
also fixed a race condition during module removal, as you are right
now cancelling the job while a new one might be scheduled directly
afterwards before the power-supply device is deregistered.
> + for (i = 0; i < PF1550_CHARGER_IRQ_NR; i++) {
> + irq = platform_get_irq(pdev, i);
> + if (irq < 0)
> + return irq;
> +
> + chg->virqs[i] = irq;
> +
> + ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
> + pf1550_charger_irq_handler,
> + IRQF_NO_SUSPEND,
> + "pf1550-charger", chg);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "failed irq request\n");
> + }
> +
> + psy_cfg.drv_data = chg;
> +
> + chg->psy_desc.name = PF1550_CHARGER_NAME;
> + chg->psy_desc.type = POWER_SUPPLY_TYPE_BATTERY;
> + chg->psy_desc.get_property = pf1550_charger_get_property;
> + chg->psy_desc.properties = pf1550_charger_props;
> + chg->psy_desc.num_properties = ARRAY_SIZE(pf1550_charger_props);
> +
> + chg->charger = devm_power_supply_register(&pdev->dev, &chg->psy_desc,
> + &psy_cfg);
> + if (IS_ERR(chg->charger))
> + return dev_err_probe(&pdev->dev, PTR_ERR(chg->charger),
> + "failed: power supply register\n");
> +
> + pf1550_dt_parse_dev_info(chg);
> +
> + return pf1550_reg_init(chg);
> +}
> +
> +static void pf1550_charger_remove(struct platform_device *pdev)
> +{
> + struct pf1550_charger *chg = platform_get_drvdata(pdev);
> +
> + cancel_delayed_work_sync(&chg->bat_sense_work);
> + cancel_delayed_work_sync(&chg->chg_sense_work);
> + cancel_delayed_work_sync(&chg->vbus_sense_work);
> +}
> +
> +static const struct platform_device_id pf1550_charger_id[] = {
> + { "pf1550-charger", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(platform, pf1550_charger_id);
> +
> +static struct platform_driver pf1550_charger_driver = {
> + .driver = {
> + .name = "pf1550-charger",
> + },
> + .probe = pf1550_charger_probe,
> + .remove = pf1550_charger_remove,
> + .id_table = pf1550_charger_id,
> +};
> +module_platform_driver(pf1550_charger_driver);
> +
> +MODULE_AUTHOR("Robin Gong <yibin.gong@freescale.com>");
> +MODULE_DESCRIPTION("PF1550 charger driver");
> +MODULE_LICENSE("GPL");
Greetings,
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v7 5/6] power: supply: pf1550: add battery charger support
2025-06-22 0:43 ` Sebastian Reichel
@ 2025-06-25 14:19 ` Samuel Kayode
2025-07-06 23:33 ` Sebastian Reichel
0 siblings, 1 reply; 17+ messages in thread
From: Samuel Kayode @ 2025-06-25 14:19 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Liam Girdwood, Mark Brown, Dmitry Torokhov, Frank Li, imx,
devicetree, linux-kernel, linux-input, linux-pm, Abel Vesa,
Abel Vesa, Robin Gong, Robin Gong, Enric Balletbo i Serra
Hi Sebastian,
Thanks a lot for the review.
On Sun, Jun 22, 2025 at 02:43:36AM +0200, Sebastian Reichel wrote:
> > +static int pf1550_get_battery_health(struct regmap *regmap, int *val)
> > +{
> > + unsigned int data;
> > + int ret;
> > +
> > + ret = regmap_read(regmap, PF1550_CHARG_REG_BATT_SNS, &data);
> > + if (ret < 0)
> > + return ret;
> > +
> > + data &= PF1550_BAT_SNS_MASK;
> > +
> > + switch (data) {
> > + case PF1550_BAT_NO_DETECT:
> > + *val = POWER_SUPPLY_HEALTH_DEAD;
>
> POWER_SUPPLY_HEALTH_NO_BATTERY ?
Yes, it should be no battery. Will update in the next version.
>
> > + break;
> > + case PF1550_BAT_NO_VBUS:
> > + case PF1550_BAT_LOW_THAN_PRECHARG:
> > + case PF1550_BAT_CHARG_FAIL:
> > + case PF1550_BAT_HIGH_THAN_PRECHARG:
> > + *val = POWER_SUPPLY_HEALTH_GOOD;
> > + break;
> > + case PF1550_BAT_OVER_VOL:
> > + *val = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
> > + break;
> > + default:
> > + *val = POWER_SUPPLY_HEALTH_UNKNOWN;
> > + break;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int pf1550_get_present(struct regmap *regmap, int *val)
> > +{
> > + unsigned int data;
> > + int ret;
> > +
> > + ret = regmap_read(regmap, PF1550_CHARG_REG_BATT_SNS, &data);
> > + if (ret < 0)
> > + return ret;
> > +
> > + data &= PF1550_BAT_SNS_MASK;
> > + *val = (data == PF1550_BAT_NO_DETECT) ? 0 : 1;
> > +
> > + return 0;
> > +}
>
> You can drop this function + property. It's meant for a battery type
> device.
The pf1550 switches the load between battery and external power depending on
whichever is available.
> > +static void pf1550_chg_bat_work(struct work_struct *work)
> > +{
> > + struct pf1550_charger *chg = container_of(to_delayed_work(work),
> > + struct pf1550_charger,
> > + bat_sense_work);
> > + unsigned int data;
> > +
> > + if (!chg->charger)
> > + return;
>
> This can't be called without a chg->charger being set.
>
Will drop.
> > + if (regmap_read(chg->pf1550->regmap, PF1550_CHARG_REG_BATT_SNS, &data)) {
> > + dev_err(chg->dev, "Read BATT_SNS error.\n");
> > + return;
> > + }
> > +
> > + switch (data & PF1550_BAT_SNS_MASK) {
> > + case PF1550_BAT_NO_VBUS:
> > + dev_dbg(chg->dev, "No valid VBUS input.\n");
> > + break;
> > + case PF1550_BAT_LOW_THAN_PRECHARG:
> > + dev_dbg(chg->dev, "VBAT < VPRECHG.LB.\n");
> > + break;
> > + case PF1550_BAT_CHARG_FAIL:
> > + dev_dbg(chg->dev, "Battery charging failed.\n");
> > + break;
> > + case PF1550_BAT_HIGH_THAN_PRECHARG:
> > + dev_dbg(chg->dev, "VBAT > VPRECHG.LB.\n");
> > + break;
> > + case PF1550_BAT_OVER_VOL:
> > + dev_dbg(chg->dev, "VBAT > VBATOV.\n");
> > + break;
> > + case PF1550_BAT_NO_DETECT:
> > + dev_dbg(chg->dev, "Battery not detected.\n");
> > + break;
> > + default:
> > + dev_err(chg->dev, "Unknown value read:%x\n",
> > + data & PF1550_CHG_SNS_MASK);
> > + }
>
> So the whole handler is just for debug purposes?
>
Yes, it is solely for debugging.
> > +}
> > +
> > +static void pf1550_chg_chg_work(struct work_struct *work)
> > +{
> > + struct pf1550_charger *chg = container_of(to_delayed_work(work),
> > + struct pf1550_charger,
> > + chg_sense_work);
> > + unsigned int data;
> > +
> > + if (!chg->charger)
> > + return;
>
> same as pf1550_chg_bat_work.
>
Will drop.
> > + if (regmap_read(chg->pf1550->regmap, PF1550_CHARG_REG_CHG_SNS, &data)) {
> > + dev_err(chg->dev, "Read CHG_SNS error.\n");
> > + return;
> > + }
> > +
> > + switch (data & PF1550_CHG_SNS_MASK) {
> > + case PF1550_CHG_PRECHARGE:
> > + dev_dbg(chg->dev, "In pre-charger mode.\n");
> > + break;
> > + case PF1550_CHG_CONSTANT_CURRENT:
> > + dev_dbg(chg->dev, "In fast-charge constant current mode.\n");
> > + break;
> > + case PF1550_CHG_CONSTANT_VOL:
> > + dev_dbg(chg->dev, "In fast-charge constant voltage mode.\n");
> > + break;
> > + case PF1550_CHG_EOC:
> > + dev_dbg(chg->dev, "In EOC mode.\n");
> > + break;
> > + case PF1550_CHG_DONE:
> > + dev_dbg(chg->dev, "In DONE mode.\n");
> > + break;
> > + case PF1550_CHG_TIMER_FAULT:
> > + dev_info(chg->dev, "In timer fault mode.\n");
> > + break;
> > + case PF1550_CHG_SUSPEND:
> > + dev_info(chg->dev, "In thermistor suspend mode.\n");
> > + break;
> > + case PF1550_CHG_OFF_INV:
> > + dev_info(chg->dev, "Input invalid, charger off.\n");
> > + break;
> > + case PF1550_CHG_BAT_OVER:
> > + dev_info(chg->dev, "Battery over-voltage.\n");
>
> dev_warn possibly?
>
Will update.
> > + break;
> > + case PF1550_CHG_OFF_TEMP:
> > + dev_info(chg->dev, "Temp high, charger off.\n");
> > + break;
> > + case PF1550_CHG_LINEAR_ONLY:
> > + dev_dbg(chg->dev, "In Linear mode, not charging.\n");
> > + break;
> > + default:
> > + dev_err(chg->dev, "Unknown value read:%x\n",
> > + data & PF1550_CHG_SNS_MASK);
> > + }
>
> also all just for debug and othewise the IRQ is not used?
>
Yes, this is also just for debug.
> > +}
> > +
> > +static void pf1550_chg_vbus_work(struct work_struct *work)
> > +{
> > + struct pf1550_charger *chg = container_of(to_delayed_work(work),
> > + struct pf1550_charger,
> > + vbus_sense_work);
> > + bool psy_changed = false;
> > + unsigned int data;
> > +
> > + if (!chg->charger)
> > + return;
>
> same as pf1550_chg_bat_work.
>
Will drop.
> > + if (regmap_read(chg->pf1550->regmap, PF1550_CHARG_REG_VBUS_SNS, &data)) {
> > + dev_err(chg->dev, "Read VBUS_SNS error.\n");
> > + return;
> > + }
> > +
> > + if (data & PF1550_VBUS_UVLO) {
> > + chg->psy_desc.type = POWER_SUPPLY_TYPE_BATTERY;
> > + psy_changed = true;
> > + dev_dbg(chg->dev, "VBUS detached.\n");
> > + }
> > + if (data & PF1550_VBUS_IN2SYS)
> > + dev_dbg(chg->dev, "VBUS_IN2SYS_SNS.\n");
> > + if (data & PF1550_VBUS_OVLO)
> > + dev_dbg(chg->dev, "VBUS_OVLO_SNS.\n");
> > + if (data & PF1550_VBUS_VALID) {
> > + chg->psy_desc.type = POWER_SUPPLY_TYPE_MAINS;
> > + psy_changed = true;
> > + dev_dbg(chg->dev, "VBUS attached.\n");
> > + }
>
> Changing the power_supply type dynamically is a big no go. FWIW I'm
> not sure how you came up with this weird type change in the first
> place. As far as I can see this is a normal charger and should just
> set POWER_SUPPLY_TYPE_MAINS in the probe routine and not touch it
> otherwise.
>
The pf1550 charger receives a VBUS power input which can be provided either by
an AC adapter or a USB bus. A depleted battery is charged using the VBUS power
input (VBUSIN). When no power is supplied to VBUSIN, the pf1550 switches the
load to the connected non-depleted battery.
I could have two power_supply_desc, one for battery and one for the external
power?
> > +
> > + if (psy_changed)
> > + power_supply_changed(chg->charger);
> > +}
> > +
> > +static irqreturn_t pf1550_charger_irq_handler(int irq, void *data)
> > +{
> > + struct pf1550_charger *chg = data;
> > + struct device *dev = chg->dev;
> > + int i, irq_type = -1;
> > +
> > + for (i = 0; i < PF1550_CHARGER_IRQ_NR; i++)
> > + if (irq == chg->virqs[i])
> > + irq_type = i;
> > +
> > + switch (irq_type) {
> > + case PF1550_CHARG_IRQ_BAT2SOCI:
> > + dev_info(dev, "BAT to SYS Overcurrent interrupt.\n");
> > + break;
> > + case PF1550_CHARG_IRQ_BATI:
> > + schedule_delayed_work(&chg->bat_sense_work,
> > + msecs_to_jiffies(10));
> > + break;
> > + case PF1550_CHARG_IRQ_CHGI:
> > + schedule_delayed_work(&chg->chg_sense_work,
> > + msecs_to_jiffies(10));
> > + break;
> > + case PF1550_CHARG_IRQ_VBUSI:
> > + schedule_delayed_work(&chg->vbus_sense_work,
> > + msecs_to_jiffies(10));
> > + break;
> > + case PF1550_CHARG_IRQ_THMI:
> > + dev_info(dev, "Thermal interrupt.\n");
> > + break;
> > + default:
> > + dev_err(dev, "unknown interrupt occurred.\n");
> > + }
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static enum power_supply_property pf1550_charger_props[] = {
> > + POWER_SUPPLY_PROP_STATUS,
> > + POWER_SUPPLY_PROP_CHARGE_TYPE,
> > + POWER_SUPPLY_PROP_HEALTH,
> > + POWER_SUPPLY_PROP_PRESENT,
> > + POWER_SUPPLY_PROP_ONLINE,
> > + POWER_SUPPLY_PROP_MODEL_NAME,
> > + POWER_SUPPLY_PROP_MANUFACTURER,
> > +};
> > +
> > +static int pf1550_charger_get_property(struct power_supply *psy,
> > + enum power_supply_property psp,
> > + union power_supply_propval *val)
> > +{
> > + struct pf1550_charger *chg = power_supply_get_drvdata(psy);
> > + struct regmap *regmap = chg->pf1550->regmap;
> > + int ret = 0;
> > +
> > + switch (psp) {
> > + case POWER_SUPPLY_PROP_STATUS:
> > + ret = pf1550_get_charger_state(regmap, &val->intval);
> > + break;
> > + case POWER_SUPPLY_PROP_CHARGE_TYPE:
> > + ret = pf1550_get_charge_type(regmap, &val->intval);
> > + break;
> > + case POWER_SUPPLY_PROP_HEALTH:
> > + ret = pf1550_get_battery_health(regmap, &val->intval);
> > + break;
> > + case POWER_SUPPLY_PROP_PRESENT:
> > + ret = pf1550_get_present(regmap, &val->intval);
> > + break;
> > + case POWER_SUPPLY_PROP_ONLINE:
> > + ret = pf1550_get_online(regmap, &val->intval);
> > + break;
> > + case POWER_SUPPLY_PROP_MODEL_NAME:
> > + val->strval = "PF1550";
> > + break;
> > + case POWER_SUPPLY_PROP_MANUFACTURER:
> > + val->strval = "NXP";
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int pf1550_set_constant_volt(struct pf1550_charger *chg,
> > + unsigned int uvolt)
> > +{
> > + unsigned int data;
> > +
> > + if (uvolt >= 3500000 && uvolt <= 4440000)
> > + data = 8 + (uvolt - 3500000) / 20000;
> > + else
> > + return dev_err_probe(chg->dev, -EINVAL,
> > + "Wrong value for constant voltage\n");
> > +
> > + dev_dbg(chg->dev, "Charging constant voltage: %u (0x%x)\n", uvolt,
> > + data);
> > +
> > + return regmap_update_bits(chg->pf1550->regmap,
> > + PF1550_CHARG_REG_BATT_REG,
> > + PF1550_CHARG_REG_BATT_REG_CHGCV_MASK, data);
> > +}
> > +
> > +static int pf1550_set_min_system_volt(struct pf1550_charger *chg,
> > + unsigned int uvolt)
> > +{
> > + unsigned int data;
> > +
> > + switch (uvolt) {
> > + case 3500000:
> > + data = 0x0;
> > + break;
> > + case 3700000:
> > + data = 0x1;
> > + break;
> > + case 4300000:
> > + data = 0x2;
> > + break;
> > + default:
> > + return dev_err_probe(chg->dev, -EINVAL,
> > + "Wrong value for minimum system voltage\n");
> > + }
> > +
> > + data <<= PF1550_CHARG_REG_BATT_REG_VMINSYS_SHIFT;
> > +
> > + dev_dbg(chg->dev, "Minimum system regulation voltage: %u (0x%x)\n",
> > + uvolt, data);
> > +
> > + return regmap_update_bits(chg->pf1550->regmap,
> > + PF1550_CHARG_REG_BATT_REG,
> > + PF1550_CHARG_REG_BATT_REG_VMINSYS_MASK, data);
> > +}
> > +
> > +static int pf1550_set_thermal_regulation_temp(struct pf1550_charger *chg,
> > + unsigned int cells)
> > +{
> > + unsigned int data;
> > +
> > + switch (cells) {
> > + case 60:
> > + data = 0x0;
> > + break;
> > + case 75:
> > + data = 0x1;
> > + break;
> > + case 90:
> > + data = 0x2;
> > + break;
> > + case 105:
> > + data = 0x3;
> > + break;
> > + default:
> > + return dev_err_probe(chg->dev, -EINVAL,
> > + "Wrong value for thermal temperature\n");
> > + }
> > +
> > + data <<= PF1550_CHARG_REG_THM_REG_CNFG_REGTEMP_SHIFT;
> > +
> > + dev_dbg(chg->dev, "Thermal regulation loop temperature: %u (0x%x)\n",
> > + cells, data);
> > +
> > + return regmap_update_bits(chg->pf1550->regmap,
> > + PF1550_CHARG_REG_THM_REG_CNFG,
> > + PF1550_CHARG_REG_THM_REG_CNFG_REGTEMP_MASK,
> > + data);
> > +}
> > +
> > +/*
> > + * Sets charger registers to proper and safe default values.
> > + */
> > +static int pf1550_reg_init(struct pf1550_charger *chg)
> > +{
> > + struct device *dev = chg->dev;
> > + unsigned int data;
> > + int ret;
> > +
> > + /* Unmask charger interrupt, mask DPMI and reserved bit */
> > + ret = regmap_write(chg->pf1550->regmap, PF1550_CHARG_REG_CHG_INT_MASK,
> > + PF1550_CHG_INT_MASK);
> > + if (ret)
> > + return dev_err_probe(dev, ret,
> > + "Error unmask charger interrupt\n");
> > +
> > + ret = regmap_read(chg->pf1550->regmap, PF1550_CHARG_REG_VBUS_SNS,
> > + &data);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Read charg vbus_sns error\n");
> > +
> > + if (data & PF1550_VBUS_VALID)
> > + chg->psy_desc.type = POWER_SUPPLY_TYPE_MAINS;
> > +
> > + ret = pf1550_set_constant_volt(chg, chg->constant_volt);
> > + if (ret)
> > + return ret;
> > +
> > + ret = pf1550_set_min_system_volt(chg, chg->min_system_volt);
> > + if (ret)
> > + return ret;
> > +
> > + ret = pf1550_set_thermal_regulation_temp(chg,
> > + chg->thermal_regulation_temp);
> > + if (ret)
> > + return ret;
> > +
> > + /* Turn on charger */
> > + ret = regmap_write(chg->pf1550->regmap, PF1550_CHARG_REG_CHG_OPER,
> > + PF1550_CHG_TURNON);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Error turn on charger\n");
> > +
> > + return 0;
> > +}
> > +
> > +static void pf1550_dt_parse_dev_info(struct pf1550_charger *chg)
> > +{
> > + struct power_supply_battery_info *info;
> > + struct device *dev = chg->dev;
> > +
> > + if (device_property_read_u32(dev->parent, "nxp,min-system-microvolt",
> > + &chg->min_system_volt))
> > + chg->min_system_volt = PF1550_DEFAULT_MIN_SYSTEM_VOLT;
> > +
> > + if (device_property_read_u32(dev->parent,
> > + "nxp,thermal-regulation-celsius",
> > + &chg->thermal_regulation_temp))
> > + chg->thermal_regulation_temp = PF1550_DEFAULT_THERMAL_TEMP;
> > +
> > + if (power_supply_get_battery_info(chg->charger, &info))
> > + chg->constant_volt = PF1550_DEFAULT_CONSTANT_VOLT;
> > + else
> > + chg->constant_volt = info->constant_charge_voltage_max_uv;
> > +}
> > +
> > +static int pf1550_charger_probe(struct platform_device *pdev)
> > +{
> > + const struct pf1550_dev *pf1550 = dev_get_drvdata(pdev->dev.parent);
> > + struct power_supply_config psy_cfg = {};
> > + struct pf1550_charger *chg;
> > + int i, irq, ret;
> > +
> > + chg = devm_kzalloc(&pdev->dev, sizeof(*chg), GFP_KERNEL);
> > + if (!chg)
> > + return -ENOMEM;
> > +
> > + chg->dev = &pdev->dev;
> > + chg->pf1550 = pf1550;
> > +
> > + if (!chg->pf1550->regmap)
> > + return dev_err_probe(&pdev->dev, -ENODEV,
> > + "failed to get regmap\n");
> > +
> > + platform_set_drvdata(pdev, chg);
> > +
> > + INIT_DELAYED_WORK(&chg->vbus_sense_work, pf1550_chg_vbus_work);
> > + INIT_DELAYED_WORK(&chg->chg_sense_work, pf1550_chg_chg_work);
> > + INIT_DELAYED_WORK(&chg->bat_sense_work, pf1550_chg_bat_work);
>
> use devm_delayed_work_autocancel(). It's not just a cleanup, but
> also fixed a race condition during module removal, as you are right
> now cancelling the job while a new one might be scheduled directly
> afterwards before the power-supply device is deregistered.
>
Will do.
> > + for (i = 0; i < PF1550_CHARGER_IRQ_NR; i++) {
> > + irq = platform_get_irq(pdev, i);
> > + if (irq < 0)
> > + return irq;
> > +
> > + chg->virqs[i] = irq;
> > +
> > + ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
> > + pf1550_charger_irq_handler,
> > + IRQF_NO_SUSPEND,
> > + "pf1550-charger", chg);
> > + if (ret)
> > + return dev_err_probe(&pdev->dev, ret,
> > + "failed irq request\n");
> > + }
> > +
> > + psy_cfg.drv_data = chg;
> > +
> > + chg->psy_desc.name = PF1550_CHARGER_NAME;
> > + chg->psy_desc.type = POWER_SUPPLY_TYPE_BATTERY;
> > + chg->psy_desc.get_property = pf1550_charger_get_property;
> > + chg->psy_desc.properties = pf1550_charger_props;
> > + chg->psy_desc.num_properties = ARRAY_SIZE(pf1550_charger_props);
> > +
> > + chg->charger = devm_power_supply_register(&pdev->dev, &chg->psy_desc,
> > + &psy_cfg);
> > + if (IS_ERR(chg->charger))
> > + return dev_err_probe(&pdev->dev, PTR_ERR(chg->charger),
> > + "failed: power supply register\n");
> > +
> > + pf1550_dt_parse_dev_info(chg);
> > +
> > + return pf1550_reg_init(chg);
> > +}
> > +
Thanks,
Sam
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v7 5/6] power: supply: pf1550: add battery charger support
2025-06-25 14:19 ` Samuel Kayode
@ 2025-07-06 23:33 ` Sebastian Reichel
2025-07-07 15:10 ` Samuel Kayode
0 siblings, 1 reply; 17+ messages in thread
From: Sebastian Reichel @ 2025-07-06 23:33 UTC (permalink / raw)
To: Samuel Kayode
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Liam Girdwood, Mark Brown, Dmitry Torokhov, Frank Li, imx,
devicetree, linux-kernel, linux-input, linux-pm, Abel Vesa,
Abel Vesa, Robin Gong, Robin Gong, Enric Balletbo i Serra
[-- Attachment #1: Type: text/plain, Size: 784 bytes --]
Hello Samuel,
On Wed, Jun 25, 2025 at 10:19:49AM -0400, Samuel Kayode wrote:
> The pf1550 charger receives a VBUS power input which can be provided either by
> an AC adapter or a USB bus. A depleted battery is charged using the VBUS power
> input (VBUSIN). When no power is supplied to VBUSIN, the pf1550 switches the
> load to the connected non-depleted battery.
>
> I could have two power_supply_desc, one for battery and one for the external
> power?
That's acceptable. But don't you have a fuel-gauge for the battery?
If you register two POWER_SUPPLY_TYPE_BATTERY devices, then your
board should have two batteries. If you have a fuel-gauge it will
very likely provide much better battery data then anything you get
out of pf1550.
Greetings,
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v7 5/6] power: supply: pf1550: add battery charger support
2025-07-06 23:33 ` Sebastian Reichel
@ 2025-07-07 15:10 ` Samuel Kayode
0 siblings, 0 replies; 17+ messages in thread
From: Samuel Kayode @ 2025-07-07 15:10 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Liam Girdwood, Mark Brown, Dmitry Torokhov, Frank Li, imx,
devicetree, linux-kernel, linux-input, linux-pm, Abel Vesa,
Abel Vesa, Robin Gong, Robin Gong, Enric Balletbo i Serra
On Mon, Jul 07, 2025 at 01:33:36AM +0200, Sebastian Reichel wrote:
> Hello Samuel,
>
> On Wed, Jun 25, 2025 at 10:19:49AM -0400, Samuel Kayode wrote:
> > The pf1550 charger receives a VBUS power input which can be provided either by
> > an AC adapter or a USB bus. A depleted battery is charged using the VBUS power
> > input (VBUSIN). When no power is supplied to VBUSIN, the pf1550 switches the
> > load to the connected non-depleted battery.
> >
> > I could have two power_supply_desc, one for battery and one for the external
> > power?
>
> That's acceptable. But don't you have a fuel-gauge for the battery?
> If you register two POWER_SUPPLY_TYPE_BATTERY devices, then your
> board should have two batteries. If you have a fuel-gauge it will
> very likely provide much better battery data then anything you get
> out of pf1550.
The PMIC unfortunately does not include a fuel-gauge.
I intend on having POWER_SUPPLY_TYPE_MAINS for the external power supply
(AC adapter or USB bus) and POWER_SUPPLY_TYPE_BATTERY for the battery.
Thanks,
Sam
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v7 6/6] MAINTAINERS: add an entry for pf1550 mfd driver
2025-06-12 14:55 [PATCH v7 0/6] add support for pf1550 PMIC MFD-based drivers Samuel Kayode via B4 Relay
` (4 preceding siblings ...)
2025-06-12 14:55 ` [PATCH v7 5/6] power: supply: pf1550: add battery charger support Samuel Kayode via B4 Relay
@ 2025-06-12 14:55 ` Samuel Kayode via B4 Relay
2025-06-12 16:03 ` Frank Li
5 siblings, 1 reply; 17+ messages in thread
From: Samuel Kayode via B4 Relay @ 2025-06-12 14:55 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Liam Girdwood, Mark Brown, Dmitry Torokhov, Sebastian Reichel,
Frank Li
Cc: imx, devicetree, linux-kernel, linux-input, linux-pm, Abel Vesa,
Abel Vesa, Robin Gong, Robin Gong, Enric Balletbo i Serra,
Samuel Kayode, Abel Vesa, Krzysztof Kozlowski
From: Samuel Kayode <samuel.kayode@savoirfairelinux.com>
Add MAINTAINERS entry for pf1550 PMIC.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Samuel Kayode <samuel.kayode@savoirfairelinux.com>
---
v6:
- Add imx mailing list
---
MAINTAINERS | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 98201e1f4ab5908ff49d32d19275e123cedb4b66..5547fdafa7e1bb11903d5d5bef246c2e1a20fbca 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17853,6 +17853,17 @@ F: Documentation/devicetree/bindings/clock/imx*
F: drivers/clk/imx/
F: include/dt-bindings/clock/imx*
+NXP PF1550 PMIC MFD DRIVER
+M: Samuel Kayode <samuel.kayode@savoirfairelinux.com>
+L: imx@lists.linux.dev
+S: Maintained
+F: Documentation/devicetree/bindings/mfd/nxp,pf1550.yaml
+F: drivers/input/misc/pf1550-onkey.c
+F: drivers/mfd/pf1550.c
+F: drivers/power/supply/pf1550-charger.c
+F: drivers/regulator/pf1550-regulator.c
+F: include/linux/mfd/pfd1550.h
+
NXP PF8100/PF8121A/PF8200 PMIC REGULATOR DEVICE DRIVER
M: Jagan Teki <jagan@amarulasolutions.com>
S: Maintained
--
2.49.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v7 6/6] MAINTAINERS: add an entry for pf1550 mfd driver
2025-06-12 14:55 ` [PATCH v7 6/6] MAINTAINERS: add an entry for pf1550 mfd driver Samuel Kayode via B4 Relay
@ 2025-06-12 16:03 ` Frank Li
0 siblings, 0 replies; 17+ messages in thread
From: Frank Li @ 2025-06-12 16:03 UTC (permalink / raw)
To: samuel.kayode
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Liam Girdwood, Mark Brown, Dmitry Torokhov, Sebastian Reichel,
imx, devicetree, linux-kernel, linux-input, linux-pm, Abel Vesa,
Abel Vesa, Robin Gong, Robin Gong, Enric Balletbo i Serra,
Krzysztof Kozlowski
On Thu, Jun 12, 2025 at 10:55:48AM -0400, Samuel Kayode via B4 Relay wrote:
> From: Samuel Kayode <samuel.kayode@savoirfairelinux.com>
>
> Add MAINTAINERS entry for pf1550 PMIC.
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: Samuel Kayode <samuel.kayode@savoirfairelinux.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> ---
> v6:
> - Add imx mailing list
> ---
> MAINTAINERS | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 98201e1f4ab5908ff49d32d19275e123cedb4b66..5547fdafa7e1bb11903d5d5bef246c2e1a20fbca 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -17853,6 +17853,17 @@ F: Documentation/devicetree/bindings/clock/imx*
> F: drivers/clk/imx/
> F: include/dt-bindings/clock/imx*
>
> +NXP PF1550 PMIC MFD DRIVER
> +M: Samuel Kayode <samuel.kayode@savoirfairelinux.com>
> +L: imx@lists.linux.dev
> +S: Maintained
> +F: Documentation/devicetree/bindings/mfd/nxp,pf1550.yaml
> +F: drivers/input/misc/pf1550-onkey.c
> +F: drivers/mfd/pf1550.c
> +F: drivers/power/supply/pf1550-charger.c
> +F: drivers/regulator/pf1550-regulator.c
> +F: include/linux/mfd/pfd1550.h
> +
> NXP PF8100/PF8121A/PF8200 PMIC REGULATOR DEVICE DRIVER
> M: Jagan Teki <jagan@amarulasolutions.com>
> S: Maintained
>
> --
> 2.49.0
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread