* [RESEND PATCH v3 0/3] add support for MCP16502 PMIC @ 2018-12-11 10:08 Andrei.Stefanescu 2018-12-11 10:09 ` [RESEND PATCH v3 1/3] regulator: dt-bindings: add MCP16502 regulator bindings Andrei.Stefanescu ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Andrei.Stefanescu @ 2018-12-11 10:08 UTC (permalink / raw) To: broonie, robh+dt, lgirdwood, mark.rutland, gregkh Cc: Cristian.Birsan, Nicolas.Ferre, Claudiu.Beznea, linux-arm-kernel, linux-kernel, devicetree, Andrei.Stefanescu The resend is because I forgot to add Mark. MCP16502 is a Power Management IC from Microchip. It has 4 Buck outputs and 2 LDOs. The buck regulators can be used in two modes: normal(FPWM) and low-power(Auto PFM). This patch series adds support for the MCP16502 PMIC. v3: - use CONFIG_SUSPEND and CONFIG_PM to fix compile errors for some configs v2: - use lpm-gpios instead of lpm-gpio in devicetree bindings documentation - describe the regulators present on the PMIC in the devicetree bindings documentation - add SPDX license inside a C++ comment - prefix macro - remove mcp16502_update_regulator and mcp16502_read - replace ?: with if-else - change some if-else with switch statements for legibility - use regmap helpers for regultor settings during runtime - make mcp16502_get_status read the status from the PMIC STS registers - use module_i2c_driver - use the PMIC's Hibernate registers for suspend-to-mem, the PMIC's Low-power registers for standby and the PMIC's Active registers for normal runtime Note about mcp16502_suspend: - mcp16502_gpio_set_mode(mcp, MCP16502_OPMODE_HIB) has now been changed to mcp16502_gpio_set_mode(mcp, MCP16502_OPMODE_LPM) for legibility. Note that the function call only sets the LPM pin of the PMIC to high. This puts the PMIC in Low-power operating mode. Hibernate operating mode is reached when the MPU sets the PWRHLD line to zero (typically when entering suspend-to-ram). Andrei Stefanescu (3): regulator: dt-bindings: add MCP16502 regulator bindings MAINTAINERS: add maintainer for MCP16502 PMIC driver regulator: mcp16502: add regulator driver for MCP16502 .../bindings/regulator/mcp16502-regulator.txt | 143 ++++++ MAINTAINERS | 7 + drivers/regulator/Kconfig | 9 + drivers/regulator/Makefile | 1 + drivers/regulator/mcp16502.c | 555 +++++++++++++++++++++ 5 files changed, 715 insertions(+) create mode 100644 Documentation/devicetree/bindings/regulator/mcp16502-regulator.txt create mode 100644 drivers/regulator/mcp16502.c -- 2.7.4 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [RESEND PATCH v3 1/3] regulator: dt-bindings: add MCP16502 regulator bindings 2018-12-11 10:08 [RESEND PATCH v3 0/3] add support for MCP16502 PMIC Andrei.Stefanescu @ 2018-12-11 10:09 ` Andrei.Stefanescu 2018-12-11 10:09 ` [RESEND PATCH v3 2/3] MAINTAINERS: add maintainer for MCP16502 PMIC driver Andrei.Stefanescu 2018-12-11 10:09 ` [RESEND PATCH v3 3/3] regulator: mcp16502: add regulator driver for MCP16502 Andrei.Stefanescu 2 siblings, 0 replies; 9+ messages in thread From: Andrei.Stefanescu @ 2018-12-11 10:09 UTC (permalink / raw) To: broonie, robh+dt, lgirdwood, mark.rutland, gregkh Cc: Cristian.Birsan, Nicolas.Ferre, Claudiu.Beznea, linux-arm-kernel, linux-kernel, devicetree, Andrei.Stefanescu This patch describes the compatible and the device tree bindings necessary for the MCP16502 PMIC. Signed-off-by: Andrei Stefanescu <andrei.stefanescu@microchip.com> Reviewed-by: Rob Herring <robh@kernel.org> --- .../bindings/regulator/mcp16502-regulator.txt | 143 +++++++++++++++++++++ 1 file changed, 143 insertions(+) create mode 100644 Documentation/devicetree/bindings/regulator/mcp16502-regulator.txt diff --git a/Documentation/devicetree/bindings/regulator/mcp16502-regulator.txt b/Documentation/devicetree/bindings/regulator/mcp16502-regulator.txt new file mode 100644 index 0000000..b8f843f --- /dev/null +++ b/Documentation/devicetree/bindings/regulator/mcp16502-regulator.txt @@ -0,0 +1,143 @@ +MCP16502 PMIC + +Required properties: +- compatible: "microchip,mcp16502" +- reg: I2C slave address +- lpm-gpios: GPIO for LPM pin. Note that this GPIO *must* remain high during + suspend-to-ram, keeping the PMIC into HIBERNATE mode. +- regulators: A node that houses a sub-node for each regulator within + the device. Each sub-node is identified using the node's + name. The content of each sub-node is defined by the + standard binding for regulators; see regulator.txt. + +Regualtors of MCP16502 PMIC: +1) VDD_IO - Buck (1.2 - 3.7 V) +2) VDD_DDR - Buck (0.6 - 1.85 V) +3) VDD_CORE - Buck (0.6 - 1.85 V) +4) VDD_OTHER - BUCK (0.6 - 1.85 V) +5) LDO1 - LDO (1.2 - 3.7 V) +6) LDO2 - LDO (1.2 - 3.7 V) + +Regulator modes: +2 - FPWM: higher precision, higher consumption +4 - AutoPFM: lower precision, lower consumption + +Each regulator is defined using the standard binding for regulators. + +Example: + +mcp16502@5b { + compatible = "microchip,mcp16502"; + reg = <0x5b>; + status = "okay"; + lpm-gpios = <&pioBU 7 GPIO_ACTIVE_HIGH>; + + regulators { + VDD_IO { + regulator-name = "VDD_IO"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <3700000>; + regulator-initial-mode = <2>; + regulator-allowed-modes = <2>, <4>; + regulator-always-on; + + regulator-state-standby { + regulator-on-in-suspend; + regulator-mode = <4>; + }; + + regulator-state-mem { + regulator-off-in-suspend; + regulator-mode = <4>; + }; + }; + + VDD_DDR { + regulator-name = "VDD_DDR"; + regulator-min-microvolt = <600000>; + regulator-max-microvolt = <1850000>; + regulator-initial-mode = <2>; + regulator-allowed-modes = <2>, <4>; + regulator-always-on; + + regulator-state-standby { + regulator-on-in-suspend; + regulator-mode = <4>; + }; + + regulator-state-mem { + regulator-on-in-suspend; + regulator-mode = <4>; + }; + }; + + VDD_CORE { + regulator-name = "VDD_CORE"; + regulator-min-microvolt = <600000>; + regulator-max-microvolt = <1850000>; + regulator-initial-mode = <2>; + regulator-allowed-modes = <2>, <4>; + regulator-always-on; + + regulator-state-standby { + regulator-on-in-suspend; + regulator-mode = <4>; + }; + + regulator-state-mem { + regulator-off-in-suspend; + regulator-mode = <4>; + }; + }; + + VDD_OTHER { + regulator-name = "VDD_OTHER"; + regulator-min-microvolt = <600000>; + regulator-max-microvolt = <1850000>; + regulator-initial-mode = <2>; + regulator-allowed-modes = <2>, <4>; + regulator-always-on; + + regulator-state-standby { + regulator-on-in-suspend; + regulator-mode = <4>; + }; + + regulator-state-mem { + regulator-off-in-suspend; + regulator-mode = <4>; + }; + }; + + LDO1 { + regulator-name = "LDO1"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <3700000>; + regulator-always-on; + + regulator-state-standby { + regulator-on-in-suspend; + }; + + regulator-state-mem { + regulator-off-in-suspend; + }; + }; + + LDO2 { + regulator-name = "LDO2"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <3700000>; + regulator-always-on; + + regulator-state-standby { + regulator-on-in-suspend; + }; + + regulator-state-mem { + regulator-off-in-suspend; + }; + }; + + }; +}; -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [RESEND PATCH v3 2/3] MAINTAINERS: add maintainer for MCP16502 PMIC driver 2018-12-11 10:08 [RESEND PATCH v3 0/3] add support for MCP16502 PMIC Andrei.Stefanescu 2018-12-11 10:09 ` [RESEND PATCH v3 1/3] regulator: dt-bindings: add MCP16502 regulator bindings Andrei.Stefanescu @ 2018-12-11 10:09 ` Andrei.Stefanescu 2018-12-11 10:09 ` [RESEND PATCH v3 3/3] regulator: mcp16502: add regulator driver for MCP16502 Andrei.Stefanescu 2 siblings, 0 replies; 9+ messages in thread From: Andrei.Stefanescu @ 2018-12-11 10:09 UTC (permalink / raw) To: broonie, robh+dt, lgirdwood, mark.rutland, gregkh Cc: Cristian.Birsan, Nicolas.Ferre, Claudiu.Beznea, linux-arm-kernel, linux-kernel, devicetree, Andrei.Stefanescu This patch adds a maintainer for the MCP16502 PMIC driver. Signed-off-by: Andrei Stefanescu <andrei.stefanescu@microchip.com> --- MAINTAINERS | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index b755a89..6a74a65 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9730,6 +9730,13 @@ M: Ludovic Desroches <ludovic.desroches@microchip.com> S: Maintained F: drivers/mmc/host/atmel-mci.c +MICROCHIP MCP16502 PMIC DRIVER +M: Andrei Stefanescu <andrei.stefanescu@microchip.com> +L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers) +S: Maintained +F: Documentation/devicetree/bindings/regulator/mcp16502-regulator.txt +F: drivers/regulator/mcp16502.c + MICROCHIP MCP3911 ADC DRIVER M: Marcus Folkesson <marcus.folkesson@gmail.com> M: Kent Gustavsson <kent@minoris.se> -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [RESEND PATCH v3 3/3] regulator: mcp16502: add regulator driver for MCP16502 2018-12-11 10:08 [RESEND PATCH v3 0/3] add support for MCP16502 PMIC Andrei.Stefanescu 2018-12-11 10:09 ` [RESEND PATCH v3 1/3] regulator: dt-bindings: add MCP16502 regulator bindings Andrei.Stefanescu 2018-12-11 10:09 ` [RESEND PATCH v3 2/3] MAINTAINERS: add maintainer for MCP16502 PMIC driver Andrei.Stefanescu @ 2018-12-11 10:09 ` Andrei.Stefanescu 2018-12-11 16:47 ` Mark Brown 2 siblings, 1 reply; 9+ messages in thread From: Andrei.Stefanescu @ 2018-12-11 10:09 UTC (permalink / raw) To: broonie, robh+dt, lgirdwood, mark.rutland, gregkh Cc: Cristian.Birsan, Nicolas.Ferre, Claudiu.Beznea, linux-arm-kernel, linux-kernel, devicetree, Andrei.Stefanescu This patch adds a regulator driver for the MCP16502 PMIC. This drivers supports basic operations through the regulator interface such as: - setting/reading voltage - setting/reading operating mode - reading current status - transitioning to/from suspend-to-ram and standby power states Signed-off-by: Andrei Stefanescu <andrei.stefanescu@microchip.com> --- drivers/regulator/Kconfig | 9 + drivers/regulator/Makefile | 1 + drivers/regulator/mcp16502.c | 555 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 565 insertions(+) create mode 100644 drivers/regulator/mcp16502.c diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig index 926cee0..719d9d6 100644 --- a/drivers/regulator/Kconfig +++ b/drivers/regulator/Kconfig @@ -567,6 +567,15 @@ config REGULATOR_MC13892 Say y here to support the regulators found on the Freescale MC13892 PMIC. +config REGULATOR_MCP16502 + tristate "Microchip MCP16502 PMIC" + depends on I2C && OF + help + Say y here to support the MCP16502 PMIC. This driver supports + basic operations (get/set voltage, get/set operating mode) + through the regulator interface. In addition it enables + suspend-to-ram/standby transition. + config REGULATOR_MT6311 tristate "MediaTek MT6311 PMIC" depends on I2C diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile index 72488ef..b12e1c9 100644 --- a/drivers/regulator/Makefile +++ b/drivers/regulator/Makefile @@ -74,6 +74,7 @@ obj-$(CONFIG_REGULATOR_MAX77802) += max77802-regulator.o obj-$(CONFIG_REGULATOR_MC13783) += mc13783-regulator.o obj-$(CONFIG_REGULATOR_MC13892) += mc13892-regulator.o obj-$(CONFIG_REGULATOR_MC13XXX_CORE) += mc13xxx-regulator-core.o +obj-$(CONFIG_REGULATOR_MCP16502) += mcp16502.o obj-$(CONFIG_REGULATOR_MT6311) += mt6311-regulator.o obj-$(CONFIG_REGULATOR_MT6323) += mt6323-regulator.o obj-$(CONFIG_REGULATOR_MT6380) += mt6380-regulator.o diff --git a/drivers/regulator/mcp16502.c b/drivers/regulator/mcp16502.c new file mode 100644 index 0000000..54db3df --- /dev/null +++ b/drivers/regulator/mcp16502.c @@ -0,0 +1,555 @@ +/* SPDX-License-Identifier: GPL-2.0 + * + * MCP16502 PMIC driver + * + * Copyright (C) 2018 Microchip Technology Inc. and its subsidiaries + * + * Author: Andrei Stefanescu <andrei.stefanescu@microchip.com> + * + * Inspired from tps65086-regulator.c + */ + +#include <linux/gpio.h> +#include <linux/i2c.h> +#include <linux/init.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/regmap.h> +#include <linux/regulator/driver.h> +#include <linux/suspend.h> + +#define VDD_LOW_SEL 0x0D +#define VDD_HIGH_SEL 0x3F + +#define MCP16502_FLT BIT(7) +#define MCP16502_ENS BIT(0) + +/* + * + * The PMIC has four sets of registers corresponding to four power modes: + * Performance, Active, Low-power, Hibernate. + * + * Registers: + * Each regulator has a register for each power mode. To access a register + * for a specific regulator and mode BASE_* and OFFSET_* need to be added. + * + * Operating modes: + * In order for the PMIC to transition to operating modes it has to be + * controlled via GPIO lines called LPM and HPM. + * + * The registers are fully configurable such that you can put all regulators in + * a low-power state while the PMIC is in Active mode. They are supposed to be + * configured at startup and then simply transition to/from a global low-power + * state by setting the GPIO lpm pin high/low. + * + * This driver keeps the PMIC in Active mode, Low-power state is set for the + * regulators by enabling/disabling operating mode (FPWM or Auto PFM). + * + * The PMIC's Low-power and Hibernate modes are used during standby/suspend. + * To enter standby/suspend the PMIC will go to Low-power mode. From there, it + * will transition to Hibernate when the PWRHLD line is set to low by the MPU. + */ + +/* + * This function is useful for iterating over all regulators and accessing their + * registers in a generic way or accessing a regulator device by its id. + */ +#define MCP16502_BASE(i) (((i) + 1) << 4) +#define MCP16502_STAT_BASE(i) ((i) + 5) + +#define MCP16502_OFFSET_MODE_A 0 +#define MCP16502_OFFSET_MODE_LPM 1 +#define MCP16502_OFFSET_MODE_HIB 2 + +#define MCP16502_OPMODE_ACTIVE REGULATOR_MODE_NORMAL +#define MCP16502_OPMODE_LPM REGULATOR_MODE_IDLE +#define MCP16502_OPMODE_HIB REGULATOR_MODE_STANDBY + +#define MCP16502_MODE_AUTO_PFM 0 +#define MCP16502_MODE_FPWM BIT(6) + +#define MCP16502_VSEL 0x3F +#define MCP16502_EN BIT(7) +#define MCP16502_MODE BIT(6) + +#define MCP16502_MIN_REG 0x0 +#define MCP16502_MAX_REG 0x65 + +static unsigned int mcp16502_of_map_mode(unsigned int mode) +{ + if (mode == REGULATOR_MODE_NORMAL || mode == REGULATOR_MODE_IDLE) + return mode; + + return REGULATOR_MODE_INVALID; +} + +#define MCP16502_REGULATOR(_name, _id, _ranges, _ops) \ + [_id] = { \ + .name = _name, \ + .regulators_node = of_match_ptr("regulators"), \ + .id = _id, \ + .ops = &(_ops), \ + .type = REGULATOR_VOLTAGE, \ + .owner = THIS_MODULE, \ + .linear_ranges = _ranges, \ + .n_linear_ranges = ARRAY_SIZE(_ranges), \ + .of_match = of_match_ptr(_name), \ + .of_map_mode = mcp16502_of_map_mode, \ + .vsel_reg = (((_id) + 1) << 4), \ + .vsel_mask = MCP16502_VSEL, \ + .enable_reg = (((_id) + 1) << 4), \ + .enable_mask = MCP16502_EN, \ + } + +enum { + BUCK1 = 0, + BUCK2, + BUCK3, + BUCK4, + LDO1, + LDO2, + NUM_REGULATORS +}; + +/* + * struct mcp16502 - PMIC representation + * @rdev: the regulators belonging to this chip + * @rmap: regmap to be used for I2C communication + * @lpm: LPM GPIO descriptor + */ +struct mcp16502 { + struct regulator_dev *rdev[NUM_REGULATORS]; + struct regmap *rmap; + struct gpio_desc *lpm; +}; + +/* + * mcp16502_gpio_set_mode() - set the GPIO corresponding value + * + * Used to prepare transitioning into hibernate or resuming from it. + */ +static void mcp16502_gpio_set_mode(struct mcp16502 *mcp, int mode) +{ + switch (mode) { + case MCP16502_OPMODE_ACTIVE: + gpiod_set_value(mcp->lpm, 0); + break; + case MCP16502_OPMODE_LPM: + case MCP16502_OPMODE_HIB: + gpiod_set_value(mcp->lpm, 1); + break; + default: + pr_err("%s: %d invalid\n", __func__, mode); + } +} + +/* + * mcp16502_get_reg() - get the PMIC's configuration register for opmode + * + * @rdev: the regulator whose register we are searching + * @opmode: the PMIC's operating mode ACTIVE, Low-power, Hibernate + */ +static int mcp16502_get_reg(struct regulator_dev *rdev, int opmode) +{ + int reg = MCP16502_BASE(rdev_get_id(rdev)); + + switch (opmode) { + case MCP16502_OPMODE_ACTIVE: + return reg + MCP16502_OFFSET_MODE_A; + case MCP16502_OPMODE_LPM: + return reg + MCP16502_OFFSET_MODE_LPM; + case MCP16502_OPMODE_HIB: + return reg + MCP16502_OFFSET_MODE_HIB; + default: + return -EINVAL; + } +} + +/* + * mcp16502_get_mode() - return the current operating mode of a regulator + * + * Note: all functions that are not part of entering/exiting standby/suspend + * use the Active mode registers. + * + * Note: this is different from the PMIC's operatig mode, it is the + * MODE bit from the regulator's register. + */ +static unsigned int mcp16502_get_mode(struct regulator_dev *rdev) +{ + unsigned int val; + int ret, reg; + struct mcp16502 *mcp = rdev_get_drvdata(rdev); + + reg = mcp16502_get_reg(rdev, MCP16502_OPMODE_ACTIVE); + if (reg < 0) + return reg; + + ret = regmap_read(mcp->rmap, reg, &val); + if (ret) + return ret; + + switch (val & MCP16502_MODE) { + case MCP16502_MODE_FPWM: + return REGULATOR_MODE_NORMAL; + case MCP16502_MODE_AUTO_PFM: + return REGULATOR_MODE_IDLE; + default: + return REGULATOR_MODE_INVALID; + } +} + +/* + * _mcp16502_set_mode() - helper for set_mode and set_suspend_mode + * + * @rdev: the regulator for which we are setting the mode + * @mode: the regulator's mode (the one from MODE bit) + * @opmode: the PMIC's operating mode: Active/Low-power/Hibernate + */ +static int _mcp16502_set_mode(struct regulator_dev *rdev, unsigned int mode, + unsigned int op_mode) +{ + int val; + int reg; + struct mcp16502 *mcp = rdev_get_drvdata(rdev); + + reg = mcp16502_get_reg(rdev, op_mode); + if (reg < 0) + return reg; + + switch (mode) { + case REGULATOR_MODE_NORMAL: + val = MCP16502_MODE_FPWM; + break; + case REGULATOR_MODE_IDLE: + val = MCP16502_MODE_AUTO_PFM; + break; + default: + return -EINVAL; + } + + reg = regmap_update_bits(mcp->rmap, reg, MCP16502_MODE, val); + return reg; +} + +/* + * mcp16502_set_mode() - regulator_ops set_mode + */ +static int mcp16502_set_mode(struct regulator_dev *rdev, unsigned int mode) +{ + return _mcp16502_set_mode(rdev, mode, MCP16502_OPMODE_ACTIVE); +} + +/* + * mcp16502_get_status() - regulator_ops get_status + */ +static int mcp16502_get_status(struct regulator_dev *rdev) +{ + int ret; + unsigned int val; + struct mcp16502 *mcp = rdev_get_drvdata(rdev); + + ret = regmap_read(mcp->rmap, MCP16502_STAT_BASE(rdev_get_id(rdev)), + &val); + if (ret) + return ret; + + if (val & MCP16502_FLT) + return REGULATOR_STATUS_ERROR; + else if (val & MCP16502_ENS) + return REGULATOR_STATUS_ON; + else if (!(val & MCP16502_ENS)) + return REGULATOR_STATUS_OFF; + + return REGULATOR_STATUS_UNDEFINED; +} + +#ifdef CONFIG_SUSPEND +/* + * mcp16502_suspend_get_target_reg() - get the reg of the target suspend PMIC + * mode + */ +static int mcp16502_suspend_get_target_reg(struct regulator_dev *rdev) +{ + switch (pm_suspend_target_state) { + case PM_SUSPEND_STANDBY: + return mcp16502_get_reg(rdev, MCP16502_OPMODE_LPM); + case PM_SUSPEND_ON: + case PM_SUSPEND_MEM: + return mcp16502_get_reg(rdev, MCP16502_OPMODE_HIB); + default: + dev_err(&rdev->dev, "invalid suspend target: %d\n", + pm_suspend_target_state); + } + + return -EINVAL; +} + +/* + * mcp16502_set_suspend_voltage() - regulator_ops set_suspend_voltage + */ +static int mcp16502_set_suspend_voltage(struct regulator_dev *rdev, int uV) +{ + struct mcp16502 *mcp = rdev_get_drvdata(rdev); + int sel = regulator_map_voltage_linear_range(rdev, uV, uV); + int reg = mcp16502_suspend_get_target_reg(rdev); + + if (sel < 0) + return sel; + + if (reg < 0) + return reg; + + return regmap_update_bits(mcp->rmap, reg, MCP16502_VSEL, sel); +} + +/* + * mcp16502_set_suspend_mode() - regulator_ops set_suspend_mode + */ +static int mcp16502_set_suspend_mode(struct regulator_dev *rdev, + unsigned int mode) +{ + switch (pm_suspend_target_state) { + case PM_SUSPEND_STANDBY: + return _mcp16502_set_mode(rdev, mode, MCP16502_OPMODE_LPM); + case PM_SUSPEND_ON: + case PM_SUSPEND_MEM: + return _mcp16502_set_mode(rdev, mode, MCP16502_OPMODE_HIB); + default: + dev_err(&rdev->dev, "invalid suspend target: %d\n", + pm_suspend_target_state); + } + + return -EINVAL; +} + +/* + * mcp16502_set_suspend_enable() - regulator_ops set_suspend_enable + */ +static int mcp16502_set_suspend_enable(struct regulator_dev *rdev) +{ + struct mcp16502 *mcp = rdev_get_drvdata(rdev); + int reg = mcp16502_suspend_get_target_reg(rdev); + + if (reg < 0) + return reg; + + return regmap_update_bits(mcp->rmap, reg, MCP16502_EN, MCP16502_EN); +} + +/* + * mcp16502_set_suspend_disable() - regulator_ops set_suspend_disable + */ +static int mcp16502_set_suspend_disable(struct regulator_dev *rdev) +{ + struct mcp16502 *mcp = rdev_get_drvdata(rdev); + int reg = mcp16502_suspend_get_target_reg(rdev); + + if (reg < 0) + return reg; + + return regmap_update_bits(mcp->rmap, reg, MCP16502_EN, 0); +} +#endif /* CONFIG_SUSPEND */ + +static const struct regulator_ops mcp16502_buck_ops = { + .list_voltage = regulator_list_voltage_linear_range, + .map_voltage = regulator_map_voltage_linear_range, + .get_voltage_sel = regulator_get_voltage_sel_regmap, + .set_voltage_sel = regulator_set_voltage_sel_regmap, + .enable = regulator_enable_regmap, + .disable = regulator_disable_regmap, + .is_enabled = regulator_is_enabled_regmap, + .get_status = mcp16502_get_status, + + .set_mode = mcp16502_set_mode, + .get_mode = mcp16502_get_mode, + +#ifdef CONFIG_SUSPEND + .set_suspend_voltage = mcp16502_set_suspend_voltage, + .set_suspend_mode = mcp16502_set_suspend_mode, + .set_suspend_enable = mcp16502_set_suspend_enable, + .set_suspend_disable = mcp16502_set_suspend_disable, +#endif /* CONFIG_SUSPEND */ +}; + +/* + * LDOs cannot change operating modes. + */ +static const struct regulator_ops mcp16502_ldo_ops = { + .list_voltage = regulator_list_voltage_linear_range, + .map_voltage = regulator_map_voltage_linear_range, + .get_voltage_sel = regulator_get_voltage_sel_regmap, + .set_voltage_sel = regulator_set_voltage_sel_regmap, + .enable = regulator_enable_regmap, + .disable = regulator_disable_regmap, + .is_enabled = regulator_is_enabled_regmap, + .get_status = mcp16502_get_status, + +#ifdef CONFIG_SUSPEND + .set_suspend_voltage = mcp16502_set_suspend_voltage, + .set_suspend_enable = mcp16502_set_suspend_enable, + .set_suspend_disable = mcp16502_set_suspend_disable, +#endif /* CONFIG_SUSPEND */ +}; + +static const struct of_device_id mcp16502_ids[] = { + { .compatible = "microchip,mcp16502", }, + {} +}; +MODULE_DEVICE_TABLE(of, mcp16502_ids); + +static const struct regulator_linear_range b1l12_ranges[] = { + REGULATOR_LINEAR_RANGE(1200000, VDD_LOW_SEL, VDD_HIGH_SEL, 50000), +}; + +static const struct regulator_linear_range b234_ranges[] = { + REGULATOR_LINEAR_RANGE(600000, VDD_LOW_SEL, VDD_HIGH_SEL, 25000), +}; + +static const struct regulator_desc mcp16502_desc[] = { + /* MCP16502_REGULATOR(_name, _id, ranges, regulator_ops) */ + MCP16502_REGULATOR("VDD_IO", BUCK1, b1l12_ranges, mcp16502_buck_ops), + MCP16502_REGULATOR("VDD_DDR", BUCK2, b234_ranges, mcp16502_buck_ops), + MCP16502_REGULATOR("VDD_CORE", BUCK3, b234_ranges, mcp16502_buck_ops), + MCP16502_REGULATOR("VDD_OTHER", BUCK4, b234_ranges, mcp16502_buck_ops), + MCP16502_REGULATOR("LDO1", LDO1, b1l12_ranges, mcp16502_ldo_ops), + MCP16502_REGULATOR("LDO2", LDO2, b1l12_ranges, mcp16502_ldo_ops) +}; + +static const struct regmap_range mcp16502_ranges[] = { + regmap_reg_range(MCP16502_MIN_REG, MCP16502_MAX_REG) +}; + +static const struct regmap_access_table mcp16502_yes_reg_table = { + .yes_ranges = mcp16502_ranges, + .n_yes_ranges = ARRAY_SIZE(mcp16502_ranges), +}; + +static const struct regmap_config mcp16502_regmap_config = { + .reg_bits = 8, + .val_bits = 8, + .max_register = MCP16502_MAX_REG, + .cache_type = REGCACHE_NONE, + .rd_table = &mcp16502_yes_reg_table, + .wr_table = &mcp16502_yes_reg_table, +}; + +/* + * set_up_regulators() - initialize all regulators + */ +static int setup_regulators(struct mcp16502 *mcp, struct device *dev, + struct regulator_config config) +{ + int i; + + for (i = 0; i < NUM_REGULATORS; i++) { + mcp->rdev[i] = devm_regulator_register(dev, + &mcp16502_desc[i], + &config); + if (IS_ERR(mcp->rdev[i])) { + dev_err(dev, + "failed to register %s regulator %ld\n", + mcp16502_desc[i].name, PTR_ERR(mcp->rdev[i])); + return PTR_ERR(mcp->rdev[i]); + } + } + + return 0; +} + +static int mcp16502_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct regulator_config config = { }; + struct device *dev; + struct mcp16502 *mcp; + int ret = 0; + + dev = &client->dev; + config.dev = dev; + + mcp = devm_kzalloc(dev, sizeof(*mcp), GFP_KERNEL); + if (!mcp) + return -ENOMEM; + + mcp->rmap = devm_regmap_init_i2c(client, &mcp16502_regmap_config); + if (IS_ERR(mcp->rmap)) { + ret = PTR_ERR(mcp->rmap); + dev_err(dev, "regmap init failed: %d\n", ret); + return ret; + } + + i2c_set_clientdata(client, mcp); + config.regmap = mcp->rmap; + config.driver_data = mcp; + + mcp->lpm = devm_gpiod_get(dev, "lpm", GPIOD_OUT_LOW); + if (IS_ERR(mcp->lpm)) { + dev_err(dev, "failed to get lpm pin: %ld\n", PTR_ERR(mcp->lpm)); + return PTR_ERR(mcp->lpm); + } + + ret = setup_regulators(mcp, dev, config); + if (ret != 0) + return ret; + + mcp16502_gpio_set_mode(mcp, MCP16502_OPMODE_ACTIVE); + + return 0; +} + +#ifdef CONFIG_SUSPEND +static int mcp16502_suspend(struct device *dev) +{ + struct i2c_client *client = to_i2c_client(dev); + struct mcp16502 *mcp = i2c_get_clientdata(client); + + mcp16502_gpio_set_mode(mcp, MCP16502_OPMODE_LPM); + + return 0; +} + +static int mcp16502_resume(struct device *dev) +{ + struct i2c_client *client = to_i2c_client(dev); + struct mcp16502 *mcp = i2c_get_clientdata(client); + + mcp16502_gpio_set_mode(mcp, MCP16502_OPMODE_ACTIVE); + + return 0; +} +#else /* !CONFIG_SUSPEND */ +#define mcp16502_suspend NULL +#define mcp16502_resume NULL +#endif /* !CONFIG_SUSPEND */ + +#ifdef CONFIG_PM +static const struct dev_pm_ops mcp16502_pm_ops = { + SET_SYSTEM_SLEEP_PM_OPS(mcp16502_suspend, mcp16502_resume) +}; +#endif +static const struct i2c_device_id mcp16502_i2c_id[] = { + { "mcp16502", 0 }, + { } +}; +MODULE_DEVICE_TABLE(i2c, mcp16502_i2c_id); + +static struct i2c_driver mcp16502_drv = { + .probe = mcp16502_probe, + .driver = { + .name = "mcp16502-regulator", + .of_match_table = of_match_ptr(mcp16502_ids), +#ifdef CONFIG_PM + .pm = &mcp16502_pm_ops, +#endif + }, + .id_table = mcp16502_i2c_id, +}; + +module_i2c_driver(mcp16502_drv); + +MODULE_VERSION("1.0"); +MODULE_LICENSE("GPL v2"); +MODULE_DESCRIPTION("MCP16502 PMIC driver"); +MODULE_AUTHOR("Andrei Stefanescu andrei.stefanescu@microchip.com"); -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [RESEND PATCH v3 3/3] regulator: mcp16502: add regulator driver for MCP16502 2018-12-11 10:09 ` [RESEND PATCH v3 3/3] regulator: mcp16502: add regulator driver for MCP16502 Andrei.Stefanescu @ 2018-12-11 16:47 ` Mark Brown 2018-12-12 8:01 ` Andrei.Stefanescu 0 siblings, 1 reply; 9+ messages in thread From: Mark Brown @ 2018-12-11 16:47 UTC (permalink / raw) To: Andrei.Stefanescu Cc: robh+dt, lgirdwood, mark.rutland, gregkh, Cristian.Birsan, Nicolas.Ferre, Claudiu.Beznea, linux-arm-kernel, linux-kernel, devicetree [-- Attachment #1: Type: text/plain, Size: 1170 bytes --] On Tue, Dec 11, 2018 at 10:09:15AM +0000, Andrei.Stefanescu@microchip.com wrote: > This patch adds a regulator driver for the MCP16502 PMIC. > This drivers supports basic operations through the > regulator interface such as: Overall this looks really good, just a couple of comments: > +++ b/drivers/regulator/mcp16502.c > @@ -0,0 +1,555 @@ > +/* SPDX-License-Identifier: GPL-2.0 > + * > + * MCP16502 PMIC driver SPDX headers need to be C++ comments - please make the entire comment block a C++ one so it looks more intentional. > +#ifdef CONFIG_SUSPEND > +static int mcp16502_suspend(struct device *dev) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct mcp16502 *mcp = i2c_get_clientdata(client); > + > + mcp16502_gpio_set_mode(mcp, MCP16502_OPMODE_LPM); > + > + return 0; > +} This puts the device into low power mode when the suspend function gets called but this might not be safe - devices using the regulator may not be suspended yet so could still need full regulation. Normally a GPIO triggered transition like this would be being done by hardware as part of the process of suspending the SoC. Is there some reason to do this manually? [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RESEND PATCH v3 3/3] regulator: mcp16502: add regulator driver for MCP16502 2018-12-11 16:47 ` Mark Brown @ 2018-12-12 8:01 ` Andrei.Stefanescu 2018-12-12 15:55 ` Mark Brown 0 siblings, 1 reply; 9+ messages in thread From: Andrei.Stefanescu @ 2018-12-12 8:01 UTC (permalink / raw) To: broonie Cc: robh+dt, lgirdwood, mark.rutland, gregkh, Cristian.Birsan, Nicolas.Ferre, Claudiu.Beznea, linux-arm-kernel, linux-kernel, devicetree Hi Mark, Thank you for the review. > SPDX headers need to be C++ comments - please make the entire comment > block a C++ one so it looks more intentional. I sent a new patch (v4) with the modified comment. >> +#ifdef CONFIG_SUSPEND >> +static int mcp16502_suspend(struct device *dev) >> +{ >> + struct i2c_client *client = to_i2c_client(dev); >> + struct mcp16502 *mcp = i2c_get_clientdata(client); >> + >> + mcp16502_gpio_set_mode(mcp, MCP16502_OPMODE_LPM); >> + >> + return 0; >> +} > This puts the device into low power mode when the suspend function gets > called but this might not be safe - devices using the regulator may not > be suspended yet so could still need full regulation. Normally a GPIO > triggered transition like this would be being done by hardware as part > of the process of suspending the SoC. Is there some reason to do this > manually? There is a line from the MPU (SHDN) which goes low only when the MPU turns off. That line is already connected to the PMIC and it differentiates between suspend-to-mem and standby. To switch to low-power, the PMIC must be controlled by the GPIO pin LPM. The suspend sequence is: - LPM pin goes high (PMIC enters Low-Power <-> Linux standby) - SHDN goes low (if target suspend state is mem) and then PMIC enters HIBERNATE Best regards, Andrei ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RESEND PATCH v3 3/3] regulator: mcp16502: add regulator driver for MCP16502 2018-12-12 8:01 ` Andrei.Stefanescu @ 2018-12-12 15:55 ` Mark Brown 2018-12-13 12:19 ` Andrei.Stefanescu 0 siblings, 1 reply; 9+ messages in thread From: Mark Brown @ 2018-12-12 15:55 UTC (permalink / raw) To: Andrei.Stefanescu Cc: robh+dt, lgirdwood, mark.rutland, gregkh, Cristian.Birsan, Nicolas.Ferre, Claudiu.Beznea, linux-arm-kernel, linux-kernel, devicetree [-- Attachment #1: Type: text/plain, Size: 1367 bytes --] On Wed, Dec 12, 2018 at 08:01:07AM +0000, Andrei.Stefanescu@microchip.com wrote: > > This puts the device into low power mode when the suspend function gets > > called but this might not be safe - devices using the regulator may not > > be suspended yet so could still need full regulation. Normally a GPIO > > triggered transition like this would be being done by hardware as part > > of the process of suspending the SoC. Is there some reason to do this > > manually? > There is a line from the MPU (SHDN) which goes low only when the MPU > turns off. That line is already connected to the PMIC and it differentiates > between suspend-to-mem and standby. To switch to low-power, the PMIC must > be controlled by the GPIO pin LPM. > The suspend sequence is: > - LPM pin goes high (PMIC enters Low-Power <-> Linux standby) > - SHDN goes low (if target suspend state is mem) and then PMIC enters > HIBERNATE This feels like it should be being controlled somewhere else, if it's actually causing a change in the PMIC state it seems like it wants to be done as late as possible in suspend to minimize the risks. At the very least suspend_late() for the driver seems appropriate. Could you submit a version with this feature at least split out into a separate patch please so we can apply the rest of the code while this is discussed? [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RESEND PATCH v3 3/3] regulator: mcp16502: add regulator driver for MCP16502 2018-12-12 15:55 ` Mark Brown @ 2018-12-13 12:19 ` Andrei.Stefanescu 2018-12-13 16:21 ` Mark Brown 0 siblings, 1 reply; 9+ messages in thread From: Andrei.Stefanescu @ 2018-12-13 12:19 UTC (permalink / raw) To: broonie Cc: robh+dt, lgirdwood, mark.rutland, gregkh, Cristian.Birsan, Nicolas.Ferre, Claudiu.Beznea, linux-arm-kernel, linux-kernel, devicetree, andr3i.stefanescu Hi Mark, On 12.12.2018 17:55, Mark Brown wrote: > On Wed, Dec 12, 2018 at 08:01:07AM +0000, Andrei.Stefanescu@microchip.com wrote: > >>> This puts the device into low power mode when the suspend function gets >>> called but this might not be safe - devices using the regulator may not >>> be suspended yet so could still need full regulation. Normally a GPIO >>> triggered transition like this would be being done by hardware as part >>> of the process of suspending the SoC. Is there some reason to do this >>> manually? >> There is a line from the MPU (SHDN) which goes low only when the MPU >> turns off. That line is already connected to the PMIC and it differentiates >> between suspend-to-mem and standby. To switch to low-power, the PMIC must >> be controlled by the GPIO pin LPM. >> The suspend sequence is: >> - LPM pin goes high (PMIC enters Low-Power <-> Linux standby) >> - SHDN goes low (if target suspend state is mem) and then PMIC enters >> HIBERNATE > This feels like it should be being controlled somewhere else, if it's > actually causing a change in the PMIC state it seems like it wants to be > done as late as possible in suspend to minimize the risks. At the very > least suspend_late() for the driver seems appropriate. I tried with both suspend_late/resume_early and suspend_noirq/resume_noirq. Everything seems to work ok. Which one do you think is more appropriate? I am guessing that during suspend_late some devices may still need to be turned on and that it would be the safest during suspend_noirq. Note: the GPIO pin used for LPM is a special one (it maintains its voltage during suspend-to-mem). It is part of the SoC and its methods for setting the values do not sleep. > > Could you submit a version with this feature at least split out into a > separate patch please so we can apply the rest of the code while this is > discussed? Sent one patch series and another patch with the current suspend/resume implementation. Best regards, Andrei ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RESEND PATCH v3 3/3] regulator: mcp16502: add regulator driver for MCP16502 2018-12-13 12:19 ` Andrei.Stefanescu @ 2018-12-13 16:21 ` Mark Brown 0 siblings, 0 replies; 9+ messages in thread From: Mark Brown @ 2018-12-13 16:21 UTC (permalink / raw) To: Andrei.Stefanescu Cc: robh+dt, lgirdwood, mark.rutland, gregkh, Cristian.Birsan, Nicolas.Ferre, Claudiu.Beznea, linux-arm-kernel, linux-kernel, devicetree, andr3i.stefanescu [-- Attachment #1: Type: text/plain, Size: 463 bytes --] On Thu, Dec 13, 2018 at 12:19:16PM +0000, Andrei.Stefanescu@microchip.com wrote: > I tried with both suspend_late/resume_early and suspend_noirq/resume_noirq. > Everything seems to work ok. > Which one do you think is more appropriate? I am guessing that during > suspend_late some devices may still need to be turned on and that it would > be the safest during suspend_noirq. I agree, noirq is going to be the safest option. Can you respin with that please? [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-12-13 16:21 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-12-11 10:08 [RESEND PATCH v3 0/3] add support for MCP16502 PMIC Andrei.Stefanescu 2018-12-11 10:09 ` [RESEND PATCH v3 1/3] regulator: dt-bindings: add MCP16502 regulator bindings Andrei.Stefanescu 2018-12-11 10:09 ` [RESEND PATCH v3 2/3] MAINTAINERS: add maintainer for MCP16502 PMIC driver Andrei.Stefanescu 2018-12-11 10:09 ` [RESEND PATCH v3 3/3] regulator: mcp16502: add regulator driver for MCP16502 Andrei.Stefanescu 2018-12-11 16:47 ` Mark Brown 2018-12-12 8:01 ` Andrei.Stefanescu 2018-12-12 15:55 ` Mark Brown 2018-12-13 12:19 ` Andrei.Stefanescu 2018-12-13 16:21 ` Mark Brown
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).