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 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
intpmic_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.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.
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.cdriver, 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.cfile consist of#ifdef PMIC_I2C {Code for handling I2C} #else {Code for handling SPI} #endifThe same approach is used at
fsl_pmic.cI'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 =====================================================================