Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36;
Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Date:   Mon, 02 Oct 2023 09:02:38 +0200
Message-Id: <CVXREP4FCX4E.3M77P8JP1T27M@otso>
Cc:     <~postmarketos/upstreaming@lists.sr.ht>,
        <phone-devel@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>,
        <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] arm64: dts: qcom: qcm6490-fairphone-fp5: Enable UFS
From:   "Luca Weiss" <luca.weiss@fairphone.com>
To:     "Konrad Dybcio" <konrad.dybcio@linaro.org>,
        "Andy Gross" <agross@kernel.org>,
        "Bjorn Andersson" <andersson@kernel.org>,
        "Rob Herring" <robh+dt@kernel.org>,
        "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
        "Conor Dooley" <conor+dt@kernel.org>,
        "Nitin Rawat" <quic_nitirawa@quicinc.com>
References: <20230929-fp5-ufs-v1-1-122941e28b06@fairphone.com>
 <cac1b912-e08b-4643-b081-834fdee30ea7@linaro.org>
In-Reply-To: <cac1b912-e08b-4643-b081-834fdee30ea7@linaro.org>
Precedence: bulk

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.
> >=20
> > 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.
> >=20
> > See also the following link for more information about the VCCQ/VCCQ2:
> > https://gerrit-public.fairphone.software/plugins/gitiles/kernel/msm-ext=
ra/devicetree/+/1590a3739e7dc29d2597307881553236d492f188/fp5/yupik-idp-pm72=
50b.dtsi#207
> >=20
> > 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. Behavio=
r
> > wise I don't see a difference between ~2.5V and ~2.9V.
> >=20
> > Should I just constrain the regulator here to min=3Dmax=3D2.952V? Or ju=
st
> > say it's okay as-is?
> >=20
> > 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_a=
ctive && on)
>                                 min_uV =3D 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