power: supply: clean up max77818 charger driver
Starting with the task to have a thorough review on the driver, I ended
up with fixing a couple of issues and cleaning up the code quite a lot.
- The termination_time is written to the wrong register. It should be
CNFG_03 instead of CNFG_09.
- A reasonable default topoff_timer should be 30 minutes according to
the reset value in data sheet.
- Fix broken interrupt code and CHGIN/WCIN insert/remove event can now
be triggered.
- Remove those verbose debugging message output.
- Improve coding style of the driver to make it a classic Linux style
driver.
- Rather than calling helpers provided max77818 mfd driver, call into
regmap functions directly, as those helpers are not so helpful, and
we may want to drop them from max77818 mfd driver later.
- Drop max77818_charger_pm_ops as the functions do nothing there.
- Use module_platform_driver() to save some code.
There are quite many function shifting and code tweaking with the
cleanup, which makes the reviewing code a lot harder (sorry about that).
So it might be easier to review the driver file rather than diff. At
end of the day, the commit shouldn't introduce any regression but only
fix issues and enable interrupt support.
But there are still a few things worth discussion and possibly further
works.
- The driver currently allows user to change charger current setting.
Should we really allow this? I quote the following from max77818
data sheet:
"Maxim recommends that CHG_CC be set to the maximum acceptable charge
rate for your battery – there is typically no need to actively adjust
the CHG_CC setting based on the capabilities of the source at CHGIN,
system load, or thermal limitations of the PCB"
- MAX77818 data sheet stats that for a particular battery/system, the
following parameters should be set properly.
Determine the following registers bit settings by considering the
characteristics of the battery:
. Charger Restart Threshold - CHG_RSTRT
Default (reset) is 150mV below CHG_CV_PRM (0x01), and we set it
3400mV, which is wrong (I'm fixing it).
. Fast-Charge Timer (tFC) - FCHGTIME
Default is 4 hours (0x01), and we set it disabled (0x00). Is this
really what we want?
. Fast-Charge Current - CHG_CC
Default is 450mA (0x09), and we set it 2800mA (0x38).
. Top-Off Time - TO_TIME
Default is 30 minutes, and we set it 0 minutes (0x00). Is this
really what we want? I'm not even sure what 0x00 means here.
. Top-Off Current - TO_ITH
Default is 150mA (0x01), we set it 200mA (0x04).
. Battery Regulation Voltage - CHG_CV_PRM
Default is 4.2V (0x16), and we set it 4.3V (0x1A).
Determine the following register bit settings by considering the
characteristics of the system:
. Low Battery Prequalification Enable: PQEN
Default is disabled, we use default.
. Minimum System Regulation Voltage: MINVSYS
Default is 3.6V (0x02), and we set it 3.4V (0x00).
. Junction Temperature Thermal Regulation Loop Setpoint: REGTEMP
Default is 115C (0x02), and we use default.
- Do we need to enable watchdog timer?
- Do we need to detect and handle over-current condition by setting
bit DISIBS in CNFG_00 to force MBATT-SYS FET off?
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
pull/10/head
Shawn Guo
Steinar Bakkemo
parent
9a508cfbbf
commit
bcdd832781