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

[Wenmeng Liu]

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