From mboxrd@z Thu Jan  1 00:00:00 1970
From: Keerthy <a0393675@ti.com>
Date: Thu, 29 Sep 2016 11:32:36 +0530
Subject: [U-Boot] [PATCH v5 2/6] power: pmic: Palmas: Add the base pmic
	support
In-Reply-To: <CAPnjgZ31pc0k9=15Pe_y4G03Da2kCFkCiVcW4shRDNsuOC8Z9Q@mail.gmail.com>
References: <1475036851-12048-1-git-send-email-j-keerthy@ti.com>
 <1475036851-12048-3-git-send-email-j-keerthy@ti.com>
 <CAPnjgZ31pc0k9=15Pe_y4G03Da2kCFkCiVcW4shRDNsuOC8Z9Q@mail.gmail.com>
Message-ID: <1a0d43cc-c823-b681-3cba-ed23af912c12@ti.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



On Wednesday 28 September 2016 11:21 PM, Simon Glass wrote:
> Hi Keerthy,
>
> On 27 September 2016 at 22:27, Keerthy <j-keerthy@ti.com> wrote:
>> Add support to bind the regulators/child nodes with the pmic.
>> Also adds the pmic i2c based read/write funtions to access pmic
>> registers.
>>
>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>> Reviewed-by: Tom Rini <trini@konsulko.com>
>> ---
>>
>> Changes in v5:
>>
>>   * Added pmic read/write functions.
>>
>>  drivers/power/pmic/Kconfig  |   7 +++
>>  drivers/power/pmic/Makefile |   1 +
>>  drivers/power/pmic/palmas.c | 108 ++++++++++++++++++++++++++++++++++++++++++++
>>  include/power/palmas.h      |  25 ++++++++++
>>  4 files changed, 141 insertions(+)
>>  create mode 100644 drivers/power/pmic/palmas.c
>>  create mode 100644 include/power/palmas.h
>>
>> diff --git a/drivers/power/pmic/Kconfig b/drivers/power/pmic/Kconfig
>> index 69f8d51..92931c5 100644
>> --- a/drivers/power/pmic/Kconfig
>> +++ b/drivers/power/pmic/Kconfig
>> @@ -135,3 +135,10 @@ config PMIC_TPS65090
>>         FETs and a battery charger. This driver provides register access
>>         only, and you can enable the regulator/charger drivers separately if
>>         required.
>> +
>> +config PMIC_PALMAS
>> +       bool "Enable driver for Texas Instruments PALMAS PMIC"
>> +       depends on DM_PMIC
>> +       ---help---
>> +       The PALMAS is a PMIC containing several LDOs, SMPS.
>> +       This driver binds the pmic children.
>> diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile
>> index 52b4f71..828c0cf 100644
>> --- a/drivers/power/pmic/Makefile
>> +++ b/drivers/power/pmic/Makefile
>> @@ -15,6 +15,7 @@ obj-$(CONFIG_PMIC_PM8916) += pm8916.o
>>  obj-$(CONFIG_PMIC_RK808) += rk808.o
>>  obj-$(CONFIG_PMIC_TPS65090) += tps65090.o
>>  obj-$(CONFIG_PMIC_S5M8767) += s5m8767.o
>> +obj-$(CONFIG_$(SPL_)PMIC_PALMAS) += palmas.o
>>
>>  obj-$(CONFIG_POWER_LTC3676) += pmic_ltc3676.o
>>  obj-$(CONFIG_POWER_MAX77696) += pmic_max77696.o
>> diff --git a/drivers/power/pmic/palmas.c b/drivers/power/pmic/palmas.c
>> new file mode 100644
>> index 0000000..1d2bd67
>> --- /dev/null
>> +++ b/drivers/power/pmic/palmas.c
>> @@ -0,0 +1,108 @@
>> +/*
>> + * (C) Copyright 2016 Texas Instruments Incorporated, <www.ti.com>
>> + * Keerthy <j-keerthy@ti.com>
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <fdtdec.h>
>> +#include <errno.h>
>> +#include <dm.h>
>> +#include <i2c.h>
>> +#include <power/pmic.h>
>> +#include <power/regulator.h>
>> +#include <power/palmas.h>
>> +#include <dm/device.h>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +static const struct pmic_child_info pmic_children_info[] = {
>> +       { .prefix = "ldo", .driver = PALMAS_LDO_DRIVER },
>> +       { .prefix = "smps", .driver = PALMAS_SMPS_DRIVER },
>> +       { },
>> +};
>> +
>> +static int palmas_write(struct udevice *dev, uint reg, const uint8_t *buff,
>> +                         int len)
>> +{
>> +       if (dm_i2c_reg_write(dev, reg, *buff)) {
>
> I think this should be dm_i2c_write(). You are only writing a single byte.

Now the v2 of this series had comments suggesting the opposite:

https://www.mail-archive.com/u-boot at lists.denx.de/msg225123.html

>
>> +               error("write error to device: %p register: %#x!", dev, reg);
>> +               return -EIO;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static int palmas_read(struct udevice *dev, uint reg, uint8_t *buff, int len)
>> +{
>> +       int ret;
>> +
>> +       ret = dm_i2c_reg_read(dev, reg);
>
> dm_i2c_read()
>
>> +       if (ret < 0) {
>> +               error("read error %d from device: %p register: %#x!", ret, dev,
>> +                     reg);
>> +               return -EIO;
>> +       }
>> +
>> +       return ret;
>> +}
>
> [...]
>
> Regards,
> Simon
>