linux-kernel - Re: [RFC PATCH 0/3] Add current load setting for qcom camss csiphy

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] [day] [month] [year] [list]

Message-ID: <1c6d8e59-eb9b-4230-a7e5-f0ffc30b8877@quicinc.com>
Date: Fri, 27 Jun 2025 17:34:15 +0800
From: Wenmeng Liu <quic_wenmliu@...cinc.com>
To: Bryan O'Donoghue <bryan.odonoghue@...aro.org>, <rfoss@...nel.org>,
        <todor.too@...il.com>, <mchehab@...nel.org>, <robh@...nel.org>,
        <krzk+dt@...nel.org>, <conor+dt@...nel.org>, <andersson@...nel.org>,
        <konradybcio@...nel.org>, <akapatra@...cinc.com>,
        <hariramp@...cinc.com>
CC: <linux-media@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>,
        <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <quic_svankada@...cinc.com>, <quic_depengs@...cinc.com>,
        <quic_vikramsa@...cinc.com>
Subject: Re: [RFC PATCH 0/3] Add current load setting for qcom camss csiphy



On 2025/6/20 16:23, Bryan O'Donoghue wrote:
> On 20/06/2025 05:07, Wenmeng Liu wrote:
>> Currently qcom camss csiphy drivers don’t set regulator’s currents
>> load properly using Linux regulator framework. This causes every
>> regulator’s initial mode set as HPM (high current mode),
>> which may have higher power consumption.
>> To address this issue, add current configuration for CSIPHY.
>>
>> Wenmeng Liu (3):
>>    media: dt-bindings: Add regulator current load
>>    media: qcom: camss: csiphy: Add regulator current load setting
>>    arm64: dts: qcom: qcs6490-rb3gen2: Add csiphy current support
>>
>>   .../bindings/media/qcom,sc7280-camss.yaml     |  6 ++++
>>   .../qcs6490-rb3gen2-vision-mezzanine.dtso     |  1 +
>>   .../media/platform/qcom/camss/camss-csiphy.c  | 29 +++++++++++++++++++
>>   .../media/platform/qcom/camss/camss-csiphy.h  |  1 +
>>   4 files changed, 37 insertions(+)
>>
> 
> How are these load-currents determined ?

According to my discussion with hw colleague,current value is decided 
based on post silicon calibration, this value is fixed for the 
corresponding chip.

> Looking at other instances of setting current for PHYs
> 
>   grep -r regulator_set_load * | grep com
>             [git:camss-bugfix-6.17] ✖
> drivers/phy/qualcomm/phy-qcom-edp.c:    ret = regulator_set_load(edp- 
>  >supplies[0].consumer, 21800); /* 1.2 V vdda-phy */
> drivers/phy/qualcomm/phy-qcom-edp.c:    ret = regulator_set_load(edp- 
>  >supplies[1].consumer, 36000); /* 0.9 V vdda-pll */
> drivers/phy/qualcomm/phy-qcom-usb-hs.c:    ret = 
> regulator_set_load(uphy->v1p8, 50000);
> drivers/phy/qualcomm/phy-qcom-usb-hs.c:    ret = 
> regulator_set_load(uphy->v3p3, 50000);
> drivers/phy/qualcomm/phy-qcom-usb-hs-28nm.c:    ret = 
> regulator_set_load(priv->vregs[VDDA_1P8].consumer, 19000);
> drivers/phy/qualcomm/phy-qcom-usb-hs-28nm.c:    ret = 
> regulator_set_load(priv->vregs[VDDA_3P3].consumer, 16000);
> drivers/phy/qualcomm/phy-qcom-usb-hs-28nm.c: regulator_set_load(priv- 
>  >vregs[VDDA_1P8].consumer, 0);
> drivers/phy/qualcomm/phy-qcom-qmp-combo.c:        ret = 
> regulator_set_load(qmp->vregs[i].consumer,
> drivers/remoteproc/qcom_q6v5_pas.c:        regulator_set_load(adsp- 
>  >cx_supply, 100000);
> drivers/remoteproc/qcom_wcnss.c:            
> regulator_set_load(bulk[i].consumer, info[i].load_uA);
> drivers/remoteproc/qcom_wcnss_iris.c: regulator_set_load(iris- 
>  >vregs[i].consumer,
> drivers/remoteproc/qcom_q6v5_mss.c:            ret = 
> regulator_set_load(regs[i].reg,
> drivers/remoteproc/qcom_q6v5_mss.c:            
> regulator_set_load(regs[i].reg, 0);
> drivers/remoteproc/qcom_q6v5_mss.c:            
> regulator_set_load(regs[i].reg, 0);
> drivers/remoteproc/qcom_q6v5_wcss.c:    regulator_set_load(wcss- 
>  >cx_supply, 100000);
> 
> I think this is the type of thing we should bury in SoC resources not in 
> DT.
> 
> I can think of how we might want to change the current depending on the 
> pixel rate.. but then I think that is something we could calculate based 
> on pixel rate with descriptions as a base in
> 
> driver/media/platfrom/qcom/camss/camss.c::static const struct 
> camss_subdev_resources csiphy_res_SoCNumber[];
> 
> ---
> bod
> 

Yes, it's more common to put the current value in the code.Will be 
updated in v2.

Thanks,
Wenmeng