[U-Boot] [PATCH 1/2] power: add PFUZE100 PMIC driver

[Tim Harvey]

On Mon, Feb 3, 2014 at 4:05 AM, Stefano Babic <sbabic at denx.de> wrote:
> Hi Tim,
>
> On 01/02/2014 20:49, Tim Harvey wrote:
>> Signed-off-by: Tim Harvey <tharvey at gateworks.com>
>> ---
>>  drivers/power/pmic/Makefile        |  1 +
>>  drivers/power/pmic/pmic_pfuze100.c | 42 +++++++++++++++++
>>  include/power/pfuze100_pmic.h      | 96 ++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 139 insertions(+)
>>  create mode 100644 drivers/power/pmic/pmic_pfuze100.c
>>  create mode 100644 include/power/pfuze100_pmic.h
>>
>> diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile
>> index 0b45ffa..4129bda 100644
>> --- a/drivers/power/pmic/Makefile
>> +++ b/drivers/power/pmic/Makefile
>> @@ -9,5 +9,6 @@ obj-$(CONFIG_POWER_MAX8998) += pmic_max8998.o
>>  obj-$(CONFIG_POWER_MAX8997) += pmic_max8997.o
>>  obj-$(CONFIG_POWER_MUIC_MAX8997) += muic_max8997.o
>>  obj-$(CONFIG_POWER_MAX77686) += pmic_max77686.o
>> +obj-$(CONFIG_POWER_PFUZE100) += pmic_pfuze100.o
>>  obj-$(CONFIG_POWER_TPS65217) += pmic_tps65217.o
>>  obj-$(CONFIG_POWER_TPS65910) += pmic_tps65910.o
>> diff --git a/drivers/power/pmic/pmic_pfuze100.c b/drivers/power/pmic/pmic_pfuze100.c
>> new file mode 100644
>> index 0000000..c382921
>> --- /dev/null
>> +++ b/drivers/power/pmic/pmic_pfuze100.c
>> @@ -0,0 +1,42 @@
>> +/*
>> + * Copyright (C) 2014 Gateworks Corporation
>> + * Tim Harvey <tharvey at gateworks.com>
>> + *
>> + * SPDX-License-Identifier:      GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <errno.h>
>> +#include <i2c.h>
>> +#include <power/pmic.h>
>> +#include <power/pfuze100_pmic.h>
>> +
>> +int pmic_init(unsigned char bus)
>> +{
>> +     static const char name[] = "PFUZE100_PMIC";
>> +     int ret;
>> +     struct pmic *p = pmic_alloc();
>> +
>> +     if (!p) {
>> +             printf("%s: POWER allocation error!\n", __func__);
>> +             return -ENOMEM;
>> +     }
>> +
>> +     p->name = name;
>> +     p->interface = PMIC_I2C;
>> +     p->number_of_regs = PMIC_NUM_OF_REGS;
>> +     p->hw.i2c.addr = CONFIG_POWER_PFUZE100_I2C_ADDR;
>> +     p->hw.i2c.tx_num = 1;
>> +     p->bus = bus;
>> +
>> +     ret = i2c_set_bus_num(p->bus);
>> +     if (ret)
>> +             return ret;
>> +     if (!i2c_probe(p->hw.i2c.addr)) {
>> +             unsigned char dev_id, rev_id;
>> +             i2c_read(p->hw.i2c.addr, PFUZE100_DEVICEID, 1, &dev_id, 1);
>> +             i2c_read(p->hw.i2c.addr, PFUZE100_REVID, 1, &rev_id, 1);
>> +             printf("PMIC:  PFUZE100 0x%02x/0x%02x\n", dev_id, rev_id);
>
> This is always printed, not only on your board - even from other board,
> if any, that will use this pmic. If you want that your board always
> prints the pmic revision, move this output to your board file - maybe in
> checkboard().

I noticed that all the pmic drivers in drivers/power/pmic print
something very generic in their pmic_init such as 'Board PMIC init'
(even though no initialization or communication with the pmic has
occurred).  I thought I would be a bit more specific and display
exactly which PMIC init has been called and provide some details about
the PMIC version.

I would agree that this can go into checkboard()

Is there value in adding the 'puts("Board PMIC init\n")?  I didn't
want to display that if the pmic isn't present (as this PMIC isn't
present on some Ventana boards).

>
>> +     }
>> +     return 0;
>
> If i2c_probe() returns with error, why does this function returns with
> zero (= no error) ?

oops!  If displaying the version info shouldn't go in pmic_init, then
I'm not sure the value of even doing the probe here either so I will
remove.

Thanks for the review!

Tim

>
> Best regards,
> Stefano Babic
>
> --
> =====================================================================
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
> =====================================================================