| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 3 May 2022 12:54:31 +0800
From: Baolin Wang <baolin.wang7@...il.com>
To: Cixi Geng <gengcixi@...il.com>
Cc: Sebastian Reichel <sre@...nel.org>,
Orson Zhai <orsonzhai@...il.com>,
Chunyan Zhang <zhang.lyra@...il.com>, chenyongzhi811@...il.com,
Linux PM <linux-pm@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] power: supply: Add enable the primary charger interface
On Thu, Apr 28, 2022 at 8:56 PM Cixi Geng <gengcixi@...il.com> wrote:
>
> From: Chen Yongzhi <Yongzhi.Chen@...soc.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@...soc.com>
> Reported-by: kernel test robot <lkp@...el.com>
> Link: https://lore.kernel.org/all/202204230206.9TgyhSb1-lkp@intel.com
> Signed-off-by: Cixi Geng <cixi.geng1@...soc.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
Powered by blists - more mailing lists