Re: [PATCH v2] power: supply: Add enable the primary charger interface

[陈永志]

Baolin Wang <baolin.wang7@xxxxxxxxx> 于2022年5月3日周二 12:53写道：
>
> On Thu, Apr 28, 2022 at 8:56 PM Cixi Geng <gengcixi@xxxxxxxxx> wrote:
> >
> > From: Chen Yongzhi <Yongzhi.Chen@xxxxxxxxxx>
> >
> > 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@xxxxxxxxxx>
> > Reported-by: kernel test robot <lkp@xxxxxxxxx>
> > Link: https://lore.kernel.org/all/202204230206.9TgyhSb1-lkp@xxxxxxxxx
> > Signed-off-by: Cixi Geng <cixi.geng1@xxxxxxxxxx>
> > ---
> >  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