On 08/01/2026 21:34, Marek Vasut wrote:
> On 1/8/26 9:09 PM, Casey Connolly wrote:
>>
>>
>> On 08/01/2026 20:55, Marek Vasut wrote:
>>> On 1/8/26 8:16 PM, Casey Connolly wrote:
>>>> When host_ref_clk_freq is 0, err will be returned before being set.
>>>> Assign a default value to fix the warning.
>>>
>>> Indeed, nice find.
>>>
>>>> +++ b/drivers/ufs/ufs-uclass.c
>>>> @@ -1854,9 +1854,9 @@ enum ufs_ref_clk_freq
>>>> ufshcd_parse_dev_ref_clk_freq(struct ufs_hba *hba, struct
>>>> }
>>>> static int ufshcd_set_dev_ref_clk(struct ufs_hba *hba)
>>>> {
>>>> - int err;
>>>> + int err = 0;
>>>> struct clk *ref_clk;
>>>> u32 host_ref_clk_freq;
>>>> u32 dev_ref_clk_freq;
>>> There is another weird piece in this function, look at this:
>>>
>>> ""
>>> diff --git a/drivers/ufs/ufs-uclass.c b/drivers/ufs/ufs-uclass.c
>>> index 3c8e4299259..da46b3ebdfd 100644
>>> --- a/drivers/ufs/ufs-uclass.c
>>> +++ b/drivers/ufs/ufs-uclass.c
>>> @@ -1868,12 +1868,11 @@ static int ufshcd_set_dev_ref_clk(struct ufs_hba
>>> *hba)
>>> }
>>>
>>> host_ref_clk_freq = ufshcd_parse_dev_ref_clk_freq(hba,
>>> ref_clk);
>>> - if (host_ref_clk_freq == REF_CLK_FREQ_INVAL)
>>> + if (host_ref_clk_freq == REF_CLK_FREQ_INVAL) {
>>> dev_err(hba->dev,
>>> "invalid ref_clk setting = %ld\n",
>>> clk_get_rate(ref_clk));
>>> -
>>> - if (host_ref_clk_freq == REF_CLK_FREQ_INVAL)
>>> - goto out;
>>> + return -EINVAL; // I think we want to bail with error ?
>>
>> I don't think we need to error out here, Qualcomm platforms don't have
>> this ref clk wired up properly yet but somehow things are working ok.>
>> + }
> This will print a warning. If on qualcomm platforms the UFS is working
> and these clock are not needed, then this should be fixed, likely by
> making the clock optional, or using fixed-clock in DT ?
I think the warning is what we want, it will be fixed properly once we
get a driver for rpmhcc which is what provides the ref clk. As for why
UFS works despite this I have no idea but I'd rather assume it's a lucky
accident.
Kind regards,
--
// Casey (she/her)
