RFC Generic PMIC driver
Get series with b4
b4 am [email protected]Versions newest first
- v1 RFC Latest0 of 2 patches 0 repliesNew
Patches in v1 No applies recorded yet
None of the 2 patches the cover letter announces are in this archive.
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.
Cover letter [U-Boot] [RFC 0/2] 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.cas 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.cread 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_initmethods 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.cdriver, 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.ccode 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) {
casePMIC_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 =====================================================================