| Message ID | 20250527-pf1550-v3-0-45f69453cd51@savoirfairelinux.com |
|---|---|
| Headers | show |
| Series | add support for pf1550 PMIC MFD-based drivers | expand |
On Tue, May 27, 2025 at 06:25:35PM -0400, Samuel Kayode via B4 Relay wrote: > @@ -0,0 +1,353 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * pf1550.c - regulator driver for the PF1550 Please make the entire comment a C++ one so things look more intentional. > + * > + * Copyright (C) 2016 Freescale Semiconductor, Inc. > + * Robin Gong <yibin.gong@freescale.com> Presumably there's been some updates since then? > +static int pf1550_set_ramp_delay(struct regulator_dev *rdev, int ramp_delay) > +{ > + int id = rdev_get_id(rdev); > + unsigned int ramp_bits; > + int ret; > + > + if (id > PF1550_VREFDDR) > + return -EACCES; > + > + 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); Shouldn't we validate the value of ramp_delay? > +static irqreturn_t pf1550_regulator_irq_handler(int irq, void *data) > +{ > + switch (irq_type) { > + case PF1550_PMIC_IRQ_SW1_LS: > + case PF1550_PMIC_IRQ_SW2_LS: > + case PF1550_PMIC_IRQ_SW3_LS: > + dev_info(dev, "lowside interrupt triggered! irq_type=%d\n", > + irq_type); > + break; > + case PF1550_PMIC_IRQ_SW1_HS: > + case PF1550_PMIC_IRQ_SW2_HS: > + case PF1550_PMIC_IRQ_SW3_HS: > + dev_info(dev, "highside interrupt triggered! irq_type=%d\n", > + irq_type); > + break; Are these under and overvoltage events which should be reported as such? > + case PF1550_PMIC_IRQ_LDO1_FAULT: > + case PF1550_PMIC_IRQ_LDO2_FAULT: > + case PF1550_PMIC_IRQ_LDO3_FAULT: > + dev_info(dev, "ldo fault triggered! irq_type=%d\n", irq_type); > + break; Similarly, we can report error events. > + case PF1550_PMIC_IRQ_TEMP_110: > + case PF1550_PMIC_IRQ_TEMP_125: > + dev_info(dev, "thermal exception triggered! irq_type=%d\n", > + irq_type); > + break; We also have an over temperature event type.
On Wed, May 28, 2025 at 08:08:17AM +0200, Krzysztof Kozlowski wrote: > > + description: > > + Temperature threshold for thermal regulation of charger in celsius. > > But this now makes me wonder whether this should be just part of thermal > zone and get the threshold from there. I assume this is temperature of > CHARGER, not the battery. If battery, you have such properties in > battery.yaml (monitored-batter). Yes, it is the charger junction temperature. > > @Sebastian, > Are there existing bindings or devices which regulate temperature based > on thermal-zones in DT? > > +examples: > > + - | > > + battery: battery-cell { > > + compatible = "simple-battery"; > > + constant-charge-voltage-max-microvolt = <4400000>; > > + operating-range-celsius = <0 75>; > > So this looks like duplicating thermal-regulation property. Yes, thermal-regulation should suffice. Thanks, Sam
This series adds support for pf1550 PMIC. It provides the core mfd driver and a set of three sub-drivers for the regulator, power supply and input subsystems. Patch 1 adds the DT binding document for the PMIC. Patches 2-5 adds the pertinent drivers. Last patch adds a MAINTAINERS entry for the drivers. Changes since v1: - DT bindings for all devices included - Add onkey driver - Add driver for the regulators - Ensure charger is activated as some variants have it off by default - Update mfd and charger driver per feedback from eballetbo@gmail.com - Add myself as maintainer for these drivers - Link to v1: https://lore.kernel.org/1523974819-8711-1-git-send-email-abel.vesa@nxp.com/ Changes since v2: - Rebase on recent mainline kernel v6.15 - Single yaml file containing dt bindings for all pf1550 devices - irq mapping done in MFD driver as suggested by Dmitry Torokhov - Drop unnecessary includes in drivers - Replace dev_err with dev_err_probe in probe method of drivers - Drop compatible string from drivers of the sub-devices - Remove dependency on OF from drivers of the sub-devices - onkey: move driver from input/keyboard into input/misc - onkey: remove dependency on OF - onkey: use onkey virqs instead of central irq - onkey: fix integer overflow for regmap_write when unmasking interrupts during pf1550_onkey_resume - charger: add support for monitored-battery which is used in setting a constant voltage for the charger. - Address other feedback from Dmitry Torokhov and Krzysztof Kozlowski - Link to v2: https://lore.kernel.org/cover.1747409892.git.samuel.kayode@savoirfairelinux.com/ Signed-off-by: Samuel Kayode <samuel.kayode@savoirfairelinux.com> --- Samuel Kayode (6): dt-bindings: mfd: add pf1550 mfd: pf1550: add core mfd driver regulator: pf1550: add support for regulator input: pf1550: add onkey support power: supply: pf1550: add battery charger support MAINTAINERS: add an entry for pf1550 mfd driver Documentation/devicetree/bindings/mfd/pf1550.yaml | 139 +++++ MAINTAINERS | 10 + drivers/input/misc/Kconfig | 11 + drivers/input/misc/Makefile | 1 + drivers/input/misc/pf1550-onkey.c | 202 +++++++ drivers/mfd/Kconfig | 14 + drivers/mfd/Makefile | 2 + drivers/mfd/pf1550.c | 277 ++++++++++ drivers/power/supply/Kconfig | 11 + drivers/power/supply/Makefile | 1 + drivers/power/supply/pf1550-charger.c | 639 ++++++++++++++++++++++ drivers/regulator/Kconfig | 9 + drivers/regulator/Makefile | 1 + drivers/regulator/pf1550-regulator.c | 353 ++++++++++++ include/linux/mfd/pf1550.h | 241 ++++++++ 15 files changed, 1911 insertions(+) --- base-commit: 0a4b866d08c6adaea2f4592d31edac6deeb4dcbd change-id: 20250527-pf1550-d401f0d07b80 Best regards,