Re: [PATCH v2] media: qcom: camss: Add support for regulator init_load_uA in CSIPHY

[Wenmeng Liu <quic_wenmliu@quicinc.com>]

On 2025/8/11 18:39, Dmitry Baryshkov wrote:
> On Tue, Jul 29, 2025 at 03:24:55PM +0800, Wenmeng Liu wrote:
>> Some Qualcomm regulators are configured with initial mode as
>> HPM (High Power Mode), which may lead to higher power consumption.
>> To reduce power usage, it's preferable to set the initial mode
>> to LPM (Low Power Mode).
>>
>> To ensure the regulator can switch from LPM to HPM when needed,
>> this patch adds current load configuration for CAMSS CSIPHY.
>> This allows the regulator framework to scale the mode dynamically
>> based on the load requirement.
>>
>> The current default value for current is uninitialized or random.
>> To address this, initial current values are added for the
>> following platforms:
>> SDM670, SM8250, SC7280, SM8550, and X1E80100.
>>
>> For SDM670, the value is set to -1, indicating that no default
>> current value is configured, the other values are derived
>> from the power grid.
>>
>> ---
>> Changes in v2:
>> - Change the source of the current value from DTS to CAMSS resource
>> - Link to v1: https://lore.kernel.org/all/20250620040736.3032667-1-quic_wenmliu@quicinc.com/
>> ---
>>
>> Signed-off-by: Wenmeng Liu <quic_wenmliu@quicinc.com>
>> ---
>>   drivers/media/platform/qcom/camss/camss-csiphy.c |  4 +++-
>>   drivers/media/platform/qcom/camss/camss.c        | 26 ++++++++++++++++++++++++
>>   drivers/media/platform/qcom/camss/camss.h        |  1 +
>>   3 files changed, 30 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c b/drivers/media/platform/qcom/camss/camss-csiphy.c
>> index 2de97f58f9ae4f91e8bba39dcadf92bea8cf6f73..7a2d80a03dbd0884b614451b55cd27dce94af637 100644
>> --- a/drivers/media/platform/qcom/camss/camss-csiphy.c
>> +++ b/drivers/media/platform/qcom/camss/camss-csiphy.c
>> @@ -707,8 +707,10 @@ int msm_csiphy_subdev_init(struct camss *camss,
>>   			return -ENOMEM;
>>   	}
>>   
>> -	for (i = 0; i < csiphy->num_supplies; i++)
>> +	for (i = 0; i < csiphy->num_supplies; i++) {
>>   		csiphy->supplies[i].supply = res->regulators[i];
>> +		csiphy->supplies[i].init_load_uA = res->regulators_current[i];
> 
> Could you possibly refactor to use devm_regulator_bulk_get_const()? It
> would save you from this data moving.
Initially, we were aiming for a minimal-change implementation.
Consider refactor for save data moving, will be refactored in the next 
version.
> 
>> +	}
>>   
>>   	ret = devm_regulator_bulk_get(camss->dev, csiphy->num_supplies,
>>   				      csiphy->supplies);
>> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
>> index e08e70b93824baa5714b3a736bc1d05405253aaa..daf21c944c2b4818b1656efc255e817551788658 100644
>> --- a/drivers/media/platform/qcom/camss/camss.c
>> +++ b/drivers/media/platform/qcom/camss/camss.c
>> @@ -750,6 +750,7 @@ static const struct camss_subdev_resources csiphy_res_670[] = {
>>   	/* CSIPHY0 */
>>   	{
>>   		.regulators = { "vdda-phy", "vdda-pll" },
>> +		.regulators_current = { -1, -1 },
> 
> If it's unset, it should be 0, not -1.
I considered existing implementations as a reference:
https://lore.kernel.org/all/20220804073608.v4.5.I55a9e65cb1c22221316629e98768ff473f47a067@changeid

but based on the implementation of regulator_bulk_get, setting it to 0 
seems to be a better approach:

drivers/regulator/core.c
consumers[i].init_load_uA > 0

Thanks,
Wenmeng