Date:	Wed, 27 Apr 2016 16:19:47 +0100
From:	Lee Jones <lee.jones@...aro.org>
To:	Laxman Dewangan <ldewangan@...dia.com>
Cc:	linus.walleij@...aro.org, robh+dt@...nel.org, mark.rutland@....com,
	gnurou@...il.com, linux-kernel@...r.kernel.org,
	linux-gpio@...r.kernel.org, devicetree@...r.kernel.org,
	ijc+devicetree@...lion.org.uk, swarren@...dia.com,
	treding@...dia.com, Mallikarjun Kasoju <mkasoju@...dia.com>
Subject: Re: [PATCH V10 2/6] mfd: max77620: add core driver for
 MAX77620/MAX20024

On Wed, 30 Mar 2016, Laxman Dewangan wrote:

> MAX77620/MAX20024 are Power Management IC from the MAXIM.
> It supports RTC, multiple GPIOs, multiple DCDC and LDOs,
> watchdog, clock etc.
> 
> Add MFD drier to provides common support for accessing the
> device; additional drivers is developed on respected subsystem
> in order to use the functionality of the device.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@...dia.com>
> Signed-off-by: Mallikarjun Kasoju <mkasoju@...dia.com>
> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@...sung.com>
> 
> ---
> Changes from V1:
> - Code cleanups per review from V1.
> - Move register acccess APIs from header to c file.
> - Remove some of non required variable, remove duplication in error message
>  and simplify some of function implementation.
> - Register RTC driver such that it can get the regmap handle form parent device
> 
> Changes from V2:
> - Run coccicheck and checkpatch in strict mode for the alignment.
> - Drop RTC driver and its i2c client registration.
> 
> Changes from V3:
> - Change all sys initcall to module driver.
> - change the max77620_read argument to unisgned int from u8.
> 
> Changes from V4:
> - Take care of fps nodes.
> - Drop the battery charger and low battery binding and related code as
>   it need to go on power driver.
> 
> Changes from V5:
> -None
> 
> Changes from V6:
> - Taken care of Lee's comment like used defines for irqs, remove max77620
>   register accesss abstractions, remove DTof module and use ID table only,
>   reduce the copyright lines.
> - Drop configuration for hard power off time chnage as it will be in
>   power driver.
> - Use direct regmap from all drivers instead of using abstractions.
> - This depends on patch
> 
> Change from V7:
> - Use MFD defines for making mfd cells.
> - Use new property name.
> 
> Changes from V8:
> - Remove the usage of MFD defines. Remove mutex_config as not needed.
> 
> Changes from V9:
> - Use the devm_regmap_add_irq_chip() for irq registration.
> 
>  drivers/mfd/Kconfig          |  15 ++
>  drivers/mfd/Makefile         |   1 +
>  drivers/mfd/max77620.c       | 544 +++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/max77620.h | 337 +++++++++++++++++++++++++++
>  4 files changed, 897 insertions(+)
>  create mode 100644 drivers/mfd/max77620.c
>  create mode 100644 include/linux/mfd/max77620.h
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index eea61e3..585d6e3 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -527,6 +527,21 @@ config MFD_MAX14577
>  	  additional drivers must be enabled in order to use the functionality
>  	  of the device.
>  
> +config MFD_MAX77620
> +	bool "Maxim Semiconductor MAX77620 and MAX20024 PMIC Support"
> +	depends on I2C=y
> +	depends on OF
> +	select MFD_CORE
> +	select REGMAP_I2C
> +	select REGMAP_IRQ
> +	select IRQ_DOMAIN
> +	help
> +	  Say yes here to add support for Maxim Semiconductor MAX77620 and
> +	  MAX20024 which are Power Management IC with General purpose pins,
> +	  RTC, regulators, clock generator, watchdog etc. This driver
> +	  provides common support for accessing the device; additional drivers
> +	  must be enabled in order to use the functionality of the device.
> +
>  config MFD_MAX77686
>  	tristate "Maxim Semiconductor MAX77686/802 PMIC Support"
>  	depends on I2C
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 5eaa6465d..921a08d 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -128,6 +128,7 @@ obj-$(CONFIG_MFD_DA9063)	+= da9063.o
>  obj-$(CONFIG_MFD_DA9150)	+= da9150-core.o
>  
>  obj-$(CONFIG_MFD_MAX14577)	+= max14577.o
> +obj-$(CONFIG_MFD_MAX77620)	+= max77620.o
>  obj-$(CONFIG_MFD_MAX77686)	+= max77686.o
>  obj-$(CONFIG_MFD_MAX77693)	+= max77693.o
>  obj-$(CONFIG_MFD_MAX77843)	+= max77843.o
> diff --git a/drivers/mfd/max77620.c b/drivers/mfd/max77620.c
> new file mode 100644
> index 0000000..41daaf7
> --- /dev/null
> +++ b/drivers/mfd/max77620.c
> @@ -0,0 +1,544 @@
> +/*
> + * Maxim MAX77620 MFD Driver
> + *
> + * Copyright (C) 2016 NVIDIA CORPORATION. All rights reserved.
> + *
> + * Author:
> + *	Laxman Dewangan <ldewangan@...dia.com>
> + *	Chaitanya Bandi <bandik@...dia.com>
> + *	Mallikarjun Kasoju <mkasoju@...dia.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/max77620.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +
> +static struct resource gpio_resources[] = {
> +	DEFINE_RES_IRQ(MAX77620_IRQ_TOP_GPIO),
> +};
> +
> +static struct resource power_resources[] = {
> +	DEFINE_RES_IRQ(MAX77620_IRQ_LBT_MBATLOW),
> +};
> +
> +static struct resource rtc_resources[] = {
> +	DEFINE_RES_IRQ(MAX77620_IRQ_TOP_RTC),
> +};
> +
> +static struct resource thermal_resources[] = {
> +	DEFINE_RES_IRQ(MAX77620_IRQ_LBT_TJALRM1),
> +	DEFINE_RES_IRQ(MAX77620_IRQ_LBT_TJALRM2),
> +};
> +
> +static const struct regmap_irq max77620_top_irqs[] = {
> +	REGMAP_IRQ_REG(MAX77620_IRQ_TOP_GLBL, 0, MAX77620_IRQ_TOP_GLBL_MASK),
> +	REGMAP_IRQ_REG(MAX77620_IRQ_TOP_SD, 0, MAX77620_IRQ_TOP_SD_MASK),
> +	REGMAP_IRQ_REG(MAX77620_IRQ_TOP_LDO, 0, MAX77620_IRQ_TOP_LDO_MASK),
> +	REGMAP_IRQ_REG(MAX77620_IRQ_TOP_GPIO, 0, MAX77620_IRQ_TOP_GPIO_MASK),
> +	REGMAP_IRQ_REG(MAX77620_IRQ_TOP_RTC, 0, MAX77620_IRQ_TOP_RTC_MASK),
> +	REGMAP_IRQ_REG(MAX77620_IRQ_TOP_32K, 0, MAX77620_IRQ_TOP_32K_MASK),
> +	REGMAP_IRQ_REG(MAX77620_IRQ_TOP_ONOFF, 0, MAX77620_IRQ_TOP_ONOFF_MASK),
> +	REGMAP_IRQ_REG(MAX77620_IRQ_LBT_MBATLOW, 1, MAX77620_IRQ_LBM_MASK),
> +	REGMAP_IRQ_REG(MAX77620_IRQ_LBT_TJALRM1, 1, MAX77620_IRQ_TJALRM1_MASK),
> +	REGMAP_IRQ_REG(MAX77620_IRQ_LBT_TJALRM2, 1, MAX77620_IRQ_TJALRM2_MASK),
> +};
> +
> +#define MAX77620_MFD_CELL_NAME(_name)				\
> +	{							\
> +		.name = (_name),				\
> +	}
> +
> +#define MAX77620_MFD_CELL_RES(_name, _res)			\
> +	{							\
> +		.name = (_name),				\
> +		.resources = (_res),				\
> +		.num_resources = ARRAY_SIZE((_res)),		\
> +	}

I'm *still* not accepting this.

> +static struct mfd_cell max77620_children[] = {
> +	MAX77620_MFD_CELL_NAME("max77620-pinctrl"),
> +	MAX77620_MFD_CELL_RES("max77620-gpio", gpio_resources),
> +	MAX77620_MFD_CELL_NAME("max77620-pmic"),
> +	MAX77620_MFD_CELL_RES("max77620-rtc", rtc_resources),
> +	MAX77620_MFD_CELL_RES("max77620-power", power_resources),
> +	MAX77620_MFD_CELL_NAME("max77620-watchdog"),
> +	MAX77620_MFD_CELL_NAME("max77620-clock"),
> +	MAX77620_MFD_CELL_RES("max77620-thermal", thermal_resources),
> +};
> +
> +static struct mfd_cell max20024_children[] = {
> +	MAX77620_MFD_CELL_NAME("max20024-pinctrl"),
> +	MAX77620_MFD_CELL_RES("max20024-gpio", gpio_resources),
> +	MAX77620_MFD_CELL_NAME("max20024-pmic"),
> +	MAX77620_MFD_CELL_RES("max77620-rtc", rtc_resources),
> +	MAX77620_MFD_CELL_RES("max20024-power", power_resources),
> +	MAX77620_MFD_CELL_NAME("max20024-watchdog"),
> +	MAX77620_MFD_CELL_NAME("max20024-clock"),
> +};

If you want this submission to be accepted this cycle, you're going to
have to convert this to the traditional way of defining MFD children.

> +static struct regmap_irq_chip max77620_top_irq_chip = {
> +	.name = "max77620-top",
> +	.irqs = max77620_top_irqs,
> +	.num_irqs = ARRAY_SIZE(max77620_top_irqs),
> +	.num_regs = 2,
> +	.status_base = MAX77620_REG_IRQTOP,
> +	.mask_base = MAX77620_REG_IRQTOPM,
> +};
> +
> +static const struct regmap_range max77620_readable_ranges[] = {
> +	regmap_reg_range(MAX77620_REG_CNFGGLBL1, MAX77620_REG_DVSSD4),
> +};
> +
> +static const struct regmap_access_table max77620_readable_table = {
> +	.yes_ranges = max77620_readable_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(max77620_readable_ranges),
> +};
> +
> +static const struct regmap_range max20024_readable_ranges[] = {
> +	regmap_reg_range(MAX77620_REG_CNFGGLBL1, MAX77620_REG_DVSSD4),
> +	regmap_reg_range(MAX20024_REG_MAX_ADD, MAX20024_REG_MAX_ADD),
> +};
> +
> +static const struct regmap_access_table max20024_readable_table = {
> +	.yes_ranges = max20024_readable_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(max20024_readable_ranges),
> +};
> +
> +static const struct regmap_range max77620_writable_ranges[] = {
> +	regmap_reg_range(MAX77620_REG_CNFGGLBL1, MAX77620_REG_DVSSD4),
> +};
> +
> +static const struct regmap_access_table max77620_writable_table = {
> +	.yes_ranges = max77620_writable_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(max77620_writable_ranges),
> +};
> +
> +static const struct regmap_range max77620_cacheable_ranges[] = {
> +	regmap_reg_range(MAX77620_REG_SD0_CFG, MAX77620_REG_LDO_CFG3),
> +	regmap_reg_range(MAX77620_REG_FPS_CFG0, MAX77620_REG_FPS_SD3),
> +};
> +
> +static const struct regmap_access_table max77620_volatile_table = {
> +	.no_ranges = max77620_cacheable_ranges,
> +	.n_no_ranges = ARRAY_SIZE(max77620_cacheable_ranges),
> +};
> +
> +static const struct regmap_config max77620_regmap_config = {
> +	.name = "power-slave",
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = MAX77620_REG_DVSSD4 + 1,
> +	.cache_type = REGCACHE_RBTREE,
> +	.rd_table = &max77620_readable_table,
> +	.wr_table = &max77620_writable_table,
> +	.volatile_table = &max77620_volatile_table,
> +};
> +
> +static const struct regmap_config max20024_regmap_config = {
> +	.name = "power-slave",
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = MAX20024_REG_MAX_ADD + 1,
> +	.cache_type = REGCACHE_RBTREE,
> +	.rd_table = &max20024_readable_table,
> +	.wr_table = &max77620_writable_table,
> +	.volatile_table = &max77620_volatile_table,
> +};
> +
> +static int max77620_get_fps_period_reg_value(struct max77620_chip *chip,
> +					     int tperiod)
> +{
> +	int base_fps_time = (chip->chip_id == MAX20024) ? 20 : 40;

What are 20 and 40?  I think you're going to need a comment to
describe what's going on in this function.

> +	int x, i;
> +
> +	for (i = 0; i < 0x7; i++) {

It's unsual to use hex values like this.

I think it should be defined.

> +		x = base_fps_time * BIT(i);

20 * (1 << {0..7})

So ...

20 * (1, 2, 4, 8, etc)

What does that does that doe exactly?

> +		if (x >= tperiod)
> +			return i;
> +	}
> +
> +	return i;
> +}
> +
> +static int max77620_config_fps(struct max77620_chip *chip,
> +			       struct device_node *fps_np)

Lots of mention of 'FPS' here, but noting to so what that is?

What does FPS stand for and what does the FPS do?

> +{
> +	struct device *dev = chip->dev;
> +	unsigned int mask = 0, config = 0;
> +	u32 param_val;
> +	int tperiod, fps_id;
> +	int ret;
> +	char fps_name[10];
> +
> +	for (fps_id = 0; fps_id < MAX77620_FPS_COUNT; fps_id++) {
> +		sprintf(fps_name, "fps%d", fps_id);
> +		if (!strcmp(fps_np->name, fps_name))
> +			break;
> +	}
> +
> +	if (fps_id == MAX77620_FPS_COUNT) {
> +		dev_err(dev, "FPS node name %s is not valid\n", fps_np->name);
> +		return -EINVAL;
> +	}
> +
> +	ret = of_property_read_u32(fps_np, "maxim,shutdown-fps-time-period-us",
> +				   &param_val);
> +	if (!ret) {
> +		mask |= MAX77620_FPS_TIME_PERIOD_MASK;
> +		chip->shutdown_fps_period[fps_id] = min(param_val, 5120U);

Please define all magic numbers.

> +		tperiod = max77620_get_fps_period_reg_value(chip,
> +				chip->shutdown_fps_period[fps_id]);
> +		config |= tperiod << MAX77620_FPS_TIME_PERIOD_SHIFT;
> +	}
> +
> +	ret = of_property_read_u32(fps_np, "maxim,suspend-fps-time-period-us",
> +				   &param_val);
> +	if (!ret)
> +		chip->suspend_fps_period[fps_id] = min(param_val, 5120U);
> +
> +	ret = of_property_read_u32(fps_np, "maxim,fps-event-source",
> +				   &param_val);
> +	if (!ret) {
> +		if (param_val > 2) {
> +			dev_err(dev, "FPS%d event-source invalid\n", fps_id);
> +			return -EINVAL;
> +		}
> +		mask |= MAX77620_FPS_EN_SRC_MASK;
> +		config |= param_val << MAX77620_FPS_EN_SRC_SHIFT;
> +		if (param_val == 2) {
> +			mask |= MAX77620_FPS_ENFPS_SW_MASK;
> +			config |= MAX77620_FPS_ENFPS_SW;
> +		}
> +	}
> +
> +	if (!chip->sleep_enable && !chip->enable_global_lpm) {
> +		ret = of_property_read_u32(fps_np,
> +				"maxim,device-state-on-disabled-event",
> +				&param_val);
> +		if (!ret) {
> +			if (param_val == 0)
> +				chip->sleep_enable = true;
> +			else if (param_val == 1)
> +				chip->enable_global_lpm = true;
> +		}
> +	}
> +
> +	ret = regmap_update_bits(chip->rmap, MAX77620_REG_FPS_CFG0 + fps_id,
> +				 mask, config);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to Reg 0x%02x update: %d\n",
> +			MAX77620_REG_FPS_CFG0 + fps_id, ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int max77620_initialise_fps(struct max77620_chip *chip)
> +{
> +	struct device *dev = chip->dev;
> +	struct device_node *fps_np, *fps_child;
> +	u8 config;
> +	int fps_id;
> +	int ret;
> +
> +	for (fps_id = 0; fps_id < MAX77620_FPS_COUNT; fps_id++) {
> +		chip->shutdown_fps_period[fps_id] = -1;
> +		chip->suspend_fps_period[fps_id] = -1;
> +	}
> +
> +	fps_np = of_get_child_by_name(dev->of_node, "fps");
> +	if (!fps_np)
> +		goto skip_fps;
> +
> +	for_each_child_of_node(fps_np, fps_child) {
> +		ret = max77620_config_fps(chip, fps_child);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	config = chip->enable_global_lpm ? MAX77620_ONOFFCNFG2_SLP_LPM_MSK : 0;
> +	ret = regmap_update_bits(chip->rmap, MAX77620_REG_ONOFFCNFG2,
> +				 MAX77620_ONOFFCNFG2_SLP_LPM_MSK, config);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to reg ONOFFCNFG2 update: %d\n", ret);

This isn't very readable for your users.

Please issue a user friendly print instead.

> +		return ret;
> +	}
> +
> +skip_fps:
> +	/* Enable wake on EN0 pin */
> +	ret = regmap_update_bits(chip->rmap, MAX77620_REG_ONOFFCNFG2,
> +				 MAX77620_ONOFFCNFG2_WK_EN0,
> +				 MAX77620_ONOFFCNFG2_WK_EN0);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to reg ONOFFCNFG2 update: %d\n", ret);

As above, and all the others below too.

> +		return ret;
> +	}
> +
> +	/* For MAX20024, SLPEN will be POR reset if CLRSE is b11 */
> +	if ((chip->chip_id == MAX20024) && chip->sleep_enable) {
> +		config = MAX77620_ONOFFCNFG1_SLPEN | MAX20024_ONOFFCNFG1_CLRSE;
> +		ret = regmap_update_bits(chip->rmap, MAX77620_REG_ONOFFCNFG1,
> +					 config, config);
> +		if (ret < 0) {
> +			dev_err(dev, "Failed to reg ONOFFCNFG1 update: %d\n",
> +				ret);
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int max77620_read_es_version(struct max77620_chip *chip)
> +{
> +	unsigned int val;
> +	u8 cid_val[6];
> +	int i;
> +	int ret;
> +
> +	for (i = MAX77620_REG_CID0; i <= MAX77620_REG_CID5; i++) {
> +		ret = regmap_read(chip->rmap, i, &val);
> +		if (ret < 0) {
> +			dev_err(chip->dev, "Failed to reg CID%d read: %d\n",
> +				i - MAX77620_REG_CID0, ret);
> +			return ret;
> +		}
> +		dev_dbg(chip->dev, "CID%d: 0x%02x\n",
> +			i - MAX77620_REG_CID0, val);
> +		cid_val[i - MAX77620_REG_CID0] = val;
> +	}
> +
> +	/* CID4 is OTP Version  and CID5 is ES version */
> +	dev_info(chip->dev, "PMIC Version OTP:0x%02X and ES:0x%X\n",
> +		 cid_val[4], MAX77620_CID5_DIDM(cid_val[5]));
> +
> +	return ret;
> +}
> +
> +static int max77620_probe(struct i2c_client *client,
> +			  const struct i2c_device_id *id)
> +{
> +	const struct regmap_config *rmap_config;
> +	struct max77620_chip *chip;
> +	struct mfd_cell *mfd_cells;
> +	int n_mfd_cells;
> +	int ret;
> +
> +	chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
> +	if (!chip)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(client, chip);
> +	chip->dev = &client->dev;
> +	chip->irq_base = -1;
> +	chip->chip_irq = client->irq;
> +	chip->chip_id = (enum max77620_chip_id)id->driver_data;
> +
> +	switch (chip->chip_id) {
> +	case MAX77620:
> +		mfd_cells = max77620_children;
> +		n_mfd_cells = ARRAY_SIZE(max77620_children);
> +		rmap_config = &max77620_regmap_config;
> +		break;
> +	case MAX20024:
> +		mfd_cells = max20024_children;
> +		n_mfd_cells = ARRAY_SIZE(max20024_children);
> +		rmap_config = &max20024_regmap_config;
> +		break;
> +	default:
> +		dev_err(chip->dev, "ChipID is invalid %d\n", chip->chip_id);
> +		return -EINVAL;
> +	}
> +
> +	chip->rmap = devm_regmap_init_i2c(client, rmap_config);
> +	if (IS_ERR(chip->rmap)) {
> +		ret = PTR_ERR(chip->rmap);
> +		dev_err(chip->dev, "Failed to regmap init: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = max77620_read_es_version(chip);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = devm_regmap_add_irq_chip(chip->dev, chip->rmap, client->irq,
> +				       IRQF_ONESHOT | IRQF_SHARED,
> +				       chip->irq_base, &max77620_top_irq_chip,
> +				       &chip->top_irq_data);
> +	if (ret < 0) {
> +		dev_err(chip->dev, "Failed to add regmap irq: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = max77620_initialise_fps(chip);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret =  mfd_add_devices(chip->dev, PLATFORM_DEVID_NONE,
> +			       mfd_cells, n_mfd_cells, NULL, 0,
> +			       regmap_irq_get_domain(chip->top_irq_data));

Use new devm_*

> +	if (ret < 0) {
> +		dev_err(chip->dev, "Failed to add sub devices: %d\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int max77620_remove(struct i2c_client *client)
> +{
> +	struct max77620_chip *chip = i2c_get_clientdata(client);
> +
> +	mfd_remove_devices(chip->dev);
> +
> +	return 0;
> +}

Remove .remove()

> +#ifdef CONFIG_PM_SLEEP
> +static int max77620_set_fps_period(struct max77620_chip *chip,
> +				   int fps_id, int time_period)
> +{
> +	int period = max77620_get_fps_period_reg_value(chip, time_period);
> +	int ret;
> +
> +	ret = regmap_update_bits(chip->rmap, MAX77620_REG_FPS_CFG0 + fps_id,
> +				 MAX77620_FPS_TIME_PERIOD_MASK,
> +				 period << MAX77620_FPS_TIME_PERIOD_SHIFT);
> +	if (ret < 0) {
> +		dev_err(chip->dev, "Failed to reg 0x%02x write: %d\n",
> +			MAX77620_REG_FPS_CFG0 + fps_id, ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int max77620_i2c_suspend(struct device *dev)
> +{
> +	struct max77620_chip *chip = dev_get_drvdata(dev);
> +	struct i2c_client *client = to_i2c_client(dev);
> +	unsigned int config;
> +	int fps;
> +	int ret;
> +
> +	for (fps = 0; fps < MAX77620_FPS_COUNT; fps++) {
> +		if (chip->suspend_fps_period[fps] < 0)
> +			continue;
> +
> +		ret = max77620_set_fps_period(chip, fps,
> +					      chip->suspend_fps_period[fps]);
> +		if (ret < 0)
> +			dev_err(dev, "Failed to FPS%d config: %d\n", fps, ret);
> +	}
> +
> +	/*
> +	 * For MAX20024: No need to configure SLPEN on suspend as
> +	 * it will be configured on Init.
> +	 */
> +	if (chip->chip_id == MAX20024)
> +		goto out;
> +
> +	config = (chip->sleep_enable) ? MAX77620_ONOFFCNFG1_SLPEN : 0;
> +	ret = regmap_update_bits(chip->rmap, MAX77620_REG_ONOFFCNFG1,
> +				 MAX77620_ONOFFCNFG1_SLPEN,
> +				 config);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to reg ONOFFCNFG1 update: %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* Disable WK_EN0 */
> +	ret = regmap_update_bits(chip->rmap, MAX77620_REG_ONOFFCNFG2,
> +				 MAX77620_ONOFFCNFG2_WK_EN0, 0);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to reg ONOFFCNFG2 update: %d\n", ret);
> +		return ret;
> +	}
> +
> +out:
> +	disable_irq(client->irq);
> +
> +	return 0;
> +}
> +
> +static int max77620_i2c_resume(struct device *dev)
> +{
> +	struct max77620_chip *chip = dev_get_drvdata(dev);
> +	struct i2c_client *client = to_i2c_client(dev);
> +	int ret;
> +	int fps;
> +
> +	for (fps = 0; fps < MAX77620_FPS_COUNT; fps++) {
> +		if (chip->shutdown_fps_period[fps] < 0)
> +			continue;
> +
> +		ret = max77620_set_fps_period(chip, fps,
> +					      chip->shutdown_fps_period[fps]);
> +		if (ret < 0)
> +			dev_err(dev, "Failed to FPS%d config: %d\n", fps, ret);
> +	}
> +
> +	/*
> +	 * For MAX20024: No need to configure WKEN0 on resume as
> +	 * it is configured on Init.
> +	 */
> +	if (chip->chip_id == MAX20024)
> +		goto out;
> +
> +	/* Enable WK_EN0 */
> +	ret = regmap_update_bits(chip->rmap, MAX77620_REG_ONOFFCNFG2,
> +				 MAX77620_ONOFFCNFG2_WK_EN0,
> +		MAX77620_ONOFFCNFG2_WK_EN0);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to reg ONOFFCNFG2 WK_EN0 update: %d\n",
> +			ret);
> +		return ret;
> +	}
> +
> +out:
> +	enable_irq(client->irq);
> +
> +	return 0;
> +}
> +#endif
> +
> +static const struct i2c_device_id max77620_id[] = {
> +	{"max77620", MAX77620},
> +	{"max20024", MAX20024},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(i2c, max77620_id);
> +
> +static const struct dev_pm_ops max77620_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(max77620_i2c_suspend, max77620_i2c_resume)
> +};
> +
> +static struct i2c_driver max77620_driver = {
> +	.driver = {
> +		.name = "max77620",
> +		.pm = &max77620_pm_ops,
> +	},
> +	.probe = max77620_probe,
> +	.remove = max77620_remove,
> +	.id_table = max77620_id,
> +};
> +
> +module_i2c_driver(max77620_driver);
> +
> +MODULE_DESCRIPTION("MAX77620/MAX20024 Multi Function Device Core Driver");
> +MODULE_AUTHOR("Laxman Dewangan <ldewangan@...dia.com>");
> +MODULE_AUTHOR("Chaitanya Bandi <bandik@...dia.com>");
> +MODULE_AUTHOR("Mallikarjun Kasoju <mkasoju@...dia.com>");
> +MODULE_ALIAS("i2c:max77620");

The I2C subsystem should do this for you.

> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mfd/max77620.h b/include/linux/mfd/max77620.h
> new file mode 100644
> index 0000000..db34107
> --- /dev/null
> +++ b/include/linux/mfd/max77620.h
> @@ -0,0 +1,337 @@
> +/*
> + * Defining registers address and its bit definitions of MAX77620 and MAX20024
> + *
> + * Copyright (C) 2016 NVIDIA CORPORATION. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + */
> +
> +#ifndef _LINUX_MFD_MAX77620_H_
> +#define _LINUX_MFD_MAX77620_H_

Just drop the LINUX part.

> +#include <linux/types.h>
> +
> +/* GLOBAL, PMIC, GPIO, FPS, ONOFFC, CID Registers */
> +#define MAX77620_REG_CNFGGLBL1			0x00
> +#define MAX77620_REG_CNFGGLBL2			0x01
> +#define MAX77620_REG_CNFGGLBL3			0x02
> +#define MAX77620_REG_CNFG1_32K			0x03
> +#define MAX77620_REG_CNFGBBC			0x04
> +#define MAX77620_REG_IRQTOP			0x05
> +#define MAX77620_REG_INTLBT			0x06
> +#define MAX77620_REG_IRQSD			0x07
> +#define MAX77620_REG_IRQ_LVL2_L0_7		0x08
> +#define MAX77620_REG_IRQ_LVL2_L8		0x09
> +#define MAX77620_REG_IRQ_LVL2_GPIO		0x0A
> +#define MAX77620_REG_ONOFFIRQ			0x0B
> +#define MAX77620_REG_NVERC			0x0C
> +#define MAX77620_REG_IRQTOPM			0x0D
> +#define MAX77620_REG_INTENLBT			0x0E
> +#define MAX77620_REG_IRQMASKSD			0x0F
> +#define MAX77620_REG_IRQ_MSK_L0_7		0x10
> +#define MAX77620_REG_IRQ_MSK_L8			0x11
> +#define MAX77620_REG_ONOFFIRQM			0x12
> +#define MAX77620_REG_STATLBT			0x13
> +#define MAX77620_REG_STATSD			0x14
> +#define MAX77620_REG_ONOFFSTAT			0x15
> +
> +/* SD and LDO Registers */
> +#define MAX77620_REG_SD0			0x16
> +#define MAX77620_REG_SD1			0x17
> +#define MAX77620_REG_SD2			0x18
> +#define MAX77620_REG_SD3			0x19
> +#define MAX77620_REG_SD4			0x1A
> +#define MAX77620_REG_DVSSD0			0x1B
> +#define MAX77620_REG_DVSSD1			0x1C
> +#define MAX77620_REG_SD0_CFG			0x1D
> +#define MAX77620_REG_SD1_CFG			0x1E
> +#define MAX77620_REG_SD2_CFG			0x1F
> +#define MAX77620_REG_SD3_CFG			0x20
> +#define MAX77620_REG_SD4_CFG			0x21
> +#define MAX77620_REG_SD_CFG2			0x22
> +#define MAX77620_REG_LDO0_CFG			0x23
> +#define MAX77620_REG_LDO0_CFG2			0x24
> +#define MAX77620_REG_LDO1_CFG			0x25
> +#define MAX77620_REG_LDO1_CFG2			0x26
> +#define MAX77620_REG_LDO2_CFG			0x27
> +#define MAX77620_REG_LDO2_CFG2			0x28
> +#define MAX77620_REG_LDO3_CFG			0x29
> +#define MAX77620_REG_LDO3_CFG2			0x2A
> +#define MAX77620_REG_LDO4_CFG			0x2B
> +#define MAX77620_REG_LDO4_CFG2			0x2C
> +#define MAX77620_REG_LDO5_CFG			0x2D
> +#define MAX77620_REG_LDO5_CFG2			0x2E
> +#define MAX77620_REG_LDO6_CFG			0x2F
> +#define MAX77620_REG_LDO6_CFG2			0x30
> +#define MAX77620_REG_LDO7_CFG			0x31
> +#define MAX77620_REG_LDO7_CFG2			0x32
> +#define MAX77620_REG_LDO8_CFG			0x33
> +#define MAX77620_REG_LDO8_CFG2			0x34
> +#define MAX77620_REG_LDO_CFG3			0x35
> +
> +#define MAX77620_LDO_SLEW_RATE_MASK		0x1
> +
> +/* LDO Configuration 3 */
> +#define MAX77620_TRACK4_MASK			BIT(5)
> +#define MAX77620_TRACK4_SHIFT			5
> +
> +/* Voltage */
> +#define MAX77620_SDX_VOLT_MASK			0xFF
> +#define MAX77620_SD0_VOLT_MASK			0x3F
> +#define MAX77620_SD1_VOLT_MASK			0x7F
> +#define MAX77620_LDO_VOLT_MASK			0x3F
> +
> +#define MAX77620_REG_GPIO0			0x36
> +#define MAX77620_REG_GPIO1			0x37
> +#define MAX77620_REG_GPIO2			0x38
> +#define MAX77620_REG_GPIO3			0x39
> +#define MAX77620_REG_GPIO4			0x3A
> +#define MAX77620_REG_GPIO5			0x3B
> +#define MAX77620_REG_GPIO6			0x3C
> +#define MAX77620_REG_GPIO7			0x3D
> +#define MAX77620_REG_PUE_GPIO			0x3E
> +#define MAX77620_REG_PDE_GPIO			0x3F
> +#define MAX77620_REG_AME_GPIO			0x40
> +#define MAX77620_REG_ONOFFCNFG1			0x41
> +#define MAX77620_REG_ONOFFCNFG2			0x42
> +
> +/* FPS Registers */
> +#define MAX77620_REG_FPS_CFG0			0x43
> +#define MAX77620_REG_FPS_CFG1			0x44
> +#define MAX77620_REG_FPS_CFG2			0x45
> +#define MAX77620_REG_FPS_LDO0			0x46
> +#define MAX77620_REG_FPS_LDO1			0x47
> +#define MAX77620_REG_FPS_LDO2			0x48
> +#define MAX77620_REG_FPS_LDO3			0x49
> +#define MAX77620_REG_FPS_LDO4			0x4A
> +#define MAX77620_REG_FPS_LDO5			0x4B
> +#define MAX77620_REG_FPS_LDO6			0x4C
> +#define MAX77620_REG_FPS_LDO7			0x4D
> +#define MAX77620_REG_FPS_LDO8			0x4E
> +#define MAX77620_REG_FPS_SD0			0x4F
> +#define MAX77620_REG_FPS_SD1			0x50
> +#define MAX77620_REG_FPS_SD2			0x51
> +#define MAX77620_REG_FPS_SD3			0x52
> +#define MAX77620_REG_FPS_SD4			0x53
> +#define MAX77620_REG_FPS_NONE			0
> +
> +#define MAX77620_FPS_SRC_MASK			0xC0
> +#define MAX77620_FPS_SRC_SHIFT			6
> +#define MAX77620_FPS_PU_PERIOD_MASK		0x38
> +#define MAX77620_FPS_PU_PERIOD_SHIFT		3
> +#define MAX77620_FPS_PD_PERIOD_MASK		0x07
> +#define MAX77620_FPS_PD_PERIOD_SHIFT		0
> +#define MAX77620_FPS_TIME_PERIOD_MASK		0x38
> +#define MAX77620_FPS_TIME_PERIOD_SHIFT		3
> +#define MAX77620_FPS_EN_SRC_MASK		0x06
> +#define MAX77620_FPS_EN_SRC_SHIFT		1
> +#define MAX77620_FPS_ENFPS_SW_MASK		0x01
> +#define MAX77620_FPS_ENFPS_SW			0x01
> +
> +#define MAX77620_REG_FPS_GPIO1			0x54
> +#define MAX77620_REG_FPS_GPIO2			0x55
> +#define MAX77620_REG_FPS_GPIO3			0x56
> +#define MAX77620_REG_FPS_RSO			0x57
> +#define MAX77620_REG_CID0			0x58
> +#define MAX77620_REG_CID1			0x59
> +#define MAX77620_REG_CID2			0x5A
> +#define MAX77620_REG_CID3			0x5B
> +#define MAX77620_REG_CID4			0x5C
> +#define MAX77620_REG_CID5			0x5D
> +
> +#define MAX77620_REG_DVSSD4			0x5E
> +#define MAX20024_REG_MAX_ADD			0x70
> +
> +#define MAX77620_CID_DIDM_MASK			0xF0
> +#define MAX77620_CID_DIDM_SHIFT			4
> +
> +/* CNCG2SD */
> +#define MAX77620_SD_CNF2_ROVS_EN_SD1		BIT(1)
> +#define MAX77620_SD_CNF2_ROVS_EN_SD0		BIT(2)
> +
> +/* Device Identification Metal */
> +#define MAX77620_CID5_DIDM(n)			(((n) >> 4) & 0xF)
> +/* Device Indentification OTP */
> +#define MAX77620_CID5_DIDO(n)			((n) & 0xF)
> +
> +/* SD CNFG1 */
> +#define MAX77620_SD_SR_MASK			0xC0
> +#define MAX77620_SD_SR_SHIFT			6
> +#define MAX77620_SD_POWER_MODE_MASK		0x30
> +#define MAX77620_SD_POWER_MODE_SHIFT		4
> +#define MAX77620_SD_CFG1_ADE_MASK		BIT(3)
> +#define MAX77620_SD_CFG1_ADE_DISABLE		0
> +#define MAX77620_SD_CFG1_ADE_ENABLE		BIT(3)
> +#define MAX77620_SD_FPWM_MASK			0x04
> +#define MAX77620_SD_FPWM_SHIFT			2
> +#define MAX77620_SD_FSRADE_MASK			0x01
> +#define MAX77620_SD_FSRADE_SHIFT		0
> +#define MAX77620_SD_CFG1_FPWM_SD_MASK		BIT(2)
> +#define MAX77620_SD_CFG1_FPWM_SD_SKIP		0
> +#define MAX77620_SD_CFG1_FPWM_SD_FPWM		BIT(2)
> +#define MAX77620_SD_CFG1_FSRADE_SD_MASK		BIT(0)
> +#define MAX77620_SD_CFG1_FSRADE_SD_DISABLE	0
> +#define MAX77620_SD_CFG1_FSRADE_SD_ENABLE	BIT(0)
> +
> +/* LDO_CNFG2 */
> +#define MAX77620_LDO_POWER_MODE_MASK		0xC0
> +#define MAX77620_LDO_POWER_MODE_SHIFT		6
> +#define MAX77620_LDO_CFG2_ADE_MASK		BIT(1)
> +#define MAX77620_LDO_CFG2_ADE_DISABLE		0
> +#define MAX77620_LDO_CFG2_ADE_ENABLE		BIT(1)
> +#define MAX77620_LDO_CFG2_SS_MASK		BIT(0)
> +#define MAX77620_LDO_CFG2_SS_FAST		BIT(0)
> +#define MAX77620_LDO_CFG2_SS_SLOW		0
> +
> +#define MAX77620_IRQ_TOP_GLBL_MASK		BIT(7)
> +#define MAX77620_IRQ_TOP_SD_MASK		BIT(6)
> +#define MAX77620_IRQ_TOP_LDO_MASK		BIT(5)
> +#define MAX77620_IRQ_TOP_GPIO_MASK		BIT(4)
> +#define MAX77620_IRQ_TOP_RTC_MASK		BIT(3)
> +#define MAX77620_IRQ_TOP_32K_MASK		BIT(2)
> +#define MAX77620_IRQ_TOP_ONOFF_MASK		BIT(1)
> +
> +#define MAX77620_IRQ_LBM_MASK			BIT(3)
> +#define MAX77620_IRQ_TJALRM1_MASK		BIT(2)
> +#define MAX77620_IRQ_TJALRM2_MASK		BIT(1)
> +
> +#define MAX77620_PWR_I2C_ADDR			0x3c
> +#define MAX77620_RTC_I2C_ADDR			0x68
> +
> +#define MAX77620_CNFG_GPIO_DRV_MASK		BIT(0)
> +#define MAX77620_CNFG_GPIO_DRV_PUSHPULL		BIT(0)
> +#define MAX77620_CNFG_GPIO_DRV_OPENDRAIN	0
> +#define MAX77620_CNFG_GPIO_DIR_MASK		BIT(1)
> +#define MAX77620_CNFG_GPIO_DIR_INPUT		BIT(1)
> +#define MAX77620_CNFG_GPIO_DIR_OUTPUT		0
> +#define MAX77620_CNFG_GPIO_INPUT_VAL_MASK	BIT(2)
> +#define MAX77620_CNFG_GPIO_OUTPUT_VAL_MASK	BIT(3)
> +#define MAX77620_CNFG_GPIO_OUTPUT_VAL_HIGH	BIT(3)
> +#define MAX77620_CNFG_GPIO_OUTPUT_VAL_LOW	0
> +#define MAX77620_CNFG_GPIO_INT_MASK		(0x3 << 4)
> +#define MAX77620_CNFG_GPIO_INT_FALLING		BIT(4)
> +#define MAX77620_CNFG_GPIO_INT_RISING		BIT(5)
> +#define MAX77620_CNFG_GPIO_DBNC_MASK		(0x3 << 6)
> +#define MAX77620_CNFG_GPIO_DBNC_None		(0x0 << 6)
> +#define MAX77620_CNFG_GPIO_DBNC_8ms		(0x1 << 6)
> +#define MAX77620_CNFG_GPIO_DBNC_16ms		(0x2 << 6)
> +#define MAX77620_CNFG_GPIO_DBNC_32ms		(0x3 << 6)
> +
> +#define MAX77620_IRQ_LVL2_GPIO_EDGE0		BIT(0)
> +#define MAX77620_IRQ_LVL2_GPIO_EDGE1		BIT(1)
> +#define MAX77620_IRQ_LVL2_GPIO_EDGE2		BIT(2)
> +#define MAX77620_IRQ_LVL2_GPIO_EDGE3		BIT(3)
> +#define MAX77620_IRQ_LVL2_GPIO_EDGE4		BIT(4)
> +#define MAX77620_IRQ_LVL2_GPIO_EDGE5		BIT(5)
> +#define MAX77620_IRQ_LVL2_GPIO_EDGE6		BIT(6)
> +#define MAX77620_IRQ_LVL2_GPIO_EDGE7		BIT(7)
> +
> +#define MAX77620_CNFG1_32K_OUT0_EN		BIT(2)
> +
> +#define MAX77620_ONOFFCNFG1_SFT_RST		BIT(7)
> +#define MAX77620_ONOFFCNFG1_MRT_MASK		0x38
> +#define MAX77620_ONOFFCNFG1_MRT_SHIFT		0x3
> +#define MAX77620_ONOFFCNFG1_SLPEN		BIT(2)
> +#define MAX77620_ONOFFCNFG1_PWR_OFF		BIT(1)
> +#define MAX20024_ONOFFCNFG1_CLRSE		0x18
> +
> +#define MAX77620_ONOFFCNFG2_SFT_RST_WK		BIT(7)
> +#define MAX77620_ONOFFCNFG2_WD_RST_WK		BIT(6)
> +#define MAX77620_ONOFFCNFG2_SLP_LPM_MSK		BIT(5)
> +#define MAX77620_ONOFFCNFG2_WK_ALARM1		BIT(2)
> +#define MAX77620_ONOFFCNFG2_WK_EN0		BIT(0)
> +
> +#define MAX77620_GLBLM_MASK			BIT(0)
> +
> +#define MAX77620_WDTC_MASK			0x3
> +#define MAX77620_WDTOFFC			BIT(4)
> +#define MAX77620_WDTSLPC			BIT(3)
> +#define MAX77620_WDTEN				BIT(2)
> +
> +#define MAX77620_TWD_MASK			0x3
> +#define MAX77620_TWD_2s				0x0
> +#define MAX77620_TWD_16s			0x1
> +#define MAX77620_TWD_64s			0x2
> +#define MAX77620_TWD_128s			0x3
> +
> +#define MAX77620_CNFGGLBL1_LBDAC_EN		BIT(7)
> +#define MAX77620_CNFGGLBL1_MPPLD		BIT(6)
> +#define MAX77620_CNFGGLBL1_LBHYST		(BIT(5) | BIT(4))
> +#define MAX77620_CNFGGLBL1_LBDAC		0x0E
> +#define MAX77620_CNFGGLBL1_LBRSTEN		BIT(0)
> +
> +/* CNFG BBC registers */
> +#define MAX77620_CNFGBBC_ENABLE			BIT(0)
> +#define MAX77620_CNFGBBC_CURRENT_MASK		0x06
> +#define MAX77620_CNFGBBC_CURRENT_SHIFT		1
> +#define MAX77620_CNFGBBC_VOLTAGE_MASK		0x18
> +#define MAX77620_CNFGBBC_VOLTAGE_SHIFT		3
> +#define MAX77620_CNFGBBC_LOW_CURRENT_DISABLE	BIT(5)
> +#define MAX77620_CNFGBBC_RESISTOR_MASK		0xC0
> +#define MAX77620_CNFGBBC_RESISTOR_SHIFT		6
> +
> +#define MAX77620_FPS_COUNT			3
> +
> +/* Interrupts */
> +enum {
> +	MAX77620_IRQ_TOP_GLBL,		/* Low-Battery */
> +	MAX77620_IRQ_TOP_SD,		/* SD power fail */
> +	MAX77620_IRQ_TOP_LDO,		/* LDO power fail */
> +	MAX77620_IRQ_TOP_GPIO,		/* TOP GPIO internal int to MAX77620 */
> +	MAX77620_IRQ_TOP_RTC,		/* RTC */
> +	MAX77620_IRQ_TOP_32K,		/* 32kHz oscillator */
> +	MAX77620_IRQ_TOP_ONOFF,		/* ON/OFF oscillator */
> +	MAX77620_IRQ_LBT_MBATLOW,	/* Thermal alarm status, > 120C */
> +	MAX77620_IRQ_LBT_TJALRM1,	/* Thermal alarm status, > 120C */
> +	MAX77620_IRQ_LBT_TJALRM2,	/* Thermal alarm status, > 140C */
> +};
> +
> +/* GPIOs */
> +enum {
> +	MAX77620_GPIO0,
> +	MAX77620_GPIO1,
> +	MAX77620_GPIO2,
> +	MAX77620_GPIO3,
> +	MAX77620_GPIO4,
> +	MAX77620_GPIO5,
> +	MAX77620_GPIO6,
> +	MAX77620_GPIO7,
> +	MAX77620_GPIO_NR,
> +};
> +
> +/* FPS Source */
> +enum max77620_fps_src {
> +	MAX77620_FPS_SRC_0,
> +	MAX77620_FPS_SRC_1,
> +	MAX77620_FPS_SRC_2,
> +	MAX77620_FPS_SRC_NONE,
> +	MAX77620_FPS_SRC_DEF,
> +};
> +
> +enum max77620_chip_id {
> +	MAX77620,
> +	MAX20024,
> +};
> +
> +struct max77620_chip {
> +	struct device *dev;
> +	struct regmap *rmap;
> +
> +	int chip_irq;
> +	int irq_base;
> +
> +	/* chip id */
> +	enum max77620_chip_id chip_id;
> +
> +	bool sleep_enable;
> +	bool enable_global_lpm;
> +	int shutdown_fps_period[MAX77620_FPS_COUNT];
> +	int suspend_fps_period[MAX77620_FPS_COUNT];
> +
> +	struct regmap_irq_chip_data *top_irq_data;
> +	struct regmap_irq_chip_data *gpio_irq_data;
> +};
> +
> +#endif /* _LINUX_MFD_MAX77620_H_ */

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Powered by blists - more mailing lists