[v3,2/2] mfd: max597x: Add support for MAX5970 and MAX5978

Message ID 20221031204129.1434059-3-Naresh.Solanki@9elements.com
State New
Headers
Series mfd: max597x: Add support for max597x |

Commit Message

Naresh Solanki Oct. 31, 2022, 8:41 p.m. UTC
  From: Patrick Rudolph <patrick.rudolph@9elements.com>

Implement a regulator driver with IRQ support for fault management.
Written against documentation [1] and [2] and tested on real hardware.

Every channel has its own regulator supplies nammed 'vss1-supply' and
'vss2-supply'. The regulator supply is used to determine the output
voltage, as the smart switch provides no output regulation.
The driver requires the 'shunt-resistor-micro-ohms' property to be
present in Device Tree to properly calculate current related
values.

Datasheet links:
1: https://datasheets.maximintegrated.com/en/ds/MAX5970.pdf
2: https://datasheets.maximintegrated.com/en/ds/MAX5978.pdf

Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
---
 drivers/mfd/Kconfig         |  12 +++++
 drivers/mfd/Makefile        |   1 +
 drivers/mfd/max597x.c       |  87 ++++++++++++++++++++++++++++++
 include/linux/mfd/max597x.h | 102 ++++++++++++++++++++++++++++++++++++
 4 files changed, 202 insertions(+)
 create mode 100644 drivers/mfd/max597x.c
 create mode 100644 include/linux/mfd/max597x.h
  

Comments

Christophe JAILLET Oct. 31, 2022, 9:19 p.m. UTC | #1
Le 31/10/2022 à 21:41, Naresh Solanki a écrit :
> From: Patrick Rudolph <patrick.rudolph@9elements.com>
> 
> Implement a regulator driver with IRQ support for fault management.
> Written against documentation [1] and [2] and tested on real hardware.
> 
> Every channel has its own regulator supplies nammed 'vss1-supply' and
> 'vss2-supply'. The regulator supply is used to determine the output
> voltage, as the smart switch provides no output regulation.
> The driver requires the 'shunt-resistor-micro-ohms' property to be
> present in Device Tree to properly calculate current related
> values.
> 
> Datasheet links:
> 1: https://datasheets.maximintegrated.com/en/ds/MAX5970.pdf
> 2: https://datasheets.maximintegrated.com/en/ds/MAX5978.pdf
> 
> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>

Hi,
a few nits below.

> ---
>   drivers/mfd/Kconfig         |  12 +++++
>   drivers/mfd/Makefile        |   1 +
>   drivers/mfd/max597x.c       |  87 ++++++++++++++++++++++++++++++
>   include/linux/mfd/max597x.h | 102 ++++++++++++++++++++++++++++++++++++
>   4 files changed, 202 insertions(+)
>   create mode 100644 drivers/mfd/max597x.c
>   create mode 100644 include/linux/mfd/max597x.h
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 8b93856de432..416fe7986b7b 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -253,6 +253,18 @@ config MFD_MADERA_SPI
>   	  Support for the Cirrus Logic Madera platform audio SoC
>   	  core functionality controlled via SPI.
>   
> +config MFD_MAX597X
> +	tristate "Maxim 597x Power Switch and Monitor"
> +	depends on I2C
> +	depends on OF
> +	select MFD_CORE
> +	select REGMAP_I2C
> +	help
> +	  This driver controls a Maxim 5970/5978 switch via I2C bus.
> +	  The MAX5970/5978 is a smart switch with no output regulation, but
> +	  fault protection and voltage and current monitoring capabilities.
> +	  Also it supports upto 4 indication LEDs.
> +
>   config MFD_CS47L15
>   	bool "Cirrus Logic CS47L15"
>   	select PINCTRL_CS47L15
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 7ed3ef4a698c..819d711fa748 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -161,6 +161,7 @@ obj-$(CONFIG_MFD_DA9063)	+= da9063.o
>   obj-$(CONFIG_MFD_DA9150)	+= da9150-core.o
>   
>   obj-$(CONFIG_MFD_MAX14577)	+= max14577.o
> +obj-$(CONFIG_MFD_MAX597X)	+= max597x.o
>   obj-$(CONFIG_MFD_MAX77620)	+= max77620.o
>   obj-$(CONFIG_MFD_MAX77650)	+= max77650.o
>   obj-$(CONFIG_MFD_MAX77686)	+= max77686.o
> diff --git a/drivers/mfd/max597x.c b/drivers/mfd/max597x.c
> new file mode 100644
> index 000000000000..68a4e7755f21
> --- /dev/null
> +++ b/drivers/mfd/max597x.c
> @@ -0,0 +1,87 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Maxim MAX5970/MAX5978 MFD Driver
> + *
> + * Copyright (c) 2022 9elements GmbH
> + *
> + * Author: Patrick Rudolph <patrick.rudolph@9elements.com>
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/max597x.h>
> +#include <linux/regmap.h>
> +
> +static const struct regmap_config max597x_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = MAX_REGISTERS,
> +};
> +
> +static const struct mfd_cell max597x_cells[] = {
> +	{ .name = "max597x-regulator", },
> +	{ .name = "max597x-iio", },
> +	{ .name = "max597x-led", },
> +};
> +
> +static int max597x_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
> +{
> +	struct max597x_data *ddata;
> +	enum max597x_chip_type chip = id->driver_data;
> +
> +	ddata = devm_kzalloc(&i2c->dev, sizeof(*ddata),
> +			GFP_KERNEL);

if (!ddata)
	return -ENOMEM;

GFP_KERNEL should be aligned with the ( on the previous line.

> +
> +	switch (chip) {
> +	case MAX597x_TYPE_MAX5970:
> +		ddata->num_switches = 2;
> +		break;
> +	case MAX597x_TYPE_MAX5978:
> +		ddata->num_switches = 1;
> +		break;
> +	}

ddata->num_switches looks unused.
Is it needed? Maybe for future use?

Should MAX5970_NUM_SWITCHES and MAX5979_NUM_SWITCHES be used instead of 
hard coding 1 and 2?

> +
> +	ddata->regmap = devm_regmap_init_i2c(i2c, &max597x_regmap_config);
> +	if (IS_ERR(ddata->regmap)) {
> +		dev_err(&i2c->dev, "Failed to initialise regmap");
> +		return -EINVAL;
> +	}
> +
> +	/* IRQ used by regulator cell */
> +	ddata->irq = i2c->irq;
> +	ddata->dev = &i2c->dev;
> +	i2c_set_clientdata(i2c, ddata);
> +
> +	return devm_mfd_add_devices(ddata->dev, PLATFORM_DEVID_AUTO,
> +				    max597x_cells, ARRAY_SIZE(max597x_cells),
> +				    NULL, 0, NULL);
> +}
> +
> +static const struct i2c_device_id max597x_table[] = {
> +	{ .name = "max5970", MAX597x_TYPE_MAX5970 },
> +	{ .name = "max5978", MAX597x_TYPE_MAX5978 },
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, max597x_table);
> +
> +static const struct of_device_id max597x_of_match[] = {
> +	{ .compatible = "maxim,max5970", .data = (void *)MAX597x_TYPE_MAX5970 },
> +	{ .compatible = "maxim,max5978", .data = (void *)MAX597x_TYPE_MAX5978 },
> +};
> +
> +MODULE_DEVICE_TABLE(of, max597x_of_match);
> +
> +static struct i2c_driver max597x_driver = {
> +	.id_table = max597x_table,
> +	.driver = {
> +		.name = "max597x",
> +		.of_match_table = of_match_ptr(max597x_of_match),
> +		},
> +	.probe = max597x_probe,
> +};
> +
> +module_i2c_driver(max597x_driver);
> +
> +MODULE_AUTHOR("Patrick Rudolph <patrick.rudolph@9elements.com>");
> +MODULE_DESCRIPTION("MAX597X Power Switch and Monitor");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mfd/max597x.h b/include/linux/mfd/max597x.h
> new file mode 100644
> index 000000000000..ae7f38f0aee2
> --- /dev/null
> +++ b/include/linux/mfd/max597x.h
> @@ -0,0 +1,102 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Maxim MAX5970/MAX5978 MFD Driver
> + *
> + * Copyright (c) 2022 9elements GmbH
> + *
> + * Author: Patrick Rudolph <patrick.rudolph@9elements.com>
> + */
> +
> +#ifndef MFD_MAX597X_H
> +#define MFD_MAX597X_H
> +
> +#include <linux/device.h>
> +#include <linux/regmap.h>
> +
> +#define MAX5970_NUM_SWITCHES 2
> +#define MAX5978_NUM_SWITCHES 1
> +#define MAX597X_NUM_LEDS     4

Unused?
Maybe the "4" below (or elsewhere) should be MAX597X_NUM_LEDS?

> +
> +enum max597x_chip_type {
> +	MAX597x_TYPE_MAX5978 = 1,
> +	MAX597x_TYPE_MAX5970,
> +};
> +
> +#define MAX5970_REG_CURRENT_L(ch)		(0x01 + (ch) * 4)
> +#define MAX5970_REG_CURRENT_H(ch)		(0x00 + (ch) * 4)
> +#define MAX5970_REG_VOLTAGE_L(ch)		(0x03 + (ch) * 4)
> +#define MAX5970_REG_VOLTAGE_H(ch)		(0x02 + (ch) * 4)
> +#define MAX5970_REG_MON_RANGE			0x18
> +#define  MAX5970_MON_MASK				0x3
> +#define  MAX5970_MON(reg, ch) \
> +						(((reg) >> ((ch) * 2)) & MAX5970_MON_MASK)

Why a line break here?

> +#define  MAX5970_MON_MAX_RANGE_UV		16000000
> +
> +#define MAX5970_REG_CH_UV_WARN_H(ch)	(0x1A + (ch) * 10)
> +#define MAX5970_REG_CH_UV_WARN_L(ch)	(0x1B + (ch) * 10)
> +#define MAX5970_REG_CH_UV_CRIT_H(ch)	(0x1C + (ch) * 10)
> +#define MAX5970_REG_CH_UV_CRIT_L(ch)	(0x1D + (ch) * 10)
> +#define MAX5970_REG_CH_OV_WARN_H(ch)	(0x1E + (ch) * 10)
> +#define MAX5970_REG_CH_OV_WARN_L(ch)	(0x1F + (ch) * 10)
> +#define MAX5970_REG_CH_OV_CRIT_H(ch)	(0x20 + (ch) * 10)
> +#define MAX5970_REG_CH_OV_CRIT_L(ch)	(0x21 + (ch) * 10)
> +
> +#define  MAX5970_VAL2REG_H(x)			(((x) >> 2) & 0xFF)
> +#define  MAX5970_VAL2REG_L(x)			((x) & 0x3)
> +
> +#define MAX5970_REG_DAC_FAST(ch)		(0x2E + (ch))
> +
> +#define MAX5970_FAST2SLOW_RATIO			200
> +
> +#define MAX5970_REG_STATUS0				0x31
> +#define  MAX5970_CB_IFAULTF(ch)			(1 << (ch))
> +#define  MAX5970_CB_IFAULTS(ch)			(1 << ((ch) + 4))
> +
> +#define MAX5970_REG_STATUS1				0x32
> +#define  STATUS1_PROT_MASK				0x3
> +#define  STATUS1_PROT(reg) \
> +	(((reg) >> 6) & STATUS1_PROT_MASK)
> +#define  STATUS1_PROT_SHUTDOWN			0
> +#define  STATUS1_PROT_CLEAR_PG			1
> +#define  STATUS1_PROT_ALERT_ONLY		2
> +
> +#define MAX5970_REG_STATUS2				0x33
> +#define  MAX5970_IRNG_MASK				0x3
> +#define  MAX5970_IRNG(reg, ch)	\
> +						(((reg) >> ((ch) * 2)) & MAX5970_IRNG_MASK)
> +
> +#define MAX5970_REG_STATUS3				0x34
> +#define  MAX5970_STATUS3_ALERT			BIT(4)
> +#define  MAX5970_STATUS3_PG(ch)			BIT(ch)
> +
> +#define MAX5970_REG_FAULT0				0x35
> +#define  UV_STATUS_WARN(ch)				BIT(ch)
> +#define  UV_STATUS_CRIT(ch)				BIT(ch + 4)
> +
> +#define MAX5970_REG_FAULT1				0x36
> +#define  OV_STATUS_WARN(ch)				BIT(ch)
> +#define  OV_STATUS_CRIT(ch)				BIT(ch + 4)
> +
> +#define MAX5970_REG_FAULT2				0x37
> +#define  OC_STATUS_WARN(ch)				BIT(ch)
> +
> +#define MAX5970_REG_CHXEN				0x3b
> +#define  CHXEN(ch)						(3 << (ch * 2))
> +
> +#define MAX5970_REG_LED_FLASH			0x43
> +
> +#define MAX_REGISTERS					0x49
> +#define ADC_MASK						0x3FF
> +
> +struct max597x_data {
> +	struct device *dev;
> +	int irq;
> +	int num_switches;
> +	struct regmap *regmap;
> +	/* Chip specific parameters needed by regulator & iio cells */
> +	u32 irng[MAX5970_NUM_SWITCHES];
> +	u32 mon_rng[MAX5970_NUM_SWITCHES];
> +	u32 shunt_micro_ohms[MAX5970_NUM_SWITCHES];

mon_rng and shunt_micro_ohms look unused.
Are they needed? Maybe for future use?

> +};
> +
> +#endif				/* _MAX597X_H */

MFD_MAX597X_H

CJ
  
kernel test robot Nov. 1, 2022, 1:18 p.m. UTC | #2
Hi Naresh,

I love your patch! Yet something to improve:

[auto build test ERROR on 6b780408be034213edfb5946889882cb29f8f159]

url:    https://github.com/intel-lab-lkp/linux/commits/Naresh-Solanki/mfd-max597x-Add-support-for-max597x/20221101-103128
base:   6b780408be034213edfb5946889882cb29f8f159
patch link:    https://lore.kernel.org/r/20221031204129.1434059-3-Naresh.Solanki%409elements.com
patch subject: [PATCH v3 2/2] mfd: max597x: Add support for MAX5970 and MAX5978
config: arc-randconfig-r031-20221102
compiler: arceb-elf-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/8165e745b1b29b3fc8d54765bec0eea4f10429c3
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Naresh-Solanki/mfd-max597x-Add-support-for-max597x/20221101-103128
        git checkout 8165e745b1b29b3fc8d54765bec0eea4f10429c3
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>, old ones prefixed by <<):

drivers/mfd/max597x: struct of_device_id is 196 bytes.  The last of 2 is:
0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x6d 0x61 0x78 0x69 0x6d 0x2c 0x6d 0x61 0x78 0x35 0x39 0x37 0x38 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x01
>> FATAL: modpost: drivers/mfd/max597x: struct of_device_id is not terminated with a NULL entry!
  
Naresh Solanki Nov. 1, 2022, 4:38 p.m. UTC | #3
Hi Christophe,

On 01-11-2022 02:49 am, Christophe JAILLET wrote:
> Le 31/10/2022 à 21:41, Naresh Solanki a écrit :
>> From: Patrick Rudolph <patrick.rudolph@9elements.com>
>>
>> Implement a regulator driver with IRQ support for fault management.
>> Written against documentation [1] and [2] and tested on real hardware.
>>
>> Every channel has its own regulator supplies nammed 'vss1-supply' and
>> 'vss2-supply'. The regulator supply is used to determine the output
>> voltage, as the smart switch provides no output regulation.
>> The driver requires the 'shunt-resistor-micro-ohms' property to be
>> present in Device Tree to properly calculate current related
>> values.
>>
>> Datasheet links:
>> 1: https://datasheets.maximintegrated.com/en/ds/MAX5970.pdf
>> 2: https://datasheets.maximintegrated.com/en/ds/MAX5978.pdf
>>
>> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
>> Signed-off-by: Marcello Sylvester Bauer <sylv@sylv.io>
>> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
> 
> Hi,
> a few nits below.
> 
>> ---
>>   drivers/mfd/Kconfig         |  12 +++++
>>   drivers/mfd/Makefile        |   1 +
>>   drivers/mfd/max597x.c       |  87 ++++++++++++++++++++++++++++++
>>   include/linux/mfd/max597x.h | 102 ++++++++++++++++++++++++++++++++++++
>>   4 files changed, 202 insertions(+)
>>   create mode 100644 drivers/mfd/max597x.c
>>   create mode 100644 include/linux/mfd/max597x.h
>>
>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>> index 8b93856de432..416fe7986b7b 100644
>> --- a/drivers/mfd/Kconfig
>> +++ b/drivers/mfd/Kconfig
>> @@ -253,6 +253,18 @@ config MFD_MADERA_SPI
>>         Support for the Cirrus Logic Madera platform audio SoC
>>         core functionality controlled via SPI.
>> +config MFD_MAX597X
>> +    tristate "Maxim 597x Power Switch and Monitor"
>> +    depends on I2C
>> +    depends on OF
>> +    select MFD_CORE
>> +    select REGMAP_I2C
>> +    help
>> +      This driver controls a Maxim 5970/5978 switch via I2C bus.
>> +      The MAX5970/5978 is a smart switch with no output regulation, but
>> +      fault protection and voltage and current monitoring capabilities.
>> +      Also it supports upto 4 indication LEDs.
>> +
>>   config MFD_CS47L15
>>       bool "Cirrus Logic CS47L15"
>>       select PINCTRL_CS47L15
>> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
>> index 7ed3ef4a698c..819d711fa748 100644
>> --- a/drivers/mfd/Makefile
>> +++ b/drivers/mfd/Makefile
>> @@ -161,6 +161,7 @@ obj-$(CONFIG_MFD_DA9063)    += da9063.o
>>   obj-$(CONFIG_MFD_DA9150)    += da9150-core.o
>>   obj-$(CONFIG_MFD_MAX14577)    += max14577.o
>> +obj-$(CONFIG_MFD_MAX597X)    += max597x.o
>>   obj-$(CONFIG_MFD_MAX77620)    += max77620.o
>>   obj-$(CONFIG_MFD_MAX77650)    += max77650.o
>>   obj-$(CONFIG_MFD_MAX77686)    += max77686.o
>> diff --git a/drivers/mfd/max597x.c b/drivers/mfd/max597x.c
>> new file mode 100644
>> index 000000000000..68a4e7755f21
>> --- /dev/null
>> +++ b/drivers/mfd/max597x.c
>> @@ -0,0 +1,87 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Maxim MAX5970/MAX5978 MFD Driver
>> + *
>> + * Copyright (c) 2022 9elements GmbH
>> + *
>> + * Author: Patrick Rudolph <patrick.rudolph@9elements.com>
>> + */
>> +
>> +#include <linux/i2c.h>
>> +#include <linux/mfd/core.h>
>> +#include <linux/mfd/max597x.h>
>> +#include <linux/regmap.h>
>> +
>> +static const struct regmap_config max597x_regmap_config = {
>> +    .reg_bits = 8,
>> +    .val_bits = 8,
>> +    .max_register = MAX_REGISTERS,
>> +};
>> +
>> +static const struct mfd_cell max597x_cells[] = {
>> +    { .name = "max597x-regulator", },
>> +    { .name = "max597x-iio", },
>> +    { .name = "max597x-led", },
>> +};
>> +
>> +static int max597x_probe(struct i2c_client *i2c, const struct 
>> i2c_device_id *id)
>> +{
>> +    struct max597x_data *ddata;
>> +    enum max597x_chip_type chip = id->driver_data;
>> +
>> +    ddata = devm_kzalloc(&i2c->dev, sizeof(*ddata),
>> +            GFP_KERNEL);
> 
> if (!ddata)
>      return -ENOMEM;
> 
Done
> GFP_KERNEL should be aligned with the ( on the previous line.
Done
> 
>> +
>> +    switch (chip) {
>> +    case MAX597x_TYPE_MAX5970:
>> +        ddata->num_switches = 2;
>> +        break;
>> +    case MAX597x_TYPE_MAX5978:
>> +        ddata->num_switches = 1;
>> +        break;
>> +    }
> 
> ddata->num_switches looks unused.
> Is it needed? Maybe for future use?
> 
Yes. Used by regulator & IIO driver.
> Should MAX5970_NUM_SWITCHES and MAX5979_NUM_SWITCHES be used instead of 
> hard coding 1 and 2?
Done
> 
>> +
>> +    ddata->regmap = devm_regmap_init_i2c(i2c, &max597x_regmap_config);
>> +    if (IS_ERR(ddata->regmap)) {
>> +        dev_err(&i2c->dev, "Failed to initialise regmap");
>> +        return -EINVAL;
>> +    }
>> +
>> +    /* IRQ used by regulator cell */
>> +    ddata->irq = i2c->irq;
>> +    ddata->dev = &i2c->dev;
>> +    i2c_set_clientdata(i2c, ddata);
>> +
>> +    return devm_mfd_add_devices(ddata->dev, PLATFORM_DEVID_AUTO,
>> +                    max597x_cells, ARRAY_SIZE(max597x_cells),
>> +                    NULL, 0, NULL);
>> +}
>> +
>> +static const struct i2c_device_id max597x_table[] = {
>> +    { .name = "max5970", MAX597x_TYPE_MAX5970 },
>> +    { .name = "max5978", MAX597x_TYPE_MAX5978 },
>> +};
>> +
>> +MODULE_DEVICE_TABLE(i2c, max597x_table);
>> +
>> +static const struct of_device_id max597x_of_match[] = {
>> +    { .compatible = "maxim,max5970", .data = (void 
>> *)MAX597x_TYPE_MAX5970 },
>> +    { .compatible = "maxim,max5978", .data = (void 
>> *)MAX597x_TYPE_MAX5978 },
>> +};
>> +
>> +MODULE_DEVICE_TABLE(of, max597x_of_match);
>> +
>> +static struct i2c_driver max597x_driver = {
>> +    .id_table = max597x_table,
>> +    .driver = {
>> +        .name = "max597x",
>> +        .of_match_table = of_match_ptr(max597x_of_match),
>> +        },
>> +    .probe = max597x_probe,
>> +};
>> +
>> +module_i2c_driver(max597x_driver);
>> +
>> +MODULE_AUTHOR("Patrick Rudolph <patrick.rudolph@9elements.com>");
>> +MODULE_DESCRIPTION("MAX597X Power Switch and Monitor");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/include/linux/mfd/max597x.h b/include/linux/mfd/max597x.h
>> new file mode 100644
>> index 000000000000..ae7f38f0aee2
>> --- /dev/null
>> +++ b/include/linux/mfd/max597x.h
>> @@ -0,0 +1,102 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Maxim MAX5970/MAX5978 MFD Driver
>> + *
>> + * Copyright (c) 2022 9elements GmbH
>> + *
>> + * Author: Patrick Rudolph <patrick.rudolph@9elements.com>
>> + */
>> +
>> +#ifndef MFD_MAX597X_H
>> +#define MFD_MAX597X_H
>> +
>> +#include <linux/device.h>
>> +#include <linux/regmap.h>
>> +
>> +#define MAX5970_NUM_SWITCHES 2
>> +#define MAX5978_NUM_SWITCHES 1
>> +#define MAX597X_NUM_LEDS     4
> 
> Unused?
> Maybe the "4" below (or elsewhere) should be MAX597X_NUM_LEDS?
Yes. The chip has 4 indiaction leds. Used by led cell.
> 
>> +
>> +enum max597x_chip_type {
>> +    MAX597x_TYPE_MAX5978 = 1,
>> +    MAX597x_TYPE_MAX5970,
>> +};
>> +
>> +#define MAX5970_REG_CURRENT_L(ch)        (0x01 + (ch) * 4)
>> +#define MAX5970_REG_CURRENT_H(ch)        (0x00 + (ch) * 4)
>> +#define MAX5970_REG_VOLTAGE_L(ch)        (0x03 + (ch) * 4)
>> +#define MAX5970_REG_VOLTAGE_H(ch)        (0x02 + (ch) * 4)
>> +#define MAX5970_REG_MON_RANGE            0x18
>> +#define  MAX5970_MON_MASK                0x3
>> +#define  MAX5970_MON(reg, ch) \
>> +                        (((reg) >> ((ch) * 2)) & MAX5970_MON_MASK)
> 
> Why a line break here?
Fixed.
> 
>> +#define  MAX5970_MON_MAX_RANGE_UV        16000000
>> +
>> +#define MAX5970_REG_CH_UV_WARN_H(ch)    (0x1A + (ch) * 10)
>> +#define MAX5970_REG_CH_UV_WARN_L(ch)    (0x1B + (ch) * 10)
>> +#define MAX5970_REG_CH_UV_CRIT_H(ch)    (0x1C + (ch) * 10)
>> +#define MAX5970_REG_CH_UV_CRIT_L(ch)    (0x1D + (ch) * 10)
>> +#define MAX5970_REG_CH_OV_WARN_H(ch)    (0x1E + (ch) * 10)
>> +#define MAX5970_REG_CH_OV_WARN_L(ch)    (0x1F + (ch) * 10)
>> +#define MAX5970_REG_CH_OV_CRIT_H(ch)    (0x20 + (ch) * 10)
>> +#define MAX5970_REG_CH_OV_CRIT_L(ch)    (0x21 + (ch) * 10)
>> +
>> +#define  MAX5970_VAL2REG_H(x)            (((x) >> 2) & 0xFF)
>> +#define  MAX5970_VAL2REG_L(x)            ((x) & 0x3)
>> +
>> +#define MAX5970_REG_DAC_FAST(ch)        (0x2E + (ch))
>> +
>> +#define MAX5970_FAST2SLOW_RATIO            200
>> +
>> +#define MAX5970_REG_STATUS0                0x31
>> +#define  MAX5970_CB_IFAULTF(ch)            (1 << (ch))
>> +#define  MAX5970_CB_IFAULTS(ch)            (1 << ((ch) + 4))
>> +
>> +#define MAX5970_REG_STATUS1                0x32
>> +#define  STATUS1_PROT_MASK                0x3
>> +#define  STATUS1_PROT(reg) \
>> +    (((reg) >> 6) & STATUS1_PROT_MASK)
>> +#define  STATUS1_PROT_SHUTDOWN            0
>> +#define  STATUS1_PROT_CLEAR_PG            1
>> +#define  STATUS1_PROT_ALERT_ONLY        2
>> +
>> +#define MAX5970_REG_STATUS2                0x33
>> +#define  MAX5970_IRNG_MASK                0x3
>> +#define  MAX5970_IRNG(reg, ch)    \
>> +                        (((reg) >> ((ch) * 2)) & MAX5970_IRNG_MASK)
>> +
>> +#define MAX5970_REG_STATUS3                0x34
>> +#define  MAX5970_STATUS3_ALERT            BIT(4)
>> +#define  MAX5970_STATUS3_PG(ch)            BIT(ch)
>> +
>> +#define MAX5970_REG_FAULT0                0x35
>> +#define  UV_STATUS_WARN(ch)                BIT(ch)
>> +#define  UV_STATUS_CRIT(ch)                BIT(ch + 4)
>> +
>> +#define MAX5970_REG_FAULT1                0x36
>> +#define  OV_STATUS_WARN(ch)                BIT(ch)
>> +#define  OV_STATUS_CRIT(ch)                BIT(ch + 4)
>> +
>> +#define MAX5970_REG_FAULT2                0x37
>> +#define  OC_STATUS_WARN(ch)                BIT(ch)
>> +
>> +#define MAX5970_REG_CHXEN                0x3b
>> +#define  CHXEN(ch)                        (3 << (ch * 2))
>> +
>> +#define MAX5970_REG_LED_FLASH            0x43
>> +
>> +#define MAX_REGISTERS                    0x49
>> +#define ADC_MASK                        0x3FF
>> +
>> +struct max597x_data {
>> +    struct device *dev;
>> +    int irq;
>> +    int num_switches;
>> +    struct regmap *regmap;
>> +    /* Chip specific parameters needed by regulator & iio cells */
>> +    u32 irng[MAX5970_NUM_SWITCHES];
>> +    u32 mon_rng[MAX5970_NUM_SWITCHES];
>> +    u32 shunt_micro_ohms[MAX5970_NUM_SWITCHES];
> 
> mon_rng and shunt_micro_ohms look unused.
> Are they needed? Maybe for future use?
Used by regulator & iio driver.
> 
>> +};
>> +
>> +#endif                /* _MAX597X_H */
> 
> MFD_MAX597X_H
> 
> CJ

Thanks,
Naresh
  

Patch

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 8b93856de432..416fe7986b7b 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -253,6 +253,18 @@  config MFD_MADERA_SPI
 	  Support for the Cirrus Logic Madera platform audio SoC
 	  core functionality controlled via SPI.
 
+config MFD_MAX597X
+	tristate "Maxim 597x Power Switch and Monitor"
+	depends on I2C
+	depends on OF
+	select MFD_CORE
+	select REGMAP_I2C
+	help
+	  This driver controls a Maxim 5970/5978 switch via I2C bus.
+	  The MAX5970/5978 is a smart switch with no output regulation, but
+	  fault protection and voltage and current monitoring capabilities.
+	  Also it supports upto 4 indication LEDs.
+
 config MFD_CS47L15
 	bool "Cirrus Logic CS47L15"
 	select PINCTRL_CS47L15
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 7ed3ef4a698c..819d711fa748 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -161,6 +161,7 @@  obj-$(CONFIG_MFD_DA9063)	+= da9063.o
 obj-$(CONFIG_MFD_DA9150)	+= da9150-core.o
 
 obj-$(CONFIG_MFD_MAX14577)	+= max14577.o
+obj-$(CONFIG_MFD_MAX597X)	+= max597x.o
 obj-$(CONFIG_MFD_MAX77620)	+= max77620.o
 obj-$(CONFIG_MFD_MAX77650)	+= max77650.o
 obj-$(CONFIG_MFD_MAX77686)	+= max77686.o
diff --git a/drivers/mfd/max597x.c b/drivers/mfd/max597x.c
new file mode 100644
index 000000000000..68a4e7755f21
--- /dev/null
+++ b/drivers/mfd/max597x.c
@@ -0,0 +1,87 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Maxim MAX5970/MAX5978 MFD Driver
+ *
+ * Copyright (c) 2022 9elements GmbH
+ *
+ * Author: Patrick Rudolph <patrick.rudolph@9elements.com>
+ */
+
+#include <linux/i2c.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/max597x.h>
+#include <linux/regmap.h>
+
+static const struct regmap_config max597x_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = MAX_REGISTERS,
+};
+
+static const struct mfd_cell max597x_cells[] = {
+	{ .name = "max597x-regulator", },
+	{ .name = "max597x-iio", },
+	{ .name = "max597x-led", },
+};
+
+static int max597x_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
+{
+	struct max597x_data *ddata;
+	enum max597x_chip_type chip = id->driver_data;
+
+	ddata = devm_kzalloc(&i2c->dev, sizeof(*ddata),
+			GFP_KERNEL);
+
+	switch (chip) {
+	case MAX597x_TYPE_MAX5970:
+		ddata->num_switches = 2;
+		break;
+	case MAX597x_TYPE_MAX5978:
+		ddata->num_switches = 1;
+		break;
+	}
+
+	ddata->regmap = devm_regmap_init_i2c(i2c, &max597x_regmap_config);
+	if (IS_ERR(ddata->regmap)) {
+		dev_err(&i2c->dev, "Failed to initialise regmap");
+		return -EINVAL;
+	}
+
+	/* IRQ used by regulator cell */
+	ddata->irq = i2c->irq;
+	ddata->dev = &i2c->dev;
+	i2c_set_clientdata(i2c, ddata);
+
+	return devm_mfd_add_devices(ddata->dev, PLATFORM_DEVID_AUTO,
+				    max597x_cells, ARRAY_SIZE(max597x_cells),
+				    NULL, 0, NULL);
+}
+
+static const struct i2c_device_id max597x_table[] = {
+	{ .name = "max5970", MAX597x_TYPE_MAX5970 },
+	{ .name = "max5978", MAX597x_TYPE_MAX5978 },
+};
+
+MODULE_DEVICE_TABLE(i2c, max597x_table);
+
+static const struct of_device_id max597x_of_match[] = {
+	{ .compatible = "maxim,max5970", .data = (void *)MAX597x_TYPE_MAX5970 },
+	{ .compatible = "maxim,max5978", .data = (void *)MAX597x_TYPE_MAX5978 },
+};
+
+MODULE_DEVICE_TABLE(of, max597x_of_match);
+
+static struct i2c_driver max597x_driver = {
+	.id_table = max597x_table,
+	.driver = {
+		.name = "max597x",
+		.of_match_table = of_match_ptr(max597x_of_match),
+		},
+	.probe = max597x_probe,
+};
+
+module_i2c_driver(max597x_driver);
+
+MODULE_AUTHOR("Patrick Rudolph <patrick.rudolph@9elements.com>");
+MODULE_DESCRIPTION("MAX597X Power Switch and Monitor");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mfd/max597x.h b/include/linux/mfd/max597x.h
new file mode 100644
index 000000000000..ae7f38f0aee2
--- /dev/null
+++ b/include/linux/mfd/max597x.h
@@ -0,0 +1,102 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Maxim MAX5970/MAX5978 MFD Driver
+ *
+ * Copyright (c) 2022 9elements GmbH
+ *
+ * Author: Patrick Rudolph <patrick.rudolph@9elements.com>
+ */
+
+#ifndef MFD_MAX597X_H
+#define MFD_MAX597X_H
+
+#include <linux/device.h>
+#include <linux/regmap.h>
+
+#define MAX5970_NUM_SWITCHES 2
+#define MAX5978_NUM_SWITCHES 1
+#define MAX597X_NUM_LEDS     4
+
+enum max597x_chip_type {
+	MAX597x_TYPE_MAX5978 = 1,
+	MAX597x_TYPE_MAX5970,
+};
+
+#define MAX5970_REG_CURRENT_L(ch)		(0x01 + (ch) * 4)
+#define MAX5970_REG_CURRENT_H(ch)		(0x00 + (ch) * 4)
+#define MAX5970_REG_VOLTAGE_L(ch)		(0x03 + (ch) * 4)
+#define MAX5970_REG_VOLTAGE_H(ch)		(0x02 + (ch) * 4)
+#define MAX5970_REG_MON_RANGE			0x18
+#define  MAX5970_MON_MASK				0x3
+#define  MAX5970_MON(reg, ch) \
+						(((reg) >> ((ch) * 2)) & MAX5970_MON_MASK)
+#define  MAX5970_MON_MAX_RANGE_UV		16000000
+
+#define MAX5970_REG_CH_UV_WARN_H(ch)	(0x1A + (ch) * 10)
+#define MAX5970_REG_CH_UV_WARN_L(ch)	(0x1B + (ch) * 10)
+#define MAX5970_REG_CH_UV_CRIT_H(ch)	(0x1C + (ch) * 10)
+#define MAX5970_REG_CH_UV_CRIT_L(ch)	(0x1D + (ch) * 10)
+#define MAX5970_REG_CH_OV_WARN_H(ch)	(0x1E + (ch) * 10)
+#define MAX5970_REG_CH_OV_WARN_L(ch)	(0x1F + (ch) * 10)
+#define MAX5970_REG_CH_OV_CRIT_H(ch)	(0x20 + (ch) * 10)
+#define MAX5970_REG_CH_OV_CRIT_L(ch)	(0x21 + (ch) * 10)
+
+#define  MAX5970_VAL2REG_H(x)			(((x) >> 2) & 0xFF)
+#define  MAX5970_VAL2REG_L(x)			((x) & 0x3)
+
+#define MAX5970_REG_DAC_FAST(ch)		(0x2E + (ch))
+
+#define MAX5970_FAST2SLOW_RATIO			200
+
+#define MAX5970_REG_STATUS0				0x31
+#define  MAX5970_CB_IFAULTF(ch)			(1 << (ch))
+#define  MAX5970_CB_IFAULTS(ch)			(1 << ((ch) + 4))
+
+#define MAX5970_REG_STATUS1				0x32
+#define  STATUS1_PROT_MASK				0x3
+#define  STATUS1_PROT(reg) \
+	(((reg) >> 6) & STATUS1_PROT_MASK)
+#define  STATUS1_PROT_SHUTDOWN			0
+#define  STATUS1_PROT_CLEAR_PG			1
+#define  STATUS1_PROT_ALERT_ONLY		2
+
+#define MAX5970_REG_STATUS2				0x33
+#define  MAX5970_IRNG_MASK				0x3
+#define  MAX5970_IRNG(reg, ch)	\
+						(((reg) >> ((ch) * 2)) & MAX5970_IRNG_MASK)
+
+#define MAX5970_REG_STATUS3				0x34
+#define  MAX5970_STATUS3_ALERT			BIT(4)
+#define  MAX5970_STATUS3_PG(ch)			BIT(ch)
+
+#define MAX5970_REG_FAULT0				0x35
+#define  UV_STATUS_WARN(ch)				BIT(ch)
+#define  UV_STATUS_CRIT(ch)				BIT(ch + 4)
+
+#define MAX5970_REG_FAULT1				0x36
+#define  OV_STATUS_WARN(ch)				BIT(ch)
+#define  OV_STATUS_CRIT(ch)				BIT(ch + 4)
+
+#define MAX5970_REG_FAULT2				0x37
+#define  OC_STATUS_WARN(ch)				BIT(ch)
+
+#define MAX5970_REG_CHXEN				0x3b
+#define  CHXEN(ch)						(3 << (ch * 2))
+
+#define MAX5970_REG_LED_FLASH			0x43
+
+#define MAX_REGISTERS					0x49
+#define ADC_MASK						0x3FF
+
+struct max597x_data {
+	struct device *dev;
+	int irq;
+	int num_switches;
+	struct regmap *regmap;
+	/* Chip specific parameters needed by regulator & iio cells */
+	u32 irng[MAX5970_NUM_SWITCHES];
+	u32 mon_rng[MAX5970_NUM_SWITCHES];
+	u32 shunt_micro_ohms[MAX5970_NUM_SWITCHES];
+};
+
+#endif				/* _MAX597X_H */