| Message ID | 20200623082016.23188-1-crag_wang@dell.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [1/1] power_supply: wilco_ec: Add permanent long life charging mode | expand |
> -----Original Message----- > From: Crag Wang <crag0715@gmail.com> > Sent: Tuesday, June 23, 2020 3:20 AM > To: Sebastian Reichel > Cc: mathewk@chromium.org; Limonciello, Mario; Wang, Crag; linux- > pm@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: [PATCH 1/1] power_supply: wilco_ec: Add permanent long life > charging mode > > > [EXTERNAL EMAIL] > > This is a long life mode for extended warranty battery, switching > to a differnt mode selection is unavailable. The power charging rate > is customized so that battery at work last longer. > > Signed-off-by: Crag Wang <crag_wang@dell.com> > --- > Documentation/ABI/testing/sysfs-class-power-wilco | 3 +++ > drivers/power/supply/power_supply_sysfs.c | 1 + > drivers/power/supply/wilco-charger.c | 5 +++++ > include/linux/power_supply.h | 1 + > 4 files changed, 10 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-class-power-wilco > b/Documentation/ABI/testing/sysfs-class-power-wilco > index da1d6ffe5e3c..1c91b17b6fd4 100644 > --- a/Documentation/ABI/testing/sysfs-class-power-wilco > +++ b/Documentation/ABI/testing/sysfs-class-power-wilco > @@ -14,6 +14,9 @@ Description: > Charging begins when level drops below > charge_control_start_threshold, and ceases when > level is above charge_control_end_threshold. > + Permanent Long Life: Last longer battery life, this mode > + is programmed once in the factory. Switching to a > + different mode is unavailable. > > What: /sys/class/power_supply/wilco- > charger/charge_control_start_threshold > Date: April 2019 > diff --git a/drivers/power/supply/power_supply_sysfs.c > b/drivers/power/supply/power_supply_sysfs.c > index bc79560229b5..af3884015ad8 100644 > --- a/drivers/power/supply/power_supply_sysfs.c > +++ b/drivers/power/supply/power_supply_sysfs.c > @@ -87,6 +87,7 @@ static const char * const POWER_SUPPLY_CHARGE_TYPE_TEXT[] > = { > [POWER_SUPPLY_CHARGE_TYPE_STANDARD] = "Standard", > [POWER_SUPPLY_CHARGE_TYPE_ADAPTIVE] = "Adaptive", > [POWER_SUPPLY_CHARGE_TYPE_CUSTOM] = "Custom", > + [POWER_SUPPLY_CHARGE_TYPE_LONGLIFE] = "Permanent Long Life", > }; > > static const char * const POWER_SUPPLY_HEALTH_TEXT[] = { > diff --git a/drivers/power/supply/wilco-charger.c > b/drivers/power/supply/wilco-charger.c > index b3c6d7cdd731..713c3018652f 100644 > --- a/drivers/power/supply/wilco-charger.c > +++ b/drivers/power/supply/wilco-charger.c > @@ -27,6 +27,7 @@ enum charge_mode { > CHARGE_MODE_AC = 3, /* Mostly AC use, used for Trickle */ > CHARGE_MODE_AUTO = 4, /* Used for Adaptive */ > CHARGE_MODE_CUSTOM = 5, /* Used for Custom */ > + CHARGE_MODE_LONGLIFE = 6, /* Used for Permanent Long Life */ Since this is normally only done in the factory context, can you please confirm does something need to be artificially done to block userland from trying to set the battery charging to this mode? Or will the EC already handle blocking it directly? > }; > > #define CHARGE_LOWER_LIMIT_MIN 50 > @@ -48,6 +49,8 @@ static int psp_val_to_charge_mode(int psp_val) > return CHARGE_MODE_AUTO; > case POWER_SUPPLY_CHARGE_TYPE_CUSTOM: > return CHARGE_MODE_CUSTOM; > + case POWER_SUPPLY_CHARGE_TYPE_LONGLIFE: > + return CHARGE_MODE_LONGLIFE; > default: > return -EINVAL; > } > @@ -67,6 +70,8 @@ static int charge_mode_to_psp_val(enum charge_mode mode) > return POWER_SUPPLY_CHARGE_TYPE_ADAPTIVE; > case CHARGE_MODE_CUSTOM: > return POWER_SUPPLY_CHARGE_TYPE_CUSTOM; > + case CHARGE_MODE_LONGLIFE: > + return POWER_SUPPLY_CHARGE_TYPE_LONGLIFE; > default: > return -EINVAL; > } > diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h > index ac1345a48ad0..528a3eaa2320 100644 > --- a/include/linux/power_supply.h > +++ b/include/linux/power_supply.h > @@ -48,6 +48,7 @@ enum { > POWER_SUPPLY_CHARGE_TYPE_STANDARD, /* normal speed */ > POWER_SUPPLY_CHARGE_TYPE_ADAPTIVE, /* dynamically adjusted speed */ > POWER_SUPPLY_CHARGE_TYPE_CUSTOM, /* use CHARGE_CONTROL_* props */ > + POWER_SUPPLY_CHARGE_TYPE_LONGLIFE, /* slow speed, longer life */ > }; > > enum { > -- > 2.20.1
> -----Original Message----- > From: Limonciello, Mario <Mario_Limonciello@Dell.com> > Sent: Tuesday, June 23, 2020 9:48 PM > To: Crag Wang; Sebastian Reichel > Cc: mathewk@chromium.org; Wang, Crag; linux-pm@vger.kernel.org; linux- > kernel@vger.kernel.org > Subject: RE: [PATCH 1/1] power_supply: wilco_ec: Add permanent long life > charging mode > > > -----Original Message----- > > From: Crag Wang <crag0715@gmail.com> > > Sent: Tuesday, June 23, 2020 3:20 AM > > To: Sebastian Reichel > > Cc: mathewk@chromium.org; Limonciello, Mario; Wang, Crag; linux- > > pm@vger.kernel.org; linux-kernel@vger.kernel.org > > Subject: [PATCH 1/1] power_supply: wilco_ec: Add permanent long life > > charging mode > > > > > > [EXTERNAL EMAIL] > > > > This is a long life mode for extended warranty battery, switching to a > > differnt mode selection is unavailable. The power charging rate is > > customized so that battery at work last longer. > > > > Signed-off-by: Crag Wang <crag_wang@dell.com> > > --- > > Documentation/ABI/testing/sysfs-class-power-wilco | 3 +++ > > drivers/power/supply/power_supply_sysfs.c | 1 + > > drivers/power/supply/wilco-charger.c | 5 +++++ > > include/linux/power_supply.h | 1 + > > 4 files changed, 10 insertions(+) > > > > diff --git a/Documentation/ABI/testing/sysfs-class-power-wilco > > b/Documentation/ABI/testing/sysfs-class-power-wilco > > index da1d6ffe5e3c..1c91b17b6fd4 100644 > > --- a/Documentation/ABI/testing/sysfs-class-power-wilco > > +++ b/Documentation/ABI/testing/sysfs-class-power-wilco > > @@ -14,6 +14,9 @@ Description: > > Charging begins when level drops below > > charge_control_start_threshold, and ceases when > > level is above charge_control_end_threshold. > > + Permanent Long Life: Last longer battery life, this mode > > + is programmed once in the factory. Switching to a > > + different mode is unavailable. > > > > What: /sys/class/power_supply/wilco- > > charger/charge_control_start_threshold > > Date: April 2019 > > diff --git a/drivers/power/supply/power_supply_sysfs.c > > b/drivers/power/supply/power_supply_sysfs.c > > index bc79560229b5..af3884015ad8 100644 > > --- a/drivers/power/supply/power_supply_sysfs.c > > +++ b/drivers/power/supply/power_supply_sysfs.c > > @@ -87,6 +87,7 @@ static const char * const > > POWER_SUPPLY_CHARGE_TYPE_TEXT[] = { > > [POWER_SUPPLY_CHARGE_TYPE_STANDARD] = "Standard", > > [POWER_SUPPLY_CHARGE_TYPE_ADAPTIVE] = "Adaptive", > > [POWER_SUPPLY_CHARGE_TYPE_CUSTOM] = "Custom", > > + [POWER_SUPPLY_CHARGE_TYPE_LONGLIFE] = "Permanent Long > Life", > > }; > > > > static const char * const POWER_SUPPLY_HEALTH_TEXT[] = { diff --git > > a/drivers/power/supply/wilco-charger.c > > b/drivers/power/supply/wilco-charger.c > > index b3c6d7cdd731..713c3018652f 100644 > > --- a/drivers/power/supply/wilco-charger.c > > +++ b/drivers/power/supply/wilco-charger.c > > @@ -27,6 +27,7 @@ enum charge_mode { > > CHARGE_MODE_AC = 3, /* Mostly AC use, used for Trickle */ > > CHARGE_MODE_AUTO = 4, /* Used for Adaptive */ > > CHARGE_MODE_CUSTOM = 5, /* Used for Custom */ > > + CHARGE_MODE_LONGLIFE = 6, /* Used for Permanent Long Life */ > > Since this is normally only done in the factory context, can you please > confirm does something need to be artificially done to block userland from > trying to set the battery charging to this mode? Or will the EC already handle > blocking it directly? This is a feature of the battery, when the EC receives this setting it will be sent to the battery and stored there. Afterwards any attempt to change this mode will return a failure. It's even better to block mode selection in the userland if long life mode already in use. > > > }; > > > > #define CHARGE_LOWER_LIMIT_MIN 50 > > @@ -48,6 +49,8 @@ static int psp_val_to_charge_mode(int psp_val) > > return CHARGE_MODE_AUTO; > > case POWER_SUPPLY_CHARGE_TYPE_CUSTOM: > > return CHARGE_MODE_CUSTOM; > > + case POWER_SUPPLY_CHARGE_TYPE_LONGLIFE: > > + return CHARGE_MODE_LONGLIFE; > > default: > > return -EINVAL; > > } > > @@ -67,6 +70,8 @@ static int charge_mode_to_psp_val(enum > charge_mode mode) > > return POWER_SUPPLY_CHARGE_TYPE_ADAPTIVE; > > case CHARGE_MODE_CUSTOM: > > return POWER_SUPPLY_CHARGE_TYPE_CUSTOM; > > + case CHARGE_MODE_LONGLIFE: > > + return POWER_SUPPLY_CHARGE_TYPE_LONGLIFE; > > default: > > return -EINVAL; > > } > > diff --git a/include/linux/power_supply.h > > b/include/linux/power_supply.h index ac1345a48ad0..528a3eaa2320 > 100644 > > --- a/include/linux/power_supply.h > > +++ b/include/linux/power_supply.h > > @@ -48,6 +48,7 @@ enum { > > POWER_SUPPLY_CHARGE_TYPE_STANDARD, /* normal speed */ > > POWER_SUPPLY_CHARGE_TYPE_ADAPTIVE, /* dynamically > adjusted speed */ > > POWER_SUPPLY_CHARGE_TYPE_CUSTOM, /* use > CHARGE_CONTROL_* props */ > > + POWER_SUPPLY_CHARGE_TYPE_LONGLIFE, /* slow speed, longer > life */ > > }; > > > > enum { > > -- > > 2.20.1
> -----Original Message----- > From: Wang, Crag <Crag_Wang@Dell.com> > Sent: Tuesday, June 23, 2020 10:51 AM > To: Limonciello, Mario; Crag Wang; Sebastian Reichel > Cc: mathewk@chromium.org; linux-pm@vger.kernel.org; linux- > kernel@vger.kernel.org > Subject: RE: [PATCH 1/1] power_supply: wilco_ec: Add permanent long life > charging mode > > > -----Original Message----- > > From: Limonciello, Mario <Mario_Limonciello@Dell.com> > > Sent: Tuesday, June 23, 2020 9:48 PM > > To: Crag Wang; Sebastian Reichel > > Cc: mathewk@chromium.org; Wang, Crag; linux-pm@vger.kernel.org; linux- > > kernel@vger.kernel.org > > Subject: RE: [PATCH 1/1] power_supply: wilco_ec: Add permanent long life > > charging mode > > > > > -----Original Message----- > > > From: Crag Wang <crag0715@gmail.com> > > > Sent: Tuesday, June 23, 2020 3:20 AM > > > To: Sebastian Reichel > > > Cc: mathewk@chromium.org; Limonciello, Mario; Wang, Crag; linux- > > > pm@vger.kernel.org; linux-kernel@vger.kernel.org > > > Subject: [PATCH 1/1] power_supply: wilco_ec: Add permanent long life > > > charging mode > > > > > > > > > [EXTERNAL EMAIL] > > > > > > This is a long life mode for extended warranty battery, switching to a > > > differnt mode selection is unavailable. The power charging rate is > > > customized so that battery at work last longer. > > > > > > Signed-off-by: Crag Wang <crag_wang@dell.com> > > > --- > > > Documentation/ABI/testing/sysfs-class-power-wilco | 3 +++ > > > drivers/power/supply/power_supply_sysfs.c | 1 + > > > drivers/power/supply/wilco-charger.c | 5 +++++ > > > include/linux/power_supply.h | 1 + > > > 4 files changed, 10 insertions(+) > > > > > > diff --git a/Documentation/ABI/testing/sysfs-class-power-wilco > > > b/Documentation/ABI/testing/sysfs-class-power-wilco > > > index da1d6ffe5e3c..1c91b17b6fd4 100644 > > > --- a/Documentation/ABI/testing/sysfs-class-power-wilco > > > +++ b/Documentation/ABI/testing/sysfs-class-power-wilco > > > @@ -14,6 +14,9 @@ Description: > > > Charging begins when level drops below > > > charge_control_start_threshold, and ceases when > > > level is above charge_control_end_threshold. > > > + Permanent Long Life: Last longer battery life, this mode > > > + is programmed once in the factory. Switching to a > > > + different mode is unavailable. > > > > > > What: /sys/class/power_supply/wilco- > > > charger/charge_control_start_threshold > > > Date: April 2019 > > > diff --git a/drivers/power/supply/power_supply_sysfs.c > > > b/drivers/power/supply/power_supply_sysfs.c > > > index bc79560229b5..af3884015ad8 100644 > > > --- a/drivers/power/supply/power_supply_sysfs.c > > > +++ b/drivers/power/supply/power_supply_sysfs.c > > > @@ -87,6 +87,7 @@ static const char * const > > > POWER_SUPPLY_CHARGE_TYPE_TEXT[] = { > > > [POWER_SUPPLY_CHARGE_TYPE_STANDARD] = "Standard", > > > [POWER_SUPPLY_CHARGE_TYPE_ADAPTIVE] = "Adaptive", > > > [POWER_SUPPLY_CHARGE_TYPE_CUSTOM] = "Custom", > > > + [POWER_SUPPLY_CHARGE_TYPE_LONGLIFE] = "Permanent Long > > Life", > > > }; > > > > > > static const char * const POWER_SUPPLY_HEALTH_TEXT[] = { diff --git > > > a/drivers/power/supply/wilco-charger.c > > > b/drivers/power/supply/wilco-charger.c > > > index b3c6d7cdd731..713c3018652f 100644 > > > --- a/drivers/power/supply/wilco-charger.c > > > +++ b/drivers/power/supply/wilco-charger.c > > > @@ -27,6 +27,7 @@ enum charge_mode { > > > CHARGE_MODE_AC = 3, /* Mostly AC use, used for Trickle */ > > > CHARGE_MODE_AUTO = 4, /* Used for Adaptive */ > > > CHARGE_MODE_CUSTOM = 5, /* Used for Custom */ > > > + CHARGE_MODE_LONGLIFE = 6, /* Used for Permanent Long Life */ > > > > Since this is normally only done in the factory context, can you please > > confirm does something need to be artificially done to block userland from > > trying to set the battery charging to this mode? Or will the EC already > handle > > blocking it directly? > > This is a feature of the battery, when the EC receives this setting it will be > sent > to the battery and stored there. Afterwards any attempt to change this mode > will return a failure. Sorry this still isn't clear to me. You're saying that if EC receives longlife setting it will be able to do this in the field? If so, then I think this patch will need to block that setting to not allow field conversions into longlife mode. > > It's even better to block mode selection in the userland if long life mode > already > in use. Yes, I agree. This sounds like a good follow up to me too. > > > > > > }; > > > > > > #define CHARGE_LOWER_LIMIT_MIN 50 > > > @@ -48,6 +49,8 @@ static int psp_val_to_charge_mode(int psp_val) > > > return CHARGE_MODE_AUTO; > > > case POWER_SUPPLY_CHARGE_TYPE_CUSTOM: > > > return CHARGE_MODE_CUSTOM; > > > + case POWER_SUPPLY_CHARGE_TYPE_LONGLIFE: > > > + return CHARGE_MODE_LONGLIFE; > > > default: > > > return -EINVAL; > > > } > > > @@ -67,6 +70,8 @@ static int charge_mode_to_psp_val(enum > > charge_mode mode) > > > return POWER_SUPPLY_CHARGE_TYPE_ADAPTIVE; > > > case CHARGE_MODE_CUSTOM: > > > return POWER_SUPPLY_CHARGE_TYPE_CUSTOM; > > > + case CHARGE_MODE_LONGLIFE: > > > + return POWER_SUPPLY_CHARGE_TYPE_LONGLIFE; > > > default: > > > return -EINVAL; > > > } > > > diff --git a/include/linux/power_supply.h > > > b/include/linux/power_supply.h index ac1345a48ad0..528a3eaa2320 > > 100644 > > > --- a/include/linux/power_supply.h > > > +++ b/include/linux/power_supply.h > > > @@ -48,6 +48,7 @@ enum { > > > POWER_SUPPLY_CHARGE_TYPE_STANDARD, /* normal speed */ > > > POWER_SUPPLY_CHARGE_TYPE_ADAPTIVE, /* dynamically > > adjusted speed */ > > > POWER_SUPPLY_CHARGE_TYPE_CUSTOM, /* use > > CHARGE_CONTROL_* props */ > > > + POWER_SUPPLY_CHARGE_TYPE_LONGLIFE, /* slow speed, longer > > life */ > > > }; > > > > > > enum { > > > -- > > > 2.20.1
> > > Since this is normally only done in the factory context, can you > > > please confirm does something need to be artificially done to block > > > userland from trying to set the battery charging to this mode? Or > > > will the EC already > > handle > > > blocking it directly? > > > > This is a feature of the battery, when the EC receives this setting it > > will be sent to the battery and stored there. Afterwards any attempt > > to change this mode will return a failure. > > Sorry this still isn't clear to me. You're saying that if EC receives longlife > setting it will be able to do this in the field? If so, then I think this patch will > need to block that setting to not allow field conversions into longlife mode. > EC does handle blocking the attempts from changing the mode. EC reads current mode ahead of a new setting. If it sees permanent long life already in use then any attempt to put the battery charging in a different mode will get failure 0x01 from EC. > > > > It's even better to block mode selection in the userland if long life > > mode already in use. > > Yes, I agree. This sounds like a good follow up to me too.
Hi kernel maintainers, I'd like to request a code review for this patch accepted, thanks. Crag
diff --git a/Documentation/ABI/testing/sysfs-class-power-wilco b/Documentation/ABI/testing/sysfs-class-power-wilco index da1d6ffe5e3c..1c91b17b6fd4 100644 --- a/Documentation/ABI/testing/sysfs-class-power-wilco +++ b/Documentation/ABI/testing/sysfs-class-power-wilco @@ -14,6 +14,9 @@ Description: Charging begins when level drops below charge_control_start_threshold, and ceases when level is above charge_control_end_threshold. + Permanent Long Life: Last longer battery life, this mode + is programmed once in the factory. Switching to a + different mode is unavailable. What: /sys/class/power_supply/wilco-charger/charge_control_start_threshold Date: April 2019 diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c index bc79560229b5..af3884015ad8 100644 --- a/drivers/power/supply/power_supply_sysfs.c +++ b/drivers/power/supply/power_supply_sysfs.c @@ -87,6 +87,7 @@ static const char * const POWER_SUPPLY_CHARGE_TYPE_TEXT[] = { [POWER_SUPPLY_CHARGE_TYPE_STANDARD] = "Standard", [POWER_SUPPLY_CHARGE_TYPE_ADAPTIVE] = "Adaptive", [POWER_SUPPLY_CHARGE_TYPE_CUSTOM] = "Custom", + [POWER_SUPPLY_CHARGE_TYPE_LONGLIFE] = "Permanent Long Life", }; static const char * const POWER_SUPPLY_HEALTH_TEXT[] = { diff --git a/drivers/power/supply/wilco-charger.c b/drivers/power/supply/wilco-charger.c index b3c6d7cdd731..713c3018652f 100644 --- a/drivers/power/supply/wilco-charger.c +++ b/drivers/power/supply/wilco-charger.c @@ -27,6 +27,7 @@ enum charge_mode { CHARGE_MODE_AC = 3, /* Mostly AC use, used for Trickle */ CHARGE_MODE_AUTO = 4, /* Used for Adaptive */ CHARGE_MODE_CUSTOM = 5, /* Used for Custom */ + CHARGE_MODE_LONGLIFE = 6, /* Used for Permanent Long Life */ }; #define CHARGE_LOWER_LIMIT_MIN 50 @@ -48,6 +49,8 @@ static int psp_val_to_charge_mode(int psp_val) return CHARGE_MODE_AUTO; case POWER_SUPPLY_CHARGE_TYPE_CUSTOM: return CHARGE_MODE_CUSTOM; + case POWER_SUPPLY_CHARGE_TYPE_LONGLIFE: + return CHARGE_MODE_LONGLIFE; default: return -EINVAL; } @@ -67,6 +70,8 @@ static int charge_mode_to_psp_val(enum charge_mode mode) return POWER_SUPPLY_CHARGE_TYPE_ADAPTIVE; case CHARGE_MODE_CUSTOM: return POWER_SUPPLY_CHARGE_TYPE_CUSTOM; + case CHARGE_MODE_LONGLIFE: + return POWER_SUPPLY_CHARGE_TYPE_LONGLIFE; default: return -EINVAL; } diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h index ac1345a48ad0..528a3eaa2320 100644 --- a/include/linux/power_supply.h +++ b/include/linux/power_supply.h @@ -48,6 +48,7 @@ enum { POWER_SUPPLY_CHARGE_TYPE_STANDARD, /* normal speed */ POWER_SUPPLY_CHARGE_TYPE_ADAPTIVE, /* dynamically adjusted speed */ POWER_SUPPLY_CHARGE_TYPE_CUSTOM, /* use CHARGE_CONTROL_* props */ + POWER_SUPPLY_CHARGE_TYPE_LONGLIFE, /* slow speed, longer life */ }; enum {
This is a long life mode for extended warranty battery, switching to a differnt mode selection is unavailable. The power charging rate is customized so that battery at work last longer. Signed-off-by: Crag Wang <crag_wang@dell.com> --- Documentation/ABI/testing/sysfs-class-power-wilco | 3 +++ drivers/power/supply/power_supply_sysfs.c | 1 + drivers/power/supply/wilco-charger.c | 5 +++++ include/linux/power_supply.h | 1 + 4 files changed, 10 insertions(+)