diff mbox

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

Message ID 1391284192-8440-2-git-send-email-tharvey@gateworks.com
State Superseded
Delegated to: Stefano Babic
Headers show

Commit Message

Tim Harvey Feb. 1, 2014, 7:49 p.m. UTC 
Signed-off-by: Tim Harvey <tharvey@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

Comments

Stefano Babic Feb. 3, 2014, 12:05 p.m. UTC | #1 
Hi Tim,

On 01/02/2014 20:49, Tim Harvey wrote:
> Signed-off-by: Tim Harvey <tharvey@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@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().

> +	}
> +	return 0;

If i2c_probe() returns with error, why does this function returns with
zero (= no error) ?

Best regards,
Stefano Babic
Tim Harvey Feb. 3, 2014, 7:30 p.m. UTC | #2 
On Mon, Feb 3, 2014 at 4:05 AM, Stefano Babic <sbabic@denx.de> wrote:
> Hi Tim,
>
> On 01/02/2014 20:49, Tim Harvey wrote:
>> Signed-off-by: Tim Harvey <tharvey@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@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@denx.de
> =====================================================================
Michael Nazzareno Trimarchi Feb. 3, 2014, 7:34 p.m. UTC | #3 
Hi

Il 01/feb/2014 20:51 "Tim Harvey" <tharvey@gateworks.com> ha scritto:
>
> Signed-off-by: Tim Harvey <tharvey@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@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)) {

Can you use ret = i2c_probe and invert condition?

Michael

> +               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);
> +       }
> +       return 0;
> +}
> diff --git a/include/power/pfuze100_pmic.h b/include/power/pfuze100_pmic.h
> new file mode 100644
> index 0000000..2a9032a
> --- /dev/null
> +++ b/include/power/pfuze100_pmic.h
> @@ -0,0 +1,96 @@
> +/*
> + *  Copyright (C) 2014 Gateworks Corporation
> + *  Tim Harvey <tharvey@gateworks.com>
> + *
> + * SPDX-License-Identifier:     GPL-2.0+
> + */
> +
> +#ifndef __PFUZE100_PMIC_H_
> +#define __PFUZE100_PMIC_H_
> +
> +/* PFUZE100 registers */
> +enum {
> +       PFUZE100_DEVICEID       = 0x00,
> +       PFUZE100_REVID          = 0x03,
> +       PFUZE100_FABID          = 0x04,
> +
> +       PFUZE100_SW1ABVOL       = 0x20,
> +       PFUZE100_SW1CVOL        = 0x2e,
> +       PFUZE100_SW2VOL         = 0x35,
> +       PFUZE100_SW3AVOL        = 0x3c,
> +       PFUZE100_SW3BVOL        = 0x43,
> +       PFUZE100_SW4VOL         = 0x4a,
> +       PFUZE100_SWBSTCON1      = 0x66,
> +       PFUZE100_VREFDDRCON     = 0x6a,
> +       PFUZE100_VSNVSVOL       = 0x6b,
> +       PFUZE100_VGEN1VOL       = 0x6c,
> +       PFUZE100_VGEN2VOL       = 0x6d,
> +       PFUZE100_VGEN3VOL       = 0x6e,
> +       PFUZE100_VGEN4VOL       = 0x6f,
> +       PFUZE100_VGEN5VOL       = 0x70,
> +       PFUZE100_VGEN6VOL       = 0x71,
> +
> +       PMIC_NUM_OF_REGS        = 0x7f,
> +};
> +
> +/*
> + * LDO Configuration
> + */
> +
> +/* VGEN1/2 Voltage Configuration */
> +#define LDOA_0_80V     0
> +#define LDOA_0_85V     1
> +#define LDOA_0_90V     2
> +#define LDOA_0_95V     3
> +#define LDOA_1_00V     4
> +#define LDOA_1_05V     5
> +#define LDOA_1_10V     6
> +#define LDOA_1_15V     7
> +#define LDOA_1_20V     8
> +#define LDOA_1_25V     9
> +#define LDOA_1_30V     10
> +#define LDOA_1_35V     11
> +#define LDOA_1_40V     12
> +#define LDOA_1_45V     13
> +#define LDOA_1_50V     14
> +#define LDOA_1_55V     15
> +
> +/* VGEN3/4/5/6 Voltage Configuration */
> +#define LDOB_1_80V     0
> +#define LDOB_1_90V     1
> +#define LDOB_2_00V     2
> +#define LDOB_2_10V     3
> +#define LDOB_2_20V     4
> +#define LDOB_2_30V     5
> +#define LDOB_2_40V     6
> +#define LDOB_2_50V     7
> +#define LDOB_2_60V     8
> +#define LDOB_2_70V     9
> +#define LDOB_2_80V     10
> +#define LDOB_2_90V     11
> +#define LDOB_3_00V     12
> +#define LDOB_3_10V     13
> +#define LDOB_3_20V     14
> +#define LDOB_3_30V     15
> +
> +#define LDO_VOL_MASK   0xf
> +#define LDO_EN         4
> +
> +/*
> + * Boost Regulator
> + */
> +
> +/* SWBST Output Voltage */
> +#define SWBST_5_00V    0
> +#define SWBST_5_05V    1
> +#define SWBST_5_10V    2
> +#define SWBST_5_15V    3
> +
> +#define SWBST_VOL_MASK 0x3
> +#define SWBST_MODE_MASK        0x6
> +#define SWBST_MODE_OFF (2 << 0)
> +#define SWBST_MODE_PFM (2 << 1)
> +#define SWBST_MODE_AUTO        (2 << 2)
> +#define SWBST_MODE_APS (2 << 3)
> +
> +#endif
> --
> 1.8.3.2
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
Stefano Babic Feb. 4, 2014, 9:04 a.m. UTC | #4 
Hi Tim,

On 03/02/2014 20:30, Tim Harvey wrote:
>> 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 do not think so

> I didn't
> want to display that if the pmic isn't present (as this PMIC isn't
> present on some Ventana boards).

Agree.

Best regards,
Stefano Babic
diff mbox

Patch

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@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);
+	}
+	return 0;
+}
diff --git a/include/power/pfuze100_pmic.h b/include/power/pfuze100_pmic.h
new file mode 100644
index 0000000..2a9032a
--- /dev/null
+++ b/include/power/pfuze100_pmic.h
@@ -0,0 +1,96 @@ 
+/*
+ *  Copyright (C) 2014 Gateworks Corporation
+ *  Tim Harvey <tharvey@gateworks.com>
+ *
+ * SPDX-License-Identifier:     GPL-2.0+
+ */
+
+#ifndef __PFUZE100_PMIC_H_
+#define __PFUZE100_PMIC_H_
+
+/* PFUZE100 registers */
+enum {
+	PFUZE100_DEVICEID	= 0x00,
+	PFUZE100_REVID		= 0x03,
+	PFUZE100_FABID		= 0x04,
+
+	PFUZE100_SW1ABVOL	= 0x20,
+	PFUZE100_SW1CVOL	= 0x2e,
+	PFUZE100_SW2VOL		= 0x35,
+	PFUZE100_SW3AVOL	= 0x3c,
+	PFUZE100_SW3BVOL	= 0x43,
+	PFUZE100_SW4VOL		= 0x4a,
+	PFUZE100_SWBSTCON1	= 0x66,
+	PFUZE100_VREFDDRCON	= 0x6a,
+	PFUZE100_VSNVSVOL	= 0x6b,
+	PFUZE100_VGEN1VOL	= 0x6c,
+	PFUZE100_VGEN2VOL	= 0x6d,
+	PFUZE100_VGEN3VOL	= 0x6e,
+	PFUZE100_VGEN4VOL	= 0x6f,
+	PFUZE100_VGEN5VOL	= 0x70,
+	PFUZE100_VGEN6VOL	= 0x71,
+
+	PMIC_NUM_OF_REGS	= 0x7f,
+};
+
+/*
+ * LDO Configuration
+ */
+
+/* VGEN1/2 Voltage Configuration */
+#define LDOA_0_80V	0
+#define LDOA_0_85V	1
+#define LDOA_0_90V	2
+#define LDOA_0_95V	3
+#define LDOA_1_00V	4
+#define LDOA_1_05V	5
+#define LDOA_1_10V	6
+#define LDOA_1_15V	7
+#define LDOA_1_20V	8
+#define LDOA_1_25V	9
+#define LDOA_1_30V	10
+#define LDOA_1_35V	11
+#define LDOA_1_40V	12
+#define LDOA_1_45V	13
+#define LDOA_1_50V	14
+#define LDOA_1_55V	15
+
+/* VGEN3/4/5/6 Voltage Configuration */
+#define LDOB_1_80V	0
+#define LDOB_1_90V	1
+#define LDOB_2_00V	2
+#define LDOB_2_10V	3
+#define LDOB_2_20V	4
+#define LDOB_2_30V	5
+#define LDOB_2_40V	6
+#define LDOB_2_50V	7
+#define LDOB_2_60V	8
+#define LDOB_2_70V	9
+#define LDOB_2_80V	10
+#define LDOB_2_90V	11
+#define LDOB_3_00V	12
+#define LDOB_3_10V	13
+#define LDOB_3_20V	14
+#define LDOB_3_30V	15
+
+#define LDO_VOL_MASK	0xf
+#define LDO_EN		4
+
+/*
+ * Boost Regulator
+ */
+
+/* SWBST Output Voltage */
+#define SWBST_5_00V	0
+#define SWBST_5_05V	1
+#define SWBST_5_10V	2
+#define SWBST_5_15V	3
+
+#define SWBST_VOL_MASK	0x3
+#define SWBST_MODE_MASK	0x6
+#define SWBST_MODE_OFF	(2 << 0)
+#define SWBST_MODE_PFM	(2 << 1)
+#define SWBST_MODE_AUTO	(2 << 2)
+#define SWBST_MODE_APS	(2 << 3)
+
+#endif