[U-Boot] [PATCH v5 2/6] power: pmic: Palmas: Add the base pmic support

[Keerthy]

On Thursday 29 September 2016 11:32 AM, Keerthy wrote:
>
>
> On Wednesday 28 September 2016 11:21 PM, Simon Glass wrote:
>> Hi Keerthy,
>>
>> On 27 September 2016 at 22:27, Keerthy <j-keerthy at 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 at ti.com>
>>> Reviewed-by: Simon Glass <sjg at chromium.org>
>>> Reviewed-by: Tom Rini <trini at 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 at 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@lists.denx.de/msg225123.html

And i thought dm_i2c_reg_write/read was specifically introduced for 
writing and reading one byte value?

int dm_i2c_reg_write(struct udevice *dev, uint offset, uint value)
{
         uint8_t val = value;

         return dm_i2c_write(dev, offset, &val, 1);
}

Regards,
Keerthy
>
>>
>>> +               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
>>