| Message ID | 20230929-fp5-ufs-v1-1-122941e28b06@fairphone.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm64: dts: qcom: qcm6490-fairphone-fp5: Enable UFS | expand |
On 29.09.2023 11:52, Luca Weiss wrote: > Enable the UFS phy and controller so that we can access the internal > storage of the phone. > > At the same time we need to bump the minimum voltage used for UFS VCC, > otherwise it doesn't initialize properly. The new range is taken from > the vcc-voltage-level property downstream. > > See also the following link for more information about the VCCQ/VCCQ2: > https://gerrit-public.fairphone.software/plugins/gitiles/kernel/msm-extra/devicetree/+/1590a3739e7dc29d2597307881553236d492f188/fp5/yupik-idp-pm7250b.dtsi#207 > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> > --- > I'm not 100% convinced about the regulator range change. For sure with > the original voltage range the UFS fails to initialize, but looking at > downstream kernel during runtime (debugfs) we see the VCC voltage > switches between 2.4V (idle?) and 2.952V (active?). But even with this > change in mainline the regulator would always stay at 2.504V which is > for sure lower than the downstream operating voltage of 2.952V. Behavior > wise I don't see a difference between ~2.5V and ~2.9V. > > Should I just constrain the regulator here to min=max=2.952V? Or just > say it's okay as-is? > > Depends on: https://lore.kernel.org/linux-arm-msm/20230927081858.15961-1-quic_nitirawa@quicinc.com/ > --- There's a little funny hack inside the driver #if defined(CONFIG_SCSI_UFSHCD_QTI) if (vreg->low_voltage_sup && !vreg->low_voltage_active && on) min_uV = vreg->max_uV; #endif so, when the ufs is in use, it's pinned to vmax Konrad
On Fri Sep 29, 2023 at 3:12 PM CEST, Konrad Dybcio wrote: > On 29.09.2023 11:52, Luca Weiss wrote: > > Enable the UFS phy and controller so that we can access the internal > > storage of the phone. > > > > At the same time we need to bump the minimum voltage used for UFS VCC, > > otherwise it doesn't initialize properly. The new range is taken from > > the vcc-voltage-level property downstream. > > > > See also the following link for more information about the VCCQ/VCCQ2: > > https://gerrit-public.fairphone.software/plugins/gitiles/kernel/msm-extra/devicetree/+/1590a3739e7dc29d2597307881553236d492f188/fp5/yupik-idp-pm7250b.dtsi#207 > > > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> > > --- > > I'm not 100% convinced about the regulator range change. For sure with > > the original voltage range the UFS fails to initialize, but looking at > > downstream kernel during runtime (debugfs) we see the VCC voltage > > switches between 2.4V (idle?) and 2.952V (active?). But even with this > > change in mainline the regulator would always stay at 2.504V which is > > for sure lower than the downstream operating voltage of 2.952V. Behavior > > wise I don't see a difference between ~2.5V and ~2.9V. > > > > Should I just constrain the regulator here to min=max=2.952V? Or just > > say it's okay as-is? > > > > Depends on: https://lore.kernel.org/linux-arm-msm/20230927081858.15961-1-quic_nitirawa@quicinc.com/ > > --- > There's a little funny hack inside the driver > > #if defined(CONFIG_SCSI_UFSHCD_QTI) > if (vreg->low_voltage_sup && !vreg->low_voltage_active && on) > min_uV = vreg->max_uV; > #endif > > so, when the ufs is in use, it's pinned to vmax Hi Konrad, Are you implying I *should* or *should not* pin the voltage range to 2.952V-2.952V for mainline? Regards Luca > > Konrad
On 10/2/23 09:02, Luca Weiss wrote: > On Fri Sep 29, 2023 at 3:12 PM CEST, Konrad Dybcio wrote: >> On 29.09.2023 11:52, Luca Weiss wrote: >>> Enable the UFS phy and controller so that we can access the internal >>> storage of the phone. >>> >>> At the same time we need to bump the minimum voltage used for UFS VCC, >>> otherwise it doesn't initialize properly. The new range is taken from >>> the vcc-voltage-level property downstream. >>> >>> See also the following link for more information about the VCCQ/VCCQ2: >>> https://gerrit-public.fairphone.software/plugins/gitiles/kernel/msm-extra/devicetree/+/1590a3739e7dc29d2597307881553236d492f188/fp5/yupik-idp-pm7250b.dtsi#207 >>> >>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> >>> --- >>> I'm not 100% convinced about the regulator range change. For sure with >>> the original voltage range the UFS fails to initialize, but looking at >>> downstream kernel during runtime (debugfs) we see the VCC voltage >>> switches between 2.4V (idle?) and 2.952V (active?). But even with this >>> change in mainline the regulator would always stay at 2.504V which is >>> for sure lower than the downstream operating voltage of 2.952V. Behavior >>> wise I don't see a difference between ~2.5V and ~2.9V. >>> >>> Should I just constrain the regulator here to min=max=2.952V? Or just >>> say it's okay as-is? >>> >>> Depends on: https://lore.kernel.org/linux-arm-msm/20230927081858.15961-1-quic_nitirawa@quicinc.com/ >>> --- >> There's a little funny hack inside the driver >> >> #if defined(CONFIG_SCSI_UFSHCD_QTI) >> if (vreg->low_voltage_sup && !vreg->low_voltage_active && on) >> min_uV = vreg->max_uV; >> #endif >> >> so, when the ufs is in use, it's pinned to vmax > > Hi Konrad, > > Are you implying I *should* or *should not* pin the voltage range to > 2.952V-2.952V for mainline? Neither, voltage scaling should be implemented :P But for now, pinning it to 2.952 const is the right temporary solution, as having working UFS is generally better than one that can only idle in a stable manner :D Konrad
diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts index 2de0b8c26c35..fea7639fc0bc 100644 --- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts +++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts @@ -182,8 +182,9 @@ vreg_l6b: ldo6 { }; vreg_l7b: ldo7 { - regulator-min-microvolt = <2400000>; - regulator-max-microvolt = <3544000>; + /* Constrained for UFS VCC */ + regulator-min-microvolt = <2504000>; + regulator-max-microvolt = <2952000>; regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; }; @@ -632,6 +633,28 @@ bluetooth: bluetooth { }; }; +&ufs_mem_hc { + reset-gpios = <&tlmm 175 GPIO_ACTIVE_LOW>; + + vcc-supply = <&vreg_l7b>; + vcc-max-microamp = <800000>; + /* + * Technically l9b enables an eLDO (supplied by s1b) which then powers + * VCCQ2 of the UFS. + */ + vccq-supply = <&vreg_l9b>; + vccq-max-microamp = <900000>; + + status = "okay"; +}; + +&ufs_mem_phy { + vdda-phy-supply = <&vreg_l10c>; + vdda-pll-supply = <&vreg_l6b>; + + status = "okay"; +}; + &usb_1 { status = "okay"; };
Enable the UFS phy and controller so that we can access the internal storage of the phone. At the same time we need to bump the minimum voltage used for UFS VCC, otherwise it doesn't initialize properly. The new range is taken from the vcc-voltage-level property downstream. See also the following link for more information about the VCCQ/VCCQ2: https://gerrit-public.fairphone.software/plugins/gitiles/kernel/msm-extra/devicetree/+/1590a3739e7dc29d2597307881553236d492f188/fp5/yupik-idp-pm7250b.dtsi#207 Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> --- I'm not 100% convinced about the regulator range change. For sure with the original voltage range the UFS fails to initialize, but looking at downstream kernel during runtime (debugfs) we see the VCC voltage switches between 2.4V (idle?) and 2.952V (active?). But even with this change in mainline the regulator would always stay at 2.504V which is for sure lower than the downstream operating voltage of 2.952V. Behavior wise I don't see a difference between ~2.5V and ~2.9V. Should I just constrain the regulator here to min=max=2.952V? Or just say it's okay as-is? Depends on: https://lore.kernel.org/linux-arm-msm/20230927081858.15961-1-quic_nitirawa@quicinc.com/ --- arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 27 ++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) --- base-commit: d85348daa4407216e47198ed35a43a66883edab6 change-id: 20230929-fp5-ufs-e2c0e21a0142 Best regards,