I had cause to look at the MP8859 driver together with the datasheet and
noticed a few small issues and missing features. This series deals with
many of these issues, the device also has support for interrupts which
are not implemented here.
Signed-off-by: Mark Brown <[email protected]>
---
Mark Brown (8):
regulator: mp8859: Specify register accessibility and enable caching
regulator: mp8859: Validate and log device identifier information
regulator: mp8859: Support enable control
regulator: mp8859: Support mode operations
regulator: mp8859: Support active discharge control
regulator: mp8859: Support status and error readback
regulator: mp8859: Report slew rate
regulator: mp8859: Implement set_current_limit()
drivers/regulator/mp8859.c | 252 ++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 251 insertions(+), 1 deletion(-)
---
base-commit: b401b621758e46812da61fa58a67c3fd8d91de0d
change-id: 20240224-regulator-mp8859-eebd7c16c374
Best regards,
--
Mark Brown <[email protected]>
Avoid needless accesses to the hardware by caching register values that
we know, marking status registers as volatile as appropriate.
Signed-off-by: Mark Brown <[email protected]>
---
drivers/regulator/mp8859.c | 38 +++++++++++++++++++++++++++++++++++++-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/drivers/regulator/mp8859.c b/drivers/regulator/mp8859.c
index b820bd6043e5..689b56680d93 100644
--- a/drivers/regulator/mp8859.c
+++ b/drivers/regulator/mp8859.c
@@ -77,11 +77,47 @@ static const struct linear_range mp8859_dcdc_ranges[] = {
REGULATOR_LINEAR_RANGE(0, VOL_MIN_IDX, VOL_MAX_IDX, 10000),
};
+static bool mp8859_readable(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case MP8859_VOUT_L_REG:
+ case MP8859_VOUT_H_REG:
+ case MP8859_VOUT_GO_REG:
+ case MP8859_IOUT_LIM_REG:
+ case MP8859_CTL1_REG:
+ case MP8859_CTL2_REG:
+ case MP8859_STATUS_REG:
+ case MP8859_INTERRUPT_REG:
+ case MP8859_MASK_REG:
+ case MP8859_ID1_REG:
+ case MP8859_MFR_ID_REG:
+ case MP8859_DEV_ID_REG:
+ case MP8859_IC_REV_REG:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static bool mp8859_volatile(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case MP8859_VOUT_GO_REG:
+ case MP8859_STATUS_REG:
+ case MP8859_INTERRUPT_REG:
+ return true;
+ default:
+ return false;
+ }
+}
+
static const struct regmap_config mp8859_regmap = {
.reg_bits = 8,
.val_bits = 8,
.max_register = MP8859_MAX_REG,
- .cache_type = REGCACHE_RBTREE,
+ .cache_type = REGCACHE_MAPLE,
+ .readable_reg = mp8859_readable,
+ .volatile_reg = mp8859_volatile,
};
static const struct regulator_ops mp8859_ops = {
--
2.39.2
Ensure that we are talking to a device which reports the expected ID
register information and log the OTP and revision information for
diagnostic purposes.
Signed-off-by: Mark Brown <[email protected]>
---
drivers/regulator/mp8859.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/drivers/regulator/mp8859.c b/drivers/regulator/mp8859.c
index 689b56680d93..b07bc63a25cb 100644
--- a/drivers/regulator/mp8859.c
+++ b/drivers/regulator/mp8859.c
@@ -147,12 +147,46 @@ static int mp8859_i2c_probe(struct i2c_client *i2c)
struct regulator_config config = {.dev = &i2c->dev};
struct regmap *regmap = devm_regmap_init_i2c(i2c, &mp8859_regmap);
struct regulator_dev *rdev;
+ unsigned int val, rev;
if (IS_ERR(regmap)) {
ret = PTR_ERR(regmap);
dev_err(&i2c->dev, "regmap init failed: %d\n", ret);
return ret;
}
+
+ ret = regmap_read(regmap, MP8859_MFR_ID_REG, &val);
+ if (ret != 0) {
+ dev_err(&i2c->dev, "Failed to read manufacturer ID: %d\n", ret);
+ return ret;
+ }
+ if (val != 0x9) {
+ dev_err(&i2c->dev, "Manufacturer ID %x != 9\n", val);
+ return -EINVAL;
+ }
+
+ ret = regmap_read(regmap, MP8859_DEV_ID_REG, &val);
+ if (ret != 0) {
+ dev_err(&i2c->dev, "Failed to read device ID: %d\n", ret);
+ return ret;
+ }
+ if (val != 0x58) {
+ dev_err(&i2c->dev, "Manufacturer ID %x != 0x58\n", val);
+ return -EINVAL;
+ }
+
+ ret = regmap_read(regmap, MP8859_IC_REV_REG, &rev);
+ if (ret != 0) {
+ dev_err(&i2c->dev, "Failed to read device revision: %d\n", ret);
+ return ret;
+ }
+ ret = regmap_read(regmap, MP8859_ID1_REG, &val);
+ if (ret != 0) {
+ dev_err(&i2c->dev, "Failed to read device ID1: %d\n", ret);
+ return ret;
+ }
+ dev_info(&i2c->dev, "MP8859-%04d revision %d\n", val, rev);
+
rdev = devm_regulator_register(&i2c->dev, &mp8859_regulators[0],
&config);
--
2.39.2
The MP8859 provides a software enable control, support it in the
regulator driver.
Signed-off-by: Mark Brown <[email protected]>
---
drivers/regulator/mp8859.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/regulator/mp8859.c b/drivers/regulator/mp8859.c
index b07bc63a25cb..a443ebe927c5 100644
--- a/drivers/regulator/mp8859.c
+++ b/drivers/regulator/mp8859.c
@@ -35,6 +35,7 @@
#define MP8859_GO_BIT 0x01
+#define MP8859_ENABLE_MASK 0x80
static int mp8859_set_voltage_sel(struct regulator_dev *rdev, unsigned int sel)
{
@@ -124,6 +125,9 @@ static const struct regulator_ops mp8859_ops = {
.set_voltage_sel = mp8859_set_voltage_sel,
.get_voltage_sel = mp8859_get_voltage_sel,
.list_voltage = regulator_list_voltage_linear_range,
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .is_enabled = regulator_is_enabled_regmap,
};
static const struct regulator_desc mp8859_regulators[] = {
@@ -136,6 +140,9 @@ static const struct regulator_desc mp8859_regulators[] = {
.n_voltages = VOL_MAX_IDX + 1,
.linear_ranges = mp8859_dcdc_ranges,
.n_linear_ranges = 1,
+ .enable_reg = MP8859_CTL1_REG,
+ .enable_mask = MP8859_ENABLE_MASK,
+ .enable_val = MP8859_ENABLE_MASK,
.ops = &mp8859_ops,
.owner = THIS_MODULE,
},
--
2.39.2
The MP8859 can actively discharge the output when disabled, add support for
controlling this feature.
Signed-off-by: Mark Brown <[email protected]>
---
drivers/regulator/mp8859.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/regulator/mp8859.c b/drivers/regulator/mp8859.c
index b97c00097148..87ef34875be1 100644
--- a/drivers/regulator/mp8859.c
+++ b/drivers/regulator/mp8859.c
@@ -36,6 +36,7 @@
#define MP8859_GO_BIT 0x01
#define MP8859_ENABLE_MASK 0x80
+#define MP8859_DISCHG_EN_MASK 0x10
#define MP8859_MODE_MASK 0x08
static int mp8859_set_voltage_sel(struct regulator_dev *rdev, unsigned int sel)
@@ -167,6 +168,7 @@ static const struct regulator_ops mp8859_ops = {
.is_enabled = regulator_is_enabled_regmap,
.set_mode = mp8859_set_mode,
.get_mode = mp8859_get_mode,
+ .set_active_discharge = regulator_set_active_discharge_regmap,
};
static const struct regulator_desc mp8859_regulators[] = {
@@ -182,6 +184,9 @@ static const struct regulator_desc mp8859_regulators[] = {
.enable_reg = MP8859_CTL1_REG,
.enable_mask = MP8859_ENABLE_MASK,
.enable_val = MP8859_ENABLE_MASK,
+ .active_discharge_reg = MP8859_CTL1_REG,
+ .active_discharge_on = MP8859_DISCHG_EN_MASK,
+ .active_discharge_mask = MP8859_DISCHG_EN_MASK,
.ops = &mp8859_ops,
.owner = THIS_MODULE,
},
--
2.39.2
The MP8859 can report if it is in regulation and detect over temperature
conditions, report this information via the relevant regulator API
calls.
Signed-off-by: Mark Brown <[email protected]>
---
drivers/regulator/mp8859.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 59 insertions(+)
diff --git a/drivers/regulator/mp8859.c b/drivers/regulator/mp8859.c
index 87ef34875be1..fc1636d69bca 100644
--- a/drivers/regulator/mp8859.c
+++ b/drivers/regulator/mp8859.c
@@ -39,6 +39,11 @@
#define MP8859_DISCHG_EN_MASK 0x10
#define MP8859_MODE_MASK 0x08
+#define MP8859_PG_MASK 0x80
+#define MP8859_OTP_MASK 0x40
+#define MP8859_OTW_MASK 0x20
+#define MP8859_CC_CV_MASK 0x10
+
static int mp8859_set_voltage_sel(struct regulator_dev *rdev, unsigned int sel)
{
int ret;
@@ -112,6 +117,58 @@ static int mp8859_set_mode(struct regulator_dev *rdev, unsigned int mode)
MP8859_MODE_MASK, val);
}
+static int mp8859_get_status(struct regulator_dev *rdev)
+{
+ unsigned int val;
+ int ret;
+
+ /* Output status is only meaingful when enabled */
+ ret = regmap_read(rdev->regmap, MP8859_CTL1_REG, &val);
+ if (ret != 0)
+ return ret;
+ if (!(val & MP8859_ENABLE_MASK))
+ return REGULATOR_STATUS_UNDEFINED;
+
+ ret = regmap_read(rdev->regmap, MP8859_STATUS_REG, &val);
+ if (ret != 0)
+ return ret;
+
+ if (val & MP8859_PG_MASK)
+ return REGULATOR_STATUS_ON;
+ else
+ return REGULATOR_STATUS_ERROR;
+}
+
+static int mp8859_get_error_flags(struct regulator_dev *rdev,
+ unsigned int *flags)
+{
+ unsigned int status, enabled;
+ int ret;
+
+ *flags = 0;
+
+ /* Output status is only meaingful when enabled */
+ ret = regmap_read(rdev->regmap, MP8859_CTL1_REG, &enabled);
+ if (ret != 0)
+ return ret;
+ enabled &= MP8859_ENABLE_MASK;
+
+ ret = regmap_read(rdev->regmap, MP8859_STATUS_REG, &status);
+ if (ret != 0)
+ return ret;
+
+ if (enabled && !(status & MP8859_PG_MASK))
+ status |= REGULATOR_ERROR_FAIL;
+ if (status & MP8859_OTP_MASK)
+ status |= REGULATOR_ERROR_OVER_TEMP;
+ if (status & MP8859_OTW_MASK)
+ status |= REGULATOR_ERROR_OVER_TEMP_WARN;
+ if (status & MP8859_CC_CV_MASK)
+ status |= REGULATOR_ERROR_OVER_CURRENT;
+
+ return 0;
+}
+
static const struct linear_range mp8859_dcdc_ranges[] = {
REGULATOR_LINEAR_RANGE(0, VOL_MIN_IDX, VOL_MAX_IDX, 10000),
};
@@ -169,6 +226,8 @@ static const struct regulator_ops mp8859_ops = {
.set_mode = mp8859_set_mode,
.get_mode = mp8859_get_mode,
.set_active_discharge = regulator_set_active_discharge_regmap,
+ .get_status = mp8859_get_status,
+ .get_error_flags = mp8859_get_error_flags,
};
static const struct regulator_desc mp8859_regulators[] = {
--
2.39.2
The MP8859 provides mode control, implement the relevant regulator API
operations.
Signed-off-by: Mark Brown <[email protected]>
---
drivers/regulator/mp8859.c | 39 +++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/drivers/regulator/mp8859.c b/drivers/regulator/mp8859.c
index a443ebe927c5..b97c00097148 100644
--- a/drivers/regulator/mp8859.c
+++ b/drivers/regulator/mp8859.c
@@ -36,6 +36,7 @@
#define MP8859_GO_BIT 0x01
#define MP8859_ENABLE_MASK 0x80
+#define MP8859_MODE_MASK 0x08
static int mp8859_set_voltage_sel(struct regulator_dev *rdev, unsigned int sel)
{
@@ -74,6 +75,42 @@ static int mp8859_get_voltage_sel(struct regulator_dev *rdev)
return val;
}
+static unsigned int mp8859_get_mode(struct regulator_dev *rdev)
+{
+ unsigned int val;
+ int ret;
+
+ ret = regmap_read(rdev->regmap, MP8859_CTL1_REG, &val);
+ if (ret != 0) {
+ dev_err(&rdev->dev, "Failed to read mode: %d\n", ret);
+ return 0;
+ }
+
+ if (val & MP8859_MODE_MASK)
+ return REGULATOR_MODE_FAST;
+ else
+ return REGULATOR_MODE_NORMAL;
+}
+
+static int mp8859_set_mode(struct regulator_dev *rdev, unsigned int mode)
+{
+ unsigned int val;
+
+ switch (mode) {
+ case REGULATOR_MODE_FAST:
+ val = MP8859_MODE_MASK;
+ break;
+ case REGULATOR_MODE_NORMAL:
+ val = 0;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return regmap_update_bits(rdev->regmap, MP8859_CTL1_REG,
+ MP8859_MODE_MASK, val);
+}
+
static const struct linear_range mp8859_dcdc_ranges[] = {
REGULATOR_LINEAR_RANGE(0, VOL_MIN_IDX, VOL_MAX_IDX, 10000),
};
@@ -128,6 +165,8 @@ static const struct regulator_ops mp8859_ops = {
.enable = regulator_enable_regmap,
.disable = regulator_disable_regmap,
.is_enabled = regulator_is_enabled_regmap,
+ .set_mode = mp8859_set_mode,
+ .get_mode = mp8859_get_mode,
};
static const struct regulator_desc mp8859_regulators[] = {
--
2.39.2
The MP8859 implements voltage changes at the rate of 1mV/us, tell the core
about this so that it can provide appropriate delays on voltage changes.
Signed-off-by: Mark Brown <[email protected]>
---
drivers/regulator/mp8859.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/regulator/mp8859.c b/drivers/regulator/mp8859.c
index fc1636d69bca..3e849e1a172f 100644
--- a/drivers/regulator/mp8859.c
+++ b/drivers/regulator/mp8859.c
@@ -81,6 +81,20 @@ static int mp8859_get_voltage_sel(struct regulator_dev *rdev)
return val;
}
+static int mp8859_set_voltage_time_sel(struct regulator_dev *rdev,
+ unsigned int from, unsigned int to)
+{
+ int change;
+
+ /* The voltage ramps at 1mV/uS, selectors are 10mV */
+ if (from > to)
+ change = from - to;
+ else
+ change = to - from;
+
+ return change * 10 * 1000;
+}
+
static unsigned int mp8859_get_mode(struct regulator_dev *rdev)
{
unsigned int val;
@@ -220,6 +234,7 @@ static const struct regulator_ops mp8859_ops = {
.set_voltage_sel = mp8859_set_voltage_sel,
.get_voltage_sel = mp8859_get_voltage_sel,
.list_voltage = regulator_list_voltage_linear_range,
+ .set_voltage_time_sel = mp8859_set_voltage_time_sel,
.enable = regulator_enable_regmap,
.disable = regulator_disable_regmap,
.is_enabled = regulator_is_enabled_regmap,
--
2.39.2
The mp8859 implements support for current limiting, provide support for
configuring this via the driver. The datasheet recommends that if the
device has hit the current limit then any changes should be implemented
via a ramp so we do so in the driver.
Signed-off-by: Mark Brown <[email protected]>
---
drivers/regulator/mp8859.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 55 insertions(+)
diff --git a/drivers/regulator/mp8859.c b/drivers/regulator/mp8859.c
index 3e849e1a172f..ab105ffd6a2e 100644
--- a/drivers/regulator/mp8859.c
+++ b/drivers/regulator/mp8859.c
@@ -35,6 +35,8 @@
#define MP8859_GO_BIT 0x01
+#define MP8859_IOUT_LIM_MASK 0x7f
+
#define MP8859_ENABLE_MASK 0x80
#define MP8859_DISCHG_EN_MASK 0x10
#define MP8859_MODE_MASK 0x08
@@ -131,6 +133,58 @@ static int mp8859_set_mode(struct regulator_dev *rdev, unsigned int mode)
MP8859_MODE_MASK, val);
}
+static int mp8859_set_current_limit(struct regulator_dev *rdev,
+ int min_uA, int max_uA)
+{
+ unsigned int cur_val, new_val;
+ int ret, i;
+
+ /* Steps of 50mA */
+ new_val = max_uA / 50000;
+ if (new_val > MP8859_IOUT_LIM_MASK)
+ return -EINVAL;
+ if (new_val == 0)
+ return -EINVAL;
+
+ /*
+ * If the regulator is limiting then ramp gradually as per
+ * datasheet, otherwise just set the value directly.
+ */
+ ret = regmap_read(rdev->regmap, MP8859_STATUS_REG, &cur_val);
+ if (ret != 0)
+ return ret;
+ if (!(cur_val & MP8859_CC_CV_MASK)) {
+ return regmap_update_bits(rdev->regmap, MP8859_IOUT_LIM_REG,
+ MP8859_IOUT_LIM_MASK, new_val);
+ }
+
+ ret = regmap_read(rdev->regmap, MP8859_IOUT_LIM_REG, &cur_val);
+ if (ret != 0)
+ return ret;
+
+ if (cur_val >= new_val) {
+ for (i = cur_val; i >= new_val; i--) {
+ ret = regmap_update_bits(rdev->regmap,
+ MP8859_IOUT_LIM_REG,
+ MP8859_IOUT_LIM_MASK,
+ cur_val - i);
+ if (ret != 0)
+ return ret;
+ }
+ } else {
+ for (i = cur_val; i <= new_val; i++) {
+ ret = regmap_update_bits(rdev->regmap,
+ MP8859_IOUT_LIM_REG,
+ MP8859_IOUT_LIM_MASK,
+ cur_val + i);
+ if (ret != 0)
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
static int mp8859_get_status(struct regulator_dev *rdev)
{
unsigned int val;
@@ -241,6 +295,7 @@ static const struct regulator_ops mp8859_ops = {
.set_mode = mp8859_set_mode,
.get_mode = mp8859_get_mode,
.set_active_discharge = regulator_set_active_discharge_regmap,
+ .set_current_limit = mp8859_set_current_limit,
.get_status = mp8859_get_status,
.get_error_flags = mp8859_get_error_flags,
};
--
2.39.2
Hi Mark,
Am 25.02.24 um 15:59 schrieb Mark Brown:
> I had cause to look at the MP8859 driver together with the datasheet and
> noticed a few small issues and missing features. This series deals with
> many of these issues, the device also has support for interrupts which
> are not implemented here.
>
> Signed-off-by: Mark Brown <[email protected]>
> ---
> Mark Brown (8):
> regulator: mp8859: Specify register accessibility and enable caching
> regulator: mp8859: Validate and log device identifier information
> regulator: mp8859: Support enable control
> regulator: mp8859: Support mode operations
> regulator: mp8859: Support active discharge control
> regulator: mp8859: Support status and error readback
> regulator: mp8859: Report slew rate
> regulator: mp8859: Implement set_current_limit()
>
> drivers/regulator/mp8859.c | 252 ++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 251 insertions(+), 1 deletion(-)
> ---
> base-commit: b401b621758e46812da61fa58a67c3fd8d91de0d
> change-id: 20240224-regulator-mp8859-eebd7c16c374
>
> Best regards,
Tested on rk3399-roc-pc.
Tested-by: Markus Reichl <[email protected]>
Gruß,
--
Markus Reichl
On Sun, 25 Feb 2024 14:59:26 +0000, Mark Brown wrote:
> I had cause to look at the MP8859 driver together with the datasheet and
> noticed a few small issues and missing features. This series deals with
> many of these issues, the device also has support for interrupts which
> are not implemented here.
>
>
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
Thanks!
[1/8] regulator: mp8859: Specify register accessibility and enable caching
commit: b65e9149bdb76e9b09f6fb76fea1f10bde256619
[2/8] regulator: mp8859: Validate and log device identifier information
commit: 6c848d772eee0f03b72542f35e1a66469030390f
[3/8] regulator: mp8859: Support enable control
commit: b79d93d99e084bf1abafba2b7aabff6a06defcd0
[4/8] regulator: mp8859: Support mode operations
commit: 673d06a858864c7dca9dd5c36a78f5f7fda793af
[5/8] regulator: mp8859: Support active discharge control
commit: d7217c91bbde48ee60d3ce67cda6557f56c6b639
[6/8] regulator: mp8859: Support status and error readback
commit: 4317ecadbeeab5464a8c34b27b73e2d2f81ef718
[7/8] regulator: mp8859: Report slew rate
commit: 6df0921e9013622091c283aa2a5be8c5d1ca3642
[8/8] regulator: mp8859: Implement set_current_limit()
commit: c8e794cfb05344af7b9ae920b5548a25a7e91fe9
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark