[U-Boot] [PATCH v2 02/20] i2c: Add TPS6586X driver

[Heiko Schocher <hs@denx.de>]

Hello Simon,

Simon Glass wrote:
> This power management chip supports battery charging and a large number
> of power supplies. This initial driver only provides the ability to adjust
> the two synchronous buck converters SM0 and SM1 in a stepwise manner.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> Changes in v2:
> - Split PMU code into separate TPS6586X driver
> 
>  drivers/power/Makefile   |    1 +
>  drivers/power/tps6586x.c |  293 ++++++++++++++++++++++++++++++++++++++++++++++
>  include/tps6586x.h       |   68 +++++++++++
>  3 files changed, 362 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/power/tps6586x.c
>  create mode 100644 include/tps6586x.h
> 
> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
> index ead00f8..9db8ea2 100644
> --- a/drivers/power/Makefile
> +++ b/drivers/power/Makefile
> @@ -28,6 +28,7 @@ LIB 	:= $(obj)libpower.o
>  COBJS-$(CONFIG_FTPMU010_POWER)	+= ftpmu010.o
>  COBJS-$(CONFIG_TWL4030_POWER)	+= twl4030.o
>  COBJS-$(CONFIG_TWL6030_POWER)	+= twl6030.o
> +COBJS-$(CONFIG_TPS6586X_POWER)	+= tps6586x.o

please hold lists sorted.

>  COBJS	:= $(COBJS-y)
>  SRCS 	:= $(COBJS:.o=.c)
> diff --git a/drivers/power/tps6586x.c b/drivers/power/tps6586x.c
> new file mode 100644
> index 0000000..5915d6f
> --- /dev/null
> +++ b/drivers/power/tps6586x.c
> @@ -0,0 +1,293 @@
> +/*
> + * Copyright (c) 2011 The Chromium OS Authors.
> + * (C) Copyright 2010,2011 NVIDIA Corporation <www.nvidia.com>
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program 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 General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#include <common.h>
> +#include <tps6586x.h>
> +#include <asm/io.h>
> +#include <i2c.h>
> +
> +/*
> + * abs() handles unsigned ints, shorts and chars and returns a signed long.
> + * TODO: Move this into common?
> + */
> +#define abs(x) ({						\
> +		long ret;					\
> +		{						\
> +			typeof((x)) __x = (x);			\
> +			ret = (__x < 0) ? -__x : __x;		\
> +		}						\
> +		ret;						\
> +	})

I thought we have such a define, but couldn't find it ... I vote
for moving it to common ...

[...]
> diff --git a/include/tps6586x.h b/include/tps6586x.h
> new file mode 100644
> index 0000000..ab88082
> --- /dev/null
> +++ b/include/tps6586x.h
> @@ -0,0 +1,68 @@
[...]
> +
> +/**

wrong comment style.

> + * Enable PWM mode for selected SM0-2
> + *
> + * @param mask	Mask of synchronous converter to enable (TPS6586X_PWM_...)
> + * @return 0 if ok, -1 on error
> + */
> +int tps6586x_set_pwm_mode(int mask);
> +
> +/**

here too, please fix globally.

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany