Re: [PATCH 2/2] power: supply: qcom_battmgr: Add thermal mitigation support

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

On 6/9/26 11:16 AM, DhruvinRajpura wrote:
> From: Dhruvin Rajpura <drajpura@qti.qualcomm.com>
> 
> Earlier commit c85c191694cb ("power: supply: remove faulty cooling
> logic") removed the automatic cooling device registration from
> the power supply framework due to inverted cooling logic and
> missing CHARGE_CONTROL_LIMIT_MAX validation across drivers. As
> a result, drivers that need thermal mitigation support must now
> explicitly register their own cooling devices with correct
> semantics.
> 
> The battery charger firmware exposes a fast charge current (FCC)
> limit property that can be used to throttle charging current in
> response to thermal events. Without explicit cooling device
> registration, the thermal framework has no way to reduce charging
> current when the device is under thermal stress, which can lead
> to excessive heat buildup during charging. Register a thermal
> cooling device that maps discrete current levels defined in the
> qcom,thermal-mitigation DT property to cooling states, where
> level 0 represents the hardware maximum current queried from
> firmware and each subsequent level reduces current to the next
> DT-defined value. Track the last applied FCC value so thermal
> limits are preserved across firmware protection domain restart
> (PDR) events. Expose CHARGE_CONTROL_LIMIT and
> CHARGE_CONTROL_LIMIT_MAX as power supply properties to allow
> userspace to observe and control the current throttle level.
> 
> Co-developed-by: Subbaraman Narayanamurthy <quic_subbaram@quicinc.com>
> Signed-off-by: Subbaraman Narayanamurthy <quic_subbaram@quicinc.com>
> Co-developed-by: Umang Chheda <quic_uchheda@quicinc.com>
> Signed-off-by: Umang Chheda <quic_uchheda@quicinc.com>
> Signed-off-by: Dhruvin Rajpura <drajpura@qti.qualcomm.com>
> ---

[...]

> +static int __battery_psy_set_charge_current(struct qcom_battmgr *battmgr,
> +					    u32 fcc_ua)
> +{
> +	int ret;
> +
> +	mutex_lock(&battmgr->lock);
> +	ret = qcom_battmgr_request_property(battmgr, BATTMGR_BAT_PROPERTY_SET,
> +					    BATT_CHG_CTRL_LIM, fcc_ua);
> +	mutex_unlock(&battmgr->lock);

I think the entirety of this function, particularly the assignment,
could use this lock

[...]

> +	if (ret < 0) {
> +		dev_err(battmgr->dev, "Unable to read CHG_CTRL_LIMIT_MAX ret :%d\n", ret);

style: "ret: %d"

[...]

> +static int qcom_battmgr_parse_dt(struct qcom_battmgr *battmgr)
> +{
> +	struct device_node *node = battmgr->dev->of_node;
> +	int len, ret;
> +
> +	ret = of_property_count_elems_of_size(node, "qcom,thermal-mitigation", sizeof(u32));
> +	if (ret == -EINVAL)
> +		return 0;               /* property not defined, thermal mitigation disabled */
> +	if (ret <= 0)
> +		return ret;              /* real error, propagate */

Does battmgr really expect the OS to house this information? Can we not
retrieve it from the running firmware?

[...]

> +	/*
> +	 * Element 0 is for normal charging current. Elements from index 1
> +	 * onwards is for thermal mitigation charging currents.
> +	 */

Documentation/ABI/testing/sysfs-class-power:

What:           /sys/class/power_supply/<supply_name>/charge_control_limit
Date:           Oct 2012
Contact:        linux-pm@vger.kernel.org
Description:
                Maximum allowable charging current. Used for charge rate
                throttling for thermal cooling or improving battery health.

                Access: Read, Write

                Valid values: Represented in microamps

My reading of your patch is that you expect a level to be passed, not a
uA value

Konrad