diff mbox

[3/4] power_supply: Introduce PSE compliant algorithm

Message ID 1391490780-6141-4-git-send-email-jenny.tc@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jenny TC Feb. 4, 2014, 5:12 a.m. UTC
As per Product Safety Engineering (PSE) specification for battery charging, the
battery characteristics and thereby the charging rates can vary on different
temperature zones. This patch introduces a PSE compliant charging algorithm with
maintenance charging support. The algorithm can be selected by the power supply
charging driver based on the type of the battery charging profile.

Signed-off-by: Jenny TC <jenny.tc@intel.com>
---
 drivers/power/Kconfig                      |   13 ++
 drivers/power/Makefile                     |    1 +
 drivers/power/charging_algo_pse.c          |  198 ++++++++++++++++++++++++++++
 include/linux/power/power_supply_charger.h |   48 +++++++
 4 files changed, 260 insertions(+)
 create mode 100644 drivers/power/charging_algo_pse.c

Comments

Pavel Machek Feb. 4, 2014, 11:36 a.m. UTC | #1
Hi!

> --- a/drivers/power/Kconfig
> +++ b/drivers/power/Kconfig
> @@ -22,6 +22,19 @@ config POWER_SUPPLY_CHARGER
>  	  drivers to keep the charging logic outside and the charger driver
>  	  just need to abstract the charger hardware.
>  
> +config POWER_SUPPLY_CHARGING_ALGO_PSE
> +	bool "PSE compliant charging algorithm"
> +	help
> +	  Say Y here to select Product Safety Engineering (PSE) compliant
> +	  charging algorithm. As per PSE standard the battery characteristics
> +	  and thereby the charging rates can vary on different temperature
> +	  zones. This config will enable PSE compliant charging algorithm with
> +	  maintenance charging support. At runtime the algorithm will be
> +	  selected by the psy charger driver based on the type of the battery
> +	  charging profile.

Information where to expect PSE compliant chargers would be nice.

> +static inline bool __is_battery_full
> +	(long volt, long cur, long iterm, unsigned long cv)
> +{

codingstyle.

> +	pr_devel("%s:current=%ld pse_mod_bprof->chrg_term_mA =%ld voltage_now=%ld full_cond=%ld",
> +			__func__, cur, iterm, volt * 100, (FULL_CV_MIN * cv));
> +
> +	return (cur > 0) && (cur <= iterm) &&
> +	((volt * 100)  >= (FULL_CV_MIN * cv));

Codingstyle. Just run checkpatch.

									Pavel
Jenny TC Feb. 20, 2014, 5:16 a.m. UTC | #2
On Tue, Feb 04, 2014 at 12:36:40PM +0100, Pavel Machek wrote:
> > --- a/drivers/power/Kconfig
> > +++ b/drivers/power/Kconfig
> > @@ -22,6 +22,19 @@ config POWER_SUPPLY_CHARGER
> >  	  drivers to keep the charging logic outside and the charger driver
> >  	  just need to abstract the charger hardware.
> >  
> > +config POWER_SUPPLY_CHARGING_ALGO_PSE
> > +	bool "PSE compliant charging algorithm"
> > +	help
> > +	  Say Y here to select Product Safety Engineering (PSE) compliant
> > +	  charging algorithm. As per PSE standard the battery characteristics
> > +	  and thereby the charging rates can vary on different temperature
> > +	  zones. This config will enable PSE compliant charging algorithm with
> > +	  maintenance charging support. At runtime the algorithm will be
> > +	  selected by the psy charger driver based on the type of the battery
> > +	  charging profile.
> 
> Information where to expect PSE compliant chargers would be nice.

This algorithm can be used with non PSE compliant chargers also. This is a SW
based charging algorithm.

-Jenny
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pavel Machek Feb. 21, 2014, 2:45 p.m. UTC | #3
On Thu 2014-02-20 10:46:55, Jenny Tc wrote:
> On Tue, Feb 04, 2014 at 12:36:40PM +0100, Pavel Machek wrote:
> > > --- a/drivers/power/Kconfig
> > > +++ b/drivers/power/Kconfig
> > > @@ -22,6 +22,19 @@ config POWER_SUPPLY_CHARGER
> > >  	  drivers to keep the charging logic outside and the charger driver
> > >  	  just need to abstract the charger hardware.
> > >  
> > > +config POWER_SUPPLY_CHARGING_ALGO_PSE
> > > +	bool "PSE compliant charging algorithm"
> > > +	help
> > > +	  Say Y here to select Product Safety Engineering (PSE) compliant
> > > +	  charging algorithm. As per PSE standard the battery characteristics
> > > +	  and thereby the charging rates can vary on different temperature
> > > +	  zones. This config will enable PSE compliant charging algorithm with
> > > +	  maintenance charging support. At runtime the algorithm will be
> > > +	  selected by the psy charger driver based on the type of the battery
> > > +	  charging profile.
> > 
> > Information where to expect PSE compliant chargers would be nice.
> 
> This algorithm can be used with non PSE compliant chargers also. This is a SW
> based charging algorithm.

Ok, but you need to explain for the users when it might be good idea
to select this option...

Or maybe this should not be user configurable and drivers should just
select it?
									Pavel
Jenny TC Feb. 26, 2014, 2:54 a.m. UTC | #4
On Fri, Feb 21, 2014 at 03:45:29PM +0100, Pavel Machek wrote:
> On Thu 2014-02-20 10:46:55, Jenny Tc wrote:
> > On Tue, Feb 04, 2014 at 12:36:40PM +0100, Pavel Machek wrote:
> > > > --- a/drivers/power/Kconfig
> > > > +++ b/drivers/power/Kconfig
> > > > @@ -22,6 +22,19 @@ config POWER_SUPPLY_CHARGER
> > > >  	  drivers to keep the charging logic outside and the charger driver
> > > >  	  just need to abstract the charger hardware.
> > > >  
> > > > +config POWER_SUPPLY_CHARGING_ALGO_PSE
> > > > +	bool "PSE compliant charging algorithm"
> > > > +	help
> > > > +	  Say Y here to select Product Safety Engineering (PSE) compliant
> > > > +	  charging algorithm. As per PSE standard the battery characteristics
> > > > +	  and thereby the charging rates can vary on different temperature
> > > > +	  zones. This config will enable PSE compliant charging algorithm with
> > > > +	  maintenance charging support. At runtime the algorithm will be
> > > > +	  selected by the psy charger driver based on the type of the battery
> > > > +	  charging profile.
> > > 
> > > Information where to expect PSE compliant chargers would be nice.
> > 
> > This algorithm can be used with non PSE compliant chargers also. This is a SW
> > based charging algorithm.
> 
> Ok, but you need to explain for the users when it might be good idea
> to select this option...
> 
> Or maybe this should not be user configurable and drivers should just
> select it?

The idea is to allow pluggable charging algorithms. Currently we have only one
charging algorithm proposed, but can have other charging algorithms (like pulse
charging, rule based charging etc.). Based on the platform need, the algorithms
can be selected. So this should be a user configurable option. I can add more
explanation on when to select this option.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij Feb. 27, 2014, 7:47 p.m. UTC | #5
On Wed, Feb 26, 2014 at 3:54 AM, Jenny Tc <jenny.tc@intel.com> wrote:

> The idea is to allow pluggable charging algorithms. Currently we have only one
> charging algorithm proposed, but can have other charging algorithms (like pulse
> charging, rule based charging etc.). Based on the platform need, the algorithms
> can be selected. So this should be a user configurable option. I can add more
> explanation on when to select this option.

Do you see a generic framework for pluggable algorithms on an abstracted
level, so that it could be used for the CC/CV charging, measurement and
temperature check algorithm that is found in e.g.
drivers/power/abx500_chargalg.c
drivers/power/ab8500_charger.c etc, or do you envision a set of pluggable
algorithms for this one hardware?

I'm asking because these drivers are a massive set of code and we may
need to start to abstract out and define library functions and frameworks
already now before it becomes impossible to contain.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij Feb. 27, 2014, 8:18 p.m. UTC | #6
On Tue, Feb 4, 2014 at 6:12 AM, Jenny TC <jenny.tc@intel.com> wrote:

> +static inline bool __is_battery_full
> +       (long volt, long cur, long iterm, unsigned long cv)

Overall I wonder if you've run checkpatch on these patches, but why
are you naming this one function with a double __underscore?
Just is_battery_full_check() or something would work fine I guess?

(...)
> +/* Parameters defining the charging range */
> +struct psy_ps_temp_chg_table {
> +       /* upper temperature limit for each zone */
> +       short int temp_up_lim; /* Degree Celsius */
> +
> +       /* charge current and voltage */
> +       short int full_chrg_vol; /* mV */
> +       short int full_chrg_cur; /* mA */
> +
> +       /*
> +       *  Maintenance charging thresholds.
> +       *  Maintenance charging voltage lower limit - Once battery hits full,
> +       *  charging will be resumed when battery voltage <= this voltage
> +       */
> +       short int maint_chrg_vol_ll; /* mV */
> +
> +       /* Charge current and voltage in maintenance charging mode */
> +       short int maint_chrg_vol_ul; /* mV */
> +       short int maint_chrg_cur;   /* mA */
> +} __packed;

Why are you packing these structs? If no real reason, remove it.
The compiler will pack what it thinks is appropriate anyway.

Convert all comments to kerneldoc.

> +#define BATTID_STR_LEN         8
> +#define BATT_TEMP_NR_RNG       6
> +
> +struct psy_pse_chrg_prof {
> +       /* battery id */
> +       char batt_id[BATTID_STR_LEN];
> +       u16 battery_type; /* Defined as POWER_SUPPLY_TECHNOLOGY_* */

Use a named enum by patching that in <linux/power_supply.h>?

> +       u16 capacity;   /* mAh */
> +       u16 voltage_max; /* mV */
> +       /* charge termination current */
> +       u16 chrg_term_mA;
> +       /* Low battery level voltage */
> +       u16 low_batt_mV;
> +       /* upper and lower temperature limits on discharging */
> +       s8 disch_temp_ul; /* Degree Celsius */
> +       s8 disch_temp_ll; /* Degree Celsius */
> +       /* number of temperature monitoring ranges */
> +       u16 temp_mon_ranges;
> +       struct psy_ps_temp_chg_table temp_mon_range[BATT_TEMP_NR_RNG];
> +       /* lowest temperature supported */
> +       s8 temp_low_lim;
> +} __packed;

Why packed, and convert to kerneldoc for this struct.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pavel Machek Feb. 27, 2014, 8:46 p.m. UTC | #7
On Wed 2014-02-26 08:24:44, Jenny Tc wrote:
> On Fri, Feb 21, 2014 at 03:45:29PM +0100, Pavel Machek wrote:
> > On Thu 2014-02-20 10:46:55, Jenny Tc wrote:
> > > On Tue, Feb 04, 2014 at 12:36:40PM +0100, Pavel Machek wrote:
> > > > > --- a/drivers/power/Kconfig
> > > > > +++ b/drivers/power/Kconfig
> > > > > @@ -22,6 +22,19 @@ config POWER_SUPPLY_CHARGER
> > > > >  	  drivers to keep the charging logic outside and the charger driver
> > > > >  	  just need to abstract the charger hardware.
> > > > >  
> > > > > +config POWER_SUPPLY_CHARGING_ALGO_PSE
> > > > > +	bool "PSE compliant charging algorithm"
> > > > > +	help
> > > > > +	  Say Y here to select Product Safety Engineering (PSE) compliant
> > > > > +	  charging algorithm. As per PSE standard the battery characteristics
> > > > > +	  and thereby the charging rates can vary on different temperature
> > > > > +	  zones. This config will enable PSE compliant charging algorithm with
> > > > > +	  maintenance charging support. At runtime the algorithm will be
> > > > > +	  selected by the psy charger driver based on the type of the battery
> > > > > +	  charging profile.
> > > > 
> > > > Information where to expect PSE compliant chargers would be nice.
> > > 
> > > This algorithm can be used with non PSE compliant chargers also. This is a SW
> > > based charging algorithm.
> > 
> > Ok, but you need to explain for the users when it might be good idea
> > to select this option...
> > 
> > Or maybe this should not be user configurable and drivers should just
> > select it?
> 
> The idea is to allow pluggable charging algorithms. Currently we have only one
> charging algorithm proposed, but can have other charging algorithms (like pulse
> charging, rule based charging etc.). Based on the platform need, the algorithms
> can be selected. So this should be a user configurable option. I can add more
> explanation on when to select this option.

Yes please. Because with current description, user has no chance.
									Pavel
Jenny TC Feb. 28, 2014, 2:52 a.m. UTC | #8
On Thu, Feb 27, 2014 at 08:47:07PM +0100, Linus Walleij wrote:
> On Wed, Feb 26, 2014 at 3:54 AM, Jenny Tc <jenny.tc@intel.com> wrote:
> 
> > The idea is to allow pluggable charging algorithms. Currently we have only one
> > charging algorithm proposed, but can have other charging algorithms (like pulse
> > charging, rule based charging etc.). Based on the platform need, the algorithms
> > can be selected. So this should be a user configurable option. I can add more
> > explanation on when to select this option.
> 
> Do you see a generic framework for pluggable algorithms on an abstracted
> level, so that it could be used for the CC/CV charging, measurement and
> temperature check algorithm that is found in e.g.
> drivers/power/abx500_chargalg.c
> drivers/power/ab8500_charger.c etc, or do you envision a set of pluggable
> algorithms for this one hardware?
> 
> I'm asking because these drivers are a massive set of code and we may
> need to start to abstract out and define library functions and frameworks
> already now before it becomes impossible to contain.

The idea of power_supply_charger driver is to move the charging logic outside of
the charger chip driver. This makes the charger chip driver as a  h/w
abstraction layer without  having any charging logic in it. power supply charger
driver invokes charging algorithm to decide the CC, CV and to stop/start
charging on different conditions (based on voltage/temperature ...). Detailed
note on using power supply charger driver can be found in
Documentation/power/power_supply_charger.txt  which is part of patch
power_supply-Introduce-generic-psy-charging-driver.patch
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jenny TC Feb. 28, 2014, 3:07 a.m. UTC | #9
On Thu, Feb 27, 2014 at 09:18:57PM +0100, Linus Walleij wrote:
> On Tue, Feb 4, 2014 at 6:12 AM, Jenny TC <jenny.tc@intel.com> wrote:
> 
> > +static inline bool __is_battery_full
> > +       (long volt, long cur, long iterm, unsigned long cv)
> 
> Overall I wonder if you've run checkpatch on these patches, but why
> are you naming this one function with a double __underscore?
> Just is_battery_full_check() or something would work fine I guess?

Just to convey that is_battery_full is a local function and not generic. You
can find similar usage in power_supply_core.c (__power_supply_changed_work)
and in other drivers. Isn't it advised to have __ prefixes?
> (...)
> > +/* Parameters defining the charging range */
> > +struct psy_ps_temp_chg_table {
> > +       /* upper temperature limit for each zone */
> > +       short int temp_up_lim; /* Degree Celsius */
> > +
> > +       /* charge current and voltage */
> > +       short int full_chrg_vol; /* mV */
> > +       short int full_chrg_cur; /* mA */
> > +
> > +       /*
> > +       *  Maintenance charging thresholds.
> > +       *  Maintenance charging voltage lower limit - Once battery hits full,
> > +       *  charging will be resumed when battery voltage <= this voltage
> > +       */
> > +       short int maint_chrg_vol_ll; /* mV */
> > +
> > +       /* Charge current and voltage in maintenance charging mode */
> > +       short int maint_chrg_vol_ul; /* mV */
> > +       short int maint_chrg_cur;   /* mA */
> > +} __packed;
> 
> Why are you packing these structs? If no real reason, remove it.
> The compiler will pack what it thinks is appropriate anyway.

The structure is part of the  battery charging profile which can be read directly
from an EEPROM or from secondary storage (emmc). So it should be packed to keep
it align with the stored format.

> > +#define BATTID_STR_LEN         8
> > +#define BATT_TEMP_NR_RNG       6
> > +
> > +struct psy_pse_chrg_prof {
> > +       /* battery id */
> > +       char batt_id[BATTID_STR_LEN];
> > +       u16 battery_type; /* Defined as POWER_SUPPLY_TECHNOLOGY_* */
> 
> Use a named enum by patching that in <linux/power_supply.h>?

This is to convey that battery_type takes value as defined by
POWER_SUPPLY_TECHNOLOGY_*
> 
> > +       u16 capacity;   /* mAh */
> > +       u16 voltage_max; /* mV */
> > +       /* charge termination current */
> > +       u16 chrg_term_mA;
> > +       /* Low battery level voltage */
> > +       u16 low_batt_mV;
> > +       /* upper and lower temperature limits on discharging */
> > +       s8 disch_temp_ul; /* Degree Celsius */
> > +       s8 disch_temp_ll; /* Degree Celsius */
> > +       /* number of temperature monitoring ranges */
> > +       u16 temp_mon_ranges;
> > +       struct psy_ps_temp_chg_table temp_mon_range[BATT_TEMP_NR_RNG];
> > +       /* lowest temperature supported */
> > +       s8 temp_low_lim;
> > +} __packed;
> 
> Why packed, and convert to kerneldoc for this struct.

Battery charging profile, may come from EEPROM/emmc which would be packed.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pavel Machek Feb. 28, 2014, 10:08 a.m. UTC | #10
On Fri 2014-02-28 08:37:27, Jenny Tc wrote:
> On Thu, Feb 27, 2014 at 09:18:57PM +0100, Linus Walleij wrote:
> > On Tue, Feb 4, 2014 at 6:12 AM, Jenny TC <jenny.tc@intel.com> wrote:
> > 
> > > +static inline bool __is_battery_full
> > > +       (long volt, long cur, long iterm, unsigned long cv)
> > 
> > Overall I wonder if you've run checkpatch on these patches, but why
> > are you naming this one function with a double __underscore?
> > Just is_battery_full_check() or something would work fine I guess?
> 
> Just to convey that is_battery_full is a local function and not generic. You
> can find similar usage in power_supply_core.c (__power_supply_changed_work)
> and in other drivers. Isn't it advised to have __ prefixes?

It is static; everybody sees it is local. __ prefix usually means
something else.

> > > +       u16 capacity;   /* mAh */
> > > +       u16 voltage_max; /* mV */
> > > +       /* charge termination current */
> > > +       u16 chrg_term_mA;
> > > +       /* Low battery level voltage */
> > > +       u16 low_batt_mV;
> > > +       /* upper and lower temperature limits on discharging */
> > > +       s8 disch_temp_ul; /* Degree Celsius */
> > > +       s8 disch_temp_ll; /* Degree Celsius */
> > > +       /* number of temperature monitoring ranges */
> > > +       u16 temp_mon_ranges;
> > > +       struct psy_ps_temp_chg_table temp_mon_range[BATT_TEMP_NR_RNG];
> > > +       /* lowest temperature supported */
> > > +       s8 temp_low_lim;
> > > +} __packed;
> > 
> > Why packed, and convert to kerneldoc for this struct.
> 
> Battery charging profile, may come from EEPROM/emmc which would be packed.

Do you need to do endianness conversion, too?
									Pavel
Jenny TC March 3, 2014, 3:11 a.m. UTC | #11
On Fri, Feb 28, 2014 at 11:08:16AM +0100, Pavel Machek wrote:
> On Fri 2014-02-28 08:37:27, Jenny Tc wrote:
> > On Thu, Feb 27, 2014 at 09:18:57PM +0100, Linus Walleij wrote:
> > > On Tue, Feb 4, 2014 at 6:12 AM, Jenny TC <jenny.tc@intel.com> wrote:
> > > 
> > > > +static inline bool __is_battery_full
> > > > +       (long volt, long cur, long iterm, unsigned long cv)
> > > 
> > > Overall I wonder if you've run checkpatch on these patches, but why
> > > are you naming this one function with a double __underscore?
> > > Just is_battery_full_check() or something would work fine I guess?
> > 
> > Just to convey that is_battery_full is a local function and not generic. You
> > can find similar usage in power_supply_core.c (__power_supply_changed_work)
> > and in other drivers. Isn't it advised to have __ prefixes?
> 
> It is static; everybody sees it is local. __ prefix usually means
> something else.

Agreed, I will remove the __ prefix in next patchset. Meanwhile I would
appreciate if anyone could help me to understand what __ prefix really means.

> > > > +       u16 capacity;   /* mAh */
> > > > +       u16 voltage_max; /* mV */
> > > > +       /* charge termination current */
> > > > +       u16 chrg_term_mA;
> > > > +       /* Low battery level voltage */
> > > > +       u16 low_batt_mV;
> > > > +       /* upper and lower temperature limits on discharging */
> > > > +       s8 disch_temp_ul; /* Degree Celsius */
> > > > +       s8 disch_temp_ll; /* Degree Celsius */
> > > > +       /* number of temperature monitoring ranges */
> > > > +       u16 temp_mon_ranges;
> > > > +       struct psy_ps_temp_chg_table temp_mon_range[BATT_TEMP_NR_RNG];
> > > > +       /* lowest temperature supported */
> > > > +       s8 temp_low_lim;
> > > > +} __packed;
> > > 
> > > Why packed, and convert to kerneldoc for this struct.
> > 
> > Battery charging profile, may come from EEPROM/emmc which would be packed.
> 
> Do you need to do endianness conversion, too?

May/may not depending on the stored format. If needed, the endianess conversion
should be done at driver level where the EEPROM/emmc reading happens.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pavel Machek March 3, 2014, 10:42 a.m. UTC | #12
Hi!

> > > Just to convey that is_battery_full is a local function and not generic. You
> > > can find similar usage in power_supply_core.c (__power_supply_changed_work)
> > > and in other drivers. Isn't it advised to have __ prefixes?
> > 
> > It is static; everybody sees it is local. __ prefix usually means
> > something else.
> 
> Agreed, I will remove the __ prefix in next patchset. Meanwhile I would
> appreciate if anyone could help me to understand what __ prefix
> really means.

If you have __foo() and foo(), the __foo() is typically worker
function, where foo() provides locking around it etc. 
									Pavel
Linus Walleij March 7, 2014, 3:34 a.m. UTC | #13
On Fri, Feb 28, 2014 at 11:07 AM, Jenny Tc <jenny.tc@intel.com> wrote:
> On Thu, Feb 27, 2014 at 09:18:57PM +0100, Linus Walleij wrote:
>> On Tue, Feb 4, 2014 at 6:12 AM, Jenny TC <jenny.tc@intel.com> wrote:
>>
>> > +static inline bool __is_battery_full
>> > +       (long volt, long cur, long iterm, unsigned long cv)
>>
>> Overall I wonder if you've run checkpatch on these patches, but why
>> are you naming this one function with a double __underscore?
>> Just is_battery_full_check() or something would work fine I guess?
>
> Just to convey that is_battery_full is a local function and not generic. You
> can find similar usage in power_supply_core.c (__power_supply_changed_work)
> and in other drivers. Isn't it advised to have __ prefixes?

The preference is different, usually __ is for compiler things, but
while I dislike it (disturbs my perception) I can sure live with it.

>> Why are you packing these structs? If no real reason, remove it.
>> The compiler will pack what it thinks is appropriate anyway.
>
> The structure is part of the  battery charging profile which can be read directly
> from an EEPROM or from secondary storage (emmc). So it should be packed to keep
> it align with the stored format.

OK I buy that. Make sure this is noted somewhere (or maybe I missed it).

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jenny TC March 7, 2014, 3:43 a.m. UTC | #14
On Fri, Mar 07, 2014 at 11:34:14AM +0800, Linus Walleij wrote:
> On Fri, Feb 28, 2014 at 11:07 AM, Jenny Tc <jenny.tc@intel.com> wrote:
> > On Thu, Feb 27, 2014 at 09:18:57PM +0100, Linus Walleij wrote:
> >> On Tue, Feb 4, 2014 at 6:12 AM, Jenny TC <jenny.tc@intel.com> wrote:
> >>
> >> > +static inline bool __is_battery_full
> >> > +       (long volt, long cur, long iterm, unsigned long cv)
> >>
> >> Overall I wonder if you've run checkpatch on these patches, but why
> >> are you naming this one function with a double __underscore?
> >> Just is_battery_full_check() or something would work fine I guess?
> >
> > Just to convey that is_battery_full is a local function and not generic. You
> > can find similar usage in power_supply_core.c (__power_supply_changed_work)
> > and in other drivers. Isn't it advised to have __ prefixes?
> 
> The preference is different, usually __ is for compiler things, but
> while I dislike it (disturbs my perception) I can sure live with it.

Fixed in patch set v7
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index f679f82..913ec36 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -22,6 +22,19 @@  config POWER_SUPPLY_CHARGER
 	  drivers to keep the charging logic outside and the charger driver
 	  just need to abstract the charger hardware.
 
+config POWER_SUPPLY_CHARGING_ALGO_PSE
+	bool "PSE compliant charging algorithm"
+	help
+	  Say Y here to select Product Safety Engineering (PSE) compliant
+	  charging algorithm. As per PSE standard the battery characteristics
+	  and thereby the charging rates can vary on different temperature
+	  zones. This config will enable PSE compliant charging algorithm with
+	  maintenance charging support. At runtime the algorithm will be
+	  selected by the psy charger driver based on the type of the battery
+	  charging profile.
+
+	depends on POWER_SUPPLY_CHARGER
+
 config PDA_POWER
 	tristate "Generic PDA/phone power driver"
 	depends on !S390
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index 405f0f4..77535fd 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -8,6 +8,7 @@  obj-$(CONFIG_POWER_SUPPLY)	+= power_supply.o
 obj-$(CONFIG_GENERIC_ADC_BATTERY)	+= generic-adc-battery.o
 
 obj-$(CONFIG_POWER_SUPPLY_CHARGER) += power_supply_charger.o
+obj-$(CONFIG_POWER_SUPPLY_CHARGING_ALGO_PSE) += charging_algo_pse.o
 obj-$(CONFIG_PDA_POWER)		+= pda_power.o
 obj-$(CONFIG_APM_POWER)		+= apm_power.o
 obj-$(CONFIG_MAX8925_POWER)	+= max8925_power.o
diff --git a/drivers/power/charging_algo_pse.c b/drivers/power/charging_algo_pse.c
new file mode 100644
index 0000000..0a0130a
--- /dev/null
+++ b/drivers/power/charging_algo_pse.c
@@ -0,0 +1,198 @@ 
+/*
+ * Copyright (C) 2012 Intel Corporation
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * 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; version 2 of the License.
+ *
+ * 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.
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ * Author: Jenny TC <jenny.tc@intel.com>
+ */
+
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/power_supply.h>
+#include <linux/thermal.h>
+#include "power_supply.h"
+#include "power_supply_charger.h"
+
+/* 98% of CV is considered as voltage to detect Full */
+#define FULL_CV_MIN 98
+
+/* Offset to exit from maintenance charging. In maintenance charging
+*  if the volatge is less than the (maintenance_lower_threshold -
+*  MAINT_EXIT_OFFSET) then system can switch to normal charging
+*/
+#define MAINT_EXIT_OFFSET 50  /* mV */
+
+static int get_tempzone(struct psy_pse_chrg_prof *pse_mod_bprof,
+		int temp)
+{
+
+	int i = 0;
+	int temp_range_cnt = min_t(u16, pse_mod_bprof->temp_mon_ranges,
+					BATT_TEMP_NR_RNG);
+
+	if ((temp < pse_mod_bprof->temp_low_lim) ||
+		(temp > pse_mod_bprof->temp_mon_range[0].temp_up_lim))
+		return -EINVAL;
+
+	for (i = 0; i < temp_range_cnt; ++i)
+		if (temp > pse_mod_bprof->temp_mon_range[i].temp_up_lim)
+			break;
+	return i-1;
+}
+
+static inline bool __is_battery_full
+	(long volt, long cur, long iterm, unsigned long cv)
+{
+	pr_devel("%s:current=%ld pse_mod_bprof->chrg_term_mA =%ld voltage_now=%ld full_cond=%ld",
+			__func__, cur, iterm, volt * 100, (FULL_CV_MIN * cv));
+
+	return (cur > 0) && (cur <= iterm) &&
+	((volt * 100)  >= (FULL_CV_MIN * cv));
+
+}
+
+static inline bool is_battery_full(struct psy_batt_props bat_prop,
+		struct psy_pse_chrg_prof *pse_mod_bprof, unsigned long cv)
+{
+
+	int i;
+	/* Software full detection. Check the battery charge current to detect
+	*  battery Full. The voltage also verified to avoid false charge
+	*  full detection.
+	*/
+	pr_devel("%s:current=%ld pse_mod_bprof->chrg_term_mA =%d bat_prop.voltage_now=%ld full_cond=%ld",
+		__func__, bat_prop.current_now, (pse_mod_bprof->chrg_term_mA),
+		bat_prop.voltage_now * 100, (FULL_CV_MIN * cv));
+
+	for (i = (MAX_CUR_VOLT_SAMPLES - 1); i >= 0; --i) {
+
+		if (!(__is_battery_full(bat_prop.voltage_now_cache[i],
+				bat_prop.current_now_cache[i],
+				pse_mod_bprof->chrg_term_mA, cv)))
+			return false;
+	}
+
+	return true;
+}
+
+static int  pse_get_bat_thresholds(struct psy_batt_chrg_prof  bprof,
+			struct psy_batt_thresholds *bat_thresh)
+{
+	struct psy_pse_chrg_prof *pse_mod_bprof =
+			(struct psy_pse_chrg_prof *) bprof.batt_prof;
+
+	if ((bprof.chrg_prof_type != PSY_CHRG_PROF_PSE) || (!pse_mod_bprof))
+		return -EINVAL;
+
+	bat_thresh->iterm = pse_mod_bprof->chrg_term_mA;
+	bat_thresh->temp_min = pse_mod_bprof->temp_low_lim;
+	bat_thresh->temp_max = pse_mod_bprof->temp_mon_range[0].temp_up_lim;
+
+	return 0;
+}
+
+static enum psy_algo_stat pse_get_next_cc_cv(struct psy_batt_props bat_prop,
+	struct psy_batt_chrg_prof  bprof,
+			unsigned long *cc, unsigned long *cv)
+{
+	int tzone;
+	struct psy_pse_chrg_prof *pse_mod_bprof =
+			(struct psy_pse_chrg_prof *) bprof.batt_prof;
+	enum psy_algo_stat algo_stat = bat_prop.algo_stat;
+	int maint_exit_volt;
+
+	*cc = *cv = 0;
+
+	/* If STATUS is discharging, assume that charger is not connected.
+	*  If charger is not connected, no need to take any action.
+	*  If charge profile type is not PSY_CHRG_PROF_PSE or the charge profile
+	*  is not present, no need to take any action.
+	*/
+
+	pr_devel("%s:battery status = %ld algo_status=%d\n",
+			__func__, bat_prop.status, algo_stat);
+
+	if ((bprof.chrg_prof_type != PSY_CHRG_PROF_PSE) || (!pse_mod_bprof))
+		return PSY_ALGO_STAT_NOT_CHARGE;
+
+	tzone = get_tempzone(pse_mod_bprof, bat_prop.temperature);
+
+	if (tzone < 0)
+		return PSY_ALGO_STAT_NOT_CHARGE;
+
+	/* Change the algo status to not charging, if battery is
+	*  not really charging or less than maintenance exit threshold.
+	*  This way algorithm can switch to normal
+	*  charging if current status is full/maintenace
+	*/
+	maint_exit_volt = pse_mod_bprof->
+			temp_mon_range[tzone].maint_chrg_vol_ll -
+				MAINT_EXIT_OFFSET;
+
+	if ((bat_prop.status == POWER_SUPPLY_STATUS_DISCHARGING) ||
+		(bat_prop.status == POWER_SUPPLY_STATUS_NOT_CHARGING) ||
+			bat_prop.voltage_now < maint_exit_volt) {
+
+		algo_stat = PSY_ALGO_STAT_NOT_CHARGE;
+
+	}
+
+	/* read cc and cv based on temperature and algorithm status*/
+	if (algo_stat == PSY_ALGO_STAT_FULL ||
+			algo_stat == PSY_ALGO_STAT_MAINT) {
+
+		/* if status is full and voltage is lower than maintenance lower
+		*  threshold change status to maintenenance
+		*/
+
+		if (algo_stat == PSY_ALGO_STAT_FULL && (bat_prop.voltage_now <=
+			pse_mod_bprof->temp_mon_range[tzone].maint_chrg_vol_ll))
+				algo_stat = PSY_ALGO_STAT_MAINT;
+
+		/* Read maintenance CC and CV */
+		if (algo_stat == PSY_ALGO_STAT_MAINT) {
+			*cv = pse_mod_bprof->temp_mon_range
+					[tzone].maint_chrg_vol_ul;
+			*cc = pse_mod_bprof->temp_mon_range
+					[tzone].maint_chrg_cur;
+		}
+	} else {
+		*cv = pse_mod_bprof->temp_mon_range[tzone].full_chrg_vol;
+		*cc = pse_mod_bprof->temp_mon_range[tzone].full_chrg_cur;
+		algo_stat = PSY_ALGO_STAT_CHARGE;
+	}
+
+	if (is_battery_full(bat_prop, pse_mod_bprof, *cv)) {
+		*cc = *cv = 0;
+		algo_stat = PSY_ALGO_STAT_FULL;
+	}
+
+	return algo_stat;
+}
+
+static int __init pse_algo_init(void)
+{
+	struct psy_charging_algo pse_algo;
+	pse_algo.chrg_prof_type = PSY_CHRG_PROF_PSE;
+	pse_algo.name = "pse_algo";
+	pse_algo.get_next_cc_cv = pse_get_next_cc_cv;
+	pse_algo.get_batt_thresholds = pse_get_bat_thresholds;
+	power_supply_register_charging_algo(&pse_algo);
+	return 0;
+}
+
+module_init(pse_algo_init);
diff --git a/include/linux/power/power_supply_charger.h b/include/linux/power/power_supply_charger.h
index e96bb3a..1b46b53 100644
--- a/include/linux/power/power_supply_charger.h
+++ b/include/linux/power/power_supply_charger.h
@@ -79,8 +79,56 @@  enum {
 
 enum psy_batt_chrg_prof_type {
 	PSY_CHRG_PROF_NONE = 0,
+	PSY_CHRG_PROF_PSE,
 };
 
+/* Product Safety Engineering (PSE) compliant charging profile */
+
+/* Parameters defining the charging range */
+struct psy_ps_temp_chg_table {
+	/* upper temperature limit for each zone */
+	short int temp_up_lim; /* Degree Celsius */
+
+	/* charge current and voltage */
+	short int full_chrg_vol; /* mV */
+	short int full_chrg_cur; /* mA */
+
+	/*
+	*  Maintenance charging thresholds.
+	*  Maintenance charging voltage lower limit - Once battery hits full,
+	*  charging will be resumed when battery voltage <= this voltage
+	*/
+	short int maint_chrg_vol_ll; /* mV */
+
+	/* Charge current and voltage in maintenance charging mode */
+	short int maint_chrg_vol_ul; /* mV */
+	short int maint_chrg_cur;   /* mA */
+} __packed;
+
+
+#define BATTID_STR_LEN		8
+#define BATT_TEMP_NR_RNG	6
+
+struct psy_pse_chrg_prof {
+	/* battery id */
+	char batt_id[BATTID_STR_LEN];
+	u16 battery_type; /* Defined as POWER_SUPPLY_TECHNOLOGY_* */
+	u16 capacity;	/* mAh */
+	u16 voltage_max; /* mV */
+	/* charge termination current */
+	u16 chrg_term_mA;
+	/* Low battery level voltage */
+	u16 low_batt_mV;
+	/* upper and lower temperature limits on discharging */
+	s8 disch_temp_ul; /* Degree Celsius */
+	s8 disch_temp_ll; /* Degree Celsius */
+	/* number of temperature monitoring ranges */
+	u16 temp_mon_ranges;
+	struct psy_ps_temp_chg_table temp_mon_range[BATT_TEMP_NR_RNG];
+	/* lowest temperature supported */
+	s8 temp_low_lim;
+} __packed;
+
 /* charging profile structure definition */
 struct psy_batt_chrg_prof {
 	enum psy_batt_chrg_prof_type chrg_prof_type;