Re: [PATCH 3/8] platform: arm64: qcom-hamoa-ec: Switch fan profile based on power supply state

[Anvesh Jain P <anvesh.p@oss.qualcomm.com>]

On 7/29/2026 4:32 PM, Konrad Dybcio wrote:
> On 7/28/26 7:44 PM, Anvesh Jain P wrote:
>> Add the EC command definitions and handler functions for switching and
>> reading the EC fan profile.
>>
>> Register a power supply notifier so the driver can react to AC/battery
>> transitions: on each PSY_EVENT_PROP_CHANGED notification, queue work
>> that checks power_supply_is_system_supplied() and, if the AC/battery
>> state has actually changed, switches the fan profile to the
>> performance profile on AC or the battery-saver profile on battery.
>> Track the last-applied state in on_ac_power to avoid redundant EC
>> writes. Run the initial sync once at probe time, and defer the actual
>> switch to a work item since the notifier callback runs in atomic
>> context.
>>
>> Serialize the fan profile write under the existing io_lock mutex,
>> alongside the other EC command sequences.
>>
>> Signed-off-by: Anvesh Jain P <anvesh.p@oss.qualcomm.com>
>> ---
> 
> [...]
> 
>> +static int qcom_ec_set_fan_profile(struct qcom_ec *ec, u8 profile_id)
>> +{
>> +	int ret;
>> +
>> +	ret = i2c_smbus_write_byte_data(ec->client, EC_FAN_PROFILE_CMD, profile_id);
> 
> Shouldn't all the accesses be serialized after patch 2?
>

It already is, both call sites hold ec->io_lock around these calls, same
as every other qcom_ec_read()/qcom_ec_write() site in the driver. None
of these low-level accessors take the lock themselves; that's
intentional so callers can wrap multi-command sequences atomically. No
gap here.

> [...]
> 
>> +static int qcom_ec_update_profile_from_power_supply(struct qcom_ec *ec)
>> +{
>> +	int on_ac_power;
>> +	u8 profile;
>> +	int ret = 0;
>> +
>> +	on_ac_power = power_supply_is_system_supplied() > 0 ? 1 : 0;
> 
> Comparisons already return a boolean value (i.e. the ternary operator is
> not necessary here)
>

Ack, will drop the ternary — on_ac_power =
power_supply_is_system_supplied() > 0; is equivalent.

>> +
>> +	profile = on_ac_power ? EC_FAN_PROFILE_BEST_PERF_PLUGGED_IN :
>> +				EC_FAN_PROFILE_BETTER_BATT_ON_BATT;
> 
> Is being on a (powerful) charger a prerequisite for entering this profile?
> Currently plugging in a 5W charger would trigger the switch as well.
> 
> Should this instead be exposed as a fan profile for userspace?
> 
> Konrad

Fair point — power_supply_is_system_supplied() only checks mains
presence, not charger wattage, so even a 5W charger triggers the "best
performance" profile. And exposing this via platform_profile instead of
an automatic AC/battery switch would let userspace make the call
instead. Will check on the feasibility of making this possible.

-- 
Best Regards,
Anvesh