LinuxLists.cc - [PATCH v3 0/2] Add MAX77857/59/MAX77831 Regulator Support

[permalink] [raw]

Subject: [PATCH v3 1/2] dt-bindings: regulator: max77857: Add ADI MAX77857/59/MAX77831 Regulator

Add ADI MAX77857/59 and MAX77831 Regulator device tree document.

Signed-off-by: Okan Sahin <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
.../bindings/regulator/adi,max77857.yaml | 86 +++++++++++++++++++
1 file changed, 86 insertions(+)
create mode 100644 Documentation/devicetree/bindings/regulator/adi,max77857.yaml

diff --git a/Documentation/devicetree/bindings/regulator/adi,max77857.yaml b/Documentation/devicetree/bindings/regulator/adi,max77857.yaml
new file mode 100644
index 000000000000..d1fa74aca721
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/adi,max77857.yaml
@@ -0,0 +1,86 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2022 Analog Devices Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/adi,max77857.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices MAX77857 Buck-Boost Converter
+
+maintainers:
+ - Ibrahim Tilki <[email protected]>
+ - Okan Sahin <[email protected]>
+
+description: Analog Devices MAX77857 Buck-Boost Converter
+
+properties:
+ compatible:
+ enum:
+ - adi,max77831
+ - adi,max77857
+ - adi,max77859
+ - adi,max77859a
+
+ reg:
+ description: I2C address of the device
+ items:
+ - enum: [0x66, 0x67, 0x6E, 0x6F]
+
+ interrupts:
+ maxItems: 1
+
+ adi,switch-frequency-hz:
+ description: Switching frequency of the Buck-Boost converter in Hz.
+ items:
+ - enum: [1200000, 1500000, 1800000, 2100000]
+
+ adi,rtop-ohms:
+ description: Top feedback resistor value in ohms for external feedback.
+ minimum: 150000
+ maximum: 330000
+
+ adi,rbot-ohms:
+ description: Bottom feedback resistor value in ohms for external feedback.
+
+dependencies:
+ adi,rtop-ohms: [ 'adi,rbot-ohms' ]
+ adi,rbot-ohms: [ 'adi,rtop-ohms' ]
+
+required:
+ - compatible
+ - reg
+
+allOf:
+ - $ref: regulator.yaml#
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - adi,max77831
+
+ then:
+ properties:
+ adi,switch-frequency-hz:
+ items:
+ enum: [1200000, 1500000, 1800000]
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ regulator@66 {
+ reg = <0x66>;
+ compatible = "adi,max77857";
+ interrupt-parent = <&gpio>;
+ interrupts = <26 IRQ_TYPE_EDGE_FALLING>;
+
+ adi,rtop-ohms = <312000>;
+ adi,rbot-ohms = <12000>;
+ };
+ };
--
2.30.2

2023-07-17 05:41:02

[permalink] [raw]

Subject: [PATCH v3 2/2] regulator: max77857: Add ADI MAX77857/59/MAX77831 Regulator Support

Regulator driver for MAX77857/59 and MAX77831.
The MAX77857 is a high-efficiency, high-performance
buck-boost converter targeted for systems requiring
a wide input voltage range (2.5V to 16V).

The MAX77859 is high-Efficiency Buck-Boost Converter
for USB-PD/PPS Applications. It has wide input range
(2.5V to 22V)

The MAX77831 is a high-efficiency, high-performance
buck-boost converter targeted for systems requiring
wide input voltage range (2.5V to 16V).

Signed-off-by: Okan Sahin <[email protected]>
---
drivers/regulator/Kconfig | 10 +
drivers/regulator/Makefile | 1 +
drivers/regulator/max77857-regulator.c | 459 +++++++++++++++++++++++++
3 files changed, 470 insertions(+)
create mode 100644 drivers/regulator/max77857-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index e5f3613c15fa..09eaa1cd90de 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -573,6 +573,16 @@ config REGULATOR_MAX77650
Semiconductor. This device has a SIMO with three independent
power rails and an LDO.

+config REGULATOR_MAX77857
+ tristate "ADI MAX77857/MAX77831 regulator support"
+ depends on I2C
+ select REGMAP_I2C
+ help
+ This driver controls a ADI MAX77857 and MAX77831 regulators.
+ via I2C bus. MAX77857 and MAX77831 are high efficiency buck-boost
+ converters with input voltage range (2.5V to 16V). Say Y here to
+ enable the regulator driver
+
config REGULATOR_MAX8649
tristate "Maxim 8649 voltage regulator"
depends on I2C
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 58dfe0147cd4..e7230846b680 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -85,6 +85,7 @@ obj-$(CONFIG_REGULATOR_MAX77686) += max77686-regulator.o
obj-$(CONFIG_REGULATOR_MAX77693) += max77693-regulator.o
obj-$(CONFIG_REGULATOR_MAX77802) += max77802-regulator.o
obj-$(CONFIG_REGULATOR_MAX77826) += max77826-regulator.o
+obj-$(CONFIG_REGULATOR_MAX77857) += max77857-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
diff --git a/drivers/regulator/max77857-regulator.c b/drivers/regulator/max77857-regulator.c
new file mode 100644
index 000000000000..c5482ffd606e
--- /dev/null
+++ b/drivers/regulator/max77857-regulator.c
@@ -0,0 +1,459 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2023 Analog Devices, Inc.
+ * ADI Regulator driver for the MAX77857
+ * MAX77859 and MAX77831.
+ */
+#include <linux/bitfield.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/util_macros.h>
+
+#define MAX77857_REG_INT_SRC 0x10
+#define MAX77857_REG_INT_MASK 0x11
+#define MAX77857_REG_CONT1 0x12
+#define MAX77857_REG_CONT2 0x13
+#define MAX77857_REG_CONT3 0x14
+
+#define MAX77857_INT_SRC_OCP BIT(0)
+#define MAX77857_INT_SRC_THS BIT(1)
+#define MAX77857_INT_SRC_HARDSHORT BIT(2)
+#define MAX77857_INT_SRC_OVP BIT(3)
+#define MAX77857_INT_SRC_POK BIT(4)
+
+#define MAX77857_ILIM_MASK GENMASK(2, 0)
+#define MAX77857_CONT1_FREQ GENMASK(4, 3)
+#define MAX77857_CONT3_FPWM BIT(5)
+
+#define MAX77859_REG_INT_SRC 0x11
+#define MAX77859_REG_CONT1 0x13
+#define MAX77859_REG_CONT2 0x14
+#define MAX77859_REG_CONT3 0x15
+#define MAX77859_REG_CONT5 0x17
+#define MAX77859_CONT2_FPWM BIT(2)
+#define MAX77859_CONT2_INTB BIT(3)
+#define MAX77859_CONT3_DVS_START BIT(2)
+#define MAX77859_VOLTAGE_SEL_MASK GENMASK(9, 0)
+
+#define MAX77859_CURRENT_MIN 1000000
+#define MAX77859_CURRENT_MAX 5000000
+#define MAX77859_CURRENT_STEP 50000
+
+enum max77857_id {
+ ID_MAX77831 = 1,
+ ID_MAX77857,
+ ID_MAX77859,
+ ID_MAX77859A,
+};
+
+static bool max77857_volatile_reg(struct device *dev, unsigned int reg)
+{
+ enum max77857_id id = (enum max77857_id)dev_get_drvdata(dev);
+
+ switch (id) {
+ case ID_MAX77831:
+ case ID_MAX77857:
+ return reg == MAX77857_REG_INT_SRC;
+ case ID_MAX77859:
+ case ID_MAX77859A:
+ return reg == MAX77859_REG_INT_SRC;
+ default:
+ return true;
+ }
+}
+
+struct regmap_config max77857_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .cache_type = REGCACHE_MAPLE,
+ .volatile_reg = max77857_volatile_reg,
+};
+
+static int max77857_get_status(struct regulator_dev *rdev)
+{
+ unsigned int val;
+ int ret;
+
+ ret = regmap_read(rdev->regmap, MAX77857_REG_INT_SRC, &val);
+ if (ret)
+ return ret;
+
+ if (FIELD_GET(MAX77857_INT_SRC_POK, val))
+ return REGULATOR_STATUS_ON;
+
+ return REGULATOR_STATUS_ERROR;
+}
+
+static unsigned int max77857_get_mode(struct regulator_dev *rdev)
+{
+ enum max77857_id id = (enum max77857_id)rdev_get_drvdata(rdev);
+ unsigned int regval;
+ int ret;
+
+ switch (id) {
+ case ID_MAX77831:
+ case ID_MAX77857:
+ ret = regmap_read(rdev->regmap, MAX77857_REG_CONT3, &regval);
+ if (ret)
+ return ret;
+
+ if (FIELD_GET(MAX77857_CONT3_FPWM, regval))
+ return REGULATOR_MODE_FAST;
+
+ break;
+ case ID_MAX77859:
+ case ID_MAX77859A:
+ ret = regmap_read(rdev->regmap, MAX77859_REG_CONT2, &regval);
+ if (ret)
+ return ret;
+
+ if (FIELD_GET(MAX77859_CONT2_FPWM, regval))
+ return REGULATOR_MODE_FAST;
+
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return REGULATOR_MODE_NORMAL;
+}
+
+static int max77857_set_mode(struct regulator_dev *rdev, unsigned int mode)
+{
+ enum max77857_id id = (enum max77857_id)rdev_get_drvdata(rdev);
+ unsigned int reg, val;
+
+ switch (id) {
+ case ID_MAX77831:
+ case ID_MAX77857:
+ reg = MAX77857_REG_CONT3;
+ val = MAX77857_CONT3_FPWM;
+ break;
+ case ID_MAX77859:
+ case ID_MAX77859A:
+ reg = MAX77859_REG_CONT2;
+ val = MAX77859_CONT2_FPWM;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ switch (mode) {
+ case REGULATOR_MODE_FAST:
+ return regmap_set_bits(rdev->regmap, reg, val);
+ case REGULATOR_MODE_NORMAL:
+ return regmap_clear_bits(rdev->regmap, reg, val);
+ default:
+ return -EINVAL;
+ }
+}
+
+static int max77857_get_error_flags(struct regulator_dev *rdev,
+ unsigned int *flags)
+{
+ unsigned int val;
+ int ret;
+
+ ret = regmap_read(rdev->regmap, MAX77857_REG_INT_SRC, &val);
+ if (ret)
+ return ret;
+
+ *flags = 0;
+
+ if (FIELD_GET(MAX77857_INT_SRC_OVP, val))
+ *flags |= REGULATOR_ERROR_OVER_VOLTAGE_WARN;
+
+ if (FIELD_GET(MAX77857_INT_SRC_OCP, val) ||
+ FIELD_GET(MAX77857_INT_SRC_HARDSHORT, val))
+ *flags |= REGULATOR_ERROR_OVER_CURRENT;
+
+ if (FIELD_GET(MAX77857_INT_SRC_THS, val))
+ *flags |= REGULATOR_ERROR_OVER_TEMP;
+
+ if (!FIELD_GET(MAX77857_INT_SRC_POK, val))
+ *flags |= REGULATOR_ERROR_FAIL;
+
+ return 0;
+}
+
+static struct linear_range max77859_lin_ranges[] = {
+ REGULATOR_LINEAR_RANGE(3200000, 0x0A0, 0x320, 20000)
+};
+
+static const unsigned int max77859_ramp_table[4] = {
+ 1000, 500, 250, 125
+};
+
+static int max77859_set_voltage_sel(struct regulator_dev *rdev,
+ unsigned int sel)
+{
+ __be16 reg;
+ int ret;
+
+ reg = cpu_to_be16(sel);
+
+ ret = regmap_bulk_write(rdev->regmap, MAX77859_REG_CONT3, &reg, 2);
+ if (ret)
+ return ret;
+
+ /* actually apply new voltage */
+ return regmap_set_bits(rdev->regmap, MAX77859_REG_CONT3,
+ MAX77859_CONT3_DVS_START);
+}
+
+int max77859_get_voltage_sel(struct regulator_dev *rdev)
+{
+ __be16 reg;
+ int ret;
+
+ ret = regmap_bulk_read(rdev->regmap, MAX77859_REG_CONT3, &reg, 2);
+ if (ret)
+ return ret;
+
+ return FIELD_GET(MAX77859_VOLTAGE_SEL_MASK, __be16_to_cpu(reg));
+}
+
+int max77859_set_current_limit(struct regulator_dev *rdev, int min_uA, int max_uA)
+{
+ u32 selector;
+
+ if (max_uA < MAX77859_CURRENT_MIN)
+ return -EINVAL;
+
+ selector = 0x12 + (max_uA - MAX77859_CURRENT_MIN) / MAX77859_CURRENT_STEP;
+
+ selector = clamp_val(selector, 0x00, 0x7F);
+
+ return regmap_write(rdev->regmap, MAX77859_REG_CONT5, selector);
+}
+
+int max77859_get_current_limit(struct regulator_dev *rdev)
+{
+ u32 selector;
+ int ret;
+
+ ret = regmap_read(rdev->regmap, MAX77859_REG_CONT5, &selector);
+ if (ret)
+ return ret;
+
+ if (selector <= 0x12)
+ return MAX77859_CURRENT_MIN;
+
+ if (selector >= 0x64)
+ return MAX77859_CURRENT_MAX;
+
+ return MAX77859_CURRENT_MIN + (selector - 0x12) * MAX77859_CURRENT_STEP;
+}
+
+static const struct regulator_ops max77859_regulator_ops = {
+ .list_voltage = regulator_list_voltage_linear_range,
+ .set_voltage_sel = max77859_set_voltage_sel,
+ .get_voltage_sel = max77859_get_voltage_sel,
+ .set_ramp_delay = regulator_set_ramp_delay_regmap,
+ .get_status = max77857_get_status,
+ .set_mode = max77857_set_mode,
+ .get_mode = max77857_get_mode,
+ .get_error_flags = max77857_get_error_flags,
+};
+
+static const struct regulator_ops max77859a_regulator_ops = {
+ .list_voltage = regulator_list_voltage_linear_range,
+ .set_voltage_sel = max77859_set_voltage_sel,
+ .get_voltage_sel = max77859_get_voltage_sel,
+ .set_current_limit = max77859_set_current_limit,
+ .get_current_limit = max77859_get_current_limit,
+ .set_ramp_delay = regulator_set_ramp_delay_regmap,
+ .get_status = max77857_get_status,
+ .set_mode = max77857_set_mode,
+ .get_mode = max77857_get_mode,
+ .get_error_flags = max77857_get_error_flags,
+};
+
+static const struct regulator_ops max77857_regulator_ops = {
+ .list_voltage = regulator_list_voltage_linear_range,
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+ .set_ramp_delay = regulator_set_ramp_delay_regmap,
+ .get_status = max77857_get_status,
+ .set_mode = max77857_set_mode,
+ .get_mode = max77857_get_mode,
+ .get_error_flags = max77857_get_error_flags,
+};
+
+static struct linear_range max77857_lin_ranges[] = {
+ REGULATOR_LINEAR_RANGE(4485000, 0x3D, 0xCC, 73500)
+};
+
+static const unsigned int max77857_switch_freq[] = {
+ 1200000, 1500000, 1800000, 2100000
+};
+
+static const unsigned int max77857_ramp_table[2][4] = {
+ { 1333, 667, 333, 227 }, /* when switch freq is 1.8MHz or 2.1MHz */
+ { 1166, 667, 333, 167 }, /* when switch freq is 1.2MHz or 1.5MHz */
+};
+
+static struct regulator_desc max77857_regulator_desc = {
+ .ops = &max77857_regulator_ops,
+ .name = "max77857",
+ .linear_ranges = max77857_lin_ranges,
+ .n_linear_ranges = ARRAY_SIZE(max77857_lin_ranges),
+ .vsel_mask = 0xFF,
+ .vsel_reg = MAX77857_REG_CONT2,
+ .ramp_delay_table = max77857_ramp_table[0],
+ .n_ramp_values = ARRAY_SIZE(max77857_ramp_table[0]),
+ .ramp_reg = MAX77857_REG_CONT3,
+ .ramp_mask = GENMASK(1, 0),
+ .ramp_delay = max77857_ramp_table[0][0],
+ .owner = THIS_MODULE,
+};
+
+static void max77857_calc_range(struct device *dev, enum max77857_id id)
+{
+ struct linear_range *range;
+ unsigned long vref_step;
+ u32 rtop = 0;
+ u32 rbot = 0;
+
+ device_property_read_u32(dev, "adi,rtop-ohms", &rtop);
+ device_property_read_u32(dev, "adi,rbot-ohms", &rbot);
+
+ if (!rbot || !rtop)
+ return;
+
+ switch (id) {
+ case ID_MAX77831:
+ case ID_MAX77857:
+ range = max77857_lin_ranges;
+ vref_step = 4900UL;
+ break;
+ case ID_MAX77859:
+ case ID_MAX77859A:
+ range = max77859_lin_ranges;
+ vref_step = 1250UL;
+ break;
+ }
+
+ range->step = DIV_ROUND_CLOSEST(vref_step * (rbot + rtop), rbot);
+ range->min = range->step * range->min_sel;
+}
+
+static int max77857_probe(struct i2c_client *client)
+{
+ const struct i2c_device_id *i2c_id;
+ struct device *dev = &client->dev;
+ struct regulator_config cfg = { };
+ struct regulator_dev *rdev;
+ struct regmap *regmap;
+ enum max77857_id id;
+ u32 switch_freq = 0;
+ int ret;
+
+ i2c_id = i2c_client_get_device_id(client);
+ if (!i2c_id)
+ return -EINVAL;
+
+ id = i2c_id->driver_data;
+
+ dev_set_drvdata(dev, (void *)id);
+
+ if (id == ID_MAX77859 || id == ID_MAX77859A) {
+ max77857_regulator_desc.ops = &max77859_regulator_ops;
+ max77857_regulator_desc.linear_ranges = max77859_lin_ranges;
+ max77857_regulator_desc.ramp_delay_table = max77859_ramp_table;
+ max77857_regulator_desc.ramp_delay = max77859_ramp_table[0];
+ }
+
+ if (id == ID_MAX77859A)
+ max77857_regulator_desc.ops = &max77859a_regulator_ops;
+
+ max77857_calc_range(dev, id);
+
+ regmap = devm_regmap_init_i2c(client, &max77857_regmap_config);
+ if (IS_ERR(regmap))
+ return dev_err_probe(dev, PTR_ERR(regmap),
+ "cannot initialize regmap\n");
+
+ device_property_read_u32(dev, "adi,switch-frequency-hz", &switch_freq);
+ if (switch_freq) {
+ switch_freq = find_closest(switch_freq, max77857_switch_freq,
+ ARRAY_SIZE(max77857_switch_freq));
+
+ if (id == ID_MAX77831 && switch_freq == 3)
+ switch_freq = 2;
+
+ switch (id) {
+ case ID_MAX77831:
+ case ID_MAX77857:
+ ret = regmap_update_bits(regmap, MAX77857_REG_CONT1,
+ MAX77857_CONT1_FREQ, switch_freq);
+
+ if (switch_freq >= 2)
+ break;
+
+ max77857_regulator_desc.ramp_delay_table = max77857_ramp_table[1];
+ max77857_regulator_desc.ramp_delay = max77857_ramp_table[1][0];
+ break;
+ case ID_MAX77859:
+ case ID_MAX77859A:
+ ret = regmap_update_bits(regmap, MAX77859_REG_CONT1,
+ MAX77857_CONT1_FREQ, switch_freq);
+ break;
+ }
+ if (ret)
+ return ret;
+ }
+
+ cfg.dev = dev;
+ cfg.driver_data = (void *)id;
+ cfg.regmap = regmap;
+ cfg.init_data = of_get_regulator_init_data(dev, dev->of_node,
+ &max77857_regulator_desc);
+ if (!cfg.init_data)
+ return -ENOMEM;
+
+ rdev = devm_regulator_register(dev, &max77857_regulator_desc, &cfg);
+ if (IS_ERR(rdev))
+ return dev_err_probe(dev, PTR_ERR(rdev),
+ "cannot register regulator\n");
+
+ return 0;
+}
+
+const struct i2c_device_id max77857_id[] = {
+ { "max77831", ID_MAX77831 },
+ { "max77857", ID_MAX77857 },
+ { "max77859", ID_MAX77859 },
+ { "max77859a", ID_MAX77859A },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, max77857_id);
+
+static const struct of_device_id max77857_of_id[] = {
+ { .compatible = "adi,max77831", .data = (void *)ID_MAX77831 },
+ { .compatible = "adi,max77857", .data = (void *)ID_MAX77857 },
+ { .compatible = "adi,max77859", .data = (void *)ID_MAX77859 },
+ { .compatible = "adi,max77859a", .data = (void *)ID_MAX77859A },
+ { }
+};
+MODULE_DEVICE_TABLE(of, max77857_of_id);
+
+struct i2c_driver max77857_driver = {
+ .driver = {
+ .name = "max77857",
+ .of_match_table = max77857_of_id,
+ },
+ .id_table = max77857_id,
+ .probe_new = max77857_probe,
+};
+module_i2c_driver(max77857_driver);
+
+MODULE_DESCRIPTION("Analog Devices MAX77857 Buck-Boost Converter Driver");
+MODULE_AUTHOR("Ibrahim Tilki <[email protected]>");
+MODULE_AUTHOR("Okan Sahin <[email protected]>");
+MODULE_LICENSE("GPL");
--
2.30.2

2023-07-17 14:16:24

[permalink] [raw]

Subject: Re: [PATCH v3 0/2] Add MAX77857/59/MAX77831 Regulator Support

On Mon, 17 Jul 2023 08:07:33 +0300, Okan Sahin wrote:
> High efficiency buck-boost regulator driver and bindings for
> MAX77857/59/MAX77831. The patches are required to be applied
> in sequence.
>
> Changes in v3:
> * Patch 1: "dt-bindings: regulator: max77857: Add ADI MAX77857/59/MAX77831
> Regulator"
> * Add second maintainer
> * Patch 2: "regulator: max77857: Add ADI MAX77857/59/MAX77831 Regulator Support"
> * Change regmap cache_type
>
> [...]

Applied to

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[1/2] dt-bindings: regulator: max77857: Add ADI MAX77857/59/MAX77831 Regulator
commit: 6d5373e98b37721987c16fd1c0f9500ecbb69f20
[2/2] regulator: max77857: Add ADI MAX77857/59/MAX77831 Regulator Support
commit: af71cccadecedad3484c2208e2c4fc8eff927d4a

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

2023-07-18 16:24:12

[permalink] [raw]

Subject: Re: [PATCH v3 2/2] regulator: max77857: Add ADI MAX77857/59/MAX77831 Regulator Support

Hi Okan,

On Mon, Jul 17, 2023 at 08:07:35AM +0300, Okan Sahin wrote:
> Regulator driver for MAX77857/59 and MAX77831.
> The MAX77857 is a high-efficiency, high-performance
> buck-boost converter targeted for systems requiring
> a wide input voltage range (2.5V to 16V).
>
> The MAX77859 is high-Efficiency Buck-Boost Converter
> for USB-PD/PPS Applications. It has wide input range
> (2.5V to 22V)
>
> The MAX77831 is a high-efficiency, high-performance
> buck-boost converter targeted for systems requiring
> wide input voltage range (2.5V to 16V).
>
> Signed-off-by: Okan Sahin <[email protected]>
> ---
> drivers/regulator/Kconfig | 10 +
> drivers/regulator/Makefile | 1 +
> drivers/regulator/max77857-regulator.c | 459 +++++++++++++++++++++++++
> 3 files changed, 470 insertions(+)
> create mode 100644 drivers/regulator/max77857-regulator.c
>
> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> index e5f3613c15fa..09eaa1cd90de 100644
> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
> @@ -573,6 +573,16 @@ config REGULATOR_MAX77650
> Semiconductor. This device has a SIMO with three independent
> power rails and an LDO.
>
> +config REGULATOR_MAX77857
> + tristate "ADI MAX77857/MAX77831 regulator support"
> + depends on I2C
> + select REGMAP_I2C
> + help
> + This driver controls a ADI MAX77857 and MAX77831 regulators.
> + via I2C bus. MAX77857 and MAX77831 are high efficiency buck-boost
> + converters with input voltage range (2.5V to 16V). Say Y here to
> + enable the regulator driver
> +
> config REGULATOR_MAX8649
> tristate "Maxim 8649 voltage regulator"
> depends on I2C
> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
> index 58dfe0147cd4..e7230846b680 100644
> --- a/drivers/regulator/Makefile
> +++ b/drivers/regulator/Makefile
> @@ -85,6 +85,7 @@ obj-$(CONFIG_REGULATOR_MAX77686) += max77686-regulator.o
> obj-$(CONFIG_REGULATOR_MAX77693) += max77693-regulator.o
> obj-$(CONFIG_REGULATOR_MAX77802) += max77802-regulator.o
> obj-$(CONFIG_REGULATOR_MAX77826) += max77826-regulator.o
> +obj-$(CONFIG_REGULATOR_MAX77857) += max77857-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
> diff --git a/drivers/regulator/max77857-regulator.c b/drivers/regulator/max77857-regulator.c
> new file mode 100644
> index 000000000000..c5482ffd606e
> --- /dev/null
> +++ b/drivers/regulator/max77857-regulator.c
> @@ -0,0 +1,459 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2023 Analog Devices, Inc.
> + * ADI Regulator driver for the MAX77857
> + * MAX77859 and MAX77831.
> + */
> +#include <linux/bitfield.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/driver.h>
> +#include <linux/regulator/machine.h>
> +#include <linux/regulator/of_regulator.h>
> +#include <linux/util_macros.h>
> +
> +#define MAX77857_REG_INT_SRC 0x10
> +#define MAX77857_REG_INT_MASK 0x11
> +#define MAX77857_REG_CONT1 0x12
> +#define MAX77857_REG_CONT2 0x13
> +#define MAX77857_REG_CONT3 0x14
> +
> +#define MAX77857_INT_SRC_OCP BIT(0)
> +#define MAX77857_INT_SRC_THS BIT(1)
> +#define MAX77857_INT_SRC_HARDSHORT BIT(2)
> +#define MAX77857_INT_SRC_OVP BIT(3)
> +#define MAX77857_INT_SRC_POK BIT(4)
> +
> +#define MAX77857_ILIM_MASK GENMASK(2, 0)
> +#define MAX77857_CONT1_FREQ GENMASK(4, 3)
> +#define MAX77857_CONT3_FPWM BIT(5)
> +
> +#define MAX77859_REG_INT_SRC 0x11
> +#define MAX77859_REG_CONT1 0x13
> +#define MAX77859_REG_CONT2 0x14
> +#define MAX77859_REG_CONT3 0x15
> +#define MAX77859_REG_CONT5 0x17
> +#define MAX77859_CONT2_FPWM BIT(2)
> +#define MAX77859_CONT2_INTB BIT(3)
> +#define MAX77859_CONT3_DVS_START BIT(2)
> +#define MAX77859_VOLTAGE_SEL_MASK GENMASK(9, 0)
> +
> +#define MAX77859_CURRENT_MIN 1000000
> +#define MAX77859_CURRENT_MAX 5000000
> +#define MAX77859_CURRENT_STEP 50000
> +
> +enum max77857_id {
> + ID_MAX77831 = 1,
> + ID_MAX77857,
> + ID_MAX77859,
> + ID_MAX77859A,
> +};
> +
> +static bool max77857_volatile_reg(struct device *dev, unsigned int reg)
> +{
> + enum max77857_id id = (enum max77857_id)dev_get_drvdata(dev);
> +
> + switch (id) {
> + case ID_MAX77831:
> + case ID_MAX77857:
> + return reg == MAX77857_REG_INT_SRC;
> + case ID_MAX77859:
> + case ID_MAX77859A:
> + return reg == MAX77859_REG_INT_SRC;
> + default:
> + return true;
> + }
> +}
> +
> +struct regmap_config max77857_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .cache_type = REGCACHE_MAPLE,
> + .volatile_reg = max77857_volatile_reg,
> +};
> +
> +static int max77857_get_status(struct regulator_dev *rdev)
> +{
> + unsigned int val;
> + int ret;
> +
> + ret = regmap_read(rdev->regmap, MAX77857_REG_INT_SRC, &val);
> + if (ret)
> + return ret;
> +
> + if (FIELD_GET(MAX77857_INT_SRC_POK, val))
> + return REGULATOR_STATUS_ON;
> +
> + return REGULATOR_STATUS_ERROR;
> +}
> +
> +static unsigned int max77857_get_mode(struct regulator_dev *rdev)
> +{
> + enum max77857_id id = (enum max77857_id)rdev_get_drvdata(rdev);
> + unsigned int regval;
> + int ret;
> +
> + switch (id) {
> + case ID_MAX77831:
> + case ID_MAX77857:
> + ret = regmap_read(rdev->regmap, MAX77857_REG_CONT3, &regval);
> + if (ret)
> + return ret;
> +
> + if (FIELD_GET(MAX77857_CONT3_FPWM, regval))
> + return REGULATOR_MODE_FAST;
> +
> + break;
> + case ID_MAX77859:
> + case ID_MAX77859A:
> + ret = regmap_read(rdev->regmap, MAX77859_REG_CONT2, &regval);
> + if (ret)
> + return ret;
> +
> + if (FIELD_GET(MAX77859_CONT2_FPWM, regval))
> + return REGULATOR_MODE_FAST;
> +
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return REGULATOR_MODE_NORMAL;
> +}
> +
> +static int max77857_set_mode(struct regulator_dev *rdev, unsigned int mode)
> +{
> + enum max77857_id id = (enum max77857_id)rdev_get_drvdata(rdev);
> + unsigned int reg, val;
> +
> + switch (id) {
> + case ID_MAX77831:
> + case ID_MAX77857:
> + reg = MAX77857_REG_CONT3;
> + val = MAX77857_CONT3_FPWM;
> + break;
> + case ID_MAX77859:
> + case ID_MAX77859A:
> + reg = MAX77859_REG_CONT2;
> + val = MAX77859_CONT2_FPWM;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + switch (mode) {
> + case REGULATOR_MODE_FAST:
> + return regmap_set_bits(rdev->regmap, reg, val);
> + case REGULATOR_MODE_NORMAL:
> + return regmap_clear_bits(rdev->regmap, reg, val);
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int max77857_get_error_flags(struct regulator_dev *rdev,
> + unsigned int *flags)
> +{
> + unsigned int val;
> + int ret;
> +
> + ret = regmap_read(rdev->regmap, MAX77857_REG_INT_SRC, &val);
> + if (ret)
> + return ret;
> +
> + *flags = 0;
> +
> + if (FIELD_GET(MAX77857_INT_SRC_OVP, val))
> + *flags |= REGULATOR_ERROR_OVER_VOLTAGE_WARN;
> +
> + if (FIELD_GET(MAX77857_INT_SRC_OCP, val) ||
> + FIELD_GET(MAX77857_INT_SRC_HARDSHORT, val))
> + *flags |= REGULATOR_ERROR_OVER_CURRENT;
> +
> + if (FIELD_GET(MAX77857_INT_SRC_THS, val))
> + *flags |= REGULATOR_ERROR_OVER_TEMP;
> +
> + if (!FIELD_GET(MAX77857_INT_SRC_POK, val))
> + *flags |= REGULATOR_ERROR_FAIL;
> +
> + return 0;
> +}
> +
> +static struct linear_range max77859_lin_ranges[] = {
> + REGULATOR_LINEAR_RANGE(3200000, 0x0A0, 0x320, 20000)
> +};
> +
> +static const unsigned int max77859_ramp_table[4] = {
> + 1000, 500, 250, 125
> +};
> +
> +static int max77859_set_voltage_sel(struct regulator_dev *rdev,
> + unsigned int sel)
> +{
> + __be16 reg;
> + int ret;
> +
> + reg = cpu_to_be16(sel);
> +
> + ret = regmap_bulk_write(rdev->regmap, MAX77859_REG_CONT3, &reg, 2);
> + if (ret)
> + return ret;
> +
> + /* actually apply new voltage */
> + return regmap_set_bits(rdev->regmap, MAX77859_REG_CONT3,
> + MAX77859_CONT3_DVS_START);
> +}
> +
> +int max77859_get_voltage_sel(struct regulator_dev *rdev)
> +{
> + __be16 reg;
> + int ret;
> +
> + ret = regmap_bulk_read(rdev->regmap, MAX77859_REG_CONT3, &reg, 2);
> + if (ret)
> + return ret;
> +
> + return FIELD_GET(MAX77859_VOLTAGE_SEL_MASK, __be16_to_cpu(reg));
> +}
> +
> +int max77859_set_current_limit(struct regulator_dev *rdev, int min_uA, int max_uA)
> +{
> + u32 selector;
> +
> + if (max_uA < MAX77859_CURRENT_MIN)
> + return -EINVAL;
> +
> + selector = 0x12 + (max_uA - MAX77859_CURRENT_MIN) / MAX77859_CURRENT_STEP;
> +
> + selector = clamp_val(selector, 0x00, 0x7F);
> +
> + return regmap_write(rdev->regmap, MAX77859_REG_CONT5, selector);
> +}
> +
> +int max77859_get_current_limit(struct regulator_dev *rdev)
> +{
> + u32 selector;
> + int ret;
> +
> + ret = regmap_read(rdev->regmap, MAX77859_REG_CONT5, &selector);
> + if (ret)
> + return ret;
> +
> + if (selector <= 0x12)
> + return MAX77859_CURRENT_MIN;
> +
> + if (selector >= 0x64)
> + return MAX77859_CURRENT_MAX;
> +
> + return MAX77859_CURRENT_MIN + (selector - 0x12) * MAX77859_CURRENT_STEP;
> +}
> +
> +static const struct regulator_ops max77859_regulator_ops = {
> + .list_voltage = regulator_list_voltage_linear_range,
> + .set_voltage_sel = max77859_set_voltage_sel,
> + .get_voltage_sel = max77859_get_voltage_sel,
> + .set_ramp_delay = regulator_set_ramp_delay_regmap,
> + .get_status = max77857_get_status,
> + .set_mode = max77857_set_mode,
> + .get_mode = max77857_get_mode,
> + .get_error_flags = max77857_get_error_flags,
> +};
> +
> +static const struct regulator_ops max77859a_regulator_ops = {
> + .list_voltage = regulator_list_voltage_linear_range,
> + .set_voltage_sel = max77859_set_voltage_sel,
> + .get_voltage_sel = max77859_get_voltage_sel,
> + .set_current_limit = max77859_set_current_limit,
> + .get_current_limit = max77859_get_current_limit,
> + .set_ramp_delay = regulator_set_ramp_delay_regmap,
> + .get_status = max77857_get_status,
> + .set_mode = max77857_set_mode,
> + .get_mode = max77857_get_mode,
> + .get_error_flags = max77857_get_error_flags,
> +};
> +
> +static const struct regulator_ops max77857_regulator_ops = {
> + .list_voltage = regulator_list_voltage_linear_range,
> + .set_voltage_sel = regulator_set_voltage_sel_regmap,
> + .get_voltage_sel = regulator_get_voltage_sel_regmap,
> + .set_ramp_delay = regulator_set_ramp_delay_regmap,
> + .get_status = max77857_get_status,
> + .set_mode = max77857_set_mode,
> + .get_mode = max77857_get_mode,
> + .get_error_flags = max77857_get_error_flags,
> +};
> +
> +static struct linear_range max77857_lin_ranges[] = {
> + REGULATOR_LINEAR_RANGE(4485000, 0x3D, 0xCC, 73500)
> +};
> +
> +static const unsigned int max77857_switch_freq[] = {
> + 1200000, 1500000, 1800000, 2100000
> +};
> +
> +static const unsigned int max77857_ramp_table[2][4] = {
> + { 1333, 667, 333, 227 }, /* when switch freq is 1.8MHz or 2.1MHz */
> + { 1166, 667, 333, 167 }, /* when switch freq is 1.2MHz or 1.5MHz */
> +};
> +
> +static struct regulator_desc max77857_regulator_desc = {
> + .ops = &max77857_regulator_ops,
> + .name = "max77857",
> + .linear_ranges = max77857_lin_ranges,
> + .n_linear_ranges = ARRAY_SIZE(max77857_lin_ranges),
> + .vsel_mask = 0xFF,
> + .vsel_reg = MAX77857_REG_CONT2,
> + .ramp_delay_table = max77857_ramp_table[0],
> + .n_ramp_values = ARRAY_SIZE(max77857_ramp_table[0]),
> + .ramp_reg = MAX77857_REG_CONT3,
> + .ramp_mask = GENMASK(1, 0),
> + .ramp_delay = max77857_ramp_table[0][0],

This breaks the build with GCC 5.x through 7.x:

drivers/regulator/max77857-regulator.c:312:16: error: initializer element is not constant
.ramp_delay = max77857_ramp_table[0][0],
^~~~~~~~~~~~~~~~~~~
drivers/regulator/max77857-regulator.c:312:16: note: (near initialization for 'max77857_regulator_desc.ramp_delay')

and clang:

drivers/regulator/max77857-regulator.c:312:16: error: initializer element is not a compile-time constant
312 | .ramp_delay = max77857_ramp_table[0][0],
| ^~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

This relies on a GCC 8.x+ change that accepts more things as
compile-time constants, which is being worked on in clang
(https://reviews.llvm.org/D76096). Since the kernel supports older
compilers, this will have to be worked around somehow. Perhaps a define
that can be used in both places?

Cheers,
Nathan

> + .owner = THIS_MODULE,
> +};
> +
> +static void max77857_calc_range(struct device *dev, enum max77857_id id)
> +{
> + struct linear_range *range;
> + unsigned long vref_step;
> + u32 rtop = 0;
> + u32 rbot = 0;
> +
> + device_property_read_u32(dev, "adi,rtop-ohms", &rtop);
> + device_property_read_u32(dev, "adi,rbot-ohms", &rbot);
> +
> + if (!rbot || !rtop)
> + return;
> +
> + switch (id) {
> + case ID_MAX77831:
> + case ID_MAX77857:
> + range = max77857_lin_ranges;
> + vref_step = 4900UL;
> + break;
> + case ID_MAX77859:
> + case ID_MAX77859A:
> + range = max77859_lin_ranges;
> + vref_step = 1250UL;
> + break;
> + }
> +
> + range->step = DIV_ROUND_CLOSEST(vref_step * (rbot + rtop), rbot);
> + range->min = range->step * range->min_sel;
> +}
> +
> +static int max77857_probe(struct i2c_client *client)
> +{
> + const struct i2c_device_id *i2c_id;
> + struct device *dev = &client->dev;
> + struct regulator_config cfg = { };
> + struct regulator_dev *rdev;
> + struct regmap *regmap;
> + enum max77857_id id;
> + u32 switch_freq = 0;
> + int ret;
> +
> + i2c_id = i2c_client_get_device_id(client);
> + if (!i2c_id)
> + return -EINVAL;
> +
> + id = i2c_id->driver_data;
> +
> + dev_set_drvdata(dev, (void *)id);
> +
> + if (id == ID_MAX77859 || id == ID_MAX77859A) {
> + max77857_regulator_desc.ops = &max77859_regulator_ops;
> + max77857_regulator_desc.linear_ranges = max77859_lin_ranges;
> + max77857_regulator_desc.ramp_delay_table = max77859_ramp_table;
> + max77857_regulator_desc.ramp_delay = max77859_ramp_table[0];
> + }
> +
> + if (id == ID_MAX77859A)
> + max77857_regulator_desc.ops = &max77859a_regulator_ops;
> +
> + max77857_calc_range(dev, id);
> +
> + regmap = devm_regmap_init_i2c(client, &max77857_regmap_config);
> + if (IS_ERR(regmap))
> + return dev_err_probe(dev, PTR_ERR(regmap),
> + "cannot initialize regmap\n");
> +
> + device_property_read_u32(dev, "adi,switch-frequency-hz", &switch_freq);
> + if (switch_freq) {
> + switch_freq = find_closest(switch_freq, max77857_switch_freq,
> + ARRAY_SIZE(max77857_switch_freq));
> +
> + if (id == ID_MAX77831 && switch_freq == 3)
> + switch_freq = 2;
> +
> + switch (id) {
> + case ID_MAX77831:
> + case ID_MAX77857:
> + ret = regmap_update_bits(regmap, MAX77857_REG_CONT1,
> + MAX77857_CONT1_FREQ, switch_freq);
> +
> + if (switch_freq >= 2)
> + break;
> +
> + max77857_regulator_desc.ramp_delay_table = max77857_ramp_table[1];
> + max77857_regulator_desc.ramp_delay = max77857_ramp_table[1][0];
> + break;
> + case ID_MAX77859:
> + case ID_MAX77859A:
> + ret = regmap_update_bits(regmap, MAX77859_REG_CONT1,
> + MAX77857_CONT1_FREQ, switch_freq);
> + break;
> + }
> + if (ret)
> + return ret;
> + }
> +
> + cfg.dev = dev;
> + cfg.driver_data = (void *)id;
> + cfg.regmap = regmap;
> + cfg.init_data = of_get_regulator_init_data(dev, dev->of_node,
> + &max77857_regulator_desc);
> + if (!cfg.init_data)
> + return -ENOMEM;
> +
> + rdev = devm_regulator_register(dev, &max77857_regulator_desc, &cfg);
> + if (IS_ERR(rdev))
> + return dev_err_probe(dev, PTR_ERR(rdev),
> + "cannot register regulator\n");
> +
> + return 0;
> +}
> +
> +const struct i2c_device_id max77857_id[] = {
> + { "max77831", ID_MAX77831 },
> + { "max77857", ID_MAX77857 },
> + { "max77859", ID_MAX77859 },
> + { "max77859a", ID_MAX77859A },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, max77857_id);
> +
> +static const struct of_device_id max77857_of_id[] = {
> + { .compatible = "adi,max77831", .data = (void *)ID_MAX77831 },
> + { .compatible = "adi,max77857", .data = (void *)ID_MAX77857 },
> + { .compatible = "adi,max77859", .data = (void *)ID_MAX77859 },
> + { .compatible = "adi,max77859a", .data = (void *)ID_MAX77859A },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, max77857_of_id);
> +
> +struct i2c_driver max77857_driver = {
> + .driver = {
> + .name = "max77857",
> + .of_match_table = max77857_of_id,
> + },
> + .id_table = max77857_id,
> + .probe_new = max77857_probe,
> +};
> +module_i2c_driver(max77857_driver);
> +
> +MODULE_DESCRIPTION("Analog Devices MAX77857 Buck-Boost Converter Driver");
> +MODULE_AUTHOR("Ibrahim Tilki <[email protected]>");
> +MODULE_AUTHOR("Okan Sahin <[email protected]>");
> +MODULE_LICENSE("GPL");
> --
> 2.30.2
>

2023-07-18 17:48:07

[permalink] [raw]

Subject: Re: [PATCH v3 2/2] regulator: max77857: Add ADI MAX77857/59/MAX77831 Regulator Support

On Tue, Jul 18, 2023 at 05:25:32PM +0000, Sahin, Okan wrote:
> >From: Nathan Chancellor <[email protected]>
> >Sent: Tuesday, July 18, 2023 6:55 PM
> >To: Sahin, Okan <[email protected]>
> >Cc: Liam Girdwood <[email protected]>; Mark Brown <[email protected]>;
> >Rob Herring <[email protected]>; Krzysztof Kozlowski
> ><[email protected]>; Conor Dooley <[email protected]>; Tilki,
> >Ibrahim <[email protected]>; [email protected];
> >[email protected]; [email protected]
> >Subject: Re: [PATCH v3 2/2] regulator: max77857: Add ADI MAX77857/59/MAX77831
> >Regulator Support
> >
> >[External]
> >
> >Hi Okan,
> >
> >On Mon, Jul 17, 2023 at 08:07:35AM +0300, Okan Sahin wrote:
> >> Regulator driver for MAX77857/59 and MAX77831.
> >> The MAX77857 is a high-efficiency, high-performance
> >> buck-boost converter targeted for systems requiring
> >> a wide input voltage range (2.5V to 16V).
> >>
> >> The MAX77859 is high-Efficiency Buck-Boost Converter
> >> for USB-PD/PPS Applications. It has wide input range
> >> (2.5V to 22V)
> >>
> >> The MAX77831 is a high-efficiency, high-performance
> >> buck-boost converter targeted for systems requiring
> >> wide input voltage range (2.5V to 16V).
> >>
> >> Signed-off-by: Okan Sahin <[email protected]>
> >> ---
> >> drivers/regulator/Kconfig | 10 +
> >> drivers/regulator/Makefile | 1 +
> >> drivers/regulator/max77857-regulator.c | 459 +++++++++++++++++++++++++
> >> 3 files changed, 470 insertions(+)
> >> create mode 100644 drivers/regulator/max77857-regulator.c
> >>
> >> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> >> index e5f3613c15fa..09eaa1cd90de 100644
> >> --- a/drivers/regulator/Kconfig
> >> +++ b/drivers/regulator/Kconfig
> >> @@ -573,6 +573,16 @@ config REGULATOR_MAX77650
> >> Semiconductor. This device has a SIMO with three independent
> >> power rails and an LDO.
> >>
> >> +config REGULATOR_MAX77857
> >> + tristate "ADI MAX77857/MAX77831 regulator support"
> >> + depends on I2C
> >> + select REGMAP_I2C
> >> + help
> >> + This driver controls a ADI MAX77857 and MAX77831 regulators.
> >> + via I2C bus. MAX77857 and MAX77831 are high efficiency buck-boost
> >> + converters with input voltage range (2.5V to 16V). Say Y here to
> >> + enable the regulator driver
> >> +
> >> config REGULATOR_MAX8649
> >> tristate "Maxim 8649 voltage regulator"
> >> depends on I2C
> >> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
> >> index 58dfe0147cd4..e7230846b680 100644
> >> --- a/drivers/regulator/Makefile
> >> +++ b/drivers/regulator/Makefile
> >> @@ -85,6 +85,7 @@ obj-$(CONFIG_REGULATOR_MAX77686) += max77686-
> >regulator.o
> >> obj-$(CONFIG_REGULATOR_MAX77693) += max77693-regulator.o
> >> obj-$(CONFIG_REGULATOR_MAX77802) += max77802-regulator.o
> >> obj-$(CONFIG_REGULATOR_MAX77826) += max77826-regulator.o
> >> +obj-$(CONFIG_REGULATOR_MAX77857) += max77857-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
> >> diff --git a/drivers/regulator/max77857-regulator.c b/drivers/regulator/max77857-
> >regulator.c
> >> new file mode 100644
> >> index 000000000000..c5482ffd606e
> >> --- /dev/null
> >> +++ b/drivers/regulator/max77857-regulator.c
> >> @@ -0,0 +1,459 @@
> >> +// SPDX-License-Identifier: GPL-2.0-only
> >> +/*
> >> + * Copyright (c) 2023 Analog Devices, Inc.
> >> + * ADI Regulator driver for the MAX77857
> >> + * MAX77859 and MAX77831.
> >> + */
> >> +#include <linux/bitfield.h>
> >> +#include <linux/i2c.h>
> >> +#include <linux/interrupt.h>
> >> +#include <linux/module.h>
> >> +#include <linux/regmap.h>
> >> +#include <linux/regulator/driver.h>
> >> +#include <linux/regulator/machine.h>
> >> +#include <linux/regulator/of_regulator.h>
> >> +#include <linux/util_macros.h>
> >> +
> >> +#define MAX77857_REG_INT_SRC 0x10
> >> +#define MAX77857_REG_INT_MASK 0x11
> >> +#define MAX77857_REG_CONT1 0x12
> >> +#define MAX77857_REG_CONT2 0x13
> >> +#define MAX77857_REG_CONT3 0x14
> >> +
> >> +#define MAX77857_INT_SRC_OCP BIT(0)
> >> +#define MAX77857_INT_SRC_THS BIT(1)
> >> +#define MAX77857_INT_SRC_HARDSHORT BIT(2)
> >> +#define MAX77857_INT_SRC_OVP BIT(3)
> >> +#define MAX77857_INT_SRC_POK BIT(4)
> >> +
> >> +#define MAX77857_ILIM_MASK GENMASK(2, 0)
> >> +#define MAX77857_CONT1_FREQ GENMASK(4, 3)
> >> +#define MAX77857_CONT3_FPWM BIT(5)
> >> +
> >> +#define MAX77859_REG_INT_SRC 0x11
> >> +#define MAX77859_REG_CONT1 0x13
> >> +#define MAX77859_REG_CONT2 0x14
> >> +#define MAX77859_REG_CONT3 0x15
> >> +#define MAX77859_REG_CONT5 0x17
> >> +#define MAX77859_CONT2_FPWM BIT(2)
> >> +#define MAX77859_CONT2_INTB BIT(3)
> >> +#define MAX77859_CONT3_DVS_START BIT(2)
> >> +#define MAX77859_VOLTAGE_SEL_MASK GENMASK(9, 0)
> >> +
> >> +#define MAX77859_CURRENT_MIN 1000000
> >> +#define MAX77859_CURRENT_MAX 5000000
> >> +#define MAX77859_CURRENT_STEP 50000
> >> +
> >> +enum max77857_id {
> >> + ID_MAX77831 = 1,
> >> + ID_MAX77857,
> >> + ID_MAX77859,
> >> + ID_MAX77859A,
> >> +};
> >> +
> >> +static bool max77857_volatile_reg(struct device *dev, unsigned int reg)
> >> +{
> >> + enum max77857_id id = (enum max77857_id)dev_get_drvdata(dev);
> >> +
> >> + switch (id) {
> >> + case ID_MAX77831:
> >> + case ID_MAX77857:
> >> + return reg == MAX77857_REG_INT_SRC;
> >> + case ID_MAX77859:
> >> + case ID_MAX77859A:
> >> + return reg == MAX77859_REG_INT_SRC;
> >> + default:
> >> + return true;
> >> + }
> >> +}
> >> +
> >> +struct regmap_config max77857_regmap_config = {
> >> + .reg_bits = 8,
> >> + .val_bits = 8,
> >> + .cache_type = REGCACHE_MAPLE,
> >> + .volatile_reg = max77857_volatile_reg,
> >> +};
> >> +
> >> +static int max77857_get_status(struct regulator_dev *rdev)
> >> +{
> >> + unsigned int val;
> >> + int ret;
> >> +
> >> + ret = regmap_read(rdev->regmap, MAX77857_REG_INT_SRC, &val);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + if (FIELD_GET(MAX77857_INT_SRC_POK, val))
> >> + return REGULATOR_STATUS_ON;
> >> +
> >> + return REGULATOR_STATUS_ERROR;
> >> +}
> >> +
> >> +static unsigned int max77857_get_mode(struct regulator_dev *rdev)
> >> +{
> >> + enum max77857_id id = (enum max77857_id)rdev_get_drvdata(rdev);
> >> + unsigned int regval;
> >> + int ret;
> >> +
> >> + switch (id) {
> >> + case ID_MAX77831:
> >> + case ID_MAX77857:
> >> + ret = regmap_read(rdev->regmap, MAX77857_REG_CONT3,
> >&regval);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + if (FIELD_GET(MAX77857_CONT3_FPWM, regval))
> >> + return REGULATOR_MODE_FAST;
> >> +
> >> + break;
> >> + case ID_MAX77859:
> >> + case ID_MAX77859A:
> >> + ret = regmap_read(rdev->regmap, MAX77859_REG_CONT2,
> >&regval);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + if (FIELD_GET(MAX77859_CONT2_FPWM, regval))
> >> + return REGULATOR_MODE_FAST;
> >> +
> >> + break;
> >> + default:
> >> + return -EINVAL;
> >> + }
> >> +
> >> + return REGULATOR_MODE_NORMAL;
> >> +}
> >> +
> >> +static int max77857_set_mode(struct regulator_dev *rdev, unsigned int mode)
> >> +{
> >> + enum max77857_id id = (enum max77857_id)rdev_get_drvdata(rdev);
> >> + unsigned int reg, val;
> >> +
> >> + switch (id) {
> >> + case ID_MAX77831:
> >> + case ID_MAX77857:
> >> + reg = MAX77857_REG_CONT3;
> >> + val = MAX77857_CONT3_FPWM;
> >> + break;
> >> + case ID_MAX77859:
> >> + case ID_MAX77859A:
> >> + reg = MAX77859_REG_CONT2;
> >> + val = MAX77859_CONT2_FPWM;
> >> + break;
> >> + default:
> >> + return -EINVAL;
> >> + }
> >> +
> >> + switch (mode) {
> >> + case REGULATOR_MODE_FAST:
> >> + return regmap_set_bits(rdev->regmap, reg, val);
> >> + case REGULATOR_MODE_NORMAL:
> >> + return regmap_clear_bits(rdev->regmap, reg, val);
> >> + default:
> >> + return -EINVAL;
> >> + }
> >> +}
> >> +
> >> +static int max77857_get_error_flags(struct regulator_dev *rdev,
> >> + unsigned int *flags)
> >> +{
> >> + unsigned int val;
> >> + int ret;
> >> +
> >> + ret = regmap_read(rdev->regmap, MAX77857_REG_INT_SRC, &val);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + *flags = 0;
> >> +
> >> + if (FIELD_GET(MAX77857_INT_SRC_OVP, val))
> >> + *flags |= REGULATOR_ERROR_OVER_VOLTAGE_WARN;
> >> +
> >> + if (FIELD_GET(MAX77857_INT_SRC_OCP, val) ||
> >> + FIELD_GET(MAX77857_INT_SRC_HARDSHORT, val))
> >> + *flags |= REGULATOR_ERROR_OVER_CURRENT;
> >> +
> >> + if (FIELD_GET(MAX77857_INT_SRC_THS, val))
> >> + *flags |= REGULATOR_ERROR_OVER_TEMP;
> >> +
> >> + if (!FIELD_GET(MAX77857_INT_SRC_POK, val))
> >> + *flags |= REGULATOR_ERROR_FAIL;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static struct linear_range max77859_lin_ranges[] = {
> >> + REGULATOR_LINEAR_RANGE(3200000, 0x0A0, 0x320, 20000)
> >> +};
> >> +
> >> +static const unsigned int max77859_ramp_table[4] = {
> >> + 1000, 500, 250, 125
> >> +};
> >> +
> >> +static int max77859_set_voltage_sel(struct regulator_dev *rdev,
> >> + unsigned int sel)
> >> +{
> >> + __be16 reg;
> >> + int ret;
> >> +
> >> + reg = cpu_to_be16(sel);
> >> +
> >> + ret = regmap_bulk_write(rdev->regmap, MAX77859_REG_CONT3, &reg, 2);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + /* actually apply new voltage */
> >> + return regmap_set_bits(rdev->regmap, MAX77859_REG_CONT3,
> >> + MAX77859_CONT3_DVS_START);
> >> +}
> >> +
> >> +int max77859_get_voltage_sel(struct regulator_dev *rdev)
> >> +{
> >> + __be16 reg;
> >> + int ret;
> >> +
> >> + ret = regmap_bulk_read(rdev->regmap, MAX77859_REG_CONT3, &reg, 2);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + return FIELD_GET(MAX77859_VOLTAGE_SEL_MASK, __be16_to_cpu(reg));
> >> +}
> >> +
> >> +int max77859_set_current_limit(struct regulator_dev *rdev, int min_uA, int
> >max_uA)
> >> +{
> >> + u32 selector;
> >> +
> >> + if (max_uA < MAX77859_CURRENT_MIN)
> >> + return -EINVAL;
> >> +
> >> + selector = 0x12 + (max_uA - MAX77859_CURRENT_MIN) /
> >MAX77859_CURRENT_STEP;
> >> +
> >> + selector = clamp_val(selector, 0x00, 0x7F);
> >> +
> >> + return regmap_write(rdev->regmap, MAX77859_REG_CONT5, selector);
> >> +}
> >> +
> >> +int max77859_get_current_limit(struct regulator_dev *rdev)
> >> +{
> >> + u32 selector;
> >> + int ret;
> >> +
> >> + ret = regmap_read(rdev->regmap, MAX77859_REG_CONT5, &selector);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + if (selector <= 0x12)
> >> + return MAX77859_CURRENT_MIN;
> >> +
> >> + if (selector >= 0x64)
> >> + return MAX77859_CURRENT_MAX;
> >> +
> >> + return MAX77859_CURRENT_MIN + (selector - 0x12) *
> >MAX77859_CURRENT_STEP;
> >> +}
> >> +
> >> +static const struct regulator_ops max77859_regulator_ops = {
> >> + .list_voltage = regulator_list_voltage_linear_range,
> >> + .set_voltage_sel = max77859_set_voltage_sel,
> >> + .get_voltage_sel = max77859_get_voltage_sel,
> >> + .set_ramp_delay = regulator_set_ramp_delay_regmap,
> >> + .get_status = max77857_get_status,
> >> + .set_mode = max77857_set_mode,
> >> + .get_mode = max77857_get_mode,
> >> + .get_error_flags = max77857_get_error_flags,
> >> +};
> >> +
> >> +static const struct regulator_ops max77859a_regulator_ops = {
> >> + .list_voltage = regulator_list_voltage_linear_range,
> >> + .set_voltage_sel = max77859_set_voltage_sel,
> >> + .get_voltage_sel = max77859_get_voltage_sel,
> >> + .set_current_limit = max77859_set_current_limit,
> >> + .get_current_limit = max77859_get_current_limit,
> >> + .set_ramp_delay = regulator_set_ramp_delay_regmap,
> >> + .get_status = max77857_get_status,
> >> + .set_mode = max77857_set_mode,
> >> + .get_mode = max77857_get_mode,
> >> + .get_error_flags = max77857_get_error_flags,
> >> +};
> >> +
> >> +static const struct regulator_ops max77857_regulator_ops = {
> >> + .list_voltage = regulator_list_voltage_linear_range,
> >> + .set_voltage_sel = regulator_set_voltage_sel_regmap,
> >> + .get_voltage_sel = regulator_get_voltage_sel_regmap,
> >> + .set_ramp_delay = regulator_set_ramp_delay_regmap,
> >> + .get_status = max77857_get_status,
> >> + .set_mode = max77857_set_mode,
> >> + .get_mode = max77857_get_mode,
> >> + .get_error_flags = max77857_get_error_flags,
> >> +};
> >> +
> >> +static struct linear_range max77857_lin_ranges[] = {
> >> + REGULATOR_LINEAR_RANGE(4485000, 0x3D, 0xCC, 73500)
> >> +};
> >> +
> >> +static const unsigned int max77857_switch_freq[] = {
> >> + 1200000, 1500000, 1800000, 2100000
> >> +};
> >> +
> >> +static const unsigned int max77857_ramp_table[2][4] = {
> >> + { 1333, 667, 333, 227 }, /* when switch freq is 1.8MHz or 2.1MHz */
> >> + { 1166, 667, 333, 167 }, /* when switch freq is 1.2MHz or 1.5MHz */
> >> +};
> >> +
> >> +static struct regulator_desc max77857_regulator_desc = {
> >> + .ops = &max77857_regulator_ops,
> >> + .name = "max77857",
> >> + .linear_ranges = max77857_lin_ranges,
> >> + .n_linear_ranges = ARRAY_SIZE(max77857_lin_ranges),
> >> + .vsel_mask = 0xFF,
> >> + .vsel_reg = MAX77857_REG_CONT2,
> >> + .ramp_delay_table = max77857_ramp_table[0],
> >> + .n_ramp_values = ARRAY_SIZE(max77857_ramp_table[0]),
> >> + .ramp_reg = MAX77857_REG_CONT3,
> >> + .ramp_mask = GENMASK(1, 0),
> >> + .ramp_delay = max77857_ramp_table[0][0],
> >
> >This breaks the build with GCC 5.x through 7.x:
> >
> > drivers/regulator/max77857-regulator.c:312:16: error: initializer element is not
> >constant
> > .ramp_delay = max77857_ramp_table[0][0],
> > ^~~~~~~~~~~~~~~~~~~
> > drivers/regulator/max77857-regulator.c:312:16: note: (near initialization for
> >'max77857_regulator_desc.ramp_delay')
> >
> >and clang:
> >
> > drivers/regulator/max77857-regulator.c:312:16: error: initializer element is not a
> >compile-time constant
> > 312 | .ramp_delay = max77857_ramp_table[0][0],
> > | ^~~~~~~~~~~~~~~~~~~~~~~~~
> > 1 error generated.
> >
> >This relies on a GCC 8.x+ change that accepts more things as
> >compile-time constants, which is being worked on in clang
> >(https://urldefense.com/v3/__https://reviews.llvm.org/D76096__;!!A3Ni8CS0y2Y!4ql
> >noZFf_EVInN-
> >MaRDQWqOHb1SEbEqkwlU06PCt1Ngw6tE41ZEE24hnL1wBMsfotRCue4-i1VwD0xw$
> >). Since the kernel supports older
> >compilers, this will have to be worked around somehow. Perhaps a define
> >that can be used in both places?
> >
> >Cheers,
> >Nathan
> >
> >> + .owner = THIS_MODULE,
> >> +};
> >> +
> >> +static void max77857_calc_range(struct device *dev, enum max77857_id id)
> >> +{
> >> + struct linear_range *range;
> >> + unsigned long vref_step;
> >> + u32 rtop = 0;
> >> + u32 rbot = 0;
> >> +
> >> + device_property_read_u32(dev, "adi,rtop-ohms", &rtop);
> >> + device_property_read_u32(dev, "adi,rbot-ohms", &rbot);
> >> +
> >> + if (!rbot || !rtop)
> >> + return;
> >> +
> >> + switch (id) {
> >> + case ID_MAX77831:
> >> + case ID_MAX77857:
> >> + range = max77857_lin_ranges;
> >> + vref_step = 4900UL;
> >> + break;
> >> + case ID_MAX77859:
> >> + case ID_MAX77859A:
> >> + range = max77859_lin_ranges;
> >> + vref_step = 1250UL;
> >> + break;
> >> + }
> >> +
> >> + range->step = DIV_ROUND_CLOSEST(vref_step * (rbot + rtop), rbot);
> >> + range->min = range->step * range->min_sel;
> >> +}
> >> +
> >> +static int max77857_probe(struct i2c_client *client)
> >> +{
> >> + const struct i2c_device_id *i2c_id;
> >> + struct device *dev = &client->dev;
> >> + struct regulator_config cfg = { };
> >> + struct regulator_dev *rdev;
> >> + struct regmap *regmap;
> >> + enum max77857_id id;
> >> + u32 switch_freq = 0;
> >> + int ret;
> >> +
> >> + i2c_id = i2c_client_get_device_id(client);
> >> + if (!i2c_id)
> >> + return -EINVAL;
> >> +
> >> + id = i2c_id->driver_data;
> >> +
> >> + dev_set_drvdata(dev, (void *)id);
> >> +
> >> + if (id == ID_MAX77859 || id == ID_MAX77859A) {
> >> + max77857_regulator_desc.ops = &max77859_regulator_ops;
> >> + max77857_regulator_desc.linear_ranges = max77859_lin_ranges;
> >> + max77857_regulator_desc.ramp_delay_table =
> >max77859_ramp_table;
> >> + max77857_regulator_desc.ramp_delay = max77859_ramp_table[0];
> >> + }
> >> +
> >> + if (id == ID_MAX77859A)
> >> + max77857_regulator_desc.ops = &max77859a_regulator_ops;
> >> +
> >> + max77857_calc_range(dev, id);
> >> +
> >> + regmap = devm_regmap_init_i2c(client, &max77857_regmap_config);
> >> + if (IS_ERR(regmap))
> >> + return dev_err_probe(dev, PTR_ERR(regmap),
> >> + "cannot initialize regmap\n");
> >> +
> >> + device_property_read_u32(dev, "adi,switch-frequency-hz", &switch_freq);
> >> + if (switch_freq) {
> >> + switch_freq = find_closest(switch_freq, max77857_switch_freq,
> >> + ARRAY_SIZE(max77857_switch_freq));
> >> +
> >> + if (id == ID_MAX77831 && switch_freq == 3)
> >> + switch_freq = 2;
> >> +
> >> + switch (id) {
> >> + case ID_MAX77831:
> >> + case ID_MAX77857:
> >> + ret = regmap_update_bits(regmap, MAX77857_REG_CONT1,
> >> + MAX77857_CONT1_FREQ,
> >switch_freq);
> >> +
> >> + if (switch_freq >= 2)
> >> + break;
> >> +
> >> + max77857_regulator_desc.ramp_delay_table =
> >max77857_ramp_table[1];
> >> + max77857_regulator_desc.ramp_delay =
> >max77857_ramp_table[1][0];
> >> + break;
> >> + case ID_MAX77859:
> >> + case ID_MAX77859A:
> >> + ret = regmap_update_bits(regmap, MAX77859_REG_CONT1,
> >> + MAX77857_CONT1_FREQ,
> >switch_freq);
> >> + break;
> >> + }
> >> + if (ret)
> >> + return ret;
> >> + }
> >> +
> >> + cfg.dev = dev;
> >> + cfg.driver_data = (void *)id;
> >> + cfg.regmap = regmap;
> >> + cfg.init_data = of_get_regulator_init_data(dev, dev->of_node,
> >> + &max77857_regulator_desc);
> >> + if (!cfg.init_data)
> >> + return -ENOMEM;
> >> +
> >> + rdev = devm_regulator_register(dev, &max77857_regulator_desc, &cfg);
> >> + if (IS_ERR(rdev))
> >> + return dev_err_probe(dev, PTR_ERR(rdev),
> >> + "cannot register regulator\n");
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +const struct i2c_device_id max77857_id[] = {
> >> + { "max77831", ID_MAX77831 },
> >> + { "max77857", ID_MAX77857 },
> >> + { "max77859", ID_MAX77859 },
> >> + { "max77859a", ID_MAX77859A },
> >> + { }
> >> +};
> >> +MODULE_DEVICE_TABLE(i2c, max77857_id);
> >> +
> >> +static const struct of_device_id max77857_of_id[] = {
> >> + { .compatible = "adi,max77831", .data = (void *)ID_MAX77831 },
> >> + { .compatible = "adi,max77857", .data = (void *)ID_MAX77857 },
> >> + { .compatible = "adi,max77859", .data = (void *)ID_MAX77859 },
> >> + { .compatible = "adi,max77859a", .data = (void *)ID_MAX77859A },
> >> + { }
> >> +};
> >> +MODULE_DEVICE_TABLE(of, max77857_of_id);
> >> +
> >> +struct i2c_driver max77857_driver = {
> >> + .driver = {
> >> + .name = "max77857",
> >> + .of_match_table = max77857_of_id,
> >> + },
> >> + .id_table = max77857_id,
> >> + .probe_new = max77857_probe,
> >> +};
> >> +module_i2c_driver(max77857_driver);
> >> +
> >> +MODULE_DESCRIPTION("Analog Devices MAX77857 Buck-Boost Converter
> >Driver");
> >> +MODULE_AUTHOR("Ibrahim Tilki <[email protected]>");
> >> +MODULE_AUTHOR("Okan Sahin <[email protected]>");
> >> +MODULE_LICENSE("GPL");
> >> --
> >> 2.30.2
> >>
>
> Hi Nathan,
>
> Should I need to fix it within the patch v4? Or should I sent another patch after merge?

I do not believe Mark rebases his tree so he would not take a v4. It
will need to be a separate patch on top.

Cheers,
Nathan

2023-07-18 17:50:01

[permalink] [raw]

Subject: Re: [PATCH v3 2/2] regulator: max77857: Add ADI MAX77857/59/MAX77831 Regulator Support

Attachments:

(No filename) (710.00 B)
signature.asc (499.00 B)
Download all attachments

2023-07-18 18:02:23

[permalink] [raw]

Subject: RE: [PATCH v3 2/2] regulator: max77857: Add ADI MAX77857/59/MAX77831 Regulator Support

>From: Nathan Chancellor <[email protected]>
>Sent: Tuesday, July 18, 2023 6:55 PM
>To: Sahin, Okan <[email protected]>
>Cc: Liam Girdwood <[email protected]>; Mark Brown <[email protected]>;
>Rob Herring <[email protected]>; Krzysztof Kozlowski
><[email protected]>; Conor Dooley <[email protected]>; Tilki,
>Ibrahim <[email protected]>; [email protected];
>[email protected]; [email protected]
>Subject: Re: [PATCH v3 2/2] regulator: max77857: Add ADI MAX77857/59/MAX77831
>Regulator Support
>
>[External]
>
>Hi Okan,
>
>On Mon, Jul 17, 2023 at 08:07:35AM +0300, Okan Sahin wrote:
>> Regulator driver for MAX77857/59 and MAX77831.
>> The MAX77857 is a high-efficiency, high-performance
>> buck-boost converter targeted for systems requiring
>> a wide input voltage range (2.5V to 16V).
>>
>> The MAX77859 is high-Efficiency Buck-Boost Converter
>> for USB-PD/PPS Applications. It has wide input range
>> (2.5V to 22V)
>>
>> The MAX77831 is a high-efficiency, high-performance
>> buck-boost converter targeted for systems requiring
>> wide input voltage range (2.5V to 16V).
>>
>> Signed-off-by: Okan Sahin <[email protected]>
>> ---
>> drivers/regulator/Kconfig | 10 +
>> drivers/regulator/Makefile | 1 +
>> drivers/regulator/max77857-regulator.c | 459 +++++++++++++++++++++++++
>> 3 files changed, 470 insertions(+)
>> create mode 100644 drivers/regulator/max77857-regulator.c
>>
>> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
>> index e5f3613c15fa..09eaa1cd90de 100644
>> --- a/drivers/regulator/Kconfig
>> +++ b/drivers/regulator/Kconfig
>> @@ -573,6 +573,16 @@ config REGULATOR_MAX77650
>> Semiconductor. This device has a SIMO with three independent
>> power rails and an LDO.
>>
>> +config REGULATOR_MAX77857
>> + tristate "ADI MAX77857/MAX77831 regulator support"
>> + depends on I2C
>> + select REGMAP_I2C
>> + help
>> + This driver controls a ADI MAX77857 and MAX77831 regulators.
>> + via I2C bus. MAX77857 and MAX77831 are high efficiency buck-boost
>> + converters with input voltage range (2.5V to 16V). Say Y here to
>> + enable the regulator driver
>> +
>> config REGULATOR_MAX8649
>> tristate "Maxim 8649 voltage regulator"
>> depends on I2C
>> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
>> index 58dfe0147cd4..e7230846b680 100644
>> --- a/drivers/regulator/Makefile
>> +++ b/drivers/regulator/Makefile
>> @@ -85,6 +85,7 @@ obj-$(CONFIG_REGULATOR_MAX77686) += max77686-
>regulator.o
>> obj-$(CONFIG_REGULATOR_MAX77693) += max77693-regulator.o
>> obj-$(CONFIG_REGULATOR_MAX77802) += max77802-regulator.o
>> obj-$(CONFIG_REGULATOR_MAX77826) += max77826-regulator.o
>> +obj-$(CONFIG_REGULATOR_MAX77857) += max77857-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
>> diff --git a/drivers/regulator/max77857-regulator.c b/drivers/regulator/max77857-
>regulator.c
>> new file mode 100644
>> index 000000000000..c5482ffd606e
>> --- /dev/null
>> +++ b/drivers/regulator/max77857-regulator.c
>> @@ -0,0 +1,459 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2023 Analog Devices, Inc.
>> + * ADI Regulator driver for the MAX77857
>> + * MAX77859 and MAX77831.
>> + */
>> +#include <linux/bitfield.h>
>> +#include <linux/i2c.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/module.h>
>> +#include <linux/regmap.h>
>> +#include <linux/regulator/driver.h>
>> +#include <linux/regulator/machine.h>
>> +#include <linux/regulator/of_regulator.h>
>> +#include <linux/util_macros.h>
>> +
>> +#define MAX77857_REG_INT_SRC 0x10
>> +#define MAX77857_REG_INT_MASK 0x11
>> +#define MAX77857_REG_CONT1 0x12
>> +#define MAX77857_REG_CONT2 0x13
>> +#define MAX77857_REG_CONT3 0x14
>> +
>> +#define MAX77857_INT_SRC_OCP BIT(0)
>> +#define MAX77857_INT_SRC_THS BIT(1)
>> +#define MAX77857_INT_SRC_HARDSHORT BIT(2)
>> +#define MAX77857_INT_SRC_OVP BIT(3)
>> +#define MAX77857_INT_SRC_POK BIT(4)
>> +
>> +#define MAX77857_ILIM_MASK GENMASK(2, 0)
>> +#define MAX77857_CONT1_FREQ GENMASK(4, 3)
>> +#define MAX77857_CONT3_FPWM BIT(5)
>> +
>> +#define MAX77859_REG_INT_SRC 0x11
>> +#define MAX77859_REG_CONT1 0x13
>> +#define MAX77859_REG_CONT2 0x14
>> +#define MAX77859_REG_CONT3 0x15
>> +#define MAX77859_REG_CONT5 0x17
>> +#define MAX77859_CONT2_FPWM BIT(2)
>> +#define MAX77859_CONT2_INTB BIT(3)
>> +#define MAX77859_CONT3_DVS_START BIT(2)
>> +#define MAX77859_VOLTAGE_SEL_MASK GENMASK(9, 0)
>> +
>> +#define MAX77859_CURRENT_MIN 1000000
>> +#define MAX77859_CURRENT_MAX 5000000
>> +#define MAX77859_CURRENT_STEP 50000
>> +
>> +enum max77857_id {
>> + ID_MAX77831 = 1,
>> + ID_MAX77857,
>> + ID_MAX77859,
>> + ID_MAX77859A,
>> +};
>> +
>> +static bool max77857_volatile_reg(struct device *dev, unsigned int reg)
>> +{
>> + enum max77857_id id = (enum max77857_id)dev_get_drvdata(dev);
>> +
>> + switch (id) {
>> + case ID_MAX77831:
>> + case ID_MAX77857:
>> + return reg == MAX77857_REG_INT_SRC;
>> + case ID_MAX77859:
>> + case ID_MAX77859A:
>> + return reg == MAX77859_REG_INT_SRC;
>> + default:
>> + return true;
>> + }
>> +}
>> +
>> +struct regmap_config max77857_regmap_config = {
>> + .reg_bits = 8,
>> + .val_bits = 8,
>> + .cache_type = REGCACHE_MAPLE,
>> + .volatile_reg = max77857_volatile_reg,
>> +};
>> +
>> +static int max77857_get_status(struct regulator_dev *rdev)
>> +{
>> + unsigned int val;
>> + int ret;
>> +
>> + ret = regmap_read(rdev->regmap, MAX77857_REG_INT_SRC, &val);
>> + if (ret)
>> + return ret;
>> +
>> + if (FIELD_GET(MAX77857_INT_SRC_POK, val))
>> + return REGULATOR_STATUS_ON;
>> +
>> + return REGULATOR_STATUS_ERROR;
>> +}
>> +
>> +static unsigned int max77857_get_mode(struct regulator_dev *rdev)
>> +{
>> + enum max77857_id id = (enum max77857_id)rdev_get_drvdata(rdev);
>> + unsigned int regval;
>> + int ret;
>> +
>> + switch (id) {
>> + case ID_MAX77831:
>> + case ID_MAX77857:
>> + ret = regmap_read(rdev->regmap, MAX77857_REG_CONT3,
>&regval);
>> + if (ret)
>> + return ret;
>> +
>> + if (FIELD_GET(MAX77857_CONT3_FPWM, regval))
>> + return REGULATOR_MODE_FAST;
>> +
>> + break;
>> + case ID_MAX77859:
>> + case ID_MAX77859A:
>> + ret = regmap_read(rdev->regmap, MAX77859_REG_CONT2,
>&regval);
>> + if (ret)
>> + return ret;
>> +
>> + if (FIELD_GET(MAX77859_CONT2_FPWM, regval))
>> + return REGULATOR_MODE_FAST;
>> +
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + return REGULATOR_MODE_NORMAL;
>> +}
>> +
>> +static int max77857_set_mode(struct regulator_dev *rdev, unsigned int mode)
>> +{
>> + enum max77857_id id = (enum max77857_id)rdev_get_drvdata(rdev);
>> + unsigned int reg, val;
>> +
>> + switch (id) {
>> + case ID_MAX77831:
>> + case ID_MAX77857:
>> + reg = MAX77857_REG_CONT3;
>> + val = MAX77857_CONT3_FPWM;
>> + break;
>> + case ID_MAX77859:
>> + case ID_MAX77859A:
>> + reg = MAX77859_REG_CONT2;
>> + val = MAX77859_CONT2_FPWM;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + switch (mode) {
>> + case REGULATOR_MODE_FAST:
>> + return regmap_set_bits(rdev->regmap, reg, val);
>> + case REGULATOR_MODE_NORMAL:
>> + return regmap_clear_bits(rdev->regmap, reg, val);
>> + default:
>> + return -EINVAL;
>> + }
>> +}
>> +
>> +static int max77857_get_error_flags(struct regulator_dev *rdev,
>> + unsigned int *flags)
>> +{
>> + unsigned int val;
>> + int ret;
>> +
>> + ret = regmap_read(rdev->regmap, MAX77857_REG_INT_SRC, &val);
>> + if (ret)
>> + return ret;
>> +
>> + *flags = 0;
>> +
>> + if (FIELD_GET(MAX77857_INT_SRC_OVP, val))
>> + *flags |= REGULATOR_ERROR_OVER_VOLTAGE_WARN;
>> +
>> + if (FIELD_GET(MAX77857_INT_SRC_OCP, val) ||
>> + FIELD_GET(MAX77857_INT_SRC_HARDSHORT, val))
>> + *flags |= REGULATOR_ERROR_OVER_CURRENT;
>> +
>> + if (FIELD_GET(MAX77857_INT_SRC_THS, val))
>> + *flags |= REGULATOR_ERROR_OVER_TEMP;
>> +
>> + if (!FIELD_GET(MAX77857_INT_SRC_POK, val))
>> + *flags |= REGULATOR_ERROR_FAIL;
>> +
>> + return 0;
>> +}
>> +
>> +static struct linear_range max77859_lin_ranges[] = {
>> + REGULATOR_LINEAR_RANGE(3200000, 0x0A0, 0x320, 20000)
>> +};
>> +
>> +static const unsigned int max77859_ramp_table[4] = {
>> + 1000, 500, 250, 125
>> +};
>> +
>> +static int max77859_set_voltage_sel(struct regulator_dev *rdev,
>> + unsigned int sel)
>> +{
>> + __be16 reg;
>> + int ret;
>> +
>> + reg = cpu_to_be16(sel);
>> +
>> + ret = regmap_bulk_write(rdev->regmap, MAX77859_REG_CONT3, &reg, 2);
>> + if (ret)
>> + return ret;
>> +
>> + /* actually apply new voltage */
>> + return regmap_set_bits(rdev->regmap, MAX77859_REG_CONT3,
>> + MAX77859_CONT3_DVS_START);
>> +}
>> +
>> +int max77859_get_voltage_sel(struct regulator_dev *rdev)
>> +{
>> + __be16 reg;
>> + int ret;
>> +
>> + ret = regmap_bulk_read(rdev->regmap, MAX77859_REG_CONT3, &reg, 2);
>> + if (ret)
>> + return ret;
>> +
>> + return FIELD_GET(MAX77859_VOLTAGE_SEL_MASK, __be16_to_cpu(reg));
>> +}
>> +
>> +int max77859_set_current_limit(struct regulator_dev *rdev, int min_uA, int
>max_uA)
>> +{
>> + u32 selector;
>> +
>> + if (max_uA < MAX77859_CURRENT_MIN)
>> + return -EINVAL;
>> +
>> + selector = 0x12 + (max_uA - MAX77859_CURRENT_MIN) /
>MAX77859_CURRENT_STEP;
>> +
>> + selector = clamp_val(selector, 0x00, 0x7F);
>> +
>> + return regmap_write(rdev->regmap, MAX77859_REG_CONT5, selector);
>> +}
>> +
>> +int max77859_get_current_limit(struct regulator_dev *rdev)
>> +{
>> + u32 selector;
>> + int ret;
>> +
>> + ret = regmap_read(rdev->regmap, MAX77859_REG_CONT5, &selector);
>> + if (ret)
>> + return ret;
>> +
>> + if (selector <= 0x12)
>> + return MAX77859_CURRENT_MIN;
>> +
>> + if (selector >= 0x64)
>> + return MAX77859_CURRENT_MAX;
>> +
>> + return MAX77859_CURRENT_MIN + (selector - 0x12) *
>MAX77859_CURRENT_STEP;
>> +}
>> +
>> +static const struct regulator_ops max77859_regulator_ops = {
>> + .list_voltage = regulator_list_voltage_linear_range,
>> + .set_voltage_sel = max77859_set_voltage_sel,
>> + .get_voltage_sel = max77859_get_voltage_sel,
>> + .set_ramp_delay = regulator_set_ramp_delay_regmap,
>> + .get_status = max77857_get_status,
>> + .set_mode = max77857_set_mode,
>> + .get_mode = max77857_get_mode,
>> + .get_error_flags = max77857_get_error_flags,
>> +};
>> +
>> +static const struct regulator_ops max77859a_regulator_ops = {
>> + .list_voltage = regulator_list_voltage_linear_range,
>> + .set_voltage_sel = max77859_set_voltage_sel,
>> + .get_voltage_sel = max77859_get_voltage_sel,
>> + .set_current_limit = max77859_set_current_limit,
>> + .get_current_limit = max77859_get_current_limit,
>> + .set_ramp_delay = regulator_set_ramp_delay_regmap,
>> + .get_status = max77857_get_status,
>> + .set_mode = max77857_set_mode,
>> + .get_mode = max77857_get_mode,
>> + .get_error_flags = max77857_get_error_flags,
>> +};
>> +
>> +static const struct regulator_ops max77857_regulator_ops = {
>> + .list_voltage = regulator_list_voltage_linear_range,
>> + .set_voltage_sel = regulator_set_voltage_sel_regmap,
>> + .get_voltage_sel = regulator_get_voltage_sel_regmap,
>> + .set_ramp_delay = regulator_set_ramp_delay_regmap,
>> + .get_status = max77857_get_status,
>> + .set_mode = max77857_set_mode,
>> + .get_mode = max77857_get_mode,
>> + .get_error_flags = max77857_get_error_flags,
>> +};
>> +
>> +static struct linear_range max77857_lin_ranges[] = {
>> + REGULATOR_LINEAR_RANGE(4485000, 0x3D, 0xCC, 73500)
>> +};
>> +
>> +static const unsigned int max77857_switch_freq[] = {
>> + 1200000, 1500000, 1800000, 2100000
>> +};
>> +
>> +static const unsigned int max77857_ramp_table[2][4] = {
>> + { 1333, 667, 333, 227 }, /* when switch freq is 1.8MHz or 2.1MHz */
>> + { 1166, 667, 333, 167 }, /* when switch freq is 1.2MHz or 1.5MHz */
>> +};
>> +
>> +static struct regulator_desc max77857_regulator_desc = {
>> + .ops = &max77857_regulator_ops,
>> + .name = "max77857",
>> + .linear_ranges = max77857_lin_ranges,
>> + .n_linear_ranges = ARRAY_SIZE(max77857_lin_ranges),
>> + .vsel_mask = 0xFF,
>> + .vsel_reg = MAX77857_REG_CONT2,
>> + .ramp_delay_table = max77857_ramp_table[0],
>> + .n_ramp_values = ARRAY_SIZE(max77857_ramp_table[0]),
>> + .ramp_reg = MAX77857_REG_CONT3,
>> + .ramp_mask = GENMASK(1, 0),
>> + .ramp_delay = max77857_ramp_table[0][0],
>
>This breaks the build with GCC 5.x through 7.x:
>
> drivers/regulator/max77857-regulator.c:312:16: error: initializer element is not
>constant
> .ramp_delay = max77857_ramp_table[0][0],
> ^~~~~~~~~~~~~~~~~~~
> drivers/regulator/max77857-regulator.c:312:16: note: (near initialization for
>'max77857_regulator_desc.ramp_delay')
>
>and clang:
>
> drivers/regulator/max77857-regulator.c:312:16: error: initializer element is not a
>compile-time constant
> 312 | .ramp_delay = max77857_ramp_table[0][0],
> | ^~~~~~~~~~~~~~~~~~~~~~~~~
> 1 error generated.
>
>This relies on a GCC 8.x+ change that accepts more things as
>compile-time constants, which is being worked on in clang
>(https://urldefense.com/v3/__https://reviews.llvm.org/D76096__;!!A3Ni8CS0y2Y!4ql
>noZFf_EVInN-
>MaRDQWqOHb1SEbEqkwlU06PCt1Ngw6tE41ZEE24hnL1wBMsfotRCue4-i1VwD0xw$
>). Since the kernel supports older
>compilers, this will have to be worked around somehow. Perhaps a define
>that can be used in both places?
>
>Cheers,
>Nathan
>
>> + .owner = THIS_MODULE,
>> +};
>> +
>> +static void max77857_calc_range(struct device *dev, enum max77857_id id)
>> +{
>> + struct linear_range *range;
>> + unsigned long vref_step;
>> + u32 rtop = 0;
>> + u32 rbot = 0;
>> +
>> + device_property_read_u32(dev, "adi,rtop-ohms", &rtop);
>> + device_property_read_u32(dev, "adi,rbot-ohms", &rbot);
>> +
>> + if (!rbot || !rtop)
>> + return;
>> +
>> + switch (id) {
>> + case ID_MAX77831:
>> + case ID_MAX77857:
>> + range = max77857_lin_ranges;
>> + vref_step = 4900UL;
>> + break;
>> + case ID_MAX77859:
>> + case ID_MAX77859A:
>> + range = max77859_lin_ranges;
>> + vref_step = 1250UL;
>> + break;
>> + }
>> +
>> + range->step = DIV_ROUND_CLOSEST(vref_step * (rbot + rtop), rbot);
>> + range->min = range->step * range->min_sel;
>> +}
>> +
>> +static int max77857_probe(struct i2c_client *client)
>> +{
>> + const struct i2c_device_id *i2c_id;
>> + struct device *dev = &client->dev;
>> + struct regulator_config cfg = { };
>> + struct regulator_dev *rdev;
>> + struct regmap *regmap;
>> + enum max77857_id id;
>> + u32 switch_freq = 0;
>> + int ret;
>> +
>> + i2c_id = i2c_client_get_device_id(client);
>> + if (!i2c_id)
>> + return -EINVAL;
>> +
>> + id = i2c_id->driver_data;
>> +
>> + dev_set_drvdata(dev, (void *)id);
>> +
>> + if (id == ID_MAX77859 || id == ID_MAX77859A) {
>> + max77857_regulator_desc.ops = &max77859_regulator_ops;
>> + max77857_regulator_desc.linear_ranges = max77859_lin_ranges;
>> + max77857_regulator_desc.ramp_delay_table =
>max77859_ramp_table;
>> + max77857_regulator_desc.ramp_delay = max77859_ramp_table[0];
>> + }
>> +
>> + if (id == ID_MAX77859A)
>> + max77857_regulator_desc.ops = &max77859a_regulator_ops;
>> +
>> + max77857_calc_range(dev, id);
>> +
>> + regmap = devm_regmap_init_i2c(client, &max77857_regmap_config);
>> + if (IS_ERR(regmap))
>> + return dev_err_probe(dev, PTR_ERR(regmap),
>> + "cannot initialize regmap\n");
>> +
>> + device_property_read_u32(dev, "adi,switch-frequency-hz", &switch_freq);
>> + if (switch_freq) {
>> + switch_freq = find_closest(switch_freq, max77857_switch_freq,
>> + ARRAY_SIZE(max77857_switch_freq));
>> +
>> + if (id == ID_MAX77831 && switch_freq == 3)
>> + switch_freq = 2;
>> +
>> + switch (id) {
>> + case ID_MAX77831:
>> + case ID_MAX77857:
>> + ret = regmap_update_bits(regmap, MAX77857_REG_CONT1,
>> + MAX77857_CONT1_FREQ,
>switch_freq);
>> +
>> + if (switch_freq >= 2)
>> + break;
>> +
>> + max77857_regulator_desc.ramp_delay_table =
>max77857_ramp_table[1];
>> + max77857_regulator_desc.ramp_delay =
>max77857_ramp_table[1][0];
>> + break;
>> + case ID_MAX77859:
>> + case ID_MAX77859A:
>> + ret = regmap_update_bits(regmap, MAX77859_REG_CONT1,
>> + MAX77857_CONT1_FREQ,
>switch_freq);
>> + break;
>> + }
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + cfg.dev = dev;
>> + cfg.driver_data = (void *)id;
>> + cfg.regmap = regmap;
>> + cfg.init_data = of_get_regulator_init_data(dev, dev->of_node,
>> + &max77857_regulator_desc);
>> + if (!cfg.init_data)
>> + return -ENOMEM;
>> +
>> + rdev = devm_regulator_register(dev, &max77857_regulator_desc, &cfg);
>> + if (IS_ERR(rdev))
>> + return dev_err_probe(dev, PTR_ERR(rdev),
>> + "cannot register regulator\n");
>> +
>> + return 0;
>> +}
>> +
>> +const struct i2c_device_id max77857_id[] = {
>> + { "max77831", ID_MAX77831 },
>> + { "max77857", ID_MAX77857 },
>> + { "max77859", ID_MAX77859 },
>> + { "max77859a", ID_MAX77859A },
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(i2c, max77857_id);
>> +
>> +static const struct of_device_id max77857_of_id[] = {
>> + { .compatible = "adi,max77831", .data = (void *)ID_MAX77831 },
>> + { .compatible = "adi,max77857", .data = (void *)ID_MAX77857 },
>> + { .compatible = "adi,max77859", .data = (void *)ID_MAX77859 },
>> + { .compatible = "adi,max77859a", .data = (void *)ID_MAX77859A },
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(of, max77857_of_id);
>> +
>> +struct i2c_driver max77857_driver = {
>> + .driver = {
>> + .name = "max77857",
>> + .of_match_table = max77857_of_id,
>> + },
>> + .id_table = max77857_id,
>> + .probe_new = max77857_probe,
>> +};
>> +module_i2c_driver(max77857_driver);
>> +
>> +MODULE_DESCRIPTION("Analog Devices MAX77857 Buck-Boost Converter
>Driver");
>> +MODULE_AUTHOR("Ibrahim Tilki <[email protected]>");
>> +MODULE_AUTHOR("Okan Sahin <[email protected]>");
>> +MODULE_LICENSE("GPL");
>> --
>> 2.30.2
>>

Hi Nathan,

Should I need to fix it within the patch v4? Or should I sent another patch after merge?

Regards,
Okan Sahin

2023-07-26 16:42:19

[permalink] [raw]

Subject: Re: [PATCH v3 2/2] regulator: max77857: Add ADI MAX77857/59/MAX77831 Regulator Support

Hi Okan,

On Tue, Jul 18, 2023 at 08:55:02AM -0700, Nathan Chancellor wrote:

<snip>

> > +static struct regulator_desc max77857_regulator_desc = {
> > + .ops = &max77857_regulator_ops,
> > + .name = "max77857",
> > + .linear_ranges = max77857_lin_ranges,
> > + .n_linear_ranges = ARRAY_SIZE(max77857_lin_ranges),
> > + .vsel_mask = 0xFF,
> > + .vsel_reg = MAX77857_REG_CONT2,
> > + .ramp_delay_table = max77857_ramp_table[0],
> > + .n_ramp_values = ARRAY_SIZE(max77857_ramp_table[0]),
> > + .ramp_reg = MAX77857_REG_CONT3,
> > + .ramp_mask = GENMASK(1, 0),
> > + .ramp_delay = max77857_ramp_table[0][0],
>
> This breaks the build with GCC 5.x through 7.x:
>
> drivers/regulator/max77857-regulator.c:312:16: error: initializer element is not constant
> .ramp_delay = max77857_ramp_table[0][0],
> ^~~~~~~~~~~~~~~~~~~
> drivers/regulator/max77857-regulator.c:312:16: note: (near initialization for 'max77857_regulator_desc.ramp_delay')
>
> and clang:
>
> drivers/regulator/max77857-regulator.c:312:16: error: initializer element is not a compile-time constant
> 312 | .ramp_delay = max77857_ramp_table[0][0],
> | ^~~~~~~~~~~~~~~~~~~~~~~~~
> 1 error generated.
>
> This relies on a GCC 8.x+ change that accepts more things as
> compile-time constants, which is being worked on in clang
> (https://reviews.llvm.org/D76096). Since the kernel supports older
> compilers, this will have to be worked around somehow. Perhaps a define
> that can be used in both places?

Was there any update on this? I do not mind sending a patch for this
myself if I have some sort of guidance on how you would prefer for this
to be fixed, should you be too busy to look into it.

Cheers,
Nathan

2023-07-27 09:19:46

[permalink] [raw]

Subject: RE: [PATCH v3 2/2] regulator: max77857: Add ADI MAX77857/59/MAX77831 Regulator Support

>-----Original Message-----
>From: Nathan Chancellor <[email protected]>
>Sent: Wednesday, July 26, 2023 7:11 PM
>To: Sahin, Okan <[email protected]>
>Cc: Liam Girdwood <[email protected]>; Mark Brown <[email protected]>;
>Rob Herring <[email protected]>; Krzysztof Kozlowski
><[email protected]>; Conor Dooley <[email protected]>;
>zzzzTilki, zzzzIbrahim <[email protected]>; [email protected];
>[email protected]; [email protected]
>Subject: Re: [PATCH v3 2/2] regulator: max77857: Add ADI MAX77857/59/MAX77831
>Regulator Support
>
>[External]
>
>Hi Okan,
>
>On Tue, Jul 18, 2023 at 08:55:02AM -0700, Nathan Chancellor wrote:
>
><snip>
>
>> > +static struct regulator_desc max77857_regulator_desc = {
>> > + .ops = &max77857_regulator_ops,
>> > + .name = "max77857",
>> > + .linear_ranges = max77857_lin_ranges,
>> > + .n_linear_ranges = ARRAY_SIZE(max77857_lin_ranges),
>> > + .vsel_mask = 0xFF,
>> > + .vsel_reg = MAX77857_REG_CONT2,
>> > + .ramp_delay_table = max77857_ramp_table[0],
>> > + .n_ramp_values = ARRAY_SIZE(max77857_ramp_table[0]),
>> > + .ramp_reg = MAX77857_REG_CONT3,
>> > + .ramp_mask = GENMASK(1, 0),
>> > + .ramp_delay = max77857_ramp_table[0][0],
>>
>> This breaks the build with GCC 5.x through 7.x:
>>
>> drivers/regulator/max77857-regulator.c:312:16: error: initializer element is not
>constant
>> .ramp_delay = max77857_ramp_table[0][0],
>> ^~~~~~~~~~~~~~~~~~~
>> drivers/regulator/max77857-regulator.c:312:16: note: (near initialization for
>'max77857_regulator_desc.ramp_delay')
>>
>> and clang:
>>
>> drivers/regulator/max77857-regulator.c:312:16: error: initializer element is not a
>compile-time constant
>> 312 | .ramp_delay = max77857_ramp_table[0][0],
>> | ^~~~~~~~~~~~~~~~~~~~~~~~~
>> 1 error generated.
>>
>> This relies on a GCC 8.x+ change that accepts more things as
>> compile-time constants, which is being worked on in clang
>>
>(https://urldefense.com/v3/__https://reviews.llvm.org/D76096__;!!A3Ni8CS0y2Y!7B
>eWxuzHgLzOprQA_madbvdR7hd0ZgmS73lUlDbgoxWUFWdDSIRXLnhyqLeRhu3uTaqpS
>kzZKwc5pHA$ ). Since the kernel supports older
>> compilers, this will have to be worked around somehow. Perhaps a define
>> that can be used in both places?
>
>Was there any update on this? I do not mind sending a patch for this
>myself if I have some sort of guidance on how you would prefer for this
>to be fixed, should you be too busy to look into it.
>
>Cheers,
>Nathan

Hi Nathan,

I thought that I should fix this issue after merging main branch that's why I did not send patch.
I sent patch v3 so should I send new patch as v4?

Regards,
Okan Sahin

2023-07-27 15:30:55

[permalink] [raw]

Subject: Re: [PATCH v3 2/2] regulator: max77857: Add ADI MAX77857/59/MAX77831 Regulator Support

On Thu, Jul 27, 2023 at 08:34:44AM +0000, Sahin, Okan wrote:
> >On Tue, Jul 18, 2023 at 08:55:02AM -0700, Nathan Chancellor wrote:
> >
> ><snip>
> >
> >> > +static struct regulator_desc max77857_regulator_desc = {
> >> > + .ops = &max77857_regulator_ops,
> >> > + .name = "max77857",
> >> > + .linear_ranges = max77857_lin_ranges,
> >> > + .n_linear_ranges = ARRAY_SIZE(max77857_lin_ranges),
> >> > + .vsel_mask = 0xFF,
> >> > + .vsel_reg = MAX77857_REG_CONT2,
> >> > + .ramp_delay_table = max77857_ramp_table[0],
> >> > + .n_ramp_values = ARRAY_SIZE(max77857_ramp_table[0]),
> >> > + .ramp_reg = MAX77857_REG_CONT3,
> >> > + .ramp_mask = GENMASK(1, 0),
> >> > + .ramp_delay = max77857_ramp_table[0][0],
> >>
> >> This breaks the build with GCC 5.x through 7.x:
> >>
> >> drivers/regulator/max77857-regulator.c:312:16: error: initializer element is not
> >constant
> >> .ramp_delay = max77857_ramp_table[0][0],
> >> ^~~~~~~~~~~~~~~~~~~
> >> drivers/regulator/max77857-regulator.c:312:16: note: (near initialization for
> >'max77857_regulator_desc.ramp_delay')
> >>
> >> and clang:
> >>
> >> drivers/regulator/max77857-regulator.c:312:16: error: initializer element is not a
> >compile-time constant
> >> 312 | .ramp_delay = max77857_ramp_table[0][0],
> >> | ^~~~~~~~~~~~~~~~~~~~~~~~~
> >> 1 error generated.
> >>
> >> This relies on a GCC 8.x+ change that accepts more things as
> >> compile-time constants, which is being worked on in clang
> >>
> >(https://urldefense.com/v3/__https://reviews.llvm.org/D76096__;!!A3Ni8CS0y2Y!7B
> >eWxuzHgLzOprQA_madbvdR7hd0ZgmS73lUlDbgoxWUFWdDSIRXLnhyqLeRhu3uTaqpS
> >kzZKwc5pHA$ ). Since the kernel supports older
> >> compilers, this will have to be worked around somehow. Perhaps a define
> >> that can be used in both places?
> >
> >Was there any update on this? I do not mind sending a patch for this
> >myself if I have some sort of guidance on how you would prefer for this
> >to be fixed, should you be too busy to look into it.
> >
> >Cheers,
> >Nathan
>
> Hi Nathan,
>
> I thought that I should fix this issue after merging main branch that's why I did not send patch.

That is an understandable position but no, this issue should be fixed
before this change makes its way to Linus, not after.

> I sent patch v3 so should I send new patch as v4?

No, you should checkout Mark's branch that contains your patch and send
a new patch on top of it just fixing this issue, like the other two
patches that have already touched this driver:

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git/log/?h=for-6.6

https://git.kernel.org/broonie/regulator/c/2920e08bef609c8b59f9996fd6852a7b97119d75
https://git.kernel.org/broonie/regulator/c/541e75954cadde0355ce7bebed5675625b2943a8

There are GCC 7.x and earlier toolchains at
https://kernel.org/pub/tools/crosstool/ and LLVM toolchains at
https://kernel.org/pub/tools/llvm/ should need to reproduce and verify
the fix.

Cheers,
Nathan

2023-08-02 23:45:11

[permalink] [raw]

Subject: Re: [PATCH v3 2/2] regulator: max77857: Add ADI MAX77857/59/MAX77831 Regulator Support

On Wed, Aug 02, 2023 at 03:52:52PM -0700, Nick Desaulniers wrote:
> Hi Okan,
> Have you sent a follow up fix? The build should not remain broken for
> so long. Otherwise I think Broonie should drop your patch.

Someone sent what's probably a fix but I was waiting for some
confirmation that the change actually works on hardware, it's not super
obvious.

Attachments:

(No filename) (363.00 B)
signature.asc (499.00 B)
Download all attachments

2023-08-03 00:09:10

[permalink] [raw]

Subject: Re: [PATCH v3 2/2] regulator: max77857: Add ADI MAX77857/59/MAX77831 Regulator Support

On Wed, Aug 02, 2023 at 04:04:26PM -0700, Nathan Chancellor wrote:
> On Thu, Aug 03, 2023 at 12:02:38AM +0100, Mark Brown wrote:
> > On Wed, Aug 02, 2023 at 03:52:52PM -0700, Nick Desaulniers wrote:
> > > Hi Okan,
> > > Have you sent a follow up fix? The build should not remain broken for
> > > so long. Otherwise I think Broonie should drop your patch.
> >
> > Someone sent what's probably a fix but I was waiting for some
> > confirmation that the change actually works on hardware, it's not super
> > obvious.
>
> Got a pointer? I don't see anything on lore:
>
> https://lore.kernel.org/all/?q=dfn:drivers/regulator/max77857-regulator.c

Oh, they didn't actuallly send it to the list :(

Attachments:

(No filename) (711.00 B)
signature.asc (499.00 B)
Download all attachments

2023-08-03 00:10:16

by Nick Desaulniers

[permalink] [raw]

Subject: Re: [PATCH v3 2/2] regulator: max77857: Add ADI MAX77857/59/MAX77831 Regulator Support

Hi Okan,
Have you sent a follow up fix? The build should not remain broken for
so long. Otherwise I think Broonie should drop your patch.

On Thu, Jul 27, 2023 at 7:51 AM Nathan Chancellor <[email protected]> wrote:
>
> On Thu, Jul 27, 2023 at 08:34:44AM +0000, Sahin, Okan wrote:
> > >On Tue, Jul 18, 2023 at 08:55:02AM -0700, Nathan Chancellor wrote:
> > >
> > ><snip>
> > >
> > >> > +static struct regulator_desc max77857_regulator_desc = {
> > >> > + .ops = &max77857_regulator_ops,
> > >> > + .name = "max77857",
> > >> > + .linear_ranges = max77857_lin_ranges,
> > >> > + .n_linear_ranges = ARRAY_SIZE(max77857_lin_ranges),
> > >> > + .vsel_mask = 0xFF,
> > >> > + .vsel_reg = MAX77857_REG_CONT2,
> > >> > + .ramp_delay_table = max77857_ramp_table[0],
> > >> > + .n_ramp_values = ARRAY_SIZE(max77857_ramp_table[0]),
> > >> > + .ramp_reg = MAX77857_REG_CONT3,
> > >> > + .ramp_mask = GENMASK(1, 0),
> > >> > + .ramp_delay = max77857_ramp_table[0][0],
> > >>
> > >> This breaks the build with GCC 5.x through 7.x:
> > >>
> > >> drivers/regulator/max77857-regulator.c:312:16: error: initializer element is not
> > >constant
> > >> .ramp_delay = max77857_ramp_table[0][0],
> > >> ^~~~~~~~~~~~~~~~~~~
> > >> drivers/regulator/max77857-regulator.c:312:16: note: (near initialization for
> > >'max77857_regulator_desc.ramp_delay')
> > >>
> > >> and clang:
> > >>
> > >> drivers/regulator/max77857-regulator.c:312:16: error: initializer element is not a
> > >compile-time constant
> > >> 312 | .ramp_delay = max77857_ramp_table[0][0],
> > >> | ^~~~~~~~~~~~~~~~~~~~~~~~~
> > >> 1 error generated.
> > >>
> > >> This relies on a GCC 8.x+ change that accepts more things as
> > >> compile-time constants, which is being worked on in clang
> > >>
> > >(https://urldefense.com/v3/__https://reviews.llvm.org/D76096__;!!A3Ni8CS0y2Y!7B
> > >eWxuzHgLzOprQA_madbvdR7hd0ZgmS73lUlDbgoxWUFWdDSIRXLnhyqLeRhu3uTaqpS
> > >kzZKwc5pHA$ ). Since the kernel supports older
> > >> compilers, this will have to be worked around somehow. Perhaps a define
> > >> that can be used in both places?
> > >
> > >Was there any update on this? I do not mind sending a patch for this
> > >myself if I have some sort of guidance on how you would prefer for this
> > >to be fixed, should you be too busy to look into it.
> > >
> > >Cheers,
> > >Nathan
> >
> > Hi Nathan,
> >
> > I thought that I should fix this issue after merging main branch that's why I did not send patch.
>
> That is an understandable position but no, this issue should be fixed
> before this change makes its way to Linus, not after.
>
> > I sent patch v3 so should I send new patch as v4?
>
> No, you should checkout Mark's branch that contains your patch and send
> a new patch on top of it just fixing this issue, like the other two
> patches that have already touched this driver:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git/log/?h=for-6.6
>
> https://git.kernel.org/broonie/regulator/c/2920e08bef609c8b59f9996fd6852a7b97119d75
> https://git.kernel.org/broonie/regulator/c/541e75954cadde0355ce7bebed5675625b2943a8
>
> There are GCC 7.x and earlier toolchains at
> https://kernel.org/pub/tools/crosstool/ and LLVM toolchains at
> https://kernel.org/pub/tools/llvm/ should need to reproduce and verify
> the fix.
>
> Cheers,
> Nathan
>

--
Thanks,
~Nick Desaulniers

2023-08-03 00:11:20

[permalink] [raw]

Subject: Re: [PATCH v3 2/2] regulator: max77857: Add ADI MAX77857/59/MAX77831 Regulator Support

On Thu, Aug 03, 2023 at 12:02:38AM +0100, Mark Brown wrote:
> On Wed, Aug 02, 2023 at 03:52:52PM -0700, Nick Desaulniers wrote:
> > Hi Okan,
> > Have you sent a follow up fix? The build should not remain broken for
> > so long. Otherwise I think Broonie should drop your patch.
>
> Someone sent what's probably a fix but I was waiting for some
> confirmation that the change actually works on hardware, it's not super
> obvious.

Got a pointer? I don't see anything on lore:

https://lore.kernel.org/all/?q=dfn:drivers/regulator/max77857-regulator.c

Cheers,
Nathan

2023-08-03 11:00:27

[permalink] [raw]

Subject: RE: [PATCH v3 2/2] regulator: max77857: Add ADI MAX77857/59/MAX77831 Regulator Support

02, 2023 at 04:04:26PM -0700, Nathan Chancellor wrote:
>> On Thu, Aug 03, 2023 at 12:02:38AM +0100, Mark Brown wrote:
>> > On Wed, Aug 02, 2023 at 03:52:52PM -0700, Nick Desaulniers wrote:
>> > > Hi Okan,
>> > > Have you sent a follow up fix? The build should not remain broken for
>> > > so long. Otherwise I think Broonie should drop your patch.
>> >
>> > Someone sent what's probably a fix but I was waiting for some
>> > confirmation that the change actually works on hardware, it's not super
>> > obvious.
>>
>> Got a pointer? I don't see anything on lore:
>>
>> https://lore.kernel.org/all/?q=dfn:drivers/regulator/max77857-regulator.c
>
>Oh, they didn't actuallly send it to the list :(

Hi Mark and Nathan,

Actually, I have received email about fix before I send new patch. As far as I understand, it was not sent to correct list, right?

Regards,
Okan Sahin

2023-08-03 12:44:44

[permalink] [raw]

Subject: RE: [PATCH v3 2/2] regulator: max77857: Add ADI MAX77857/59/MAX77831 Regulator Support

>
>On Thu, Aug 03, 2023 at 09:10:26AM +0000, Sahin, Okan wrote:
>> 02, 2023 at 04:04:26PM -0700, Nathan Chancellor wrote:
>
>> >Oh, they didn't actuallly send it to the list :(
>
>> Actually, I have received email about fix before I send new patch. As
>> far as I understand, it was not sent to correct list, right?
>
>It wasn't sent to any list.

I see. Let me clone your regulator repository, and send you a new patch.

2023-08-03 12:57:46