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

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

[Nathan Chancellor]

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 <okan.sahin@analog.com>
> ---
>  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 <Ibrahim.Tilki@analog.com>");
> +MODULE_AUTHOR("Okan Sahin <Okan.Sahin@analog.com>");
> +MODULE_LICENSE("GPL");
> -- 
> 2.30.2
> 

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

[Sahin, Okan]

>From: Nathan Chancellor <nathan@kernel.org>
>Sent: Tuesday, July 18, 2023 6:55 PM
>To: Sahin, Okan <Okan.Sahin@analog.com>
>Cc: Liam Girdwood <lgirdwood@gmail.com>; Mark Brown <broonie@kernel.org>;
>Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski
><krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; Tilki,
>Ibrahim <Ibrahim.Tilki@analog.com>; linux-kernel@vger.kernel.org;
>devicetree@vger.kernel.org; llvm@lists.linux.dev
>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 <okan.sahin@analog.com>
>> ---
>>  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 <Ibrahim.Tilki@analog.com>");
>> +MODULE_AUTHOR("Okan Sahin <Okan.Sahin@analog.com>");
>> +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

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

[Nathan Chancellor]

On Tue, Jul 18, 2023 at 05:25:32PM +0000, Sahin, Okan wrote:
> >From: Nathan Chancellor <nathan@kernel.org>
> >Sent: Tuesday, July 18, 2023 6:55 PM
> >To: Sahin, Okan <Okan.Sahin@analog.com>
> >Cc: Liam Girdwood <lgirdwood@gmail.com>; Mark Brown <broonie@kernel.org>;
> >Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski
> ><krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; Tilki,
> >Ibrahim <Ibrahim.Tilki@analog.com>; linux-kernel@vger.kernel.org;
> >devicetree@vger.kernel.org; llvm@lists.linux.dev
> >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 <okan.sahin@analog.com>
> >> ---
> >>  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 <Ibrahim.Tilki@analog.com>");
> >> +MODULE_AUTHOR("Okan Sahin <Okan.Sahin@analog.com>");
> >> +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

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

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 697 bytes --]

On Tue, Jul 18, 2023 at 05:25:32PM +0000, Sahin, Okan wrote:
> >From: Nathan Chancellor <nathan@kernel.org>
> >Sent: Tuesday, July 18, 2023 6:55 PM
> >To: Sahin, Okan <Okan.Sahin@analog.com>
> >Cc: Liam Girdwood <lgirdwood@gmail.com>; Mark Brown <broonie@kernel.org>;
> >Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski
> ><krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; Tilki,
> >Ibrahim <Ibrahim.Tilki@analog.com>; linux-kernel@vger.kernel.org;

Please delete unneeded context from mails when replying.  Doing this
makes it much easier to find your reply in the message, helping ensure
it won't be missed by people scrolling through the irrelevant quoted
material.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Nathan Chancellor]

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

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

[Sahin, Okan]

>-----Original Message-----
>From: Nathan Chancellor <nathan@kernel.org>
>Sent: Wednesday, July 26, 2023 7:11 PM
>To: Sahin, Okan <Okan.Sahin@analog.com>
>Cc: Liam Girdwood <lgirdwood@gmail.com>; Mark Brown <broonie@kernel.org>;
>Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski
><krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>;
>zzzzTilki, zzzzIbrahim <Ibrahim.Tilki@analog.com>; linux-kernel@vger.kernel.org;
>devicetree@vger.kernel.org; llvm@lists.linux.dev
>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

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

[Nathan Chancellor]

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

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

[Nick Desaulniers]

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 <nathan@kernel.org> 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

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

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 355 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Nathan Chancellor]

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

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

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 711 bytes --]

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 :(

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Sahin, Okan]

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

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

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 346 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Sahin, Okan]

>
>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.