[PATCH 12/13] ufs: core: remove link_startup_again logic

[Neil Armstrong]

Hi,

On 10/09/2024 13:36, Neha Malcom Francis wrote:
> Hi Neil
> 
> On 10/09/24 14:50, Neil Armstrong wrote:
>> The link_startup_again logic was added in Linux to handle device
>> that were set in LinkDown state, which should not be the case since U-boot
>> doesn't set LinkDown state are init, and Linux sets the device active
>> in ufshcd_init() for the first link startup.
>>
>> While it worked to far, it breaks link startup for Qualcomm Controllers v5,
>> let's just remove the logic.
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong at linaro.org>
> 
> I had sent a review comment here https://patchwork.ozlabs.org/project/uboot/patch/20240528-topic-sm8x50-ufs-core-link-startup-again-v1-1-146ca43e80b6@linaro.org/

Indeed I missed it, I'll fix the commit msg.

Nevertheless, as I explain in the cover letter the double link startup is
not done in Linux for the initial power-up:
https://elixir.bootlin.com/linux/v6.10.9/source/drivers/ufs/core/ufshcd.c#L10548

ufshcd_set_ufs_dev_active(hba) is called at ufshcd_init() right before
scheduling an ufshcd_async_scan that will call ufshcd_device_init() then ufshcd_link_startup().

The comment in probe says:
	/*
	 * We are assuming that device wasn't put in sleep/power-down
	 * state exclusively during the boot stage before kernel.
	 * This assumption helps avoid doing link startup twice during
	 * ufshcd_probe_hba().
	 */
we can assume the same from U-boot.

Neil
> 
> Would like Bhupesh to also have a look at this.
> 
>> ---
>>   drivers/ufs/ufs.c | 8 --------
>>   1 file changed, 8 deletions(-)
>>
>> diff --git a/drivers/ufs/ufs.c b/drivers/ufs/ufs.c
>> index d2df5c26f76..e34e4586224 100644
>> --- a/drivers/ufs/ufs.c
>> +++ b/drivers/ufs/ufs.c
>> @@ -462,9 +462,7 @@ static int ufshcd_link_startup(struct ufs_hba *hba)
>>   {
>>       int ret;
>>       int retries = DME_LINKSTARTUP_RETRIES;
>> -    bool link_startup_again = true;
>> -link_startup:
>>       do {
>>           ufshcd_ops_link_startup_notify(hba, PRE_CHANGE);
>> @@ -490,12 +488,6 @@ link_startup:
>>           /* failed to get the link up... retire */
>>           goto out;
>> -    if (link_startup_again) {
>> -        link_startup_again = false;
>> -        retries = DME_LINKSTARTUP_RETRIES;
>> -        goto link_startup;
>> -    }
>> -
>>       /* Mark that link is up in PWM-G1, 1-lane, SLOW-AUTO mode */
>>       ufshcd_init_pwr_info(hba);
>>
>