From mboxrd@z Thu Jan  1 00:00:00 1970
From: Przemyslaw Marczak <p.marczak@samsung.com>
Date: Tue, 04 Nov 2014 18:46:03 +0100
Subject: [U-Boot] [PATCH] power: LP8720 regulator support
In-Reply-To: <20141104155806.GL15133@bill-the-cat>
References: <1414517557-2457-1-git-send-email-contact@paulk.fr>
	<1414839697.2912.4.camel@collins> <20141104155806.GL15133@bill-the-cat>
Message-ID: <545910DB.1040205@samsung.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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@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@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!
>
>
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

Thanks for cc Tom, I have some comments in a next message.

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.

Best regards,
-- 
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com