Message-ID: <50c29a62-1fdb-4de2-8887-0d551eee5ec0@google.com>
Date: Tue, 6 Jan 2026 15:41:18 -0800
From: Amit Sunil Dhamne <amitsd@...gle.com>
To: André Draszik <andre.draszik@...aro.org>,
 Sebastian Reichel <sre@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Lee Jones <lee@...nel.org>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Badhri Jagan Sridharan <badhri@...gle.com>,
 Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
 Peter Griffin <peter.griffin@...aro.org>,
 Tudor Ambarus <tudor.ambarus@...aro.org>,
 Alim Akhtar <alim.akhtar@...sung.com>
Cc: linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
 devicetree@...r.kernel.org, linux-usb@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-samsung-soc@...r.kernel.org,
 RD Babiera <rdbabiera@...gle.com>, Kyle Tso <kyletso@...gle.com>
Subject: Re: [PATCH v3 4/5] power: supply: max77759: add charger driver

Hi Andre',

On 1/5/26 9:32 AM, André Draszik wrote:
> Hi Amit,
>
> I haven't done a full review, but a few things caught my eye.
>
> On Sat, 2025-12-27 at 00:04 +0000, Amit Sunil Dhamne via B4 Relay wrote:
>> From: Amit Sunil Dhamne <amitsd@...gle.com>
>>
>> Add support for MAX77759 battery charger driver. This is a 4A 1-Cell
>> Li+/LiPoly dual input switch mode charger. While the device can support
>> USB & wireless charger inputs, this implementation only supports USB
>> input. This implementation supports both buck and boost modes.
>>
>> Signed-off-by: Amit Sunil Dhamne <amitsd@...gle.com>
>> ---
>>   MAINTAINERS                             |   6 +
>>   drivers/power/supply/Kconfig            |  11 +
>>   drivers/power/supply/Makefile           |   1 +
>>   drivers/power/supply/max77759_charger.c | 764 ++++++++++++++++++++++++++++++++
>>   4 files changed, 782 insertions(+)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index dc731d37c8fe..26a9654ab75e 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -15539,6 +15539,12 @@ F:	drivers/mfd/max77759.c
>>   F:	drivers/nvmem/max77759-nvmem.c
>>   F:	include/linux/mfd/max77759.h
>>   
>> +MAXIM MAX77759 BATTERY CHARGER DRIVER
>> +M:	Amit Sunil Dhamne <amitsd@...gle.com>
>> +L:	linux-kernel@...r.kernel.org
>> +S:	Maintained
>> +F:	drivers/power/supply/max77759_charger.c
>> +
>>   MAXIM MAX77802 PMIC REGULATOR DEVICE DRIVER
>>   M:	Javier Martinez Canillas <javier@...hile0.org>
>>   L:	linux-kernel@...r.kernel.org
>> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
>> index 92f9f7aae92f..e172fd980fde 100644
>> --- a/drivers/power/supply/Kconfig
>> +++ b/drivers/power/supply/Kconfig
>> @@ -1132,4 +1132,15 @@ config FUEL_GAUGE_MM8013
>>   	  the state of charge, temperature, cycle count, actual and design
>>   	  capacity, etc.
>>   
>> +config CHARGER_MAX77759
>> +	tristate "MAX77759 Charger Driver"
>> +	depends on MFD_MAX77759 && REGULATOR
>> +	default MFD_MAX77759
>> +	help
>> +	  Say M or Y here to enable the MAX77759 Charger Driver. MAX77759
>> +	  charger is a function of the MAX77759 PMIC. This is a dual input
>> +	  switch-mode charger. This driver supports buck and OTG boost modes.
>> +
>> +	  If built as a module, it will be called max77759_charger.
>> +
> It might make sense to add this block near the existing MAX77... charger drivers,
> while updating the tristate string and keeping alphabetical order of entries.
>
>>   endif # POWER_SUPPLY
>> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
>> index 4b79d5abc49a..6af905875ad5 100644
>> --- a/drivers/power/supply/Makefile
>> +++ b/drivers/power/supply/Makefile
>> @@ -128,3 +128,4 @@ obj-$(CONFIG_CHARGER_SURFACE)	+= surface_charger.o
>>   obj-$(CONFIG_BATTERY_UG3105)	+= ug3105_battery.o
>>   obj-$(CONFIG_CHARGER_QCOM_SMB2)	+= qcom_smbx.o
>>   obj-$(CONFIG_FUEL_GAUGE_MM8013)	+= mm8013.o
>> +obj-$(CONFIG_CHARGER_MAX77759)	+= max77759_charger.o
>> diff --git a/drivers/power/supply/max77759_charger.c b/drivers/power/supply/max77759_charger.c
>> new file mode 100644
>> index 000000000000..3d255b069fb9
>> --- /dev/null
>> +++ b/drivers/power/supply/max77759_charger.c
>> @@ -0,0 +1,764 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * max77759_charger.c - Battery charger driver for MAX77759 charger device.
>> + *
>> + * Copyright 2025 Google LLC.
>> + */
>> +
>> +#include <linux/bitfield.h>
>> +#include <linux/cleanup.h>
>> +#include <linux/device.h>
>> +#include <linux/devm-helpers.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/irq.h>
>> +#include <linux/math64.h>
>> +#include <linux/mfd/max77759.h>
>> +#include <linux/module.h>
>> +#include <linux/mod_devicetable.h>
>> +#include <linux/mutex.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/power_supply.h>
>> +#include <linux/regmap.h>
>> +#include <linux/regulator/driver.h>
>> +#include <linux/string_choices.h>
>> +
>> +/* Default values for Fast Charge Current & Float Voltage */
>> +#define CHG_CC_DEFAULT_UA			2266770
>> +#define CHG_FV_DEFAULT_MV			4300
>> +
>> +#define FOREACH_IRQ(S)			\
>> +	S(AICL),			\
>> +	S(CHGIN),			\
>> +	S(CHG),				\
>> +	S(INLIM),			\
>> +	S(BAT_OILO),			\
>> +	S(CHG_STA_CC),			\
>> +	S(CHG_STA_CV),			\
>> +	S(CHG_STA_TO),			\
>> +	S(CHG_STA_DONE)
>> +
>> +#define GENERATE_ENUM(e)		e
>> +#define GENERATE_STRING(s)		#s
>> +
>> +enum {
>> +	FOREACH_IRQ(GENERATE_ENUM)
>> +};
>> +
>> +static const char *const chgr_irqs_str[] = {
>> +	FOREACH_IRQ(GENERATE_STRING)
>> +};
>> +
>> +static int irqs[ARRAY_SIZE(chgr_irqs_str)];
> No global variables please, this is not a singleton.
>
>> [...]
>>
>> +static int set_input_current_limit(struct max77759_charger *chg, int ilim_ua)
>> +{
>> +	u32 regval;
>> +
>> +	if (ilim_ua < 0)
>> +		return -EINVAL;
>> +
>> +	if (ilim_ua == 0)
>> +		ilim_ua = MAX77759_CHGR_CHGIN_ILIM_MIN_UA;
>> +	else if (ilim_ua > MAX77759_CHGR_CHGIN_ILIM_MAX_UA)
>> +		ilim_ua = MAX77759_CHGR_CHGIN_ILIM_MAX_UA;
> What if ilim_ua == 1 (or any other value < min_uA)? You could use clamp()
> instead of open-coding.
>
>> +
>> +	regval = val_to_regval(ilim_ua, MAX77759_CHGR_CHGIN_ILIM_MIN_UA,
>> +			       MAX77759_CHGR_CHGIN_ILIM_STEP_UA,
>> +			       MAX77759_CHGR_CHGIN_ILIM_REG_OFFSET);
>> +	return regmap_update_bits(chg->regmap, MAX77759_CHGR_REG_CHG_CNFG_09,
>> +				  MAX77759_CHGR_REG_CHG_CNFG_09_CHGIN_ILIM,
>> +				  regval);
>> +}
>> +
>> +static const enum power_supply_property max77759_charger_props[] = {
>> +	POWER_SUPPLY_PROP_ONLINE,
>> +	POWER_SUPPLY_PROP_PRESENT,
>> +	POWER_SUPPLY_PROP_STATUS,
>> +	POWER_SUPPLY_PROP_CHARGE_TYPE,
>> +	POWER_SUPPLY_PROP_HEALTH,
>> +	POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX,
>> +	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
>> +	POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
>> +};
>> +
>> +static int max77759_charger_get_property(struct power_supply *psy,
>> +					 enum power_supply_property psp,
>> +					 union power_supply_propval *pval)
>> +{
>> +	struct max77759_charger *chg = power_supply_get_drvdata(psy);
>> +	int ret;
>> +
>> +	switch (psp) {
>> +	case POWER_SUPPLY_PROP_ONLINE:
>> +		ret = get_online(chg);
>> +		break;
>> +	case POWER_SUPPLY_PROP_PRESENT:
>> +		ret = charger_input_valid(chg);
>> +		break;
>> +	case POWER_SUPPLY_PROP_STATUS:
>> +		ret = get_status(chg);
>> +		break;
>> +	case POWER_SUPPLY_PROP_CHARGE_TYPE:
>> +		ret = get_charge_type(chg);
>> +		break;
>> +	case POWER_SUPPLY_PROP_HEALTH:
>> +		ret = get_health(chg);
>> +		break;
>> +	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX:
>> +		ret = get_fast_charge_current(chg);
>> +		break;
>> +	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX:
>> +		ret = get_float_voltage(chg);
>> +		break;
>> +	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
>> +		ret = get_input_current_limit(chg);
>> +		break;
>> +	default:
>> +		ret = -EINVAL;
>> +	}
>> +
>> +	pval->intval = ret;
>> +	return ret < 0 ? ret : 0;
>> +}
>> +
>> +static const struct power_supply_desc max77759_charger_desc = {
>> +	.name = "max77759-charger",
>> +	.type = POWER_SUPPLY_TYPE_USB,
>> +	.properties = max77759_charger_props,
>> +	.num_properties = ARRAY_SIZE(max77759_charger_props),
>> +	.get_property = max77759_charger_get_property,
>> +};
>> +
>> +static int charger_set_mode(struct max77759_charger *chg,
>> +			    enum max77759_chgr_mode mode)
>> +{
>> +	int ret;
>> +
>> +	guard(mutex)(&chg->lock);
>> +
>> +	if (chg->mode == mode)
>> +		return 0;
>> +
>> +	if ((mode == MAX77759_CHGR_MODE_CHG_BUCK_ON ||
>> +	     mode == MAX77759_CHGR_MODE_OTG_BOOST_ON) &&
>> +	    chg->mode != MAX77759_CHGR_MODE_OFF) {
>> +		dev_err(chg->dev, "Invalid mode transition from %d to %d",
>> +			chg->mode, mode);
>> +		return -EINVAL;
>> +	}
>> +
>> +	ret = regmap_update_bits(chg->regmap, MAX77759_CHGR_REG_CHG_CNFG_00,
>> +				 MAX77759_CHGR_REG_CHG_CNFG_00_MODE, mode);
>> +	if (ret)
>> +		return ret;
>> +
>> +	chg->mode = mode;
>> +	return 0;
>> +}
>> +
>> +static int enable_chgin_otg(struct regulator_dev *rdev)
>> +{
>> +	struct max77759_charger *chg = rdev_get_drvdata(rdev);
>> +
>> +	return charger_set_mode(chg, MAX77759_CHGR_MODE_OTG_BOOST_ON);
>> +}
>> +
>> +static int disable_chgin_otg(struct regulator_dev *rdev)
>> +{
>> +	struct max77759_charger *chg = rdev_get_drvdata(rdev);
>> +
>> +	return charger_set_mode(chg, MAX77759_CHGR_MODE_OFF);
>> +}
>> +
>> +static int chgin_otg_status(struct regulator_dev *rdev)
>> +{
>> +	struct max77759_charger *chg = rdev_get_drvdata(rdev);
>> +
>> +	guard(mutex)(&chg->lock);
>> +	return chg->mode == MAX77759_CHGR_MODE_OTG_BOOST_ON;
>> +}
>> +
>> +static const struct regulator_ops chgin_otg_reg_ops = {
>> +	.enable = enable_chgin_otg,
>> +	.disable = disable_chgin_otg,
>> +	.is_enabled = chgin_otg_status,
>> +};
>> +
>> +static const struct regulator_desc chgin_otg_reg_desc = {
>> +	.name = "chgin-otg",
>> +	.of_match = of_match_ptr("chgin-otg-regulator"),
>> +	.owner = THIS_MODULE,
>> +	.ops = &chgin_otg_reg_ops,
>> +	.fixed_uV = 5000000,
>> +	.n_voltages = 1,
>> +};
>> +
>> +static irqreturn_t irq_handler(int irq, void *data)
>> +{
>> +	struct max77759_charger *chg = data;
>> +	struct device *dev = chg->dev;
>> +	u32 chgint_ok;
>> +	int i;
>> +
>> +	regmap_read(chg->regmap, MAX77759_CHGR_REG_CHG_INT_OK, &chgint_ok);
> You might want to check the return value and return IRQ_NONE if it didn't
> work?
>
>> +
>> +	for (i = 0; i < ARRAY_SIZE(irqs); i++) {
>> +		if (irqs[i] == irq)
>> +			break;
>> +	}
>> +
>> +	switch (i) {
>> +	case AICL:
>> +		dev_dbg(dev, "AICL mode: %s",
>> +			str_no_yes(chgint_ok & MAX77759_CHGR_REG_CHG_INT_AICL));
>> +		break;
>> +	case CHGIN:
>> +		dev_dbg(dev, "CHGIN input valid: %s",
>> +			str_yes_no(chgint_ok & MAX77759_CHGR_REG_CHG_INT_CHGIN));
>> +		break;
>> +	case CHG:
>> +		dev_dbg(dev, "CHG status okay/off: %s",
>> +			str_yes_no(chgint_ok & MAX77759_CHGR_REG_CHG_INT_CHG));
>> +		break;
>> +	case INLIM:
>> +		dev_dbg(dev, "Current Limit reached: %s",
>> +			str_no_yes(chgint_ok & MAX77759_CHGR_REG_CHG_INT_INLIM));
>> +		break;
>> +	case BAT_OILO:
>> +		dev_dbg(dev, "Battery over-current threshold crossed");
>> +		break;
>> +	case CHG_STA_CC:
>> +		dev_dbg(dev, "Charger reached CC stage");
>> +		break;
>> +	case CHG_STA_CV:
>> +		dev_dbg(dev, "Charger reached CV stage");
>> +		break;
>> +	case CHG_STA_TO:
>> +		dev_dbg(dev, "Charger reached TO stage");
>> +		break;
>> +	case CHG_STA_DONE:
>> +		dev_dbg(dev, "Charger reached TO stage");
>> +		break;
> Are the above debug messages really all needed?
>
>> +	default:
>> +		dev_err(dev, "Unrecognized irq: %d", i);
>> +		return IRQ_HANDLED;
> I'm not sure it should return IRQ_HANDLED in this case.
>
>> +	}
>> +
>> +	power_supply_changed(chg->psy);
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int max77759_init_irqhandler(struct max77759_charger *chg)
>> +{
>> +	struct device *dev = chg->dev;
>> +	unsigned long irq_flags;
>> +	struct irq_data *irqd;
>> +	int i, ret;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(chgr_irqs_str); i++) {
>> +		ret = platform_get_irq_byname(to_platform_device(dev),
>> +					      chgr_irqs_str[i]);
>> +		if (ret < 0) {
>> +			dev_err(dev,
>> +				"Failed to get irq resource for %s, ret=%d",
>> +				chgr_irqs_str[i], ret);
>> +			return ret;
>> +		}
> You should use return dev_err_probe() here, and drop the additional dev_err_probe()
> in max77759_charger_probe().
>
>> +
>> +		irqs[i] = ret;
>> +		irq_flags = IRQF_ONESHOT;
>> +		irqd = irq_get_irq_data(irqs[i]);
>> +		if (irqd)
>> +			irq_flags |= irqd_get_trigger_type(irqd);
> The above three lines are not needed, and then you can also drop irq_flags and
> use its value in the below call directly.
>
>> +
>> +		ret = devm_request_threaded_irq(dev, irqs[i], NULL, irq_handler,
>> +						irq_flags, dev_name(dev), chg);
>> +		if (ret) {
>> +			dev_err(dev,
>> +				"Unable to register irq handler for %s, ret=%d",
>> +				chgr_irqs_str[i], ret);
>> +			return ret;
> dev_err_probe() please.
>
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int max77759_charger_init(struct max77759_charger *chg)
>> +{
>> +	struct power_supply_battery_info *info;
>> +	u32 regval, fast_chg_curr, fv;
>> +	int ret;
>> +
>> +	regmap_read(chg->regmap, MAX77759_CHGR_REG_CHG_CNFG_00, &regval);
>> +	chg->mode = FIELD_GET(MAX77759_CHGR_REG_CHG_CNFG_00_MODE, regval);
>> +	ret = charger_set_mode(chg, MAX77759_CHGR_MODE_OFF);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (power_supply_get_battery_info(chg->psy, &info)) {
>> +		fv = CHG_FV_DEFAULT_MV;
>> +		fast_chg_curr = CHG_CC_DEFAULT_UA;
>> +	} else {
>> +		fv = info->constant_charge_voltage_max_uv / 1000;
>> +		fast_chg_curr = info->constant_charge_current_max_ua;
>> +	}
>> +
>> +	ret = set_fast_charge_current_limit(chg, fast_chg_curr);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = set_float_voltage_limit(chg, fv);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = unlock_prot_regs(chg, true);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Disable wireless charging input */
>> +	regmap_update_bits(chg->regmap, MAX77759_CHGR_REG_CHG_CNFG_12,
>> +			   MAX77759_CHGR_REG_CHG_CNFG_12_WCINSEL, 0);
>> +
>> +	regmap_update_bits(chg->regmap, MAX77759_CHGR_REG_CHG_CNFG_18,
>> +			   MAX77759_CHGR_REG_CHG_CNFG_18_WDTEN, 0);
> I think it's good practice to check return values.
>
>> +
>> +	return unlock_prot_regs(chg, false);
>> +}
>> +
>> +static void psy_work_item(struct work_struct *work)
>> +{
>> +	struct max77759_charger *chg =
>> +		container_of(work, struct max77759_charger, psy_work);
>> +	union power_supply_propval current_limit = { 0 }, online = { 0 };
>> +	int ret;
>> +
>> +	power_supply_get_property(chg->tcpm_psy, POWER_SUPPLY_PROP_CURRENT_MAX,
>> +				  &current_limit);
>> +	power_supply_get_property(chg->tcpm_psy, POWER_SUPPLY_PROP_ONLINE,
>> +				  &online);
> Would it make sense to rework this and check the return values? Then you can also
> drop the greedy init at function entry.
>
>> +
>> +	if (online.intval && current_limit.intval) {
>> +		ret = set_input_current_limit(chg, current_limit.intval);
>> +		if (ret)
>> +			dev_err(chg->dev,
>> +				"Unable to set current limit, ret=%d", ret);
>> +
>> +		charger_set_mode(chg, MAX77759_CHGR_MODE_CHG_BUCK_ON);
>> +	} else {
>> +		charger_set_mode(chg, MAX77759_CHGR_MODE_OFF);
>> +	}
>> +}
>> +
>> +static int psy_changed(struct notifier_block *nb, unsigned long evt, void *data)
>> +{
>> +	struct max77759_charger *chg = container_of(nb, struct max77759_charger,
>> +						    nb);
>> +	const char *psy_name = "tcpm-source";
>> +	struct power_supply *psy = data;
>> +
>> +	if (!strnstr(psy->desc->name, psy_name, strlen(psy_name)) ||
>> +	    evt != PSY_EVENT_PROP_CHANGED)
>> +		return NOTIFY_OK;
>> +
>> +	chg->tcpm_psy = psy;
>> +	schedule_work(&chg->psy_work);
> Maybe add a newline here.
>
>> +	return NOTIFY_OK;
>> +}
>> +
>> +static void max_tcpci_unregister_psy_notifier(void *nb)
>> +{
>> +	power_supply_unreg_notifier(nb);
>> +}
>> +
>> +static int max77759_charger_probe(struct platform_device *pdev)
>> +{
>> +	struct regulator_config chgin_otg_reg_cfg;
>> +	struct power_supply_config psy_cfg;
>> +	struct device *dev = &pdev->dev;
>> +	struct max77759_charger *chg;
>> +	int ret;
>> +
>> +	device_set_of_node_from_dev(dev, dev->parent);
>> +	chg = devm_kzalloc(dev, sizeof(*chg), GFP_KERNEL);
>> +	if (!chg)
>> +		return -ENOMEM;
>> +
>> +	platform_set_drvdata(pdev, chg);
>> +	chg->dev = dev;
>> +	chg->regmap = dev_get_regmap(dev->parent, "charger");
>> +	if (!chg->regmap)
>> +		return dev_err_probe(dev, -ENODEV, "Missing regmap");
>> +
>> +	ret = devm_mutex_init(dev, &chg->lock);
>> +	if (ret)
>> +		return dev_err_probe(dev, ret, "Failed to initialize lock");
>> +
>> +	psy_cfg.fwnode = dev_fwnode(dev);
>> +	psy_cfg.drv_data = chg;
>> +	chg->psy = devm_power_supply_register(dev, &max77759_charger_desc,
>> +					      &psy_cfg);
>> +	if (IS_ERR(chg->psy))
>> +		return dev_err_probe(dev, -EPROBE_DEFER,
>> +				     "Failed to register psy, ret=%ld",
>> +				     PTR_ERR(chg->psy));
>> +
>> +	ret = max77759_charger_init(chg);
>> +	if (ret)
>> +		return dev_err_probe(dev, ret,
>> +				     "Failed to initialize max77759 charger");
>> +
>> +	chgin_otg_reg_cfg.dev = dev;
>> +	chgin_otg_reg_cfg.driver_data = chg;
>> +	chgin_otg_reg_cfg.of_node = dev_of_node(dev);
>> +	chg->chgin_otg_rdev = devm_regulator_register(dev, &chgin_otg_reg_desc,
>> +						      &chgin_otg_reg_cfg);
>> +	if (IS_ERR(chg->chgin_otg_rdev))
>> +		return dev_err_probe(dev, PTR_ERR(chg->chgin_otg_rdev),
>> +				     "Failed to register chgin otg regulator");
>> +
>> +	ret = devm_work_autocancel(dev, &chg->psy_work, psy_work_item);
>> +	if (ret)
>> +		return dev_err_probe(dev, ret, "Failed to initialize psy work");
>> +
>> +	chg->nb.notifier_call = psy_changed;
>> +	ret = power_supply_reg_notifier(&chg->nb);
>> +	if (ret)
>> +		return dev_err_probe(dev, ret,
>> +				     "Unable to register psy notifier");
>> +
>> +	ret = devm_add_action_or_reset(dev, max_tcpci_unregister_psy_notifier,
>> +				       &chg->nb);
>> +	if (ret)
>> +		return ret;
> You could print a message here as well.

Thanks for the detailed review! I will fix all the flagged issues in the 
next rev.


BR,

Amit

>
> Cheers,
> Andre'

Powered by blists - more mailing lists