Power: DA9052 Battery driver v4
PATCH 4/11 Power: DA9052 Battery driver v4
Driver for DA9052 battery charger. This driver depends on DA9052 MFD core dirver for definitions and methods.
Tested on Samsung SMDK6410 board with DA9052-BC and DA9053-BA evaluation boards.
---
Changes since v3
- Included power_supply.h file
- Corrected the definition of DA9052_BAT_THRESHOLD macro
- Changed definition of charger_type_enum
- Added API da9052_battery_read_end_current
- Added API da9052_battery_check_status
- Changed the way of handling the interrupts in da9052_bat_irq Changes since v2
- Correct code styling for inline functions
- Remove averaging algorithm
- Set use_for_apm thru board specific parameter
--- drivers/power/Kconfig | 7 + drivers/power/Makefile | 1 + drivers/power/da9052-battery.c | 611 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 619 insertions(+), 0 deletions(-) create mode 100644 drivers/power/da9052-battery.c
drivers/power/Kconfig +7 -0
@@ -150,6 +150,13 @@ config BATTERY_DA9030 Say Y here to enable support for batteries charger integrated into DA9030 PMIC. +config BATTERY_DA9052 + tristate "Dialog DA9052 Battery" + depends on PMIC_DA9052 + help + Say Y here to enable support for batteries charger integrated into + DA9052 PMIC. + config BATTERY_MAX17040 tristate "Maxim MAX17040 Fuel Gauge" depends on I2C
drivers/power/Makefile +1 -0
@@ -25,6 +25,7 @@ obj-$(CONFIG_BATTERY_WM97XX) += wm97xx_battery.o obj-$(CONFIG_BATTERY_BQ20Z75) += bq20z75.o obj-$(CONFIG_BATTERY_BQ27x00) += bq27x00_battery.o obj-$(CONFIG_BATTERY_DA9030) += da9030_battery.o +obj-$(CONFIG_BATTERY_DA9052) += da9052-battery.o obj-$(CONFIG_BATTERY_MAX17040) += max17040_battery.o obj-$(CONFIG_BATTERY_MAX17042) += max17042_battery.o obj-$(CONFIG_BATTERY_Z2) += z2_battery.o
drivers/power/da9052-battery.c new file +611 -0
611 lines not on this page · raw message
Signature · 1 line
1.7.1
Quoted 8 lines · Ashish Jangam
Driver for DA9052 battery charger. This driver depends on DA9052 MFD core dirver for definitions and methods.
Tested on Samsung SMDK6410 board with DA9052-BC and DA9053-BA evaluation boards.
Signed-off-by David Dajun Chen <[email protected]>Signed-off-by Ashish Jangam <[email protected]>---
Ashish,
Much thanks for your work! The code itself looks OK to me. But there are some issues regarding readability and coding style. No big deal, but please consider fixing.
[...]
[..]
drivers/power/da9052-battery.c line 213
Quoted 5 lines
drivers/power/da9052-battery.c new file +5
-0+static int da9052_battery_check_status (struct da9052_battery *battery, +int *status) +{ +uint8_t v[2 ] = {0 ,0 }; +uint8_t bat_status, chg_end;
In kernel code we try to use 'uXX' types. I.e. u8 here.
drivers/power/da9052-battery.c line 214
Quoted 1 line
drivers/power/da9052-battery.c new file +1
-0+int ret, chg_current, chg_end_current;
Each declaration should be on its own line.
int ret;
int chr_current;
Quoted 5 lines
drivers/power/da9052-battery.c new file +5
-0+ + ret =da9052_group_read (battery->da9052,DA9052_STATUS_A_REG ,2 , v); +if (ret <0 ) +return ret; +
drivers/power/da9052-battery.c line 229
Quoted 10 lines
drivers/power/da9052-battery.c new file +10
-0+ bat_status = v[0 ]; + chg_end = v[1 ]; + +/* Preference to WALL(DCIN) charger unit */ +if (((bat_status &DA9052_STATUSA_DCINSEL ) && + (bat_status &DA9052_STATUSA_DCINDET )) + || + ((bat_status &DA9052_STATUSA_VBUSSEL ) && + (bat_status &DA9052_STATUSA_VBUSDET )) + ) {
I can't parse it.
Maybe
bool dcinsel = bat_status & DA9052_STATUSA_DCINSEL; bool dcindet = bat_status & DA9052_STATUSA_DCINDET; bool vbussel = bat_status & DA9052_STATUSA_VBUSSEL; bool vbusdet = bat_status & DA9052_STATUSA_VBUSDET; bool dc = dcinsel && dcindet;
bool vbus = vbussel && vbusdet;
if (dc || vbus) {
...
} else if (dcindet || vbusdet) {
...
} else {
...
}Note that it does not add a single line of code, but things become much more readable.
Quoted 37 lines
drivers/power/da9052-battery.c new file +37
-0+ battery->charger_type =DA9052_CHARGER ; + +/* If charging end flag is set and Charging current is greater +* than charging end limit then battery is charging +*/ +if ((chg_end &DA9052_STATUSB_CHGEND ) !=0 ) { + ret =da9052_battery_read_current (battery, + &chg_current); +if (ret <0 ) +return ret; + ret =da9052_battery_read_end_current (battery, + &chg_end_current); +if (ret <0 ) +return ret; + +if (chg_current >= chg_end_current) + battery->status =POWER_SUPPLY_STATUS_CHARGING ; +else + battery->status = +POWER_SUPPLY_STATUS_NOT_CHARGING ; + } +/* If Charging end flag is cleared then battery is charging */ +else + battery->status =POWER_SUPPLY_STATUS_CHARGING ; + }else if (bat_status &DA9052_STATUSA_DCINDET || + bat_status &DA9052_STATUSA_VBUSDET ) { + battery->charger_type =DA9052_CHARGER ; + battery->status =POWER_SUPPLY_STATUS_NOT_CHARGING ; + }else { + battery->charger_type =DA9052_NOCHARGER ; + battery->status =POWER_SUPPLY_STATUS_DISCHARGING ; + } + +if (status !=NULL ) + *status = battery->status; +return 0 ; +}
[...]
drivers/power/da9052-battery.c line 321
Quoted 11 lines
drivers/power/da9052-battery.c new file +11
-0+unsigned char select_temperature (unsigned char temp_index,int bat_temperature) +{ +int temp_temperature; + + temp_temperature = (temperature_lookup_ref[temp_index] + + temperature_lookup_ref[temp_index +1 ]) /2 ; + +if (bat_temperature >= temp_temperature) { + temp_index +=1 ; +return temp_index; + }else
should be "} else {", per coding style.
Quoted 19 lines
drivers/power/da9052-battery.c new file +19
-0+return temp_index; +} + +static int da9052_battery_read_capacity (struct da9052_battery *battery, +int *capacity) +{ +int bat_temperature, bat_voltage; +int vbat_lower, vbat_upper, level_upper, level_lower; +int ret, flag, index, access_index =0 ; + + ret =da9052_battery_read_volt (battery, &bat_voltage); +if (ret <0 ) +return ret; + + bat_temperature =da9052_adc_temperature_read (battery->da9052); +if (bat_temperature <0 ) +return bat_temperature; + +for (index =0 ; index < (DA9052_NO_OF_LOOKUP_TABLE -1 ); index++) {
drivers/power/da9052-battery.c line 350
Quoted 10 lines
drivers/power/da9052-battery.c new file +10
-0+if (bat_temperature <= temperature_lookup_ref[0 ]) { + access_index =0 ; +break ; + }else if (bat_temperature > + temperature_lookup_ref[DA9052_NO_OF_LOOKUP_TABLE ]) { + access_index =DA9052_NO_OF_LOOKUP_TABLE -1 ; +break ; + }else if ((bat_temperature >= temperature_lookup_ref[index]) && + (bat_temperature >= temperature_lookup_ref[index +1 ] + )) {
Braces placement is weird. The longer identifiers you use, the less columns you have. Maybe try to use shorter variable names?
Quoted 21 lines
drivers/power/da9052-battery.c new file +21
-0+ access_index =select_temperature (index, + bat_temperature); +break ; + } + } +if (bat_voltage >= vbat_vs_capacity_look_up[access_index][0 ][0 ]) { + *capacity =100 ; +return 0 ; + } +if (bat_voltage <= vbat_vs_capacity_look_up[access_index] + [DA9052_LOOK_UP_TABLE_SIZE -1 ][0 ]) { + *capacity =0 ; +return 0 ; + } + flag =0 ; + +for (index =0 ; index < (DA9052_LOOK_UP_TABLE_SIZE -1 ); index++) { +if ((bat_voltage <= + vbat_vs_capacity_look_up[access_index][index][0 ]) && + (bat_voltage >= + vbat_vs_capacity_look_up[access_index][index +1 ][0 ])) {
drivers/power/da9052-battery.c line 381
Quoted 10 lines
drivers/power/da9052-battery.c new file +10
-0+ vbat_upper = + vbat_vs_capacity_look_up[access_index][index][0 ]; + vbat_lower = + vbat_vs_capacity_look_up[access_index][index +1 ][0 ]; + level_upper = + vbat_vs_capacity_look_up[access_index][index][1 ]; + level_lower = + vbat_vs_capacity_look_up[access_index][index +1 ][1 ]; + flag =1 ; +break ;
I can't parse it.
You don't have to use 24-character variable name to document your code. For example, you can rename vbat_vs_capacity_look_up to "vc_tbl" and just add a comment that the table is used to lookup voltage via capacity.
s/vbat_vs_capacity_look_up/vc_tbl/g
s/access_index/i/g
s/index/j/g
[...]
drivers/power/da9052-battery.c line 424
Quoted 3 lines
drivers/power/da9052-battery.c new file +3
-0+static irqreturn_t da9052_bat_irq (int irq,void *data) +{ +struct da9052_battery *battery = (struct da9052_battery *)data;
Needless cast.
Thanks!
Signature · 2 lines
Anton Vorontsov Email: [email protected]