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.>
+ }
>
> err = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR,
> QUERY_ATTR_IDN_REF_CLK_FREQ, 0, 0,
> &dev_ref_clk_freq);
> ""
>
> How about we simply get rid of the whole trivial fail path and directly
> return a return value instead of the 'goto' statements ? That should
> cover all the weird corner cases too.
I was worried you'd say that :P sure I'll give it a whirl.
Kind regards,
--
// Casey (she/her)
