Am 05/10/2012 10:16, schrieb Lukasz Majewski:
Now it is possible to provide specific function per PMIC/power device instance.
Signed-off-by: Lukasz Majewski l.majewski@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com
Changes for v2:
- New at patch v2
Hi Lucasz,
include/power/battery.h | 38 ++++++++++++++++++++++++++++++++++++++ include/power/pmic.h | 23 ++++++++++++++++++++++- 2 files changed, 60 insertions(+), 1 deletions(-) create mode 100644 include/power/battery.h
diff --git a/include/power/battery.h b/include/power/battery.h new file mode 100644 index 0000000..e2fec68 --- /dev/null +++ b/include/power/battery.h @@ -0,0 +1,38 @@ +/*
- Copyright (C) 2012 Samsung Electronics
- Lukasz Majewski l.majewski@samsung.com
- See file CREDITS for list of people who contributed to this
- project.
- This program is free software; you can redistribute it and/or
- modify it under the terms of the GNU General Public License as
- published by the Free Software Foundation; either version 2 of
- the License, or (at your option) any later version.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
- along with this program; if not, write to the Free Software
- Foundation, Inc., 59 Temple Place, Suite 330, Boston,
- MA 02111-1307 USA
- */
+#ifndef __POWER_BATTERY_H_ +#define __POWER_BATTERY_H_
+struct battery {
- unsigned int version;
- unsigned int state_of_chrg;
- unsigned int time_to_empty;
- unsigned int capacity;
- unsigned int voltage_uV;
- unsigned int state;
+};
+int power_bat_init(unsigned char bus); +#endif /* __POWER_BATTERY_H_ */ diff --git a/include/power/pmic.h b/include/power/pmic.h index 3583342..5ec7bae 100644 --- a/include/power/pmic.h +++ b/include/power/pmic.h @@ -27,8 +27,9 @@ #include <common.h> #include <linux/list.h> #include <i2c.h> +#include <power/power_chrg.h>
-enum { PMIC_I2C, PMIC_SPI, }; +enum { PMIC_I2C, PMIC_SPI, PMIC_NONE}; enum { I2C_PMIC, I2C_NUM, }; enum { PMIC_READ, PMIC_WRITE, }; enum { PMIC_BYTE_ORDER_REVERSED, PMIC_BYTE_ORDER_NORMAL, }; @@ -48,6 +49,21 @@ struct p_spi { u32 (*prepare_tx)(u32 reg, u32 *val, u32 write); };
+struct pmic; +struct power_battery {
- struct battery *bat;
- int (*fg_battery_check) (struct pmic *p, struct pmic *bat);
- int (*fg_battery_update) (struct pmic *p, struct pmic *bat);
- int (*chrg_type) (struct pmic *p);
- int (*chrg_bat_present) (struct pmic *p);
- int (*chrg_state) (struct pmic *p, int state, int current);
- /* Keep info about power devices involved with battery operation */
- struct pmic *chrg, *fg, *muic;
+};
struct pmic { const char *name; unsigned char bus; @@ -59,6 +75,11 @@ struct pmic { struct p_spi spi; } hw;
- struct power_battery *pwr_bat;
- int (*battery_init) (struct pmic *p, struct pmic *bat);
If we add entry points to the pmic structure, I do not expect to pass the pointer of the structure itself, or I could call the function with another pointer:
p->battery_init(p1, bat);
So p should be hide, stored during the initialization of PMIC itself.
- int (*battery_charge) (struct pmic *bat);
- void (*low_power_mode) (void);
- struct list_head list;
};
I have some concerns regarding this structure. It seems to me very coupled to the PMIC you are using.
Reading this and the next patches it seems to me that is due because your PMIC includes other three PMICs. However, IMHO the interface presented before this patch is very clear.
I have expected something as:
P = get_pmic("CHRG"); p->chrg_state(current);
And if the problem is that this PMIC really descend from another one, we could add a "struct pmic *parent" in the pmic itself. You already introduced lists, and we can work with pointer instead of hard-code the subpmics as in : struct pmic *chrg, *fg, *muic;
In this case, if we have a pmic that logically can be seen with a different number of pmics, we can still support. You introduce PMIC_NONE to tell that this pmic does not have an own data transfer and rely on another one. But again, this could be solved using
if (p->parent) p->i2c....
or something like this to use the transfer method of the "main" PMIC.
Best regards, Stefano Babic