From 19253cac2a9021733e047ab0c04594c7c21182a9 Mon Sep 17 00:00:00 2001 From: Jad Keskes Date: Wed, 17 Jun 2026 10:46:22 +0100 Subject: [PATCH] regulator: max14577: fix set_mode clobbering enable on MAX77836 LDOs So the PWRMD field in CNFG1_LDO is both the enable bit and the mode. You can't change one without stepping on the other. The problem is that enable() from the regulator core just writes enable_mask (which is PWRMD_NORMAL). If you'd called set_mode(LPM) then disabled and re-enabled, the mode gets reset to NORMAL. And set_mode updates the register through the same field, so it can accidentally enable a disabled regulator. Fix it by storing the mode in per-regulator data. A custom enable writes whatever mode was last set. set_mode only touches hardware if the regulator is already on; otherwise it just caches the value. Add of_map_mode while here so the initial mode can be wired from DT. Signed-off-by: Jad Keskes Acked-by: Lee Jones Link: https://patch.msgid.link/20260617094622.1846471-1-inasj268@gmail.com Signed-off-by: Mark Brown --- drivers/regulator/max14577-regulator.c | 103 ++++++++++++++++++++++++- include/linux/mfd/max14577-private.h | 3 + 2 files changed, 102 insertions(+), 4 deletions(-) diff --git a/drivers/regulator/max14577-regulator.c b/drivers/regulator/max14577-regulator.c index c9d8d5e31cbd..cd592c5de148 100644 --- a/drivers/regulator/max14577-regulator.c +++ b/drivers/regulator/max14577-regulator.c @@ -123,15 +123,88 @@ static const struct regulator_desc max14577_supported_regulators[] = { [MAX14577_CHARGER] = MAX14577_CHARGER_REG, }; +struct max77836_ldo { + struct max14577 *max14577; + unsigned int mode; +}; + +static int max77836_ldo_enable(struct regulator_dev *rdev) +{ + struct max77836_ldo *ldo = rdev_get_drvdata(rdev); + + return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg, + MAX77836_CNFG1_LDO_PWRMD_MASK, ldo->mode); +} + +static int max77836_ldo_disable(struct regulator_dev *rdev) +{ + return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg, + MAX77836_CNFG1_LDO_PWRMD_MASK, + MAX77836_CNFG1_LDO_PWRMD_OFF); +} + +static unsigned int max77836_ldo_get_mode(struct regulator_dev *rdev) +{ + struct max77836_ldo *ldo = rdev_get_drvdata(rdev); + + switch (ldo->mode) { + case MAX77836_CNFG1_LDO_PWRMD_LPM: + return REGULATOR_MODE_IDLE; + case MAX77836_CNFG1_LDO_PWRMD_NORMAL: + return REGULATOR_MODE_NORMAL; + default: + return REGULATOR_MODE_INVALID; + } +} + +static int max77836_ldo_set_mode(struct regulator_dev *rdev, + unsigned int mode) +{ + struct max77836_ldo *ldo = rdev_get_drvdata(rdev); + unsigned int val; + + switch (mode) { + case REGULATOR_MODE_NORMAL: + val = MAX77836_CNFG1_LDO_PWRMD_NORMAL; + break; + case REGULATOR_MODE_IDLE: + val = MAX77836_CNFG1_LDO_PWRMD_LPM; + break; + default: + return -EINVAL; + } + + ldo->mode = val; + + /* Only touch hardware if the regulator is already on */ + if (regulator_is_enabled_regmap(rdev)) + return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg, + MAX77836_CNFG1_LDO_PWRMD_MASK, val); + + return 0; +} + +static unsigned int max77836_ldo_of_map_mode(unsigned int mode) +{ + switch (mode) { + case REGULATOR_MODE_NORMAL: + case REGULATOR_MODE_IDLE: + return mode; + default: + return REGULATOR_MODE_INVALID; + } +} + static const struct regulator_ops max77836_ldo_ops = { .is_enabled = regulator_is_enabled_regmap, - .enable = regulator_enable_regmap, - .disable = regulator_disable_regmap, + .enable = max77836_ldo_enable, + .disable = max77836_ldo_disable, .list_voltage = regulator_list_voltage_linear, .map_voltage = regulator_map_voltage_linear, .get_voltage_sel = regulator_get_voltage_sel_regmap, .set_voltage_sel = regulator_set_voltage_sel_regmap, - /* TODO: add .set_suspend_mode */ + .get_mode = max77836_ldo_get_mode, + .set_mode = max77836_ldo_set_mode, }; #define MAX77836_LDO_REG(num) { \ @@ -147,6 +220,7 @@ static const struct regulator_ops max77836_ldo_ops = { .uV_step = MAX77836_REGULATOR_LDO_VOLTAGE_STEP, \ .enable_reg = MAX77836_LDO_REG_CNFG1_LDO ## num, \ .enable_mask = MAX77836_CNFG1_LDO_PWRMD_MASK, \ + .of_map_mode = max77836_ldo_of_map_mode, \ .vsel_reg = MAX77836_LDO_REG_CNFG1_LDO ## num, \ .vsel_mask = MAX77836_CNFG1_LDO_TV_MASK, \ } @@ -205,7 +279,6 @@ static int max14577_regulator_probe(struct platform_device *pdev) } config.dev = max14577->dev; - config.driver_data = max14577; for (i = 0; i < supported_regulators_size; i++) { struct regulator_dev *regulator; @@ -217,6 +290,28 @@ static int max14577_regulator_probe(struct platform_device *pdev) config.init_data = pdata->regulators[i].initdata; config.of_node = pdata->regulators[i].of_node; } + + /* + * LDOs need per-regulator driver data to store their mode. + * The charger and safeout share the core MFD struct. + */ + if (dev_type == MAXIM_DEVICE_TYPE_MAX77836 && + (supported_regulators[i].id == MAX77836_LDO1 || + supported_regulators[i].id == MAX77836_LDO2)) { + struct max77836_ldo *ldo; + + ldo = devm_kzalloc(&pdev->dev, sizeof(*ldo), + GFP_KERNEL); + if (!ldo) + return -ENOMEM; + + ldo->max14577 = max14577; + ldo->mode = MAX77836_CNFG1_LDO_PWRMD_NORMAL; + config.driver_data = ldo; + } else { + config.driver_data = max14577; + } + config.regmap = max14577_get_regmap(max14577, supported_regulators[i].id); diff --git a/include/linux/mfd/max14577-private.h b/include/linux/mfd/max14577-private.h index dd51a37fa37f..5957e15b568e 100644 --- a/include/linux/mfd/max14577-private.h +++ b/include/linux/mfd/max14577-private.h @@ -350,6 +350,9 @@ enum max77836_pmic_reg { #define MAX77836_CNFG1_LDO_PWRMD_SHIFT 6 #define MAX77836_CNFG1_LDO_TV_SHIFT 0 #define MAX77836_CNFG1_LDO_PWRMD_MASK (0x3 << MAX77836_CNFG1_LDO_PWRMD_SHIFT) +#define MAX77836_CNFG1_LDO_PWRMD_OFF (0x0 << MAX77836_CNFG1_LDO_PWRMD_SHIFT) +#define MAX77836_CNFG1_LDO_PWRMD_LPM (0x1 << MAX77836_CNFG1_LDO_PWRMD_SHIFT) +#define MAX77836_CNFG1_LDO_PWRMD_NORMAL (0x3 << MAX77836_CNFG1_LDO_PWRMD_SHIFT) #define MAX77836_CNFG1_LDO_TV_MASK (0x3f << MAX77836_CNFG1_LDO_TV_SHIFT) /* LDO1/LDO2 CONFIG2 register */