* [PATCH v2] power: supply: Add enable the primary charger interface
@ 2022-04-28 12:56 Cixi Geng
2022-05-03 4:54 ` Baolin Wang
0 siblings, 1 reply; 4+ messages in thread
From: Cixi Geng @ 2022-04-28 12:56 UTC (permalink / raw)
To: sre, orsonzhai, baolin.wang7, zhang.lyra, chenyongzhi811
Cc: linux-pm, linux-kernel
From: Chen Yongzhi <Yongzhi.Chen@unisoc.com>
In the case of charging multiple charging ICs,the primary
charging IC often needs to be turned off in the fast
charging stage, and only using the charger pump to charge,
need to add a new power_supply_property attribute.
Signed-off-by: Chen Yongzhi <Yongzhi.Chen@unisoc.com>
Reported-by: kernel test robot <lkp@intel.com>
Link: https://lore.kernel.org/all/202204230206.9TgyhSb1-lkp@intel.com
Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
---
drivers/power/supply/sc2731_charger.c | 52 +++++++++++++++++++++++++--
include/linux/power_supply.h | 1 +
2 files changed, 50 insertions(+), 3 deletions(-)
diff --git a/drivers/power/supply/sc2731_charger.c b/drivers/power/supply/sc2731_charger.c
index 9ac17cf7a126..c15f9b75e6a8 100644
--- a/drivers/power/supply/sc2731_charger.c
+++ b/drivers/power/supply/sc2731_charger.c
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-2.0
-// Copyright (C) 2018 Spreadtrum Communications Inc.
+// Copyright (C) 2022 Spreadtrum Communications Inc.
#include <linux/module.h>
#include <linux/platform_device.h>
@@ -146,6 +146,24 @@ static int sc2731_charger_get_status(struct sc2731_charger_info *info)
return POWER_SUPPLY_STATUS_CHARGING;
}
+static int sc2731_charger_set_status(struct sc2731_charger_info *info, int val)
+{
+ int ret = 0;
+
+ if (!val && info->charging) {
+ sc2731_charger_stop_charge(info);
+ info->charging = false;
+ } else if (val && !info->charging) {
+ ret = sc2731_charger_start_charge(info);
+ if (ret)
+ dev_err(info->dev, "start charge failed\n");
+ else
+ info->charging = true;
+ }
+
+ return ret;
+}
+
static int sc2731_charger_get_current(struct sc2731_charger_info *info,
u32 *cur)
{
@@ -204,7 +222,7 @@ sc2731_charger_usb_set_property(struct power_supply *psy,
const union power_supply_propval *val)
{
struct sc2731_charger_info *info = power_supply_get_drvdata(psy);
- int ret;
+ int ret = 0;
mutex_lock(&info->lock);
@@ -214,6 +232,12 @@ sc2731_charger_usb_set_property(struct power_supply *psy,
}
switch (psp) {
+ case POWER_SUPPLY_PROP_STATUS:
+ ret = sc2731_charger_set_status(info, val->intval);
+ if (ret < 0)
+ dev_err(info->dev, "set charge status failed\n");
+ break;
+
case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
ret = sc2731_charger_set_current(info, val->intval / 1000);
if (ret < 0)
@@ -227,6 +251,15 @@ sc2731_charger_usb_set_property(struct power_supply *psy,
dev_err(info->dev, "set input current limit failed\n");
break;
+ case POWER_SUPPLY_PROP_CHARGE_ENABLED:
+ if (val->intval == true) {
+ ret = sc2731_charger_start_charge(info);
+ if (ret)
+ dev_err(info->dev, "start charge failed\n");
+ } else if (val->intval == false) {
+ sc2731_charger_stop_charge(info);
+ }
+ break;
default:
ret = -EINVAL;
}
@@ -241,7 +274,7 @@ static int sc2731_charger_usb_get_property(struct power_supply *psy,
{
struct sc2731_charger_info *info = power_supply_get_drvdata(psy);
int ret = 0;
- u32 cur;
+ u32 cur, enabled;
mutex_lock(&info->lock);
@@ -277,6 +310,16 @@ static int sc2731_charger_usb_get_property(struct power_supply *psy,
}
break;
+ case POWER_SUPPLY_PROP_CHARGE_ENABLED:
+ ret = regmap_read(info->regmap, info->base + SC2731_CHG_CFG0, &enabled);
+ if (ret) {
+ dev_err(info->dev, "get sc2731 charge enabled failed\n");
+ goto out;
+ }
+
+ val->intval = enabled & SC2731_CHARGER_PD;
+
+ break;
default:
ret = -EINVAL;
}
@@ -292,8 +335,10 @@ static int sc2731_charger_property_is_writeable(struct power_supply *psy,
int ret;
switch (psp) {
+ case POWER_SUPPLY_PROP_STATUS:
case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+ case POWER_SUPPLY_PROP_CHARGE_ENABLED:
ret = 1;
break;
@@ -308,6 +353,7 @@ static enum power_supply_property sc2731_usb_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT,
POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
+ POWER_SUPPLY_PROP_CHARGE_ENABLED,
};
static const struct power_supply_desc sc2731_charger_desc = {
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index cb380c1d9459..1dfe194d8a5e 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -167,6 +167,7 @@ enum power_supply_property {
POWER_SUPPLY_PROP_PRECHARGE_CURRENT,
POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT,
POWER_SUPPLY_PROP_CALIBRATE,
+ POWER_SUPPLY_PROP_CHARGE_ENABLED,
POWER_SUPPLY_PROP_MANUFACTURE_YEAR,
POWER_SUPPLY_PROP_MANUFACTURE_MONTH,
POWER_SUPPLY_PROP_MANUFACTURE_DAY,
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] power: supply: Add enable the primary charger interface
2022-04-28 12:56 [PATCH v2] power: supply: Add enable the primary charger interface Cixi Geng
@ 2022-05-03 4:54 ` Baolin Wang
2022-05-12 3:22 ` 陈永志
0 siblings, 1 reply; 4+ messages in thread
From: Baolin Wang @ 2022-05-03 4:54 UTC (permalink / raw)
To: Cixi Geng
Cc: Sebastian Reichel, Orson Zhai, Chunyan Zhang, chenyongzhi811,
Linux PM, LKML
On Thu, Apr 28, 2022 at 8:56 PM Cixi Geng <gengcixi@gmail.com> wrote:
>
> From: Chen Yongzhi <Yongzhi.Chen@unisoc.com>
>
> In the case of charging multiple charging ICs,the primary
> charging IC often needs to be turned off in the fast
> charging stage, and only using the charger pump to charge,
> need to add a new power_supply_property attribute.
I'm still confused why introducing a new
POWER_SUPPLY_PROP_CHARGE_ENABLED property to control the charging, but
you already controlled the charging by POWER_SUPPLY_PROP_STATUS?
>
> Signed-off-by: Chen Yongzhi <Yongzhi.Chen@unisoc.com>
> Reported-by: kernel test robot <lkp@intel.com>
> Link: https://lore.kernel.org/all/202204230206.9TgyhSb1-lkp@intel.com
> Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
> ---
> drivers/power/supply/sc2731_charger.c | 52 +++++++++++++++++++++++++--
> include/linux/power_supply.h | 1 +
> 2 files changed, 50 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/power/supply/sc2731_charger.c b/drivers/power/supply/sc2731_charger.c
> index 9ac17cf7a126..c15f9b75e6a8 100644
> --- a/drivers/power/supply/sc2731_charger.c
> +++ b/drivers/power/supply/sc2731_charger.c
> @@ -1,5 +1,5 @@
> // SPDX-License-Identifier: GPL-2.0
> -// Copyright (C) 2018 Spreadtrum Communications Inc.
> +// Copyright (C) 2022 Spreadtrum Communications Inc.
Do not add unrelated changes.
>
> #include <linux/module.h>
> #include <linux/platform_device.h>
> @@ -146,6 +146,24 @@ static int sc2731_charger_get_status(struct sc2731_charger_info *info)
> return POWER_SUPPLY_STATUS_CHARGING;
> }
>
> +static int sc2731_charger_set_status(struct sc2731_charger_info *info, int val)
> +{
> + int ret = 0;
> +
> + if (!val && info->charging) {
> + sc2731_charger_stop_charge(info);
> + info->charging = false;
> + } else if (val && !info->charging) {
> + ret = sc2731_charger_start_charge(info);
> + if (ret)
> + dev_err(info->dev, "start charge failed\n");
Duplicate error information, since you already print errors in
sc2731_charger_usb_set_property()
> + else
> + info->charging = true;
> + }
> +
> + return ret;
> +}
> +
> static int sc2731_charger_get_current(struct sc2731_charger_info *info,
> u32 *cur)
> {
> @@ -204,7 +222,7 @@ sc2731_charger_usb_set_property(struct power_supply *psy,
> const union power_supply_propval *val)
> {
> struct sc2731_charger_info *info = power_supply_get_drvdata(psy);
> - int ret;
> + int ret = 0;
>
> mutex_lock(&info->lock);
>
> @@ -214,6 +232,12 @@ sc2731_charger_usb_set_property(struct power_supply *psy,
> }
>
> switch (psp) {
> + case POWER_SUPPLY_PROP_STATUS:
> + ret = sc2731_charger_set_status(info, val->intval);
> + if (ret < 0)
> + dev_err(info->dev, "set charge status failed\n");
> + break;
> +
> case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
> ret = sc2731_charger_set_current(info, val->intval / 1000);
> if (ret < 0)
> @@ -227,6 +251,15 @@ sc2731_charger_usb_set_property(struct power_supply *psy,
> dev_err(info->dev, "set input current limit failed\n");
> break;
>
> + case POWER_SUPPLY_PROP_CHARGE_ENABLED:
> + if (val->intval == true) {
> + ret = sc2731_charger_start_charge(info);
> + if (ret)
> + dev_err(info->dev, "start charge failed\n");
> + } else if (val->intval == false) {
> + sc2731_charger_stop_charge(info);
> + }
> + break;
> default:
> ret = -EINVAL;
> }
> @@ -241,7 +274,7 @@ static int sc2731_charger_usb_get_property(struct power_supply *psy,
> {
> struct sc2731_charger_info *info = power_supply_get_drvdata(psy);
> int ret = 0;
> - u32 cur;
> + u32 cur, enabled;
>
> mutex_lock(&info->lock);
>
> @@ -277,6 +310,16 @@ static int sc2731_charger_usb_get_property(struct power_supply *psy,
> }
> break;
>
> + case POWER_SUPPLY_PROP_CHARGE_ENABLED:
> + ret = regmap_read(info->regmap, info->base + SC2731_CHG_CFG0, &enabled);
> + if (ret) {
> + dev_err(info->dev, "get sc2731 charge enabled failed\n");
> + goto out;
> + }
> +
> + val->intval = enabled & SC2731_CHARGER_PD;
> +
> + break;
> default:
> ret = -EINVAL;
> }
> @@ -292,8 +335,10 @@ static int sc2731_charger_property_is_writeable(struct power_supply *psy,
> int ret;
>
> switch (psp) {
> + case POWER_SUPPLY_PROP_STATUS:
> case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
> case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
> + case POWER_SUPPLY_PROP_CHARGE_ENABLED:
> ret = 1;
> break;
>
> @@ -308,6 +353,7 @@ static enum power_supply_property sc2731_usb_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT,
> POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
> + POWER_SUPPLY_PROP_CHARGE_ENABLED,
> };
>
> static const struct power_supply_desc sc2731_charger_desc = {
> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> index cb380c1d9459..1dfe194d8a5e 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -167,6 +167,7 @@ enum power_supply_property {
> POWER_SUPPLY_PROP_PRECHARGE_CURRENT,
> POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT,
> POWER_SUPPLY_PROP_CALIBRATE,
> + POWER_SUPPLY_PROP_CHARGE_ENABLED,
> POWER_SUPPLY_PROP_MANUFACTURE_YEAR,
> POWER_SUPPLY_PROP_MANUFACTURE_MONTH,
> POWER_SUPPLY_PROP_MANUFACTURE_DAY,
> --
> 2.25.1
>
--
Baolin Wang
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] power: supply: Add enable the primary charger interface
2022-05-03 4:54 ` Baolin Wang
@ 2022-05-12 3:22 ` 陈永志
2022-05-13 2:15 ` Baolin Wang
0 siblings, 1 reply; 4+ messages in thread
From: 陈永志 @ 2022-05-12 3:22 UTC (permalink / raw)
To: Baolin Wang
Cc: Cixi Geng, Sebastian Reichel, Orson Zhai, Chunyan Zhang, Linux PM, LKML
Baolin Wang <baolin.wang7@gmail.com> 于2022年5月3日周二 12:53写道：
>
> On Thu, Apr 28, 2022 at 8:56 PM Cixi Geng <gengcixi@gmail.com> wrote:
> >
> > From: Chen Yongzhi <Yongzhi.Chen@unisoc.com>
> >
> > In the case of charging multiple charging ICs,the primary
> > charging IC often needs to be turned off in the fast
> > charging stage, and only using the charger pump to charge,
> > need to add a new power_supply_property attribute.
>
> I'm still confused why introducing a new
> POWER_SUPPLY_PROP_CHARGE_ENABLED property to control the charging, but
> you already controlled the charging by POWER_SUPPLY_PROP_STATUS?
>
Our purpose is to achieve two different stop charging states:
POWER_SUPPLY_PROP_STATUS: The software status stops charging, and the
hardware also stops charging；
POWER_SUPPLY_PROP_CHARGE_ENABLED: The hardware is stopped charging,
the software is still charging；
Our don't want to change the charge_status switch due to the
switching of charging and discharging of the charging IC in the
charging scenario of multiple charging ICs, and let the upper layer
perceive this switching
> >
> > Signed-off-by: Chen Yongzhi <Yongzhi.Chen@unisoc.com>
> > Reported-by: kernel test robot <lkp@intel.com>
> > Link: https://lore.kernel.org/all/202204230206.9TgyhSb1-lkp@intel.com
> > Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
> > ---
> > drivers/power/supply/sc2731_charger.c | 52 +++++++++++++++++++++++++--
> > include/linux/power_supply.h | 1 +
> > 2 files changed, 50 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/power/supply/sc2731_charger.c b/drivers/power/supply/sc2731_charger.c
> > index 9ac17cf7a126..c15f9b75e6a8 100644
> > --- a/drivers/power/supply/sc2731_charger.c
> > +++ b/drivers/power/supply/sc2731_charger.c
> > @@ -1,5 +1,5 @@
> > // SPDX-License-Identifier: GPL-2.0
> > -// Copyright (C) 2018 Spreadtrum Communications Inc.
> > +// Copyright (C) 2022 Spreadtrum Communications Inc.
>
> Do not add unrelated changes.
>
ok, do not change
> >
> > #include <linux/module.h>
> > #include <linux/platform_device.h>
> > @@ -146,6 +146,24 @@ static int sc2731_charger_get_status(struct sc2731_charger_info *info)
> > return POWER_SUPPLY_STATUS_CHARGING;
> > }
> >
> > +static int sc2731_charger_set_status(struct sc2731_charger_info *info, int val)
> > +{
> > + int ret = 0;
> > +
> > + if (!val && info->charging) {
> > + sc2731_charger_stop_charge(info);
> > + info->charging = false;
> > + } else if (val && !info->charging) {
> > + ret = sc2731_charger_start_charge(info);
> > + if (ret)
> > + dev_err(info->dev, "start charge failed\n");
>
> Duplicate error information, since you already print errors in
fix it
> sc2731_charger_usb_set_property()
>
> > + else
> > + info->charging = true;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > static int sc2731_charger_get_current(struct sc2731_charger_info *info,
> > u32 *cur)
> > {
> > @@ -204,7 +222,7 @@ sc2731_charger_usb_set_property(struct power_supply *psy,
> > const union power_supply_propval *val)
> > {
> > struct sc2731_charger_info *info = power_supply_get_drvdata(psy);
> > - int ret;
> > + int ret = 0;
> >
> > mutex_lock(&info->lock);
> >
> > @@ -214,6 +232,12 @@ sc2731_charger_usb_set_property(struct power_supply *psy,
> > }
> >
> > switch (psp) {
> > + case POWER_SUPPLY_PROP_STATUS:
> > + ret = sc2731_charger_set_status(info, val->intval);
> > + if (ret < 0)
> > + dev_err(info->dev, "set charge status failed\n");
> > + break;
> > +
> > case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
> > ret = sc2731_charger_set_current(info, val->intval / 1000);
> > if (ret < 0)
> > @@ -227,6 +251,15 @@ sc2731_charger_usb_set_property(struct power_supply *psy,
> > dev_err(info->dev, "set input current limit failed\n");
> > break;
> >
> > + case POWER_SUPPLY_PROP_CHARGE_ENABLED:
> > + if (val->intval == true) {
> > + ret = sc2731_charger_start_charge(info);
> > + if (ret)
> > + dev_err(info->dev, "start charge failed\n");
> > + } else if (val->intval == false) {
> > + sc2731_charger_stop_charge(info);
> > + }
> > + break;
> > default:
> > ret = -EINVAL;
> > }
> > @@ -241,7 +274,7 @@ static int sc2731_charger_usb_get_property(struct power_supply *psy,
> > {
> > struct sc2731_charger_info *info = power_supply_get_drvdata(psy);
> > int ret = 0;
> > - u32 cur;
> > + u32 cur, enabled;
> >
> > mutex_lock(&info->lock);
> >
> > @@ -277,6 +310,16 @@ static int sc2731_charger_usb_get_property(struct power_supply *psy,
> > }
> > break;
> >
> > + case POWER_SUPPLY_PROP_CHARGE_ENABLED:
> > + ret = regmap_read(info->regmap, info->base + SC2731_CHG_CFG0, &enabled);
> > + if (ret) {
> > + dev_err(info->dev, "get sc2731 charge enabled failed\n");
> > + goto out;
> > + }
> > +
> > + val->intval = enabled & SC2731_CHARGER_PD;
> > +
> > + break;
> > default:
> > ret = -EINVAL;
> > }
> > @@ -292,8 +335,10 @@ static int sc2731_charger_property_is_writeable(struct power_supply *psy,
> > int ret;
> >
> > switch (psp) {
> > + case POWER_SUPPLY_PROP_STATUS:
> > case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
> > case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
> > + case POWER_SUPPLY_PROP_CHARGE_ENABLED:
> > ret = 1;
> > break;
> >
> > @@ -308,6 +353,7 @@ static enum power_supply_property sc2731_usb_props[] = {
> > POWER_SUPPLY_PROP_STATUS,
> > POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT,
> > POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
> > + POWER_SUPPLY_PROP_CHARGE_ENABLED,
> > };
> >
> > static const struct power_supply_desc sc2731_charger_desc = {
> > diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> > index cb380c1d9459..1dfe194d8a5e 100644
> > --- a/include/linux/power_supply.h
> > +++ b/include/linux/power_supply.h
> > @@ -167,6 +167,7 @@ enum power_supply_property {
> > POWER_SUPPLY_PROP_PRECHARGE_CURRENT,
> > POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT,
> > POWER_SUPPLY_PROP_CALIBRATE,
> > + POWER_SUPPLY_PROP_CHARGE_ENABLED,
> > POWER_SUPPLY_PROP_MANUFACTURE_YEAR,
> > POWER_SUPPLY_PROP_MANUFACTURE_MONTH,
> > POWER_SUPPLY_PROP_MANUFACTURE_DAY,
> > --
> > 2.25.1
> >
>
>
> --
> Baolin Wang
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] power: supply: Add enable the primary charger interface
2022-05-12 3:22 ` 陈永志
@ 2022-05-13 2:15 ` Baolin Wang
0 siblings, 0 replies; 4+ messages in thread
From: Baolin Wang @ 2022-05-13 2:15 UTC (permalink / raw)
To: 陈永志
Cc: Cixi Geng, Sebastian Reichel, Orson Zhai, Chunyan Zhang, Linux PM, LKML
Hi,
On Thu, May 12, 2022 at 11:22 AM 陈永志 <chenyongzhi811@gmail.com> wrote:
>
> Baolin Wang <baolin.wang7@gmail.com> 于2022年5月3日周二 12:53写道：
> >
> > On Thu, Apr 28, 2022 at 8:56 PM Cixi Geng <gengcixi@gmail.com> wrote:
> > >
> > > From: Chen Yongzhi <Yongzhi.Chen@unisoc.com>
> > >
> > > In the case of charging multiple charging ICs,the primary
> > > charging IC often needs to be turned off in the fast
> > > charging stage, and only using the charger pump to charge,
> > > need to add a new power_supply_property attribute.
> >
> > I'm still confused why introducing a new
> > POWER_SUPPLY_PROP_CHARGE_ENABLED property to control the charging, but
> > you already controlled the charging by POWER_SUPPLY_PROP_STATUS?
> >
> Our purpose is to achieve two different stop charging states:
> POWER_SUPPLY_PROP_STATUS: The software status stops charging, and the
> hardware also stops charging；
> POWER_SUPPLY_PROP_CHARGE_ENABLED: The hardware is stopped charging,
> the software is still charging；
Please separate it into two patches, one patch adds charging control
with POWER_SUPPLY_PROP_STATUS attribute, and describe the use case in
detail.
The second patch introduces the new POWER_SUPPLY_PROP_CHARGE_ENABLED
attribute with explicit description why you want a new attribute.
Cause I'm still confused about what you want, and your commit message
is useless. If the hardware stopped charging, why does the software
still need to charge?
> Our don't want to change the charge_status switch due to the
> switching of charging and discharging of the charging IC in the
> charging scenario of multiple charging ICs, and let the upper layer
> perceive this switching
> > >
snip.
--
Baolin Wang
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-05-13 2:15 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-28 12:56 [PATCH v2] power: supply: Add enable the primary charger interface Cixi Geng
2022-05-03 4:54 ` Baolin Wang
2022-05-12 3:22 ` 陈永志
2022-05-13 2:15 ` Baolin Wang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.