Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220428125610.66647-1-gengcixi@gmail.com> <CADBw62r8eGRNcXH1cAZvYQdKCgBjxUVnxhLsa=Oyzs-uwavRTA@mail.gmail.com>
In-Reply-To: <CADBw62r8eGRNcXH1cAZvYQdKCgBjxUVnxhLsa=Oyzs-uwavRTA@mail.gmail.com>
From:   =?UTF-8?B?6ZmI5rC45b+X?= <chenyongzhi811@gmail.com>
Date:   Thu, 12 May 2022 11:22:44 +0800
Message-ID: <CAJejCsY+DX0JywDS_dk=1P-fvyjUc4i1e67uM_WW64E3YVvzQg@mail.gmail.com>
Subject: Re: [PATCH v2] power: supply: Add enable the primary charger interface
To:     Baolin Wang <baolin.wang7@gmail.com>
Cc:     Cixi Geng <gengcixi@gmail.com>, Sebastian Reichel <sre@kernel.org>,
        Orson Zhai <orsonzhai@gmail.com>,
        Chunyan Zhang <zhang.lyra@gmail.com>,
        Linux PM <linux-pm@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Baolin Wang <baolin.wang7@gmail.com> =E4=BA=8E2022=E5=B9=B45=E6=9C=883=E6=
=97=A5=E5=91=A8=E4=BA=8C 12:53=E5=86=99=E9=81=93=EF=BC=9A
>
> 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=EF=BC=9B
POWER_SUPPLY_PROP_CHARGE_ENABLED: The hardware is stopped charging,
the software is still charging=EF=BC=9B
  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/supp=
ly/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 =3D 0;
> > +
> > +       if (!val && info->charging) {
> > +               sc2731_charger_stop_charge(info);
> > +               info->charging =3D false;
> > +       } else if (val && !info->charging) {
> > +               ret =3D 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 =3D 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 =3D power_supply_get_drvdata(p=
sy);
> > -       int ret;
> > +       int ret =3D 0;
> >
> >         mutex_lock(&info->lock);
> >
> > @@ -214,6 +232,12 @@ sc2731_charger_usb_set_property(struct power_suppl=
y *psy,
> >         }
> >
> >         switch (psp) {
> > +       case POWER_SUPPLY_PROP_STATUS:
> > +               ret =3D 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 =3D sc2731_charger_set_current(info, val->intval / =
1000);
> >                 if (ret < 0)
> > @@ -227,6 +251,15 @@ sc2731_charger_usb_set_property(struct power_suppl=
y *psy,
> >                         dev_err(info->dev, "set input current limit fai=
led\n");
> >                 break;
> >
> > +       case POWER_SUPPLY_PROP_CHARGE_ENABLED:
> > +               if (val->intval =3D=3D true) {
> > +                       ret =3D sc2731_charger_start_charge(info);
> > +                       if (ret)
> > +                               dev_err(info->dev, "start charge failed=
\n");
> > +               } else if (val->intval =3D=3D false) {
> > +                       sc2731_charger_stop_charge(info);
> > +               }
> > +               break;
> >         default:
> >                 ret =3D -EINVAL;
> >         }
> > @@ -241,7 +274,7 @@ static int sc2731_charger_usb_get_property(struct p=
ower_supply *psy,
> >  {
> >         struct sc2731_charger_info *info =3D power_supply_get_drvdata(p=
sy);
> >         int ret =3D 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 =3D regmap_read(info->regmap, info->base + SC2731_C=
HG_CFG0, &enabled);
> > +               if (ret) {
> > +                       dev_err(info->dev, "get sc2731 charge enabled f=
ailed\n");
> > +                       goto out;
> > +               }
> > +
> > +               val->intval =3D enabled & SC2731_CHARGER_PD;
> > +
> > +               break;
> >         default:
> >                 ret =3D -EINVAL;
> >         }
> > @@ -292,8 +335,10 @@ static int sc2731_charger_property_is_writeable(st=
ruct 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 =3D 1;
> >                 break;
> >
> > @@ -308,6 +353,7 @@ static enum power_supply_property sc2731_usb_props[=
] =3D {
> >         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 =3D {
> > 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