[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <811c7c41-c52a-4c61-b35d-b921568429c3@quicinc.com>
Date: Thu, 8 May 2025 18:39:29 +0800
From: Ziqi Chen <quic_ziqichen@...cinc.com>
To: Luca Weiss <luca.weiss@...rphone.com>, <quic_cang@...cinc.com>,
<bvanassche@....org>, <mani@...nel.org>, <beanhuo@...ron.com>,
<avri.altman@....com>, <junwoo80.lee@...sung.com>,
<martin.petersen@...cle.com>, <quic_nguyenb@...cinc.com>,
<quic_nitirawa@...cinc.com>, <quic_rampraka@...cinc.com>,
<neil.armstrong@...aro.org>, <konrad.dybcio@....qualcomm.com>
CC: <linux-arm-msm@...r.kernel.org>, <linux-scsi@...r.kernel.org>,
"Manivannan
Sadhasivam" <manivannan.sadhasivam@...aro.org>,
"James E.J. Bottomley"
<James.Bottomley@...senpartnership.com>,
open list
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/3] scsi: ufs: qcom: Map devfreq OPP freq to UniPro
Core Clock freq
On 5/7/2025 5:59 PM, Luca Weiss wrote:
> On Wed May 7, 2025 at 11:09 AM CEST, Ziqi Chen wrote:
>> Hi Luca,
>>
>> On 5/7/2025 4:19 PM, Luca Weiss wrote:
>>> Hi Ziqi,
>>>
>>> On Wed May 7, 2025 at 9:44 AM CEST, Ziqi Chen wrote:
>>>> From: Can Guo <quic_cang@...cinc.com>
>>>>
>>>> On some platforms, the devfreq OPP freq may be different than the unipro
>>>> core clock freq. Implement ufs_qcom_opp_freq_to_clk_freq() and use it to
>>>> find the unipro core clk freq.
>>>>
>>>> Signed-off-by: Can Guo <quic_cang@...cinc.com>
>>>> Co-developed-by: Ziqi Chen <quic_ziqichen@...cinc.com>
>>>> Signed-off-by: Ziqi Chen <quic_ziqichen@...cinc.com>
>>>> ---
>>>> drivers/ufs/host/ufs-qcom.c | 81 ++++++++++++++++++++++++++++++++-----
>>>> 1 file changed, 71 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
>>>> index 7f10926100a5..804c8ccd8d03 100644
>>>> --- a/drivers/ufs/host/ufs-qcom.c
>>>> +++ b/drivers/ufs/host/ufs-qcom.c
>>>>
>>>> +static unsigned long ufs_qcom_opp_freq_to_clk_freq(struct ufs_hba *hba,
>>>> + unsigned long freq, char *name)
>>>> +{
>>>> + struct ufs_clk_info *clki;
>>>> + struct dev_pm_opp *opp;
>>>> + unsigned long clk_freq;
>>>> + int idx = 0;
>>>> + bool found = false;
>>>> +
>>>> + opp = dev_pm_opp_find_freq_exact_indexed(hba->dev, freq, 0, true);
>>>> + if (IS_ERR(opp)) {
>>>> + dev_err(hba->dev, "Failed to find OPP for exact frequency %lu\n", freq);
>>>
>>> I'm hitting this print on bootup:
>>>
>>> [ 0.512515] ufshcd-qcom 1d84000.ufshc: Failed to find OPP for exact frequency 18446744073709551615
>>> [ 0.512571] ufshcd-qcom 1d84000.ufshc: Failed to find OPP for exact frequency 18446744073709551615
>>>
>>> Doesn't look like it's intended? The number is (2^64 - 1)
>>>
>> Yes, this is expected. During link startup, the frequency
>> ULONG_MAX will be passed to ufs_qcom_set_core_clk_ctrl() and
>> ufs_qcom_cfg_timer(). This frequency cannot be found through the API
>> dev_pm_opp_find_freq_exact_indexed(). Therefore, we handle the
>> frequency ULONG_MAX separately within Ufs_qcom_set_core_clk_ctrl()
>> and ufs_qcom_cfg_timer().
>>
>> This print only be print twice during link startup. If you think print
>> such print during bootup is not make sense, I can improve the code and
>> update a new vwesion.
>
> I'll let others comment on what should happen but certainly this large
> number looks more like a mistake, like an integer overflow, if you don't
> dig into what this number is supposed to represent.
>
> Perhaps an idea could be to just skip the print (or even more code) for
> ULONG_MAX since an opp for that is not supposed to exist anyways?
>
> I didn't check the code now but for other frequencies this would be an
> actual error I imagine where it should be visible.
>
sure, let me clear away this prints in next version.
BRs,
Ziqi
> Regards
> Luca
>
>>
>> BRs.
>> Ziqi
>>
>>> Regards
>>> Luca
>>>
>
Powered by blists - more mailing lists