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;
Message-ID: <3267eb4b-7154-200a-ec10-d795acc029f1@linaro.org>
Date:   Mon, 2 Oct 2023 11:14:36 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.1
Subject: Re: [PATCH] arm64: dts: qcom: qcm6490-fairphone-fp5: Enable UFS
Content-Language: en-US
To:     Luca Weiss <luca.weiss@fairphone.com>,
        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>
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
References: <20230929-fp5-ufs-v1-1-122941e28b06@fairphone.com>
 <cac1b912-e08b-4643-b081-834fdee30ea7@linaro.org>
 <CVXREP4FCX4E.3M77P8JP1T27M@otso>
From:   Konrad Dybcio <konrad.dybcio@linaro.org>
In-Reply-To: <CVXREP4FCX4E.3M77P8JP1T27M@otso>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


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