Re: [PATCH v1 1/2] ufs: core: Configure only active lanes during link

From: Palash Kambar

Date: Thu Mar 26 2026 - 12:24:46 EST




On 3/23/2026 9:13 PM, Bart Van Assche wrote:
> On 3/10/26 11:09 PM, palash.kambar@xxxxxxxxxxxxxxxx wrote:
>> [ ... ]
>> +static int ufshcd_get_connected_tx_lanes(struct ufs_hba *hba, u32 *tx_lanes)
>> +{
>> +    return ufshcd_dme_get(hba,
>> +                  UIC_ARG_MIB(PA_CONNECTEDTXDATALANES), tx_lanes);
>> +}
>> +
>> +static int ufshcd_get_connected_rx_lanes(struct ufs_hba *hba, u32 *rx_lanes)
>> +{
>> +    return ufshcd_dme_get(hba,
>> +                  UIC_ARG_MIB(PA_CONNECTEDRXDATALANES), rx_lanes);
>> +}
>
> The body of the above two functions is very short. Please remove these
> functions and instead inline these function into their only caller.

Sure Bart, will address this.
>
>> +static void ufshcd_validate_link_params(struct ufs_hba *hba)
>> +{
>> +    int val = 0;
>> +
>> +    if (ufshcd_get_connected_tx_lanes(hba, &val))
>> +        return;
>
> Shouldn't it be reported if ufshcd_get_connected_tx_lanes() fails?
>
>> +    if (ufshcd_get_connected_rx_lanes(hba, &val))
>> +        return;
>
> Same question here - shouldn't it be reported if
> ufshcd_get_connected_rx_lanes() fails?
>
> Why does this patch only call dev_err() in case of a mismatch instead of adjusting hba->lanes_per_direction or making initialization fail?

Idea was just to warn and catch the failure scenario. But I agree on your feedback. Will fix it.

>
> Thanks,
>
> Bart.