linux-kernel - Re: [PATCH v8 5/6] power: supply: pf1550: add battery charger support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e2veigexln4ma5meguxqh6jh2r2fhj2d47pv4exjzwrhlazn7d@raknfsiucqls>
Date: Fri, 11 Jul 2025 09:02:18 +0000
From: Sean Nyekjaer <sean@...nix.com>
To: Samuel Kayode <samuel.kayode@...oirfairelinux.com>
Cc: Lee Jones <lee@...nel.org>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>, Dmitry Torokhov <dmitry.torokhov@...il.com>, Sebastian Reichel <sre@...nel.org>, Frank Li <Frank.li@....com>, imx@...ts.linux.dev, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, linux-input@...r.kernel.org, linux-pm@...r.kernel.org, Abel Vesa <abelvesa@...nel.org>, Abel Vesa <abelvesa@...ux.com>, Robin Gong <b38343@...escale.com>, Robin Gong <yibin.gong@....com>, Enric Balletbo i Serra <eballetbo@...il.com>
Subject: Re: [PATCH v8 5/6] power: supply: pf1550: add battery charger support

Hi,

On Mon, Jul 07, 2025 at 05:37:24PM +0100, Samuel Kayode wrote:
> Add support for the battery charger for pf1550 PMIC.
> 
> Signed-off-by: Samuel Kayode <samuel.kayode@...oirfairelinux.com>
> ---

[...]

> diff --git a/drivers/power/supply/pf1550-charger.c b/drivers/power/supply/pf1550-charger.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..7a6bd9c30d60280f1e1c50d4e1ddaf0a4998b9f0
> --- /dev/null
> +++ b/drivers/power/supply/pf1550-charger.c
> @@ -0,0 +1,632 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * charger driver for the PF1550
> + *
> + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> + * Robin Gong <yibin.gong@...escale.com>
> + *
> + * Portions Copyright (c) 2025 Savoir-faire Linux Inc.
> + * Samuel Kayode <samuel.kayode@...oirfairelinux.com>
> + */
> +
> +#include <linux/devm-helpers.h>
> +#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_DEFAULT_CONSTANT_VOLT	4200000
> +#define PF1550_DEFAULT_MIN_SYSTEM_VOLT	3500000
> +#define PF1550_DEFAULT_THERMAL_TEMP	75

Default is 95

> +#define PF1550_CHARGER_IRQ_NR		5
> +
> +struct pf1550_charger {
> +	struct device *dev;
> +	const struct pf1550_ddata *pf1550;
> +	struct power_supply *charger;
> +	struct power_supply *battery;
> +	struct delayed_work vbus_sense_work;
> +	struct delayed_work chg_sense_work;
> +	struct delayed_work bat_sense_work;
> +	int virqs[PF1550_CHARGER_IRQ_NR];
> +

[...]

> +
> +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;

>From the datasheet 80, 95, 110 and 125c is supported

> +	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");

data is unused here :/

> +
> +	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");

There are 3 modes for the charger operation:
0: charger = off, linear = off
1: charger = off, linear = on
2: charger = on, linear = on

The driver is hardcoded to use no. 2.

We are using the mode 1, and setting it to 2 causes my system to boot loop.

I don't know how we should select mode, maybe it could be an option from
the devicetree or use power_supply_get_battery_info() to look for a
battery and the only select between 1 or 2, but 0 would also be a valid
option.

/Sean