linux-kernel - Re: [PATCH 5/5] power: supply: qcom-battmgr: Add charge control support

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a6deee53-6106-4e50-8d53-f1a87e50e9f0@oss.qualcomm.com>
Date: Mon, 26 May 2025 16:43:03 +0800
From: Fenglin Wu <fenglin.wu@....qualcomm.com>
To: Neil Armstrong <neil.armstrong@...aro.org>,
        György Kurucz <me@...uczgy.com>,
        Sebastian Reichel <sre@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>
Cc: Subbaraman Narayanamurthy <subbaraman.narayanamurthy@....qualcomm.com>,
        David Collins <david.collins@....qualcomm.com>,
        linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, kernel@....qualcomm.com
Subject: Re: [PATCH 5/5] power: supply: qcom-battmgr: Add charge control
 support


On 5/26/2025 4:27 PM, neil.armstrong@...aro.org wrote:
> On 26/05/2025 08:19, Fenglin Wu wrote:
>>
>> On 5/24/2025 5:29 AM, György Kurucz wrote:
>>> Hi!
>>>
>>>> +static int qcom_battmgr_set_charge_control(struct qcom_battmgr 
>>>> *battmgr,
>>>> +                       u32 target_soc, u32 delta_soc)
>>>> +{
>>>> +    struct qcom_battmgr_charge_ctrl_request request = {
>>>> +        .hdr.owner = cpu_to_le32(PMIC_GLINK_OWNER_BATTMGR),
>>>> +        .hdr.type = cpu_to_le32(PMIC_GLINK_REQ_RESP),
>>>> +        .hdr.opcode = cpu_to_le32(BATTMGR_CHG_CTRL_LIMIT_EN),
>>>> +        .enable = cpu_to_le32(1),
>>>> +        .target_soc = cpu_to_le32(target_soc),
>>>> +        .delta_soc = cpu_to_le32(delta_soc),
>>>> +    };
>>>> +
>>>> +    return qcom_battmgr_request(battmgr, &request, sizeof(request));
>>>> +}
>>>> +
>>>> +static int qcom_battmgr_set_charge_start_threshold(struct 
>>>> qcom_battmgr *battmgr, int soc)
>>>> +{
>>>> +    u32 target_soc, delta_soc;
>>>> +    int ret;
>>>> +
>>>> +    if (soc < CHARGE_CTRL_START_THR_MIN ||
>>>> +            soc > CHARGE_CTRL_START_THR_MAX) {
>>>> +        dev_err(battmgr->dev, "charge control start threshold 
>>>> exceed range: [%u - %u]\n",
>>>> +                CHARGE_CTRL_START_THR_MIN, 
>>>> CHARGE_CTRL_START_THR_MAX);
>>>> +        return -EINVAL;
>>>> +    }
>>>> +
>>>> +    /*
>>>> +     * If the new start threshold is larger than the old end 
>>>> threshold,
>>>> +     * move the end threshold one step (DELTA_SOC) after the new 
>>>> start
>>>> +     * threshold.
>>>> +     */
>>>> +    if (soc > battmgr->info.charge_ctrl_end) {
>>>> +        target_soc = soc + CHARGE_CTRL_DELTA_SOC;
>>>> +        target_soc = min_t(u32, target_soc, CHARGE_CTRL_END_THR_MAX);
>>>> +        delta_soc = target_soc - soc;
>>>> +        delta_soc = min_t(u32, delta_soc, CHARGE_CTRL_DELTA_SOC);
>>>> +    } else {
>>>> +        target_soc =  battmgr->info.charge_ctrl_end;
>>>> +        delta_soc = battmgr->info.charge_ctrl_end - soc;
>>>> +    }
>>>> +
>>>> +    mutex_lock(&battmgr->lock);
>>>> +    ret = qcom_battmgr_set_charge_control(battmgr, target_soc, 
>>>> delta_soc);
>>>> +    mutex_unlock(&battmgr->lock);
>>>> +    if (!ret) {
>>>> +        battmgr->info.charge_ctrl_start = soc;
>>>> +        battmgr->info.charge_ctrl_end = target_soc;
>>>> +    }
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static int qcom_battmgr_set_charge_end_threshold(struct 
>>>> qcom_battmgr *battmgr, int soc)
>>>> +{
>>>> +    u32 delta_soc = CHARGE_CTRL_DELTA_SOC;
>>>> +    int ret;
>>>> +
>>>> +    if (soc < CHARGE_CTRL_END_THR_MIN ||
>>>> +            soc > CHARGE_CTRL_END_THR_MAX) {
>>>> +        dev_err(battmgr->dev, "charge control end threshold exceed 
>>>> range: [%u - %u]\n",
>>>> +                CHARGE_CTRL_END_THR_MIN, CHARGE_CTRL_END_THR_MAX);
>>>> +        return -EINVAL;
>>>> +    }
>>>> +
>>>> +    if (battmgr->info.charge_ctrl_start && soc > 
>>>> battmgr->info.charge_ctrl_start)
>>>> +        delta_soc = soc - battmgr->info.charge_ctrl_start;
>>>> +
>>>> +    mutex_lock(&battmgr->lock);
>>>> +    ret = qcom_battmgr_set_charge_control(battmgr, soc, delta_soc);
>>>> +    mutex_unlock(&battmgr->lock);
>>>> +    if (!ret) {
>>>> +        battmgr->info.charge_ctrl_start = soc - delta_soc;
>>>> +        battmgr->info.charge_ctrl_end = soc;
>>>> +    }
>>>> +
>>>> +    return 0;
>>>> +}
>>>
>>> These function names sound quite generic, but AFAIU this patch is 
>>> only adding charge control support for the SM8550. Is sc8280xp and 
>>> x1e80100 also expected to be supported using the same 
>>> qcom_battmgr_charge_ctrl_request format?
>>
>> No, sc8280xp and x1e80100 don't support it. So I didn't add the 
>> support for them.
>
> And what about SM8650 and SM8750 ?
>
> Neil
>
Both SM8650 and SM8750 support charge control functionality. I saw 
SM8650 has already used "qcom,sm8550-pmic-glink" as fallback compatible 
string, so it will have it enabled by default when the change gets 
accepted. SM8750 platform can also use "qcom,sm8550-pmic-glink" as 
fallback to support it when uploading the DT change.

Fenglin

>>
>> These are generic functions are similar to 
>> "qcom_battmgr_update_charge_time" and "qcom_battmgr_update_info" 
>> which are only used for sc8280xp platform. Even right now charge 
>> control is only supported in mobile platforms starting from SM8550, 
>> however, it could be potentially supported in battery management 
>> firmware of any future platforms and the same functions could be reused.
>>
>>> Thanks,
>>> György
>>
>