Skip to contentSkip to conversation

misc:max8998 Support for MAX8998 PMIC driver

PATCH New Lukasz Majewski 3 participants 40 messages Sep 1, 2011 – Oct 12, 2011

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover misc:max8998 Support for MAX8998 PMIC driver

This patch series add support for MAXIM's MAX8998 power IC chip. It also enables usage of it on the GONI reference target.

Lukasz Majewski (2):
misc:max8998 Support for MAX8998 PMIC driver misc:samsung:s5p Enable MAX8998 support at GONI reference target

 drivers/misc/Makefile       |    1 +
 drivers/misc/max8998_pmic.c |   74 ++++++++++++++++++++++++++++++++++++
 include/configs/s5p_goni.h  |    2 +
 include/max8998_pmic.h      |   87 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 164 insertions(+), 0 deletions(-)
 create mode 100644 drivers/misc/max8998_pmic.c
 create mode 100644 include/max8998_pmic.h
Signature · 1 line
1.7.2.3
#1 lore raw

PATCH 1/2 misc:max8998 Support for MAX8998 PMIC driver

This commit adds support for MAX8998 driver. CONFIG_MAX8998_PMIC flag shall be defined on an architecture supporting it.

---
 drivers/misc/Makefile       |    1 +
 drivers/misc/max8998_pmic.c |   74 ++++++++++++++++++++++++++++++++++++
 include/max8998_pmic.h      |   87 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 162 insertions(+), 0 deletions(-)
 create mode 100644 drivers/misc/max8998_pmic.c
 create mode 100644 include/max8998_pmic.h
drivers/misc/Makefile +1 -0
index b152486..551bea9 100644
@@ -35,6 +35,7 @@ COBJS-$(CONFIG_NS87308) += ns87308.o
 COBJS-$(CONFIG_PDSP188x) += pdsp188x.o
 COBJS-$(CONFIG_STATUS_LED) += status_led.o
 COBJS-$(CONFIG_TWL4030_LED) += twl4030_led.o
+COBJS-$(CONFIG_MAX8998_PMIC) += max8998_pmic.o
 
 COBJS	:= $(COBJS-y)
 SRCS	:= $(COBJS:.o=.c)
drivers/misc/max8998_pmic.c new file +74 -0

74 lines not on this page · raw message

include/max8998_pmic.h new file +87 -0

87 lines not on this page · raw message

Signature · 1 line
1.7.2.3
#2 lore raw

PATCH 2/2 misc:samsung:s5p Enable MAX8998 support at GONI reference target

In reply to Lukasz Majewski

Adds support for MAX8998 PMIC at S5PC110 GONI target.

---
 include/configs/s5p_goni.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
include/configs/s5p_goni.h +2 -0
index 886c8be..aef3a9d 100644
@@ -223,6 +223,8 @@
 
 #define CONFIG_SYS_INIT_SP_ADDR	(CONFIG_SYS_LOAD_ADDR - 0x1000000)
 
+#define CONFIG_MAX8998_PMIC
+
 #include <asm/arch/gpio.h>
 /*
  * I2C Settings
Signature · 1 line
1.7.2.3
#3 lore raw

cover misc:max8998 Support for MAX8998 PMIC driver

In reply to Lukasz Majewski
Quoted 7 lines · On Thu, 01 Sep 2011 11:13:30 +0200 Lukasz Majewski <[email protected]>…

This patch series add support for MAXIM's MAX8998 power IC chip. It also enables usage of it on the GONI reference target.

Lukasz Majewski (2):
misc:max8998 Support for MAX8998 PMIC driver misc:samsung:s5p Enable MAX8998 support at GONI reference target

Dear all,

Is there any feedback/comments for those patches?
MAX8998 PMIC patches were posted some time ago and no response has been received.

BTW. On the "u-boot Custodians" page I haven't found anyone responsible for driver/misc. Is there appropriate person for this, or shall it be addressed to Wolfgang?

Signature · 6 lines
Best regards,

Lukasz Majewski

Samsung Poland R&D Center
Platform Group
#4 lore raw

cover misc:max8998 Support for MAX8998 PMIC driver

Quoted 6 lines · Lukasz Majewski
Quoted 3 lines · On Thu, 01 Sep 2011 11:13:30 +0200 Lukasz Majewski <[email protected]>…

This patch series add support for MAXIM's MAX8998 power IC chip. It also enables usage of it on the GONI reference target.

Quoted 7 lines
Quoted 4 lines

Lukasz Majewski (2):
misc:max8998 Support for MAX8998 PMIC driver misc:samsung:s5p Enable MAX8998 support at GONI reference target

Dear all,

Hi Lukasz,

Quoted 3 lines

Is there any feedback/comments for those patches?
MAX8998 PMIC patches were posted some time ago and no response has been received.

#5 lore raw

cover misc:max8998 Support for MAX8998 PMIC driver

Dear Stefano Babic,

Quoted 7 lines · In message <[email protected]> you wrote:
Quoted 3 lines

Is there appropriate person for this, or shall it be addressed to Wolfgang?

I do not know - however, your driver is not strictly related to a specific SOC, and could be used by any architecture. We can consider it as general code, and I added Wolfgang in CC to ask his opinion.

If nobody else takes it, I will have to.

But I will not protest if you just go ahead and put it on your own plate :-)

Best regards,

Wolfgang Denk

Signature · 8 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Science makes godlike -- it is all over with priests and  gods  when
man becomes scientific. Moral: science is the forbidden as such -- it
alone  is  forbidden. Science is the *first* sin, the *original* sin.
*This alone is  morality.*  ``Thou  shalt  not  know''  --  the  rest
follows."                                       - Friedrich Nietzsche
#6 lore raw

cover misc:max8998 Support for MAX8998 PMIC driver

In reply to Stefano Babic

Hi Stefano,

Thanks for your reply.

Quoted 7 lines

From my site I promise I will get a look at it. At the moment I have a big question. I see your code is quite similar to other ones (fsl_pmic.c is what I know better..). I remember there was already a discussion about another pmic, whose patch reassembled some drivers in u-boot.

Quoted 8 lines

My big question is: if we have a similar mechanism to access the PMICs (SPI/I2C), should we not to found a way to generalize it ? This kind of driver has only some pmic_read/pmic_write functions.

Maybe getting rid of special manufacturer names as fsl_pmic, max_pmic... and using a general access for this kind of chips ? From code your patch is not very distant from what we currently have. Any opinion about this ?

Yes, you are right. I've looked to the fsl_pmic driver when I was preparing patch for MAX8998.

As you have written, dealing with those two PMICs boils down to reading/writing their internal registers via I2C by i2c.h API.

We can think of a common PMIC framework, which is consisting of methods for reading/writing internal registers, enabling outputs of those devices.

PMIC specific data (like register map, I2C addresses) can be stored in *.h files.

I can prepare some common framework and post proposition on the mailing list. On the beginning I'll focus on MAX8998 and FSL.

Any comments/ideas are welcome.

Quoted 16 lines
Quoted 3 lines

BTW. On the "u-boot Custodians" page I haven't found anyone responsible for driver/misc.

You are right.

Quoted 2 lines

Is there appropriate person for this, or shall it be addressed to Wolfgang?

I do not know - however, your driver is not strictly related to a specific SOC, and could be used by any architecture. We can consider it as general code, and I added Wolfgang in CC to ask his opinion.

Best regards,
Stefano Babic

Signature · 6 lines
Best regards,

Lukasz Majewski

Samsung Poland R&D Center
Platform Group
#7 lore raw

cover misc:max8998 Support for MAX8998 PMIC driver

Quoted 3 lines · Lukasz Majewski

Hi Stefano,

Thanks for your reply.

Hi Lucasz,

Quoted 7 lines
Quoted 4 lines

Maybe getting rid of special manufacturer names as fsl_pmic, max_pmic... and using a general access for this kind of chips ? From code your patch is not very distant from what we currently have. Any opinion about this ?

Yes, you are right. I've looked to the fsl_pmic driver when I was preparing patch for MAX8998.

I have supposed...

Quoted 7 lines

As you have written, dealing with those two PMICs boils down to reading/writing their internal registers via I2C by i2c.h API.

We can think of a common PMIC framework, which is consisting of methods for reading/writing internal registers, enabling outputs of those devices.

Exactly !

Quoted 3 lines

PMIC specific data (like register map, I2C addresses) can be stored in *.h files.

Yes, this is what I meant ;-)

Quoted 3 lines

I can prepare some common framework and post proposition on the mailing list. On the beginning I'll focus on MAX8998 and FSL.

Agree.

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#8 lore raw

cover Generic PMIC driver

In reply to Lukasz Majewski

Dear all,

I'd like to propose a new approach for PMIC generic driver.

In my opinion following issues needs discussion:
1. In proposed
int pmic_reg_read(struct pmic *p, u32 *val) the val is returned by pointer. Now at fsl_pmic.c read value is returned by return clause.

I think, that passing pointer is a better approach,since errors from i2c_read/spi_read can be caught in upper layers.

2. Since I haven't got a chance to test SPI part of the fsl_pmic.c driver, I've focused mainly on I2C and place stubs for SPI.

3. Suggestions for struct pmic's additional fields for SPI are more than welcome :-)

4. Now the pmic_core.c file consist of
#ifdef PMIC_I2C
{Code for handling I2C}
#else
{Code for handling SPI}
#endif

The same approach is used at fsl_pmic.c

I'm wondering if this approach shouldn't be replaced with on-time checking if SPI or I2C interface is available.
This check can be performed by:

struct pmic *p;
if (p->interface == PMIC_I2C) {

} else {

}

It would allow to remove obscure #ifdefs, but on the other hand it will reduce execution speed of the driver.

Thanks in advance,
Lukasz

Lukasz Majewski (2):
misc:pmic New generic pmic driver
misc:pmic:max8998: Support for max8998 pmic

 arch/arm/lib/board.c       |    5 +
 board/samsung/goni/goni.c  |   18 ++++
 drivers/misc/Makefile      |    1 +
 drivers/misc/pmic_core.c   |  236 ++++++++++++++++++++++++++++++++++++++++++++
 include/configs/s5p_goni.h |    3 +
 include/max8998_pmic.h     |   84 ++++++++++++++++
 include/pmic.h             |   60 +++++++++++
 7 files changed, 407 insertions(+), 0 deletions(-)
 create mode 100644 drivers/misc/pmic_core.c
 create mode 100644 include/max8998_pmic.h
 create mode 100644 include/pmic.h
Signature · 1 line
1.7.2.3
#9 lore raw

RFC 1/2 misc:pmic New generic pmic driver

This commit adds new PMIC core driver
PMIC IC devices connected via I2C or SPI can be used.

---
 arch/arm/lib/board.c     |    5 +
 drivers/misc/Makefile    |    1 +
 drivers/misc/pmic_core.c |  236 ++++++++++++++++++++++++++++++++++++++++++++++
 include/pmic.h           |   60 ++++++++++++
 4 files changed, 302 insertions(+), 0 deletions(-)
 create mode 100644 drivers/misc/pmic_core.c
 create mode 100644 include/pmic.h
arch/arm/lib/board.c +5 -0
index c899839..bd2c619 100644
@@ -48,6 +48,7 @@
 #include <nand.h>
 #include <onenand_uboot.h>
 #include <mmc.h>
+#include <pmic.h>
 
 #ifdef CONFIG_BITBANGMII
 #include <miiphy.h>
@@ -580,6 +581,10 @@ void board_init_r(gd_t *id, ulong dest_addr)
 		copy_filename(BootFile, s, sizeof(BootFile));
 #endif
 
+#if defined(CONFIG_PMIC)
+	pmic_init();
+#endif
+
 #ifdef BOARD_LATE_INIT
 	board_late_init();
 #endif
drivers/misc/Makefile +1 -0
index b152486..f710a29 100644
@@ -35,6 +35,7 @@ COBJS-$(CONFIG_NS87308) += ns87308.o
 COBJS-$(CONFIG_PDSP188x) += pdsp188x.o
 COBJS-$(CONFIG_STATUS_LED) += status_led.o
 COBJS-$(CONFIG_TWL4030_LED) += twl4030_led.o
+COBJS-$(CONFIG_PMIC) += pmic_core.o
 
 COBJS	:= $(COBJS-y)
 SRCS	:= $(COBJS:.o=.c)
drivers/misc/pmic_core.c new file +236 -0

236 lines not on this page · raw message

include/pmic.h new file +60 -0

60 lines not on this page · raw message

Signature · 1 line
1.7.2.3
#10 lore raw

RFC 2/2 misc:pmic:max8998: Support for max8998 pmic

In reply to Lukasz Majewski

This commit enables the max8998 PMIC to run with pmic_core.c generic driver.

---
 board/samsung/goni/goni.c  |   18 +++++++++
 include/configs/s5p_goni.h |    3 ++
 include/max8998_pmic.h     |   84 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 105 insertions(+), 0 deletions(-)
 create mode 100644 include/max8998_pmic.h
board/samsung/goni/goni.c +18 -0
index e24cd29..3e0cb74 100644
@@ -25,6 +25,8 @@
 #include <common.h>
 #include <asm/arch/gpio.h>
 #include <asm/arch/mmc.h>
+#include <pmic.h>
+#include <max8998_pmic.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -59,6 +61,22 @@ void dram_init_banksize(void)
 	gd->bd->bi_dram[2].size = PHYS_SDRAM_3_SIZE;
 }
 
+int board_pmic_init(struct pmic *p)
+{
+	static char name[] = "MAX8998_PMIC";
+
+	puts("Board PMIC init\n");
+
+	p->name = name;
+	p->interface = PMIC_I2C;
+	p->number_of_regs = PMIC_NUM_OF_REGS;
+	p->hw.i2c.addr = MAX8998_I2C_ADDR;
+	p->hw.i2c.tx_num = 1;
+	p->hw.i2c.bus_num = I2C_PMIC;
+
+	return 0;
+}
+
 #ifdef CONFIG_DISPLAY_BOARDINFO
 int checkboard(void)
 {
include/configs/s5p_goni.h +3 -0
index 886c8be..e26dcb2 100644
@@ -223,6 +223,9 @@
 
 #define CONFIG_SYS_INIT_SP_ADDR	(CONFIG_SYS_LOAD_ADDR - 0x1000000)
 
+#define CONFIG_PMIC
+#define CONFIG_PMIC_I2C
+
 #include <asm/arch/gpio.h>
 /*
  * I2C Settings
include/max8998_pmic.h new file +84 -0

84 lines not on this page · raw message

Signature · 1 line
1.7.2.3
#11 lore raw

cover Generic PMIC driver

In reply to Lukasz Majewski
Quoted 1 line · Lukasz Majewski

Dear all,

Hi Lukasz,

Quoted 2 lines

I'd like to propose a new approach for PMIC generic driver.

Fine !

Quoted 4 lines

In my opinion following issues needs discussion:
1. In proposed
int pmic_reg_read(struct pmic *p, u32 *val) the val is returned by pointer.

And the register to be read/write is embedded in the structure. For readness I will let it as separate parameter, but this is my personal taste.

Who is responsible to allocate the pmic structure ? It could be (there is no use case at the moment) that the pmic can be programmed before the relocation, when malloc() is not available.

This is the main reason because everything is decided at compile time with CONFIG_ macros. However, we can also decide if it is *really* required that PMIC can be accessed before relocation.

Quoted 1 line

Now at fsl_pmic.c read value is returned by return clause.

Yes, we can change - of course, we need to update in one-shot also the boards already using a pmic.

Quoted 3 lines

I think, that passing pointer is a better approach,since errors from i2c_read/spi_read can be caught in upper layers.

Using a pointer is the common way to implement a read routine, so I cannot argue. The side effect is that the calling code will be filled with the same and boring check:

	ret = pmic_read_reg(....);
	if (ret) {
		<...error handling, most case only print>
	}
	ret = pmic_read_reg(....);
	if (ret) {
		<...error handling, most case only print>
	}
Quoted 3 lines

2. Since I haven't got a chance to test SPI part of the fsl_pmic.c driver, I've focused mainly on I2C and place stubs for SPI.

Ok, right - this is a RFC. I will do no not review the details in your sample code, we are discussing the interface ;-).

For the "real" patch, I think we should merge this new code with fsl_pmic.c, too.

Quoted 2 lines

3. Suggestions for struct pmic's additional fields for SPI are more than welcome :-)

We should have the parameters to setup a SPUI connection:
- SPI chipselect
- SPI frequency
- SPI mode
- SPI bus

Quoted 17 lines

4. Now the pmic_core.c file consist of

   #ifdef PMIC_I2C
   {Code for handling I2C}
   #else
   {Code for handling SPI}
   #endif

The same approach is used at fsl_pmic.c

I'm wondering if this approach shouldn't be replaced with on-time checking if SPI or I2C interface is available.
This check can be performed by:

struct pmic *p;
if (p->interface == PMIC_I2C) {

Quoted 6 lines

} else {

}

It would allow to remove obscure #ifdefs, but on the other hand it will reduce execution speed of the driver.

Well, I do not worry about time execution. The time to execute the if branch is maybe negligible compared to the time to make a I2C/SPI transfer.

However, we increase the footprint, and surely only one mechanism is required on the same board.

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#12 lore raw

cover Generic PMIC driver

Hi Stefano,

Thanks for the reply.

Quoted 9 lines
Quoted 5 lines

In my opinion following issues needs discussion:
1. In proposed
int struct pmic *p, u32 *val) the val is returned by pointer.

And the register to be read/write is embedded in the structure. For readness I will let it as separate parameter, but this is my personal taste.

For the first version of the pmic_core.c driver we might stick to this proposition.

Quoted 3 lines

Who is responsible to allocate the pmic structure ? It could be (there is no use case at the moment) that the pmic can be programmed before the relocation, when malloc() is not available.

In my opinion on the beginning we should focus on basic scenarios. Therefore the pmic struct is defined static at pmic_core.c as follows:
static struct pmic pmic;

It is simple and efficient (since it's scope is limited to the translation unit). We shall NOT use malloc() allocation for the first version of the driver.

Quoted 4 lines
Quoted 1 line

Now at fsl_pmic.c read value is returned by return clause.

Yes, we can change - of course, we need to update in one-shot also the boards already using a pmic.

Some work for the iMX target boards (e.g. MX51evk) need to be done to comply with new framework.

As fair as I looked into the code, only iMX power_init methods need to be fixed.

Quoted 7 lines
Quoted 2 lines

I think, that passing pointer is a better approach,since errors from i2c_read/spi_read can be caught in upper layers.

Using a pointer is the common way to implement a read routine, so I cannot argue. The side effect is that the calling code will be filled with the same and boring check:

Quoted 9 lines
	ret = pmic_read_reg(....);
	if (ret) {
		<...error handling, most case only print>
	}
	ret = pmic_read_reg(....);
	if (ret) {
		<...error handling, most case only print>
	}

I think, that error checking is always welcome :-).

Quoted 9 lines
Quoted 2 lines

2. Since I haven't got a chance to test SPI part of the fsl_pmic.c driver, I've focused mainly on I2C and place stubs for SPI.

Ok, right - this is a RFC. I will do no not review the details in your sample code, we are discussing the interface ;-).

For the "real" patch, I think we should merge this new code with fsl_pmic.c, too.

In my opinion we shall reuse fsl_pmic.c code as much as possible (especially the SPI code - since I cannot test it).

Quoted 10 lines
Quoted 3 lines

3. Suggestions for struct pmic's additional fields for SPI are more than welcome :-)

We should have the parameters to setup a SPUI connection:
- SPI chipselect
- SPI frequency
- SPI mode
- SPI bus

OK.

Quoted 23 lines
Quoted 16 lines

4. Now the pmic_core.c file consist of

   #ifdef PMIC_I2C
   {Code for handling I2C}
   #else
   {Code for handling SPI}
   #endif

The same approach is used at fsl_pmic.c

I'm wondering if this approach shouldn't be replaced with on-time checking if SPI or I2C interface is available.
This check can be performed by:

struct pmic *p;
if (p->interface == PMIC_I2C) {

Quoted 6 lines

} else {

}

It would allow to remove obscure #ifdefs, but on the other hand it will reduce execution speed of the driver.

Quoted 7 lines

Well, I do not worry about time execution. The time to execute the if branch is maybe negligible compared to the time to make a I2C/SPI transfer.

However, we increase the footprint, and surely only one mechanism is required on the same board.

So I will try to use switch-case construct.

switch (p->interface) {
case PMIC_I2C:

case PMIC_SPI:

}

Quoted 3 lines

Best regards,
Stefano Babic

Signature · 6 lines
Best regards,

Lukasz Majewski

Samsung Poland R&D Center
Platform Group
#13 lore raw

cover Generic PMIC driver

Quoted 4 lines · Lukasz Majewski
Quoted 3 lines

Who is responsible to allocate the pmic structure ? It could be (there is no use case at the moment) that the pmic can be programmed before the relocation, when malloc() is not available.

Quoted 8 lines

In my opinion on the beginning we should focus on basic scenarios. Therefore the pmic struct is defined static at pmic_core.c as follows:
static struct pmic pmic;

It is simple and efficient (since it's scope is limited to the translation unit). We shall NOT use malloc() allocation for the first version of the driver.

Agree, we must not use malloc.

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

Now at fsl_pmic.c read value is returned by return clause.

Yes, we can change - of course, we need to update in one-shot also the boards already using a pmic.

Some work for the iMX target boards (e.g. MX51evk) need to be done to comply with new framework.

Yes, of course - but if we improve our code, this is not a drawback ;-)

Do not miss the RTC inside the PMIC (drivers/rtc/mc13783-rtc.c)

Quoted 2 lines

As fair as I looked into the code, only iMX power_init methods need to be fixed.

This depends on the board code. There are 6 boards using the PMIC, plus the RTC driver. For this reason and to avoid to change all boards, maybe it is not bad to have a compatibility way.

Quoted 17 lines
Quoted 8 lines
Quoted 3 lines

I think, that passing pointer is a better approach,since errors from i2c_read/spi_read can be caught in upper layers.

Using a pointer is the common way to implement a read routine, so I cannot argue. The side effect is that the calling code will be filled with the same and boring check:

Quoted 9 lines
	ret = pmic_read_reg(....);
	if (ret) {
		<...error handling, most case only print>
	}
	ret = pmic_read_reg(....);
	if (ret) {
		<...error handling, most case only print>
	}
Quoted 1 line

I think, that error checking is always welcome :-).

Nothing against checking. We can also

Quoted 4 lines
Quoted 4 lines
Quoted 3 lines

2. Since I haven't got a chance to test SPI part of the fsl_pmic.c driver, I've focused mainly on I2C and place stubs for SPI.

Quoted 8 lines
Quoted 6 lines

Ok, right - this is a RFC. I will do no not review the details in your sample code, we are discussing the interface ;-).

For the "real" patch, I think we should merge this new code with fsl_pmic.c, too.

In my opinion we shall reuse fsl_pmic.c code as much as possible (especially the SPI code - since I cannot test it).

Agree.

Quoted 7 lines
Quoted 7 lines

Well, I do not worry about time execution. The time to execute the if branch is maybe negligible compared to the time to make a I2C/SPI transfer.

However, we increase the footprint, and surely only one mechanism is required on the same board.

Quoted 8 lines

So I will try to use switch-case construct.

switch (p->interface) {
case PMIC_I2C:

case PMIC_SPI:

}

There is probably a drawback, that is you must link code you do not need (for example, a board has no I2C at all, but we call and link i2c_write or another i2 function. The same if a board has only SPI. But I wait for your proposal !

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#14 lore raw

cover misc:pmic: New PMIC generic driver

In reply to Lukasz Majewski

This patch series adds new generic PMIC framework for u-boot. It is supposed to handle various types of pmic IC's (both I2C and SPI).

This patch series has been tested on Samsung's GONI reference target.

Moreover a special patch for mx51evk target board has been prepared. It is compiling, but I cannot test if the driver works. Other iMX targets shall be corrected in a similar way that mx51evk.

The old fsl_pmic.c driver has been preserved.

Lukasz Majewski (3):
misc:pmic New generic pmic driver
misc:pmic: Enable PMIC handling at u-boot startup code misc:pmic:mx: Code modification for mx51evk board

 arch/arm/lib/board.c              |    5 +
 board/freescale/mx51evk/mx51evk.c |   68 ++++++++++++------
 drivers/misc/Makefile             |    6 +-
 drivers/misc/pmic_core.c          |  148 +++++++++++++++++++++++++++++++++++++
 drivers/misc/pmic_fsl.c           |   57 ++++++++++++++
 drivers/misc/pmic_i2c.c           |   92 +++++++++++++++++++++++
 drivers/misc/pmic_max8998.c       |   43 +++++++++++
 drivers/misc/pmic_spi.c           |  109 +++++++++++++++++++++++++++
 include/configs/mx51evk.h         |    6 +-
 include/configs/s5p_goni.h        |    4 +
 include/fsl_pmic.h                |    5 +-
 include/max8998_pmic.h            |   84 +++++++++++++++++++++
 include/pmic.h                    |   71 ++++++++++++++++++
 13 files changed, 670 insertions(+), 28 deletions(-)
 create mode 100644 drivers/misc/pmic_core.c
 create mode 100644 drivers/misc/pmic_fsl.c
 create mode 100644 drivers/misc/pmic_i2c.c
 create mode 100644 drivers/misc/pmic_max8998.c
 create mode 100644 drivers/misc/pmic_spi.c
 create mode 100644 include/max8998_pmic.h
 create mode 100644 include/pmic.h
Signature · 1 line
1.7.2.3
#15 lore raw

PATCH 1/3 misc:pmic New generic pmic driver

This commit adds new PMIC core driver.

PMIC IC devices connected via I2C or SPI can be used. Separate files: pmic_i2c.c and pmic_spi.c are responsible for handling transmission specific files for I2C or SPI busses.

pmic_fsl.c and pmic_max8998.c are PMIC specific files. They define pmic_init() method for early init.

New flags added:
CONFIG_PMIC - enable PMIC general device. CONFIG_PMIC_I2C/SPI - specify the interface to be used. CONFIG_PMIC_MAX8998/FSL - add routines specific for PMIC chip.

---
 drivers/misc/Makefile       |    6 ++-
 drivers/misc/pmic_core.c    |  148 +++++++++++++++++++++++++++++++++++++++++++
 drivers/misc/pmic_fsl.c     |   57 +++++++++++++++++
 drivers/misc/pmic_i2c.c     |   92 +++++++++++++++++++++++++++
 drivers/misc/pmic_max8998.c |   43 +++++++++++++
 drivers/misc/pmic_spi.c     |  109 +++++++++++++++++++++++++++++++
 include/configs/mx51evk.h   |    6 ++-
 include/configs/s5p_goni.h  |    4 +
 include/fsl_pmic.h          |    5 +-
 include/max8998_pmic.h      |   84 ++++++++++++++++++++++++
 include/pmic.h              |   71 +++++++++++++++++++++
 11 files changed, 619 insertions(+), 6 deletions(-)
 create mode 100644 drivers/misc/pmic_core.c
 create mode 100644 drivers/misc/pmic_fsl.c
 create mode 100644 drivers/misc/pmic_i2c.c
 create mode 100644 drivers/misc/pmic_max8998.c
 create mode 100644 drivers/misc/pmic_spi.c
 create mode 100644 include/max8998_pmic.h
 create mode 100644 include/pmic.h
drivers/misc/Makefile +5 -1
index b152486..b388e5a 100644
@@ -28,13 +28,17 @@ LIB	:= $(obj)libmisc.o
 COBJS-$(CONFIG_ALI152X) += ali512x.o
 COBJS-$(CONFIG_DS4510)  += ds4510.o
 COBJS-$(CONFIG_FSL_LAW) += fsl_law.o
-COBJS-$(CONFIG_FSL_PMIC) += fsl_pmic.o
 COBJS-$(CONFIG_GPIO_LED) += gpio_led.o
 COBJS-$(CONFIG_FSL_MC9SDZ60) += mc9sdz60.o
 COBJS-$(CONFIG_NS87308) += ns87308.o
 COBJS-$(CONFIG_PDSP188x) += pdsp188x.o
 COBJS-$(CONFIG_STATUS_LED) += status_led.o
 COBJS-$(CONFIG_TWL4030_LED) += twl4030_led.o
+COBJS-$(CONFIG_PMIC) += pmic_core.o
+COBJS-$(CONFIG_PMIC_I2C) += pmic_i2c.o
+COBJS-$(CONFIG_PMIC_SPI) += pmic_spi.o
+COBJS-$(CONFIG_PMIC_FSL) += pmic_fsl.o
+COBJS-$(CONFIG_PMIC_MAX8998) += pmic_max8998.o
 
 COBJS	:= $(COBJS-y)
 SRCS	:= $(COBJS:.o=.c)
drivers/misc/pmic_core.c new file +148 -0

148 lines not on this page · raw message

drivers/misc/pmic_fsl.c new file +57 -0

57 lines not on this page · raw message

drivers/misc/pmic_i2c.c new file +92 -0

92 lines not on this page · raw message

drivers/misc/pmic_max8998.c new file +43 -0

43 lines not on this page · raw message

drivers/misc/pmic_spi.c new file +109 -0

109 lines not on this page · raw message

include/configs/mx51evk.h +5 -1
index 7d05dc8..bbc54f0 100644
@@ -69,11 +69,15 @@
 
 #define CONFIG_MXC_SPI
 
-#define CONFIG_FSL_PMIC
+#define CONFIG_PMIC
+#define CONFIG_PMIC_SPI
+#define CONFIG_PMIC_FSL
+
 #define CONFIG_FSL_PMIC_BUS	0
 #define CONFIG_FSL_PMIC_CS	0
 #define CONFIG_FSL_PMIC_CLK	2500000
 #define CONFIG_FSL_PMIC_MODE	(SPI_MODE_0 | SPI_CS_HIGH)
+#define CONFIG_FSL_PMIC_BITLEN	32
 
 /*
  * MMC Configs
include/configs/s5p_goni.h +4 -0
index 886c8be..aa51114 100644
@@ -223,6 +223,10 @@
 
 #define CONFIG_SYS_INIT_SP_ADDR	(CONFIG_SYS_LOAD_ADDR - 0x1000000)
 
+#define CONFIG_PMIC
+#define CONFIG_PMIC_I2C
+#define CONFIG_PMIC_MAX8998
+
 #include <asm/arch/gpio.h>
 /*
  * I2C Settings
include/fsl_pmic.h +1 -4
index e3abde6..742f2e1 100644
@@ -99,6 +99,7 @@ enum {
 	REG_TEST2,
 	REG_TEST3,
 	REG_TEST4,
+	PMIC_NUM_OF_REGS,
 };
 
 /* REG_POWER_MISC */
@@ -121,8 +122,4 @@ enum {
 /* Interrupt status 1 */
 #define RTCRSTI		(1 << 7)
 
-void pmic_show_pmic_info(void);
-void pmic_reg_write(u32 reg, u32 value);
-u32 pmic_reg_read(u32 reg);
-
 #endif
include/max8998_pmic.h new file +84 -0

84 lines not on this page · raw message

include/pmic.h new file +71 -0

71 lines not on this page · raw message

Signature · 1 line
1.7.2.3
#16 lore raw

PATCH 1/3 misc:pmic New generic pmic driver

Am 26/09/2011 17:10, schrieb Lukasz Majewski:

Quoted 9 lines

This commit adds new PMIC core driver.

PMIC IC devices connected via I2C or SPI can be used. Separate files: pmic_i2c.c and pmic_spi.c are responsible for handling transmission specific files for I2C or SPI busses.

pmic_fsl.c and pmic_max8998.c are PMIC specific files. They define pmic_init() method for early init.

Quoted 8 lines

New flags added:
CONFIG_PMIC - enable PMIC general device. CONFIG_PMIC_I2C/SPI - specify the interface to be used. CONFIG_PMIC_MAX8998/FSL - add routines specific for PMIC chip.

Hi Lukasz,

drivers/misc/pmic_core.c line 87

Quoted 10 lines
drivers/misc/pmic_core.c new file +10 -0
+
+		if (!(i % 8))
+			printf("\n0x%02x: ", i);
+
+		printf("%08x ", val);
+	}
+	puts("\n");
+}
+
+

Drop the second newline-

drivers/misc/pmic_fsl.c line 42

Quoted 6 lines
drivers/misc/pmic_fsl.c new file +6 -0
+int pmic_init(void)
+{
+	struct pmic *p = get_pmic();
+	static const char name[] = "FSL_PMIC";
+
+	puts("Board PMIC init\n");

This contains no info - you can drop or replace puts with debug().

drivers/misc/pmic_fsl.c line 57

Quoted 6 lines
drivers/misc/pmic_fsl.c new file +6 -0
+	p->hw.spi.bitlen = CONFIG_FSL_PMIC_BITLEN;
+	p->hw.spi.flags = SPI_XFER_BEGIN | SPI_XFER_END;
+	p->hw.spi.prepare_tx = pmic_spi_prepare_tx;
+
+	return 0;
+}

Ok - I do not know if CONFIG_FSL_PMIC_BITLEN is really needed, but we can leave it.

drivers/misc/pmic_i2c.c line 85

Quoted 4 lines
drivers/misc/pmic_i2c.c new file +4 -0
+int pmic_probe(struct pmic *p)
+{
+	i2c_set_bus_num(p->bus);
+	printf("PMIC:%s probed!\n", p->name);

Probably this comes from your testing - you can drop it or use debug()

drivers/misc/pmic_i2c.c line 87

Quoted 2 lines
drivers/misc/pmic_i2c.c new file +2 -0
+	if (i2c_probe(pmic_i2c_addr)) {
+		puts("Can't find max8998\n");

...and we have not always a max8998 !

drivers/misc/pmic_max8998.c line 33

Quoted 6 lines
drivers/misc/pmic_max8998.c new file +6 -0
+int pmic_init(void)
+{
+	struct pmic *p = get_pmic();
+	static const char name[] = "MAX8998_PMIC";
+
+	puts("Board PMIC init\n");

Replace with debug()

include/configs/mx51evk.h

Quoted 5 lines
drivers/misc/pmic_spi.c +0 -0
include/configs/mx51evk.h +0 -0
index 7d05dc8..bbc54f0 100644

You can leave away the MX boards (and we have to change all of them or none of them - there are also MX3 boards that can use the new code).

include/configs/s5p_goni.h

Quoted 3 lines

* MMC Configs

include/configs/s5p_goni.h +0 -0
index 886c8be..aa51114 100644

Put changes to board configuration in a separate patch.

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#17 lore raw

PATCH 2/3 misc:pmic: Enable PMIC handling at u-boot startup code

In reply to Lukasz Majewski

The pmic_init() initialization handler has been added to board_init_r function.

---
 arch/arm/lib/board.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
arch/arm/lib/board.c +5 -0
index 85320bc..ea9e3d6 100644
@@ -48,6 +48,7 @@
 #include <nand.h>
 #include <onenand_uboot.h>
 #include <mmc.h>
+#include <pmic.h>
 
 #ifdef CONFIG_BITBANGMII
 #include <miiphy.h>
@@ -578,6 +579,10 @@ void board_init_r(gd_t *id, ulong dest_addr)
 		copy_filename(BootFile, s, sizeof(BootFile));
 #endif
 
+#if defined(CONFIG_PMIC)
+	pmic_init();
+#endif
+
 #ifdef BOARD_LATE_INIT
 	board_late_init();
 #endif
Signature · 1 line
1.7.2.3
#18 lore raw

PATCH 2/3 misc:pmic: Enable PMIC handling at u-boot startup code

Am 26/09/2011 17:10, schrieb Lukasz Majewski:

Quoted 23 lines

The pmic_init() initialization handler has been added to board_init_r function.

---
 arch/arm/lib/board.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
arch/arm/lib/board.c +1 -0
index 85320bc..ea9e3d6 100644
@@ -48,6 +48,7 @@
 #include <nand.h>
 #include <onenand_uboot.h>
 #include <mmc.h>
+#include <pmic.h>
 
 #ifdef CONFIG_BITBANGMII
 #include <miiphy.h>

arch/arm/lib/board.c void board_init_r(gd_t *id, ulong dest_addr) · line 588

Quoted 10 lines
arch/arm/lib/board.c +4 -0
@@ -578,6 +579,10 @@ void board_init_r(gd_t *id, ulong dest_addr)
 		copy_filename(BootFile, s, sizeof(BootFile));
 #endif
 
+#if defined(CONFIG_PMIC)
+	pmic_init();
+#endif
+
 #ifdef BOARD_LATE_INIT
 	board_late_init();
 #endif

I do not think this is correct. Only a few boards have a PMIC, and we put the initialization in the general code for all ARM. And we cannot decide *when* the PMIC must be initialized. Maybe a board requires some other parts (GPIO initialization,...) before accessing the PMIC. It is better to let the board maintainers to decide when to call this function. So simply drop this patch, and put the call to pmic_init() in your s5p_goni code.

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#19 lore raw

PATCH 3/3 misc:pmic:mx: Code modification for mx51evk board

In reply to Lukasz Majewski

This is an example code for adjusting the mx51evk example board to new pmic_core driver.

---
 board/freescale/mx51evk/mx51evk.c |   68 +++++++++++++++++++++++++------------
 1 files changed, 46 insertions(+), 22 deletions(-)
board/freescale/mx51evk/mx51evk.c +46 -22
index 94ea1f2..46562f0 100644
@@ -34,6 +34,7 @@
 #include <fsl_esdhc.h>
 #include <fsl_pmic.h>
 #include <mc13892.h>
+#include <pmic.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -181,72 +182,94 @@ static void setup_iomux_spi(void)
 static void power_init(void)
 {
 	unsigned int val;
+	struct pmic *p = get_pmic();
 	struct mxc_ccm_reg *mxc_ccm = (struct mxc_ccm_reg *)MXC_CCM_BASE;
 
 	/* Write needed to Power Gate 2 register */
-	val = pmic_reg_read(REG_POWER_MISC);
+	if (pmic_reg_read(p, REG_POWER_MISC, &val))
+		puts("PMIC write error!\n");
 	val &= ~PWGT2SPIEN;
-	pmic_reg_write(REG_POWER_MISC, val);
+	if (pmic_reg_write(p, REG_POWER_MISC, &val))
+		puts("PMIC write error!\n");
 
 	/* Externally powered */
-	val = pmic_reg_read(REG_CHARGE);
+	if (pmic_reg_read(p, REG_CHARGE, &val))
+		puts("PMIC write error!\n");
 	val |= ICHRG0 | ICHRG1 | ICHRG2 | ICHRG3 | CHGAUTOB;
-	pmic_reg_write(REG_CHARGE, val);
+	if (pmic_reg_write(p, REG_CHARGE, &val))
+		puts("PMIC write error!\n");
 
 	/* power up the system first */
-	pmic_reg_write(REG_POWER_MISC, PWUP);
+	val = PWUP;
+	if (pmic_reg_write(p, REG_POWER_MISC, &val))
+		puts("PMIC write error!\n");
 
 	/* Set core voltage to 1.1V */
-	val = pmic_reg_read(REG_SW_0);
+	if (pmic_reg_read(p, REG_SW_0, &val))
+		puts("PMIC write error!\n");
 	val = (val & ~SWx_VOLT_MASK) | SWx_1_100V;
-	pmic_reg_write(REG_SW_0, val);
+	if (pmic_reg_write(p, REG_SW_0, &val))
+		puts("PMIC write error!\n");
 
 	/* Setup VCC (SW2) to 1.25 */
-	val = pmic_reg_read(REG_SW_1);
+	if (pmic_reg_read(p, REG_SW_1, &val))
+		puts("PMIC write error!\n");
 	val = (val & ~SWx_VOLT_MASK) | SWx_1_250V;
-	pmic_reg_write(REG_SW_1, val);
+	if (pmic_reg_write(p, REG_SW_1, &val))
+		puts("PMIC write error!\n");
 
 	/* Setup 1V2_DIG1 (SW3) to 1.25 */
-	val = pmic_reg_read(REG_SW_2);
+	if (pmic_reg_read(p, REG_SW_2, &val))
+		puts("PMIC write error!\n");
 	val = (val & ~SWx_VOLT_MASK) | SWx_1_250V;
-	pmic_reg_write(REG_SW_2, val);
-	udelay(50);
+	if (pmic_reg_write(p, REG_SW_2, &val))
+		puts("PMIC write error!\n");
 
 	/* Raise the core frequency to 800MHz */
 	writel(0x0, &mxc_ccm->cacrr);
 
 	/* Set switchers in Auto in NORMAL mode & STANDBY mode */
 	/* Setup the switcher mode for SW1 & SW2*/
-	val = pmic_reg_read(REG_SW_4);
+	if (pmic_reg_read(p, REG_SW_4, &val))
+		puts("PMIC write error!\n");
 	val = (val & ~((SWMODE_MASK << SWMODE1_SHIFT) |
 		(SWMODE_MASK << SWMODE2_SHIFT)));
 	val |= (SWMODE_AUTO_AUTO << SWMODE1_SHIFT) |
 		(SWMODE_AUTO_AUTO << SWMODE2_SHIFT);
-	pmic_reg_write(REG_SW_4, val);
+	if (pmic_reg_write(p, REG_SW_4, &val))
+		puts("PMIC write error!\n");
 
 	/* Setup the switcher mode for SW3 & SW4 */
-	val = pmic_reg_read(REG_SW_5);
+	if (pmic_reg_read(p, REG_SW_5, &val))
+		puts("PMIC write error!\n");
 	val = (val & ~((SWMODE_MASK << SWMODE3_SHIFT) |
 		(SWMODE_MASK << SWMODE4_SHIFT)));
 	val |= (SWMODE_AUTO_AUTO << SWMODE3_SHIFT) |
 		(SWMODE_AUTO_AUTO << SWMODE4_SHIFT);
-	pmic_reg_write(REG_SW_5, val);
+	if (pmic_reg_write(p, REG_SW_5, &val))
+		puts("PMIC write error!\n");
 
 	/* Set VDIG to 1.65V, VGEN3 to 1.8V, VCAM to 2.6V */
-	val = pmic_reg_read(REG_SETTING_0);
+	if (pmic_reg_read(p, REG_SETTING_0, &val))
+		puts("PMIC write error!\n");
 	val &= ~(VCAM_MASK | VGEN3_MASK | VDIG_MASK);
 	val |= VDIG_1_65 | VGEN3_1_8 | VCAM_2_6;
-	pmic_reg_write(REG_SETTING_0, val);
+	if (pmic_reg_write(p, REG_SETTING_0, &val))
+		puts("PMIC write error!\n");
 
 	/* Set VVIDEO to 2.775V, VAUDIO to 3V, VSD to 3.15V */
-	val = pmic_reg_read(REG_SETTING_1);
+	if (pmic_reg_read(p, REG_SETTING_1, &val))
+		puts("PMIC write error!\n");
 	val &= ~(VVIDEO_MASK | VSD_MASK | VAUDIO_MASK);
 	val |= VSD_3_15 | VAUDIO_3_0 | VVIDEO_2_775;
-	pmic_reg_write(REG_SETTING_1, val);
+	if (pmic_reg_write(p, REG_SETTING_1, &val))
+		puts("PMIC write error!\n");
 
 	/* Configure VGEN3 and VCAM regulators to use external PNP */
 	val = VGEN3CONFIG | VCAMCONFIG;
-	pmic_reg_write(REG_MODE_1, val);
+	if (pmic_reg_write(p, REG_MODE_1, &val))
+		puts("PMIC write error!\n");
+
 	udelay(200);
 
 	gpio_direction_output(46, 0);
@@ -257,7 +280,8 @@ static void power_init(void)
 	/* Enable VGEN3, VCAM, VAUDIO, VVIDEO, VSD regulators */
 	val = VGEN3EN | VGEN3CONFIG | VCAMEN | VCAMCONFIG |
 		VVIDEOEN | VAUDIOEN  | VSDEN;
-	pmic_reg_write(REG_MODE_1, val);
+	if (pmic_reg_write(p, REG_MODE_1, &val))
+		puts("PMIC write error!\n");
 
 	udelay(500);
 
Signature · 1 line
1.7.2.3
#20 lore raw

PATCH 3/3 misc:pmic:mx: Code modification for mx51evk board

Am 26/09/2011 17:10, schrieb Lukasz Majewski:

Quoted 6 lines

This is an example code for adjusting the mx51evk example board to new pmic_core driver.

Thnaks for example. In the next version you do not need to repost this code.

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#21 lore raw

cover misc:pmic: New PMIC generic driver

In reply to Lukasz Majewski
Quoted 10 lines · Lukasz Majewski

This patch series adds new generic PMIC framework for u-boot. It is supposed to handle various types of pmic IC's (both I2C and SPI).

This patch series has been tested on Samsung's GONI reference target.

Moreover a special patch for mx51evk target board has been prepared. It is compiling, but I cannot test if the driver works. Other iMX targets shall be corrected in a similar way that mx51evk.

Hi Lukasz,

I cannot review now your patches, but I will be able to test the SPI part. Give me only some days..

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#22 lore raw

cover misc:pmic: New PMIC generic driver

Hi Stefano,

Quoted 2 lines

I cannot review now your patches,

Ok, no problem. I will wait :-)

Signature · 6 lines
Best regards,

Lukasz Majewski

Samsung Poland R&D Center
Platform Group
#23 lore raw

cover misc:pmic: New PMIC generic driver

In reply to Lukasz Majewski

Am 26/09/2011 17:10, schrieb Lukasz Majewski:

Quoted 4 lines

This patch series adds new generic PMIC framework for u-boot. It is supposed to handle various types of pmic IC's (both I2C and SPI).

Hi Lukasz,

Quoted 4 lines

This patch series has been tested on Samsung's GONI reference target.

Moreover a special patch for mx51evk target board has been prepared. It is compiling, but I cannot test if the driver works.

I have tested the SPI part on a different board instead of MX51EVK(vision2). I think you can forget for your patchset about MX5 boards - they can be switched to the general pmic_core when the fsl_pmic file will be dropped and after merging your patches. Thanks in any case to have provided an example ;-)

Quoted 3 lines

Other iMX targets shall be corrected in a similar way that mx51evk.

The old fsl_pmic.c driver has been preserved.

Ok - it will be removed after including you patches.

The write function is symmetric to the read function:

int pmic_reg_write(struct pmic *p, u32 reg, u32 *val)

However, we do not want to change the value pointed by val. I think it is should better to pass a u32 instead of a pointer.

Quoted 6 lines

Lukasz Majewski (3):
misc:pmic New generic pmic driver
misc:pmic: Enable PMIC handling at u-boot startup code misc:pmic:mx: Code modification for mx51evk board

Quoted 13 lines
 arch/arm/lib/board.c              |    5 +
 board/freescale/mx51evk/mx51evk.c |   68 ++++++++++++------
 drivers/misc/Makefile             |    6 +-
 drivers/misc/pmic_core.c          |  148 +++++++++++++++++++++++++++++++++++++
 drivers/misc/pmic_fsl.c           |   57 ++++++++++++++
 drivers/misc/pmic_i2c.c           |   92 +++++++++++++++++++++++
 drivers/misc/pmic_max8998.c       |   43 +++++++++++
 drivers/misc/pmic_spi.c           |  109 +++++++++++++++++++++++++++
 include/configs/mx51evk.h         |    6 +-
 include/configs/s5p_goni.h        |    4 +
 include/fsl_pmic.h                |    5 +-
 include/max8998_pmic.h            |   84 +++++++++++++++++++++
 include/pmic.h                    |   71 ++++++++++++++++++

Logically I see your patchset split into 3 patches, but differently as you did:
- adding pmic core driver
- adding support for max8998
- adding pmic to the s5p_goni

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#24 lore raw

v2 cover misc:pmic: New PMIC generic driver

In reply to Lukasz Majewski

This patch series adds new generic PMIC framework for u-boot. It is supposed to handle various types of pmic IC's (both, equippled with I2C or SPI bus).

This patch series has been tested on Samsung's GONI reference target.
---
Changes for v2:
- New logical structure of patch series
- printf/puts replaced with debug() macro where applicable
- pmic_reg_write(...,u32 *val) replaced with u32 val
- pmic_init() moved to board_init() function

Lukasz Majewski (3):
misc:pmic:core New generic PMIC driver misc:pmic:max8998 MAX8998 support at a new PMIC driver. misc:pmic:samsung Enable PMIC driver at GONI target

 board/samsung/goni/goni.c   |    4 +
 drivers/misc/Makefile       |    4 +
 drivers/misc/pmic_core.c    |  147 +++++++++++++++++++++++++++++++++++++++++++
 drivers/misc/pmic_i2c.c     |   92 +++++++++++++++++++++++++++
 drivers/misc/pmic_max8998.c |   43 +++++++++++++
 drivers/misc/pmic_spi.c     |  109 ++++++++++++++++++++++++++++++++
 include/configs/s5p_goni.h  |    4 +
 include/max8998_pmic.h      |   84 ++++++++++++++++++++++++
 include/pmic.h              |   71 +++++++++++++++++++++
 9 files changed, 558 insertions(+), 0 deletions(-)
 create mode 100644 drivers/misc/pmic_core.c
 create mode 100644 drivers/misc/pmic_i2c.c
 create mode 100644 drivers/misc/pmic_max8998.c
 create mode 100644 drivers/misc/pmic_spi.c
 create mode 100644 include/max8998_pmic.h
 create mode 100644 include/pmic.h
Signature · 1 line
1.7.2.3
#25 lore raw

PATCH v2 1/3 misc:pmic:core New generic PMIC driver

I2C or SPI PMIC devices can be accessed. Separate files: pmic_i2c.c and pmic_spi.c are responsible for handling transmission over I2C or SPI bus.

New flags:
CONFIG_PMIC - enable PMIC general device. CONFIG_PMIC_I2C/SPI - specify the interface to be used.

---
Changes for v2:
- pmic_reg_write(..., u32 *val) - u32 *val replaced with u32 val.
- debug() macros added instead of plain printf()

---
 drivers/misc/Makefile    |    3 +
 drivers/misc/pmic_core.c |  147 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/misc/pmic_i2c.c  |   92 +++++++++++++++++++++++++++++
 drivers/misc/pmic_spi.c  |  109 ++++++++++++++++++++++++++++++++++
 include/pmic.h           |   71 ++++++++++++++++++++++
 5 files changed, 422 insertions(+), 0 deletions(-)
 create mode 100644 drivers/misc/pmic_core.c
 create mode 100644 drivers/misc/pmic_i2c.c
 create mode 100644 drivers/misc/pmic_spi.c
 create mode 100644 include/pmic.h
drivers/misc/Makefile +3 -0
index b152486..91c5bfa 100644
@@ -35,6 +35,9 @@ COBJS-$(CONFIG_NS87308) += ns87308.o
 COBJS-$(CONFIG_PDSP188x) += pdsp188x.o
 COBJS-$(CONFIG_STATUS_LED) += status_led.o
 COBJS-$(CONFIG_TWL4030_LED) += twl4030_led.o
+COBJS-$(CONFIG_PMIC) += pmic_core.o
+COBJS-$(CONFIG_PMIC_I2C) += pmic_i2c.o
+COBJS-$(CONFIG_PMIC_SPI) += pmic_spi.o
 
 COBJS	:= $(COBJS-y)
 SRCS	:= $(COBJS:.o=.c)
drivers/misc/pmic_core.c new file +147 -0

147 lines not on this page · raw message

drivers/misc/pmic_i2c.c new file +92 -0

92 lines not on this page · raw message

drivers/misc/pmic_spi.c new file +109 -0

109 lines not on this page · raw message

include/pmic.h new file +71 -0

71 lines not on this page · raw message

Signature · 1 line
1.7.2.3
#26 lore raw

PATCH v2 1/3 misc:pmic:core New generic PMIC driver

Quoted 4 lines · Lukasz Majewski

I2C or SPI PMIC devices can be accessed. Separate files: pmic_i2c.c and pmic_spi.c are responsible for handling transmission over I2C or SPI bus.

Quoted 8 lines

New flags:
CONFIG_PMIC - enable PMIC general device. CONFIG_PMIC_I2C/SPI - specify the interface to be used.

---

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#27 lore raw

PATCH v2 1/3 misc:pmic:core New generic PMIC driver

In reply to Lukasz Majewski
Quoted 4 lines · Lukasz Majewski

I2C or SPI PMIC devices can be accessed. Separate files: pmic_i2c.c and pmic_spi.c are responsible for handling transmission over I2C or SPI bus.

Quoted 8 lines

New flags:
CONFIG_PMIC - enable PMIC general device. CONFIG_PMIC_I2C/SPI - specify the interface to be used.

---

Hi Lukasz,

Quoted 10 lines
include/pmic.h new file +10 -0
+
+struct p_spi {
+	unsigned char cs;
+	unsigned char mode;
+	unsigned char bitlen;
+	unsigned int  clk;
+	unsigned int  flags;
+	u32 (*prepare_tx)(u32 reg, u32 *val, u32 write);
+};
+

include/pmic.h line 55

Quoted 10 lines
include/pmic.h new file +10 -0
+struct pmic {
+	const char *name;
+	unsigned char bus;
+	unsigned char interface;
+	unsigned char number_of_regs;
+	union hw {
+		struct p_i2c i2c;
+		struct p_spi spi;
+	} hw;
+};

I have found an issue with this structure. In some case a char is not enough. The i.MX SPI driver uses the LSB byte to set a chipselect provided directly by the controller, and the MSB if a GPIO is selected. You can see an example in efikamx.h. It is better to replace all unsigned char in this structure with unsigned int to avoid these problems.

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#28 lore raw

PATCH v2 1/3 misc:pmic:core New generic PMIC driver

Hi Stefano,

I suppose you are talking about cases like the following:

Quoted 1 line

#define CONFIG_SPI_FLASH_CS (1 | 121 << 8)

Yes, this construct will cause range overflow.

include/pmic.h line 40

Quoted 4 lines
include/pmic.h new file +4 -0
+struct p_spi {
+	unsigned char cs;
+	unsigned char mode;
+	unsigned char bitlen;

Therefore the above variables need to be at least unsigned shorts. As you proposed they shall be unsigned int.

On technical question:
Shall I prepare the whole v3 patch series or only v3 for this single patch ([PATCH v2 1/3] misc:pmic:core New generic PMIC drive)?

What would be easier for You or Wolfgang to pick?

Signature · 6 lines
Best regards,

Lukasz Majewski

Samsung Poland R&D Center
Platform Group
#29 lore raw

PATCH v2 1/3 misc:pmic:core New generic PMIC driver

Quoted 2 lines · Lukasz Majewski

Hi Stefano,

Hi Lukasz,

Quoted 6 lines

I suppose you are talking about cases like the following:

Quoted 1 line

#define CONFIG_SPI_FLASH_CS (1 | 121 << 8)

Yes, this construct will cause range overflow.

Quoted 7 lines

include/pmic.h line 40

Quoted 4 lines
include/pmic.h new file +4 -0
+struct p_spi {
+	unsigned char cs;
+	unsigned char mode;
+	unsigned char bitlen;

Therefore the above variables need to be at least unsigned shorts. As you proposed they shall be unsigned int.

Yes

Quoted 4 lines

On technical question:
Shall I prepare the whole v3 patch series or only v3 for this single patch ([PATCH v2 1/3] misc:pmic:core New generic PMIC drive)?

Post only v3 for the single patch.

Quoted 2 lines

What would be easier for You or Wolfgang to pick?

As Wolfgang asked, I will take care of your patchset and I will merge it with u-boot-imx. It will go into mainline after my pull request.

Best regards,
Stefano

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#30 lore raw

PATCH v2 2/3 misc:pmic:max8998 MAX8998 support at a new PMIC driver.

In reply to Lukasz Majewski

This commit adds support for MAX8998 PMIC driver.

---
Changes for v2:
- separate commit for MAX8998 prepared

---
 drivers/misc/Makefile       |    1 +
 drivers/misc/pmic_max8998.c |   43 ++++++++++++++++++++++
 include/max8998_pmic.h      |   84 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 128 insertions(+), 0 deletions(-)
 create mode 100644 drivers/misc/pmic_max8998.c
 create mode 100644 include/max8998_pmic.h
drivers/misc/Makefile +1 -0
index 91c5bfa..6d82c22 100644
@@ -38,6 +38,7 @@ COBJS-$(CONFIG_TWL4030_LED) += twl4030_led.o
 COBJS-$(CONFIG_PMIC) += pmic_core.o
 COBJS-$(CONFIG_PMIC_I2C) += pmic_i2c.o
 COBJS-$(CONFIG_PMIC_SPI) += pmic_spi.o
+COBJS-$(CONFIG_PMIC_MAX8998) += pmic_max8998.o
 
 COBJS	:= $(COBJS-y)
 SRCS	:= $(COBJS:.o=.c)
drivers/misc/pmic_max8998.c new file +43 -0

43 lines not on this page · raw message

include/max8998_pmic.h new file +84 -0

84 lines not on this page · raw message

Signature · 1 line
1.7.2.3
#31 lore raw

PATCH v2 2/3 misc:pmic:max8998 MAX8998 support at a new PMIC driver.

Quoted 8 lines · Lukasz Majewski

This commit adds support for MAX8998 PMIC driver.

---
Changes for v2:
- separate commit for MAX8998 prepared

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#32 lore raw

PATCH v2 2/3 misc:pmic:max8998 MAX8998 support at a new PMIC driver.

In reply to Lukasz Majewski
Quoted 9 lines · Lukasz Majewski

This commit adds support for MAX8998 PMIC driver.

---
Changes for v2:
- separate commit for MAX8998 prepared
---

Applied to u-boot-imx, thanks.

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#33 lore raw

PATCH v2 3/3 misc:pmic:samsung Enable PMIC driver at GONI target

In reply to Lukasz Majewski

Enable support for new PMIC driver at GONI reference target.

---
Changes for v2:
- pmic_init() function call removed from arch/arm/lib/board.c and moved to s5p_goni board_init() function.
- prepare separate patch for enabling new PMIC driver at GONI

---
 board/samsung/goni/goni.c  |    4 ++++
 include/configs/s5p_goni.h |    4 ++++
 2 files changed, 8 insertions(+), 0 deletions(-)
board/samsung/goni/goni.c +4 -0
index e24cd29..e191bfb 100644
@@ -25,6 +25,7 @@
 #include <common.h>
 #include <asm/arch/gpio.h>
 #include <asm/arch/mmc.h>
+#include <pmic.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -38,6 +39,9 @@ int board_init(void)
 	gd->bd->bi_arch_number = MACH_TYPE_GONI;
 	gd->bd->bi_boot_params = PHYS_SDRAM_1 + 0x100;
 
+#if defined(CONFIG_PMIC)
+	pmic_init();
+#endif
 	return 0;
 }
 
include/configs/s5p_goni.h +4 -0
index 886c8be..aa51114 100644
@@ -223,6 +223,10 @@
 
 #define CONFIG_SYS_INIT_SP_ADDR	(CONFIG_SYS_LOAD_ADDR - 0x1000000)
 
+#define CONFIG_PMIC
+#define CONFIG_PMIC_I2C
+#define CONFIG_PMIC_MAX8998
+
 #include <asm/arch/gpio.h>
 /*
  * I2C Settings
Signature · 1 line
1.7.2.3
#34 lore raw

PATCH v2 3/3 misc:pmic:samsung Enable PMIC driver at GONI target

Quoted 6 lines · Lukasz Majewski

Enable support for new PMIC driver at GONI reference target.

---

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#35 lore raw

PATCH v2 3/3 misc:pmic:samsung Enable PMIC driver at GONI target

In reply to Lukasz Majewski
Quoted 11 lines · Lukasz Majewski

Enable support for new PMIC driver at GONI reference target.

---
Changes for v2:
- pmic_init() function call removed from arch/arm/lib/board.c and moved to s5p_goni board_init() function.
- prepare separate patch for enabling new PMIC driver at GONI
---

Applied to u-boot-imx, thanks.

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#36 lore raw

v2 cover misc:pmic: New PMIC generic driver

In reply to Lukasz Majewski
Quoted 12 lines · Lukasz Majewski

This patch series adds new generic PMIC framework for u-boot. It is supposed to handle various types of pmic IC's (both, equippled with I2C or SPI bus).

This patch series has been tested on Samsung's GONI reference target.
---
Changes for v2:
- New logical structure of patch series
- printf/puts replaced with debug() macro where applicable
- pmic_reg_write(...,u32 *val) replaced with u32 val
- pmic_init() moved to board_init() function

Quoted 5 lines

Lukasz Majewski (3):
misc:pmic:core New generic PMIC driver misc:pmic:max8998 MAX8998 support at a new PMIC driver. misc:pmic:samsung Enable PMIC driver at GONI target

Hi Lukasz,

I retested your patches, they looks ok for me. I have only seen you dropped also the changes to fsl_pmic.h and pmic_fsl.c, not only the ones for MX51. Because I need in any case these changes to test your code, I will merge also these files (I resubmit now a patch with your signed-off with the code extracted from the V1, as fourth patch of your patchset..).

I tested also the RTC mc13783, this uses also PMIC code. It looks ok ;-).

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#37 lore raw

PATCH v2 4/4 misc: pmic: Freescale PMIC switches to generic PMIC driver

In reply to Lukasz Majewski

---
Changes:
this code was part of V1, missing in V2, required for Freescale PMIC

 drivers/misc/Makefile   |    1 +
 drivers/misc/pmic_fsl.c |   57 +++++++++++++++++++++++++++++++++++++++++++++++
 include/fsl_pmic.h      |    5 +---
 3 files changed, 59 insertions(+), 4 deletions(-)
 create mode 100644 drivers/misc/pmic_fsl.c
drivers/misc/Makefile +1 -0
index 6d82c22..f732a95 100644
@@ -36,6 +36,7 @@ COBJS-$(CONFIG_PDSP188x) += pdsp188x.o
 COBJS-$(CONFIG_STATUS_LED) += status_led.o
 COBJS-$(CONFIG_TWL4030_LED) += twl4030_led.o
 COBJS-$(CONFIG_PMIC) += pmic_core.o
+COBJS-$(CONFIG_PMIC_FSL) += pmic_fsl.o
 COBJS-$(CONFIG_PMIC_I2C) += pmic_i2c.o
 COBJS-$(CONFIG_PMIC_SPI) += pmic_spi.o
 COBJS-$(CONFIG_PMIC_MAX8998) += pmic_max8998.o
drivers/misc/pmic_fsl.c new file +57 -0

57 lines not on this page · raw message

include/fsl_pmic.h +1 -4
index e3abde6..742f2e1 100644
@@ -99,6 +99,7 @@ enum {
 	REG_TEST2,
 	REG_TEST3,
 	REG_TEST4,
+	PMIC_NUM_OF_REGS,
 };
 
 /* REG_POWER_MISC */
@@ -121,8 +122,4 @@ enum {
 /* Interrupt status 1 */
 #define RTCRSTI		(1 << 7)
 
-void pmic_show_pmic_info(void);
-void pmic_reg_write(u32 reg, u32 value);
-u32 pmic_reg_read(u32 reg);
-
 #endif
Signature · 1 line
1.7.1
#38 lore raw

PATCH v3 1/3 misc:pmic:core New generic PMIC driver

In reply to Lukasz Majewski

I2C or SPI PMIC devices can be accessed. Separate files: pmic_i2c.c and pmic_spi.c are responsible for handling transmission over I2C or SPI bus.

New flags:
CONFIG_PMIC - enable PMIC general device. CONFIG_PMIC_I2C/SPI - specify the interface to be used.

---
Changes for v2:
- pmic_reg_write(..., u32 *val) - u32 *val replaced with u32 val.
- debug() macros added instead of plain printf() Changes for v3:
- at 'struct p_spi' replace cs, mode and bitlen unsigned char variables to unsigned int

---
 drivers/misc/Makefile    |    3 +
 drivers/misc/pmic_core.c |  147 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/misc/pmic_i2c.c  |   92 +++++++++++++++++++++++++++++
 drivers/misc/pmic_spi.c  |  109 ++++++++++++++++++++++++++++++++++
 include/pmic.h           |   71 ++++++++++++++++++++++
 5 files changed, 422 insertions(+), 0 deletions(-)
 create mode 100644 drivers/misc/pmic_core.c
 create mode 100644 drivers/misc/pmic_i2c.c
 create mode 100644 drivers/misc/pmic_spi.c
 create mode 100644 include/pmic.h
drivers/misc/Makefile +3 -0
index b152486..91c5bfa 100644
@@ -35,6 +35,9 @@ COBJS-$(CONFIG_NS87308) += ns87308.o
 COBJS-$(CONFIG_PDSP188x) += pdsp188x.o
 COBJS-$(CONFIG_STATUS_LED) += status_led.o
 COBJS-$(CONFIG_TWL4030_LED) += twl4030_led.o
+COBJS-$(CONFIG_PMIC) += pmic_core.o
+COBJS-$(CONFIG_PMIC_I2C) += pmic_i2c.o
+COBJS-$(CONFIG_PMIC_SPI) += pmic_spi.o
 
 COBJS	:= $(COBJS-y)
 SRCS	:= $(COBJS:.o=.c)
drivers/misc/pmic_core.c new file +147 -0

147 lines not on this page · raw message

drivers/misc/pmic_i2c.c new file +92 -0

92 lines not on this page · raw message

drivers/misc/pmic_spi.c new file +109 -0

109 lines not on this page · raw message

include/pmic.h new file +71 -0

71 lines not on this page · raw message

Signature · 1 line
1.7.2.3
#39 lore raw

PATCH v3 1/3 misc:pmic:core New generic PMIC driver

Quoted 4 lines · Lukasz Majewski

I2C or SPI PMIC devices can be accessed. Separate files: pmic_i2c.c and pmic_spi.c are responsible for handling transmission over I2C or SPI bus.

Quoted 8 lines

New flags:
CONFIG_PMIC - enable PMIC general device. CONFIG_PMIC_I2C/SPI - specify the interface to be used.

---

Applied to u-boot-imx, thanks.

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================