[U-Boot] [PATCH] power: LP8720 regulator support

[Paul Kocialkowski]

Le mardi 04 novembre 2014 à 18:46 +0100, Przemyslaw Marczak a écrit :
> Hello,
> 
> On 11/04/2014 04:58 PM, Tom Rini wrote:
> > On Sat, Nov 01, 2014 at 12:01:37PM +0100, Paul Kocialkowski wrote:
> >> Le mardi 28 octobre 2014 à 18:32 +0100, Paul Kocialkowski a écrit :
> >>> This adds support for the LP8720 i2c regulator, as found in e.g. the LG
> >>> Optimus Black (P970), codename sniper. This code supports setting up and
> >>> enabling one of the 5 LDOs that the IC provides.
> >>> Other more advanced features are unsupported.
> >>>
> >>> Signed-off-by: Paul Kocialkowski <contact at paulk.fr>
> >>> ---
> >>>   drivers/power/Makefile |    1 +
> >>>   drivers/power/lp8720.c |  107 ++++++++++++++++++++++++++++++++++++++++++++++++
> >>>   include/lp8720.h       |   93 +++++++++++++++++++++++++++++++++++++++++
> >>>   3 files changed, 201 insertions(+)
> >>>   create mode 100644 drivers/power/lp8720.c
> >>>   create mode 100644 include/lp8720.h
> >>>
> >>> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
> >>> index dc64e4d..65be5a0 100644
> >>> --- a/drivers/power/Makefile
> >>> +++ b/drivers/power/Makefile
> >>> @@ -9,6 +9,7 @@ obj-$(CONFIG_AXP152_POWER)	+= axp152.o
> >>>   obj-$(CONFIG_AXP209_POWER)	+= axp209.o
> >>>   obj-$(CONFIG_EXYNOS_TMU)	+= exynos-tmu.o
> >>>   obj-$(CONFIG_FTPMU010_POWER)	+= ftpmu010.o
> >>> +obj-$(CONFIG_LP8720_POWER)	+= lp8720.o
> >>>   obj-$(CONFIG_TPS6586X_POWER)	+= tps6586x.o
> >>>   obj-$(CONFIG_TWL4030_POWER)	+= twl4030.o
> >>>   obj-$(CONFIG_TWL6030_POWER)	+= twl6030.o
> >>> diff --git a/drivers/power/lp8720.c b/drivers/power/lp8720.c
> >>> new file mode 100644
> >>> index 0000000..ac7fc11
> >>> --- /dev/null
> >>> +++ b/drivers/power/lp8720.c
> >>> @@ -0,0 +1,107 @@
> >>> +/*
> >>> + * Copyright (C) 2014 Paul Kocialkowski <contact at paulk.fr>
> >>> + *
> >>> + * SPDX-License-Identifier:	GPL-2.0+
> >>> + */
> >>> +
> >>> +#include <common.h>
> >>> +#include <i2c.h>
> >>> +#include <asm/gpio.h>
> >>> +#include <lp8720.h>
> >>> +
> >>> +static struct lp8720_info lp8720_info;
> >>> +
> >>> +static int lp8720_write(u8 reg, u8 val)
> >>> +{
> >>> +	return i2c_write(lp8720_info.chip_idsel, reg, 1, &val, 1);
> >>> +}
> >>> +
> >>> +static int lp8720_read(u8 reg, u8 *val)
> >>> +{
> >>> +	return i2c_read(lp8720_info.chip_idsel, reg, 1, val, 1);
> >>> +}
> >>
> >> Should I ifdef for I2C and GPIO support? It seems that GPIO support only
> >> has board-sepcific config options, so this may be hard. There is
> >> CONFIG_DM_GPIO for driver model, but it is apparently not always used,
> >> especially not on SPL.
> >
> > Don't code for what you can't test.  Which way are you using this on the
> > Optimus?  And in general, any comments here Przemyslaw?  Thanks!

I am using that code both on SPL and regular U-Boot. I'll just drop the
idea of adding ifdef on GPIO and go with it all the way. On the Optimus
Black, a GPIO is used to enable the regulators.

> Paul,
> there are no patches for the board, which you mentioned about. Usually 
> we don't add dead code to U-Boot. If you are going to add new board - 
> just add it with all required drivers in a one patchset - then it is 
> consistent and easy to review.

I understand that. I'll keep your previous comments aside and will
rework the LP8720 code a bit later. I'll focus on having all the changes
I need on already-existing code accepted first and then submit new code
(that relies on these new features), including the Optimus Black board,
configs and the the LP8720 regulator.

Thanks!

-- 
Paul Kocialkowski, Replicant developer

Replicant is a fully free Android distribution

Website: http://www.replicant.us/
Redmine: http://redmine.replicant.us/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141105/287f9616/attachment.pgp>