diff mbox

[2/3] power: core: Add variables related temperature to power_supply_info.

Message ID 1412679518-21499-3-git-send-email-jonghwa3.lee@samsung.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Jonghwa Lee Oct. 7, 2014, 10:58 a.m. UTC
To represent thermal limitation of power_supply, it adds 'temperature_max,
temperature_min' to power_supply_info structure.

Signed-off-by: Jonghwa Lee <jonghwa3.lee@samsung.com>
---
 include/linux/power_supply.h |    2 ++
 1 file changed, 2 insertions(+)

Comments

Jenny TC Nov. 10, 2014, 11:16 a.m. UTC | #1
> @@ -241,6 +241,8 @@ struct power_supply_info {
>  	int charge_empty_design;
>  	int energy_full_design;
>  	int energy_empty_design;
> +	int temperature_max;
> +	int temperature_min;
>  	int use_for_apm;
>  };


The CC,CV and restart threshold would vary based on the battery temperature
So I would suggest to have temperature zone table as part  of battery info
along with other attributes.

int iterm; //charge termination current (used to stop charging)
int temp_zone_count; // number of temperature zone tables present
struct batt_temp_mon_table temp_mon_tbl[MAX_TEMP_MON_TABLE]; //temperature zone table array

struct  batt_temp_mon_table {
	 short int temp_max;
	 short int cc;
	 short int cv;
	 short int vbat_vchk_drop_uv;
	 short int temp_min;
};

-Jenny

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jonghwa Lee Nov. 11, 2014, 12:30 a.m. UTC | #2
On 2014? 11? 10? 20:16, Tc, Jenny wrote:

>> @@ -241,6 +241,8 @@ struct power_supply_info {
>>  	int charge_empty_design;
>>  	int energy_full_design;
>>  	int energy_empty_design;
>> +	int temperature_max;
>> +	int temperature_min;
>>  	int use_for_apm;
>>  };
> 
> 
> The CC,CV and restart threshold would vary based on the battery temperature
> So I would suggest to have temperature zone table as part  of battery info
> along with other attributes.
> 
> int iterm; //charge termination current (used to stop charging)
> int temp_zone_count; // number of temperature zone tables present
> struct batt_temp_mon_table temp_mon_tbl[MAX_TEMP_MON_TABLE]; //temperature zone table array
> 
> struct  batt_temp_mon_table {
> 	 short int temp_max;
> 	 short int cc;
> 	 short int cv;
> 	 short int vbat_vchk_drop_uv;
> 	 short int temp_min;
> };
> 


IMO, throttling cc/cv according the temperature can be done via thermal fw
interface. However voltage drop and charging termination current can be added here.


Jonghwa

> -Jenny
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jenny TC Nov. 11, 2014, 4:56 a.m. UTC | #3
PiA+IFRoZSBDQyxDViBhbmQgcmVzdGFydCB0aHJlc2hvbGQgd291bGQgdmFyeSBiYXNlZCBvbiB0
aGUgYmF0dGVyeSB0ZW1wZXJhdHVyZQ0KPiA+IFNvIEkgd291bGQgc3VnZ2VzdCB0byBoYXZlIHRl
bXBlcmF0dXJlIHpvbmUgdGFibGUgYXMgcGFydCAgb2YgYmF0dGVyeSBpbmZvDQo+ID4gYWxvbmcg
d2l0aCBvdGhlciBhdHRyaWJ1dGVzLg0KPiA+DQo+ID4gaW50IGl0ZXJtOyAvL2NoYXJnZSB0ZXJt
aW5hdGlvbiBjdXJyZW50ICh1c2VkIHRvIHN0b3AgY2hhcmdpbmcpDQo+ID4gaW50IHRlbXBfem9u
ZV9jb3VudDsgLy8gbnVtYmVyIG9mIHRlbXBlcmF0dXJlIHpvbmUgdGFibGVzIHByZXNlbnQNCj4g
PiBzdHJ1Y3QgYmF0dF90ZW1wX21vbl90YWJsZSB0ZW1wX21vbl90YmxbTUFYX1RFTVBfTU9OX1RB
QkxFXTsNCj4gLy90ZW1wZXJhdHVyZSB6b25lIHRhYmxlIGFycmF5DQo+ID4NCj4gPiBzdHJ1Y3Qg
IGJhdHRfdGVtcF9tb25fdGFibGUgew0KPiA+IAkgc2hvcnQgaW50IHRlbXBfbWF4Ow0KPiA+IAkg
c2hvcnQgaW50IGNjOw0KPiA+IAkgc2hvcnQgaW50IGN2Ow0KPiA+IAkgc2hvcnQgaW50IHZiYXRf
dmNoa19kcm9wX3V2Ow0KPiA+IAkgc2hvcnQgaW50IHRlbXBfbWluOw0KPiA+IH07DQo+ID4NCj4g
DQo+IA0KPiBJTU8sIHRocm90dGxpbmcgY2MvY3YgYWNjb3JkaW5nIHRoZSB0ZW1wZXJhdHVyZSBj
YW4gYmUgZG9uZSB2aWEgdGhlcm1hbCBmdw0KPiBpbnRlcmZhY2UuIEhvd2V2ZXIgdm9sdGFnZSBk
cm9wIGFuZCBjaGFyZ2luZyB0ZXJtaW5hdGlvbiBjdXJyZW50IGNhbiBiZSBhZGRlZCBoZXJlLg0K
DQpUaGUgQ0MvQ1YgZm9yIGVhY2ggYmF0dGVyeSB0ZW1wZXJhdHVyZSB6b25lIGlzIGRlZmluZWQg
YXMgcGFydCBvZiBiYXR0ZXJ5IHNwZWMuIFRoaXMgaXMNCmFzIHBlciB0aGUgSkVJVEEvUFNFIHN0
YW5kYXJkcy4gU28gSU1PLCB0aGlzIGlzIGEgYmF0dGVyeSBjaGFyZ2luZyBpbmZvcm1hdGlvbg0K
KGNoYXJnaW5nIG9iamVjdCkgcmF0aGVyIHRoYW4gYSB0aGVybWFsIHRocm90dGxpbmcgaW5mb3Jt
YXRpb24uDQoNCkFsc28gdGhlIGJhdHRlcnkgaW5mb3JtYXRpb24gbWF5IG5vdCBmaXQgaW50byBh
IHN0YW5kYXJkIGZvcm1hdC4gRGlmZmVyZW50IHN0YW5kYXJkcyBoYXZlDQpkaWZmZXJlbnQgZm9y
bWF0IGZvciBjaGFyZ2luZyBvYmplY3QuIFNvIEkgd291bGQgc3VnZ2VzdCB0byBtYWtlIGl0IGZs
ZXhpYmxlIGVub3VnaCB0bw0Kc3VwcG9ydCBkaWZmZXJlbnQgY2hhcmdpbmcgb2JqZWN0IGZvcm1h
dC4gRm9yIGV4YW1wbGUgTUlQSSBCSUYgY2hhcmdpbmcgb2JqZWN0IGZvcm1hdA0KKGh0dHBzOi8v
bWVtYmVycy5taXBpLm9yZy93Zy9CSUYvZG9jdW1lbnQvMTE1MTgpIGFuZCBNSVBJIEJJRiBSdWxl
IGJhc2VkIGNoYXJnaW5nIGFsZ29yaXRobQ0KKGh0dHA6Ly9taXBpLm9yZy9zaXRlcy9kZWZhdWx0
L2ZpbGVzL21pcGlfQklGX3J1bGUtYmFzZWQtY2hhcmdpbmdfd2hpdGUtcGFwZXJfMS5wZGYpDQpo
YXMgZGlmZmVyZW50IGNoYXJnaW5nIG9iamVjdCBmb3JtYXQuIFRoaXMgaXMgd2h5IHRoZSBwYXRj
aCAgaHR0cHM6Ly9sa21sLm9yZy9sa21sLzIwMTQvOC8xMy8zNTUNCmhhcyBvcHRpb24gdG8gc3Vw
cG9ydCBkaWZmZXJlbnQgY2hhcmdpbmcgb2JqZWN0cyBhbmQgZGlmZmVyZW50IGNoYXJnaW5nIGFs
Z29yaXRobXMuDQoNCi1KZW5ueQ0K
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pavel Machek Nov. 11, 2014, 8:42 p.m. UTC | #4
On Tue 2014-11-11 04:56:31, Tc, Jenny wrote:
> > > The CC,CV and restart threshold would vary based on the battery temperature
> > > So I would suggest to have temperature zone table as part  of battery info
> > > along with other attributes.
> > >
> > > int iterm; //charge termination current (used to stop charging)
> > > int temp_zone_count; // number of temperature zone tables present
> > > struct batt_temp_mon_table temp_mon_tbl[MAX_TEMP_MON_TABLE];
> > //temperature zone table array
> > >
> > > struct  batt_temp_mon_table {
> > > 	 short int temp_max;
> > > 	 short int cc;
> > > 	 short int cv;
> > > 	 short int vbat_vchk_drop_uv;
> > > 	 short int temp_min;
> > > };
> > >
> > 
> > 
> > IMO, throttling cc/cv according the temperature can be done via thermal fw
> > interface. However voltage drop and charging termination current can be added here.
> 
> The CC/CV for each battery temperature zone is defined as part of battery spec. This is
> as per the JEITA/PSE standards. So IMO, this is a battery charging information
> (charging object) rather than a thermal throttling information.
> 
> Also the battery information may not fit into a standard format. Different standards have
> different format for charging object. So I would suggest to make it flexible enough to
> support different charging object format. For example MIPI BIF charging object format
> (https://members.mipi.org/wg/BIF/document/11518) and MIPI BIF Rule based charging algorithm
> (http://mipi.org/sites/default/files/mipi_BIF_rule-based-charging_white-paper_1.pdf)
> has different charging object format. This is why the patch  https://lkml.org/lkml/2014/8/13/355
> has option to support different charging objects and different charging algorithms.

Yes, and this is also why your patches are not being
merged. Overengineered, too complex. Citing standards will not improve
the patches.

And yes, adding cc/cv to the thermal interface seems like a good idea
to me.

									Pavel
Jenny TC Nov. 12, 2014, 3:45 a.m. UTC | #5
> > The CC/CV for each battery temperature zone is defined as part of battery spec.
> This is
> > as per the JEITA/PSE standards. So IMO, this is a battery charging information
> > (charging object) rather than a thermal throttling information.
> >
> > Also the battery information may not fit into a standard format. Different standards
> have
> > different format for charging object. So I would suggest to make it flexible enough to
> > support different charging object format. For example MIPI BIF charging object
> format
> > (https://members.mipi.org/wg/BIF/document/11518) and MIPI BIF Rule based
> charging algorithm
> > (http://mipi.org/sites/default/files/mipi_BIF_rule-based-charging_white-paper_1.pdf)
> > has different charging object format. This is why the patch
> https://lkml.org/lkml/2014/8/13/355
> > has option to support different charging objects and different charging algorithms.
> 
> Yes, and this is also why your patches are not being
> merged. Overengineered, too complex. Citing standards will not improve
> the patches.
> 
> And yes, adding cc/cv to the thermal interface seems like a good idea
> to me.

Sorry to disagree with you. IMHO it's a charging profile and not a thermal profile. The
cc/cv information is defined as part of battery spec. If the intention here is to provide a
place for battery info, then cc/cv should be part of battery info. The latest charger chips
allows to configure CC/CV for different temperature zone. IMHO adding these information
to thermal profile doesn't seems to be the right approach since the thermal subsystem
need to be aware of the charging subsystem constraints.

The standards were cited to point where the industry is moving. Anyway let the maintainer
take a final call -  should we align with industry standards or stick to legacy charging
methodologies?

-Jenny
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" 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/include/linux/power_supply.h b/include/linux/power_supply.h
index 99306aa..0c8588c 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -241,6 +241,8 @@  struct power_supply_info {
 	int charge_empty_design;
 	int energy_full_design;
 	int energy_empty_design;
+	int temperature_max;
+	int temperature_min;
 	int use_for_apm;
 };