RE: [RFC V1] COMMIT 1: DA9210 driver files

[Opensource [Steve Twiss]]

On Fri, 21 Jun 2013, Guennadi Liakhovetski wrote:

>
>On Thu, 20 Jun 2013, Steve Twiss wrote:
>
>> From: Steve Twiss <stwiss.opensource@xxxxxxxxxxx>
>>
>> This is the regulator driver for the Dialog DA9210 Multi-phase Buck.
>> The patch is relative to linux-next next-20130620
>>
>> The regulator implements the functions for .enable, .set_voltage,
>> .get_voltage, .set_current_limit, and .get_current_limit. It uses the
>> kernel standard functions for .disable, .is_enabled and .list_voltage.
>>
>> This file contains a regulator driver and I2C driver combined into the
>> same file and has been tested on a Samsung SMDK6410 connected to a
>> Dialog
>> DA9210 Evaluation Board through a I2C connection.
>>
>> It would be appreciated if you could add any comments you may have
>> about the driver.
>>
>> Signed-off-by: Steve Twiss <stwiss.opensource@xxxxxxxxxxx>
>> Signed-off-by: David Dajun Chen <david.chen@xxxxxxxxxxx>
>> ---
>> Checks performed with linux-next/next-20130620/scripts/checkpatch.pl
>>  Kconfig             total: 0 errors, 0 warnings, 535 lines checked
>>  da9210-regulator.c  total: 0 errors, 0 warnings, 334 lines checked
>>  Makefile            total: 0 errors, 0 warnings, 77 lines checked
>>  da9210-regulator.h  total: 0 errors, 0 warnings, 286 lines checked
>>
>>  drivers/regulator/Kconfig                  |    7 +
>>  drivers/regulator/Makefile                 |    1 +
>>  drivers/regulator/da9210-regulator.c       |  334 ++++++++++++++++++++++++++++
>>  include/linux/regulator/da9210-regulator.h |  286
>> ++++++++++++++++++++++++
>>  4 files changed, 628 insertions(+), 0 deletions(-)  create mode
>> 100644 drivers/regulator/da9210-regulator.c
>>  create mode 100644 include/linux/regulator/da9210-regulator.h
>>
>> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
>> index 9296425..aea2fc8 100644
>> --- a/drivers/regulator/Kconfig
>> +++ b/drivers/regulator/Kconfig
>> @@ -293,6 +293,13 @@ config REGULATOR_LP8788
>>  	help
>>  	  This driver supports LP8788 voltage regulator chip.
>>
>> +config REGULATOR_DA9210
>> +	bool "Dialog Semiconductor DA9210 Regulator"
>
>You don't support modular build? I think you should.
>
>> +	depends on I2C=y
>
>Then you won't need =y either
>

Yes I should have added this option.
I will add this and re-submit.

>> +	select REGMAP_I2C
>> +	help
>> +	  Support for the Dialog Semiconductor DA9210 chip.
>> +
>>  config REGULATOR_PCF50633
>>  	tristate "NXP PCF50633 regulator driver"
>>  	depends on MFD_PCF50633
>> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
>> index 26e6c4a..0155e2a 100644
>> --- a/drivers/regulator/Makefile
>> +++ b/drivers/regulator/Makefile
>> @@ -20,6 +20,7 @@ obj-$(CONFIG_REGULATOR_AS3711) += as3711-regulator.o
>>  obj-$(CONFIG_REGULATOR_DA903X)	+= da903x.o
>>  obj-$(CONFIG_REGULATOR_DA9052)	+= da9052-regulator.o
>>  obj-$(CONFIG_REGULATOR_DA9055)	+= da9055-regulator.o
>> +obj-$(CONFIG_REGULATOR_DA9210) += da9210-regulator.o
>>  obj-$(CONFIG_REGULATOR_DBX500_PRCMU) += dbx500-prcmu.o
>>  obj-$(CONFIG_REGULATOR_DB8500_PRCMU) += db8500-prcmu.o
>>  obj-$(CONFIG_REGULATOR_FAN53555) += fan53555.o diff --git
>> a/drivers/regulator/da9210-regulator.c
>> b/drivers/regulator/da9210-regulator.c
>> new file mode 100644
>> index 0000000..5e6691e
>> --- /dev/null
>> +++ b/drivers/regulator/da9210-regulator.c
>> @@ -0,0 +1,334 @@
>> +
>> +/* da9210-regulator.c - Regulator device driver for DA9210
>> + * Copyright (C) 2013  Dialog Semiconductor Ltd.
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Library General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Library General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Library General Public
>> + * License along with this library; if not, write to the
>> + * Free Software Foundation, Inc., 51 Franklin St, Fifth Floor,
>> + * Boston, MA  02110-1301, USA.
>> + */
>> +
>> +#include <linux/err.h>
>> +#include <linux/i2c.h>
>> +#include <linux/module.h>
>> +#include <linux/regulator/driver.h>
>> +#include <linux/regulator/machine.h>
>> +#include <linux/regulator/da9210-regulator.h>
>> +#include <linux/regmap.h>
>
>This driver doesn't support DT. This is ok for the first submission, I think, but we'll
>definitely need it soon.
>
>Some headers are also missing here, e.g. slab.h, init.h
>

Added <linux/init.h> and <linux/slab.h>

>> +
>> +#define DRIVER_NAME	"da9210"
>> +
>> +struct da9210_regulator_info {
>> +	int min_uV;
>> +	int max_uV;
>> +	unsigned step_uV;
>> +	unsigned n_steps;
>> +
>> +	unsigned n_current_limits;
>> +	const int *current_limits;
>> +};
>> +
>> +struct da9210 {
>> +	struct i2c_client *i2c;
>> +	struct device *dev;
>> +	struct mutex io_mutex;
>> +	const struct da9210_regulator_info *info;
>> +	struct regulator_desc desc;
>> +	struct regulator_dev *rdev;
>> +	struct regmap *regmap;
>> +};
>> +
>> +static struct regmap_config da9210_regmap_config = {
>
>you can make this const
>

Yes. Have made it static const struct regmap_config da9210_regmap_config 

>> +	.reg_bits = 8,
>> +	.val_bits = 8,
>> +};
>> +
>> +static int da9210_enable(struct regulator_dev *rdev); static int
>> +da9210_set_voltage(struct regulator_dev *rdev, int min_uV,
>> +			      int max_uV, unsigned *selector); static int
>> +da9210_get_voltage(struct regulator_dev *rdev); static int
>> +da9210_set_current_limit(struct regulator_dev *rdev, int min_uA,
>> +				    int max_uA);
>> +static int da9210_get_current_limit(struct regulator_dev *rdev);
>> +
>> +static struct regulator_ops da9210_buck_ops = {
>> +	.enable = da9210_enable,
>
>Why don't you just assign regulator_enable_regmap here?
>

Oh! 
Done: now using regulator_enable_regmap instead of function just containing regulator_enable_regmap.

>> +	.disable = regulator_disable_regmap,
>> +	.is_enabled = regulator_is_enabled_regmap,
>> +	.set_voltage = da9210_set_voltage,
>> +	.get_voltage = da9210_get_voltage,
>> +	.list_voltage = regulator_list_voltage_linear,
>> +	.set_current_limit = da9210_set_current_limit,
>> +	.get_current_limit = da9210_get_current_limit, };
>> +
>> +/* Default limits measured in millivolts and milliamps */
>> +#define	DA9210_MIN_MV	300
>> +#define	DA9210_MAX_MV	1570
>> +#define	DA9210_MIN_MA	1600
>> +#define	DA9210_MAX_MA	4600
>> +#define	DA9210_STEP_MV	10
>> +
>> +/* Current limits for buck (uA) indices corresponds with register
>> +values */ static const int da9210_buck_limits[] = {
>> +	1600000, 1800000, 2000000, 2200000, 2400000, 2600000, 2800000,
>3000000,
>> +	3200000, 3400000, 3600000, 3800000, 4000000, 4200000, 4400000,
>> +4600000 };
>> +
>> +static const struct da9210_regulator_info da9210_info = {
>> +	.min_uV = (DA9210_MIN_MV * 1000),
>> +	.max_uV = (DA9210_MAX_MV * 1000),
>> +	.step_uV = (DA9210_STEP_MV * 1000),
>> +	.n_steps = (((DA9210_MAX_MV) - (DA9210_MIN_MV)) / (DA9210_STEP_MV))
>> ++ 1,
>
>Way too many parenthesis above. Strictly speaking you don't need this struct, you
>could just use macros / constant variables and remove a bunch of pointer dereferences,
>but it's up to you, I think.
>

Yep. 
Cleaned that up.

>> +	.current_limits = da9210_buck_limits,
>> +	.n_current_limits = ARRAY_SIZE(da9210_buck_limits), };
>> +
>> +static struct regulator_consumer_supply __initdata def_da9210_consumers[] = {
>> +	REGULATOR_SUPPLY("DA9210", NULL),
>> +};
>> +
>> +static struct regulator_init_data __initdata default_da9210_constraints = {
>> +	.constraints = {
>> +			.name = "DA9210-DEFAULT",
>> +			.min_uV = (DA9210_MIN_MV * 1000),
>> +			.max_uV = (DA9210_MAX_MV * 1000),
>> +			.min_uA = (DA9210_MIN_MA * 1000),
>> +			.max_uA = (DA9210_MAX_MA * 1000),
>> +			.uV_offset = 0,
>> +			.always_on = 0,
>> +			.boot_on = 0,
>> +			.apply_uV = 1,
>> +			.valid_ops_mask = REGULATOR_CHANGE_VOLTAGE |
>> +					  REGULATOR_CHANGE_CURRENT |
>> +					  REGULATOR_CHANGE_STATUS,
>> +			},
>> +	.num_consumer_supplies = ARRAY_SIZE(def_da9210_consumers),
>> +	.consumer_supplies = def_da9210_consumers, };
>
>Oops, no, none of the above two structs belong here. Please, remove.
>

I have received a similar comment from Mark Brown.
This part will need to be re-written to follow his suggestions.

>> +
>> +static int da9210_set_voltage(struct regulator_dev *rdev, int min_uV,
>> +			      int max_uV, unsigned *selector) {
>> +	struct da9210 *chip = rdev_get_drvdata(rdev);
>> +	int val;
>> +	int ret;
>> +
>> +	val = regulator_map_voltage_linear(rdev, min_uV, max_uV);
>> +	if (val < 0)
>> +		return -EINVAL;
>
>"return val" would be better
>

Yes. Done it.

>> +
>> +	ret = regmap_update_bits(chip->regmap, DA9210_REG_VBUCK_A,
>> +				 DA9210_VBUCK_MASK, val);
>> +	return ret;
>
>just
>
>+	return regmap_update_bits(chip->regmap, DA9210_REG_VBUCK_A,
>+				  DA9210_VBUCK_MASK, val);
>

Yes. That done that too.

>> +}
>> +
>> +static int da9210_get_voltage_sel(struct regulator_dev *rdev) {
>> +	struct da9210 *chip = rdev_get_drvdata(rdev);
>> +	unsigned int data;
>> +	int sel;
>> +	int ret;
>> +
>> +	ret = regmap_read(chip->regmap, DA9210_REG_VBUCK_A, &data);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	sel = (data & DA9210_VBUCK_MASK) >> DA9210_VBUCK_SHIFT;
>> +	sel -= DA9210_VBUCK_BIAS;
>> +	if (sel < 0)
>> +		sel = 0;
>> +	if (sel >= chip->info->n_steps)
>> +		sel = chip->info->n_steps - 1;
>> +
>> +	return sel;
>> +}
>> +
>> +static int da9210_get_voltage(struct regulator_dev *rdev) {
>> +	struct da9210 *chip = rdev_get_drvdata(rdev);
>> +	int sel = da9210_get_voltage_sel(rdev);
>> +
>> +	if (sel < 0)
>> +		return sel;
>> +
>> +	return (chip->info->step_uV * sel) + chip->info->min_uV;
>
>superfluous parenthesis
>

Mark Brown also commented that this was core functionality, so I need to review this again.

>> +}
>> +
>> +static int da9210_set_current_limit(struct regulator_dev *rdev, int min_uA,
>> +				    int max_uA)
>> +{
>> +	struct da9210 *chip = rdev_get_drvdata(rdev);
>> +	unsigned int sel;
>> +	int i;
>> +
>> +	if (!chip->info->current_limits)
>> +		return -EINVAL;
>
>It's not really possible :) But, well, if you want this check...
>

Ah, this was my attempt at being rigorous
I have removed the check.

>> +
>> +	/* search for closest to maximum */
>> +	for (i = chip->info->n_current_limits - 1; i >= 0; i--) {
>> +		if (min_uA <= chip->info->current_limits[i] &&
>> +		    max_uA >= chip->info->current_limits[i]) {
>> +			sel = i;
>> +			sel = sel << DA9210_BUCK_ILIM_SHIFT;
>> +			return regmap_update_bits(chip->regmap,
>> +						  DA9210_REG_BUCK_ILIM,
>> +						  DA9210_BUCK_ILIM_MASK, sel);
>> +		}
>> +	}
>> +
>> +	return -EINVAL;
>> +}
>> +
>> +static int da9210_get_current_limit(struct regulator_dev *rdev) {
>> +	struct da9210 *chip = rdev_get_drvdata(rdev);
>> +	unsigned int data;
>> +	int sel;
>> +	int ret;
>> +
>> +	ret = regmap_read(chip->regmap, DA9210_REG_BUCK_ILIM, &data);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	sel = (data & DA9210_BUCK_ILIM_MASK) >> DA9210_BUCK_ILIM_SHIFT;
>> +	if (sel < 0)
>
>Don't think sel can be < 0 here.
>
>> +		sel = 0;
>> +	if (sel >= chip->info->n_current_limits)
>
>Your sel is (x & 0xf) >> 0, so, it's 0 <= sel < 16. n_current_limits above == 16, so, this is
>impossible too. If you had simple constants instead of structs, this would have been
>simpler to follow.
>

Yes. This would be simpler if there were constants instead of pointer dereferences
all over the place. I will clean this method up and re-submit it.

>> +		sel = chip->info->n_current_limits - 1;
>> +
>> +	return chip->info->current_limits[sel]; }
>> +
>> +static int da9210_enable(struct regulator_dev *rdev) {
>> +	return regulator_enable_regmap(rdev); }
>> +
>> +/* I2C driver interface functions.
>> + */
>> +
>> +static int da9210_i2c_probe(struct i2c_client *i2c,
>> +			    const struct i2c_device_id *id) {
>> +	struct da9210 *chip;
>> +	const struct regulator_init_data *init_data = NULL;
>> +	struct regulator_dev *rdev = NULL;
>> +	struct regulator_config config = { };
>> +	int error;
>> +
>> +	chip = devm_kzalloc(&i2c->dev, sizeof(struct da9210), GFP_KERNEL);
>> +	if (NULL == chip) {
>> +		dev_err(&i2c->dev,
>> +			"Cannot kzalloc memory for regulator structure\n");
>> +		return -ENOMEM;
>> +	}
>> +
>> +	chip->regmap = devm_regmap_init_i2c(i2c, &da9210_regmap_config);
>> +	if (IS_ERR(chip->regmap)) {
>> +		error = PTR_ERR(chip->regmap);
>> +		dev_err(&i2c->dev, "Failed to allocate register map: %d\n",
>> +			error);
>> +		return error;
>> +	}
>> +
>> +	chip->dev = &i2c->dev;
>> +	chip->i2c = i2c;
>
>you don't use either of the above two pointers. To be precise, you do use
>chip->dev a couple of times below, but you could just use &i2c->dev
>chip->there
>too.
>

Removed unused dev and i2c.

>> +	chip->info = &da9210_info;
>> +
>> +	mutex_init(&chip->io_mutex);
>
>unused?
>

Removed that too.

>> +
>> +	chip->desc.name = "DA9210";
>> +	chip->desc.ops = &da9210_buck_ops;
>> +	chip->desc.n_voltages = chip->info->n_steps;
>> +	chip->desc.type = REGULATOR_VOLTAGE;
>> +	chip->desc.owner = THIS_MODULE;
>> +	chip->desc.id = 0;
>> +	chip->desc.min_uV = chip->info->min_uV;
>> +	chip->desc.uV_step = chip->info->step_uV;
>> +	chip->desc.enable_reg = DA9210_REG_BUCK_CONT;
>> +	chip->desc.enable_mask = DA9210_BUCK_EN;
>> +
>> +	if (!i2c->dev.platform_data)
>> +		init_data = &default_da9210_constraints;
>> +	else
>> +		init_data = i2c->dev.platform_data;
>> +
>> +	config.dev = &i2c->dev;
>> +	config.init_data = init_data;
>> +	config.driver_data = chip;
>> +	config.regmap = chip->regmap;
>> +
>> +	rdev = regulator_register(&chip->desc, &config);
>> +	if (IS_ERR(rdev)) {
>> +		dev_err(chip->dev, "Failed to register DA9210 regulator\n");
>> +		chip->rdev = NULL;
>
>You don't need this assignment
>

Removed setting chip->rdev=NULL.

>> +		return PTR_ERR(rdev);
>> +	}
>> +
>> +	chip->rdev = rdev;
>> +
>> +	i2c_set_clientdata(i2c, chip);
>> +
>> +	dev_info(chip->dev, "Device DA9210 detected.\n");
>> +	return 0;
>> +}
>> +
>> +static int da9210_i2c_remove(struct i2c_client *i2c) {
>> +	struct da9210 *chip = i2c_get_clientdata(i2c);
>> +	regulator_unregister(chip->rdev);
>> +	return 0;
>> +}
>> +
>> +static const struct i2c_device_id da9210_i2c_id[] = {
>> +	{DRIVER_NAME, 0},
>> +	{},
>> +};
>> +
>> +MODULE_DEVICE_TABLE(i2c, da9210_i2c_id);
>> +
>> +static struct i2c_driver da9210_regulator_driver = {
>> +	.driver = {
>> +		.name = DRIVER_NAME,
>> +		.owner = THIS_MODULE,
>> +		},
>> +	.probe = da9210_i2c_probe,
>> +	.remove = da9210_i2c_remove,
>> +	.id_table = da9210_i2c_id,
>> +};
>> +
>> +static int __init da9210_regulator_init(void) {
>> +	int ret;
>> +
>> +	ret = i2c_add_driver(&da9210_regulator_driver);
>> +	if (0 != ret)
>> +		pr_err("Failed to register da9210 I2C driver\n");
>> +
>> +	return ret;
>> +}
>> +
>> +subsys_initcall(da9210_regulator_init);
>> +
>> +static void __exit da9210_regulator_cleanup(void) {
>> +	i2c_del_driver(&da9210_regulator_driver);
>> +}
>> +
>> +module_exit(da9210_regulator_cleanup);
>> +
>> +MODULE_AUTHOR("S Twiss <stwiss.opensource@xxxxxxxxxxx>");
>> +MODULE_DESCRIPTION("Regulator device driver for Dialog DA9210");
>> +MODULE_LICENSE("GPL v2"); MODULE_ALIAS("platform:" DRIVER_NAME);
>> diff --git a/include/linux/regulator/da9210-regulator.h
>> b/include/linux/regulator/da9210-regulator.h
>> new file mode 100644
>> index 0000000..e6748d6
>> --- /dev/null
>> +++ b/include/linux/regulator/da9210-regulator.h
>
>You don't need this header under include/... Please, either move it to drivers/regulator/
>or just move all these defines into the .c file.
>
>Thanks
>Guennadi
>

I was stumped where this should I go: since this sort of thing normally goes
into ./include/linux/mfd 

I thought about cutting down the majority of this content and putting it 
directly into the C file, but there have been requests for the full list of registers 
(in case somebody wants to implement additional functionality).

I will resubmit this with the header file in ./drivers/regulator 

>> @@ -0,0 +1,286 @@
>> +
>> +/* da9210-regulator.h - Regulator definitions for DA9210
>> + * Copyright (C) 2013  Dialog Semiconductor Ltd.
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Library General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Library General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Library General Public
>> + * License along with this library; if not, write to the
>> + * Free Software Foundation, Inc., 51 Franklin St, Fifth Floor,
>> + * Boston, MA  02110-1301, USA.
>> + */
>> +
>> +#ifndef __DA9210_REGISTERS_H__
>> +#define __DA9210_REGISTERS_H__
>> +
>> +/* Page selection I2C or SPI always in the begining of any page. */
>> +/* Page 0 : I2C access 0x000 - 0x0FF	SPI access 0x000 - 0x07F */
>> +/* Page 1 :				SPI access 0x080 - 0x0FF */
>> +/* Page 2 : I2C access 0x100 - 0x17F	SPI access 0x100 - 0x17F */
>> +#define	DA9210_REG_PAGE_CON			0x00
>> +
>> +/* System Control and Event Registers */
>> +#define	DA9210_REG_STATUS_A			0x50
>> +#define	DA9210_REG_STATUS_B			0x51
>> +#define	DA9210_REG_EVENT_A			0x52
>> +#define	DA9210_REG_EVENT_B			0x53
>> +#define	DA9210_REG_MASK_A			0x54
>> +#define	DA9210_REG_MASK_B			0x55
>> +#define	DA9210_REG_CONTROL_A			0x56
>> +
>> +/* GPIO Control Registers */
>> +#define	DA9210_REG_GPIO_0_1			0x58
>> +#define	DA9210_REG_GPIO_2_3			0x59
>> +#define	DA9210_REG_GPIO_4_5			0x5A
>> +#define	DA9210_REG_GPIO_6			0x5B
>> +
>> +/* Regulator Registers */
>> +#define	DA9210_REG_BUCK_CONT			0x5D
>> +#define	DA9210_REG_BUCK_ILIM			0xD0
>> +#define	DA9210_REG_BUCK_CONF1			0xD1
>> +#define	DA9210_REG_BUCK_CONF2			0xD2
>> +#define DA9210_REG_VBACK_AUTO			0xD4
>> +#define DA9210_REG_VBACK_BASE			0xD5
>> +#define DA9210_REG_VBACK_MAX_DVC_IF		0xD6
>> +#define DA9210_REG_VBACK_DVC			0xD7
>> +#define	DA9210_REG_VBUCK_A			0xD8
>> +#define	DA9210_REG_VBUCK_B			0xD9
>> +
>> +/* I2C Interface Settings */
>> +#define DA9210_REG_INTERFACE			0x105
>> +
>> +/* OTP */
>> +#define	DA9210_REG_OPT_COUNT			0x140
>> +#define	DA9210_REG_OPT_ADDR			0x141
>> +#define	DA9210_REG_OPT_DATA			0x142
>> +
>> +/* Customer Trim and Configuration */
>> +#define	DA9210_REG_CONFIG_A			0x143
>> +#define	DA9210_REG_CONFIG_B			0x144
>> +#define	DA9210_REG_CONFIG_C			0x145
>> +#define	DA9210_REG_CONFIG_D			0x146
>> +#define	DA9210_REG_CONFIG_E			0x147
>> +
>> +
>> +/*
>> + * Registers bits
>> + */
>> +/* DA9210_REG_PAGE_CON (addr=0x00) */
>> +#define	DA9210_PEG_PAGE_SHIFT			0
>> +#define	DA9210_REG_PAGE_MASK			0x0F
>> +/* On I2C registers 0x00 - 0xFF */
>> +#define	DA9210_REG_PAGE0			0
>> +/* On I2C registers 0x100 - 0x1FF */
>> +#define	DA9210_REG_PAGE2			2
>> +#define	DA9210_PAGE_WRITE_MODE			0x00
>> +#define	DA9210_REPEAT_WRITE_MODE		0x40
>> +#define	DA9210_PAGE_REVERT			0x80
>> +
>> +/* DA9210_REG_STATUS_A (addr=0x50) */
>> +#define	DA9210_GPI0				0x01
>> +#define	DA9210_GPI1				0x02
>> +#define	DA9210_GPI2				0x04
>> +#define	DA9210_GPI3				0x08
>> +#define	DA9210_GPI4				0x10
>> +#define	DA9210_GPI5				0x20
>> +#define	DA9210_GPI6				0x40
>> +
>> +/* DA9210_REG_EVENT_A (addr=0x52) */
>> +#define	DA9210_E_GPI0				0x01
>> +#define	DA9210_E_GPI1				0x02
>> +#define	DA9210_E_GPI2				0x04
>> +#define	DA9210_E_GPI3				0x08
>> +#define	DA9210_E_GPI4				0x10
>> +#define	DA9210_E_GPI5				0x20
>> +#define	DA9210_E_GPI6				0x40
>> +
>> +/* DA9210_REG_EVENT_B (addr=0x53) */
>> +#define	DA9210_E_OVCURR				0x01
>> +#define	DA9210_E_NPWRGOOD			0x02
>> +#define	DA9210_E_TEMP_WARN			0x04
>> +#define	DA9210_E_TEMP_CRIT			0x08
>> +#define	DA9210_E_VMAX				0x10
>> +
>> +/* DA9210_REG_MASK_A (addr=0x54) */
>> +#define	DA9210_M_GPI0				0x01
>> +#define	DA9210_M_GPI1				0x02
>> +#define	DA9210_M_GPI2				0x04
>> +#define	DA9210_M_GPI3				0x08
>> +#define	DA9210_M_GPI4				0x10
>> +#define	DA9210_M_GPI5				0x20
>> +#define	DA9210_M_GPI6				0x40
>> +
>> +/* DA9210_REG_MASK_B (addr=0x55) */
>> +#define	DA9210_M_OVCURR				0x01
>> +#define	DA9210_M_NPWRGOOD			0x02
>> +#define	DA9210_M_TEMP_WARN			0x04
>> +#define	DA9210_M_TEMP_CRIT			0x08
>> +#define	DA9210_M_VMAX				0x10
>> +
>> +/* DA9210_REG_CONTROL_A (addr=0x56) */
>> +#define	DA9210_DEBOUNCING_SHIFT			0
>> +#define	DA9210_DEBOUNCING_MASK			0x07
>> +#define	DA9210_SLEW_RATE_SHIFT			3
>> +#define	DA9210_SLEW_RATE_MASK			0x18
>> +#define	DA9210_V_LOCK				0x20
>> +
>> +/* DA9210_REG_GPIO_0_1 (addr=0x58) */
>> +#define	DA9210_GPIO0_PIN_SHIFT			0
>> +#define	DA9210_GPIO0_PIN_MASK			0x03
>> +#define		DA9210_GPIO0_PIN_GPI		0x00
>> +#define		DA9210_GPIO0_PIN_GPO_OD		0x02
>> +#define		DA9210_GPIO0_PIN_GPO		0x03
>> +#define	DA9210_GPIO0_TYPE			0x04
>> +#define		DA9210_GPIO0_TYPE_GPI		0x00
>> +#define		DA9210_GPIO0_TYPE_GPO		0x04
>> +#define	DA9210_GPIO0_MODE			0x08
>> +#define	DA9210_GPIO1_PIN_SHIFT			4
>> +#define	DA9210_GPIO1_PIN_MASK			0x30
>> +#define		DA9210_GPIO1_PIN_GPI		0x00
>> +#define		DA9210_GPIO1_PIN_VERROR		0x10
>> +#define		DA9210_GPIO1_PIN_GPO_OD		0x20
>> +#define		DA9210_GPIO1_PIN_GPO		0x30
>> +#define	DA9210_GPIO1_TYPE_SHIFT			0x40
>> +#define		DA9210_GPIO1_TYPE_GPI		0x00
>> +#define		DA9210_GPIO1_TYPE_GPO		0x40
>> +#define	DA9210_GPIO1_MODE			0x80
>> +
>> +/* DA9210_REG_GPIO_2_3 (addr=0x59) */
>> +#define	DA9210_GPIO2_PIN_SHIFT			0
>> +#define	DA9210_GPIO2_PIN_MASK			0x03
>> +#define		DA9210_GPIO2_PIN_GPI		0x00
>> +#define		DA9210_GPIO5_PIN_BUCK_CLK	0x10
>> +#define		DA9210_GPIO2_PIN_GPO_OD		0x02
>> +#define		DA9210_GPIO2_PIN_GPO		0x03
>> +#define	DA9210_GPIO2_TYPE			0x04
>> +#define		DA9210_GPIO2_TYPE_GPI		0x00
>> +#define		DA9210_GPIO2_TYPE_GPO		0x04
>> +#define	DA9210_GPIO2_MODE			0x08
>> +#define	DA9210_GPIO3_PIN_SHIFT			4
>> +#define	DA9210_GPIO3_PIN_MASK			0x30
>> +#define		DA9210_GPIO3_PIN_GPI		0x00
>> +#define		DA9210_GPIO3_PIN_IERROR		0x10
>> +#define		DA9210_GPIO3_PIN_GPO_OD		0x20
>> +#define		DA9210_GPIO3_PIN_GPO		0x30
>> +#define	DA9210_GPIO3_TYPE_SHIFT			0x40
>> +#define		DA9210_GPIO3_TYPE_GPI		0x00
>> +#define		DA9210_GPIO3_TYPE_GPO		0x40
>> +#define	DA9210_GPIO3_MODE			0x80
>> +
>> +/* DA9210_REG_GPIO_4_5 (addr=0x5A) */
>> +#define	DA9210_GPIO4_PIN_SHIFT			0
>> +#define	DA9210_GPIO4_PIN_MASK			0x03
>> +#define		DA9210_GPIO4_PIN_GPI		0x00
>> +#define		DA9210_GPIO4_PIN_GPO_OD		0x02
>> +#define		DA9210_GPIO4_PIN_GPO		0x03
>> +#define	DA9210_GPIO4_TYPE			0x04
>> +#define		DA9210_GPIO4_TYPE_GPI		0x00
>> +#define		DA9210_GPIO4_TYPE_GPO		0x04
>> +#define	DA9210_GPIO4_MODE			0x08
>> +#define	DA9210_GPIO5_PIN_SHIFT			4
>> +#define	DA9210_GPIO5_PIN_MASK			0x30
>> +#define		DA9210_GPIO5_PIN_GPI		0x00
>> +#define		DA9210_GPIO5_PIN_INTERFACE	0x01
>> +#define		DA9210_GPIO5_PIN_GPO_OD		0x20
>> +#define		DA9210_GPIO5_PIN_GPO		0x30
>> +#define	DA9210_GPIO5_TYPE_SHIFT			0x40
>> +#define		DA9210_GPIO5_TYPE_GPI		0x00
>> +#define		DA9210_GPIO5_TYPE_GPO		0x40
>> +#define	DA9210_GPIO5_MODE			0x80
>> +
>> +/* DA9210_REG_GPIO_6 (addr=0x5B) */
>> +#define	DA9210_GPIO6_PIN_SHIFT			0
>> +#define	DA9210_GPIO6_PIN_MASK			0x03
>> +#define		DA9210_GPIO6_PIN_GPI		0x00
>> +#define		DA9210_GPIO6_PIN_INTERFACE	0x01
>> +#define		DA9210_GPIO6_PIN_GPO_OD		0x02
>> +#define		DA9210_GPIO6_PIN_GPO		0x03
>> +#define	DA9210_GPIO6_TYPE			0x04
>> +#define		DA9210_GPIO6_TYPE_GPI		0x00
>> +#define		DA9210_GPIO6_TYPE_GPO		0x04
>> +#define	DA9210_GPIO6_MODE			0x08
>> +
>> +/* DA9210_REG_BUCK_CONT (addr=0x5D) */
>> +#define	DA9210_BUCK_EN				0x01
>> +#define	DA9210_BUCK_GPI_SHIFT			1
>> +#define DA9210_BUCK_GPI_MASK			0x06
>> +#define		DA9210_BUCK_GPI_OFF		0x00
>> +#define		DA9210_BUCK_GPI_GPIO0		0x02
>> +#define		DA9210_BUCK_GPI_GPIO3		0x04
>> +#define		DA9210_BUCK_GPI_GPIO4		0x06
>> +#define	DA9210_BUCK_PD_DIS			0x08
>> +#define	DA9210_VBUCK_SEL			0x10
>> +#define		DA9210_VBUCK_SEL_A		0x00
>> +#define		DA9210_VBUCK_SEL_B		0x10
>> +#define	DA9210_VBUCK_GPI_SHIFT			5
>> +#define	DA9210_VBUCK_GPI_MASK			0x60
>> +#define		DA9210_VBUCK_GPI_OFF		0x00
>> +#define		DA9210_VBUCK_GPI_GPIO0		0x20
>> +#define		DA9210_VBUCK_GPI_GPIO3		0x40
>> +#define		DA9210_VBUCK_GPI_GPIO4		0x60
>> +#define	DA9210_DVC_CTRL_EN			0x80
>> +
>> +/* DA9210_REG_BUCK_ILIM (addr=0xD0) */
>> +#define DA9210_BUCK_ILIM_SHIFT			0
>> +#define DA9210_BUCK_ILIM_MASK			0x0F
>> +#define DA9210_BUCK_IALARM			0x10
>> +
>> +/* DA9210_REG_BUCK_CONF1 (addr=0xD1) */
>> +#define DA9210_BUCK_MODE_SHIFT			0
>> +#define DA9210_BUCK_MODE_MASK			0x03
>> +#define		DA9210_BUCK_MODE_MANUAL		0x00
>> +#define		DA9210_BUCK_MODE_SLEEP		0x01
>> +#define		DA9210_BUCK_MODE_SYNC		0x02
>> +#define		DA9210_BUCK_MODE_AUTO		0x03
>> +#define DA9210_STARTUP_CTRL_SHIFT		2
>> +#define DA9210_STARTUP_CTRL_MASK		0x1C
>> +#define DA9210_PWR_DOWN_CTRL_SHIFT		5
>> +#define DA9210_PWR_DOWN_CTRL_MASK		0xE0
>> +
>> +/* DA9210_REG_BUCK_CONF2 (addr=0xD2) */
>> +#define DA9210_PHASE_SEL_SHIFT			0
>> +#define DA9210_PHASE_SEL_MASK			0x03
>> +#define DA9210_FREQ_SEL				0x40
>> +
>> +/* DA9210_REG_BUCK_AUTO (addr=0xD4) */
>> +#define DA9210_VBUCK_AUTO_SHIFT			0
>> +#define DA9210_VBUCK_AUTO_MASK			0x7F
>> +
>> +/* DA9210_REG_BUCK_BASE (addr=0xD5) */
>> +#define DA9210_VBUCK_BASE_SHIFT			0
>> +#define DA9210_VBUCK_BASE_MASK			0x7F
>> +
>> +/* DA9210_REG_VBUCK_MAX_DVC_IF (addr=0xD6) */
>> +#define DA9210_VBUCK_MAX_SHIFT			0
>> +#define DA9210_VBUCK_MAX_MASK			0x7F
>> +#define DA9210_DVC_STEP_SIZE			0x80
>> +#define		DA9210_DVC_STEP_SIZE_10MV	0x00
>> +#define		DA9210_DVC_STEP_SIZE_20MV	0x80
>> +
>> +/* DA9210_REG_VBUCK_DVC (addr=0xD7) */
>> +#define DA9210_VBUCK_DVC_SHIFT			0
>> +#define DA9210_VBUCK_DVC_MASK			0x7F
>> +
>> +/* DA9210_REG_VBUCK_A/B (addr=0xD8/0xD9) */
>> +#define DA9210_VBUCK_SHIFT			0
>> +#define DA9210_VBUCK_MASK			0x7F
>> +#define DA9210_VBUCK_BIAS			0
>> +#define DA9210_BUCK_SL				0x80
>> +
>> +/* DA9210_REG_INTERFACE (addr=0x105) */
>> +#define DA9210_IF_BASE_ADDR_SHIFT		4
>> +#define DA9210_IF_BASE_ADDR_MASK		0xF0
>> +
>> +/* DA9210_REG_CONFIG_E (addr=0x147) */
>> +#define DA9210_STAND_ALONE			0x01
>> +
>> +#endif	/* __DA9210_REGISTERS_H__ */
>> +
>> --
>> end-of-patch for RFC V1
>>
>
>---
>Guennadi Liakhovetski, Ph.D.
>Freelance Open-Source Software Developer
>http://www.open-technology.de/

Thank you for the comments.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/