Re: [PATCH v6 11/11] scsi: ufs: core: Move code out of an if-statement

[Neil Armstrong <neil.armstrong@linaro.org>]

Le 31/10/2024 à 18:51, Bart Van Assche a écrit :
> On 10/31/24 7:46 AM, Neil Armstrong wrote:
>> This change regresses the Qualcomm SM8650 Platforms, QRD and HDK boards fails to boot:
>> https://git.codelinaro.org/linaro/qcomlt/ci/staging/cdba-tester/-/jobs/182758#L1200
>>
>> [    5.155432] ufshcd-qcom 1d84000.ufshc: Resource ufs_mem not provided
>> [    5.155439] ufshcd-qcom 1d84000.ufshc: MCQ mode is disabled, err=-19
>> [    5.155443] ufshcd-qcom 1d84000.ufshc: ufshcd_add_scsi_host: failed to initialize (legacy doorbell mode not supported)
>> [    5.155874] ufshcd-qcom 1d84000.ufshc: error -EINVAL: Initialization failed with error -22
>>
>> then causes system crash:
>> [   15.400948] Internal error: Oops: 0000000096000006 [#1] PREEMPT SMP
>> [   15.667218] Call trace:
>> [   15.669833]  _raw_spin_lock_irqsave+0x34/0x8c (P)
>> [   15.674829]  pm_runtime_get_if_active+0x24/0x9c (L)
>> [   15.679998]  pm_runtime_get_if_active+0x24/0x9c
>> [   15.684811]  ufshcd_rtc_work+0x138/0x1b4
>> [   15.688991]  process_one_work+0x148/0x288
>> [   15.693258]  worker_thread+0x2cc/0x3d4
>> [   15.697248]  kthread+0x110/0x114
>> [   15.700703]  ret_from_fork+0x10/0x20
>> [   15.704516] Code: b9000841 d503201f 52800001 52800022 (88e17c02)
>> [   15.710956] ---[ end trace 0000000000000000 ]---
> 
> Hi Neil,
> 
> Thank you for the very detailed report. I think that two bugs are being
> reported:
> * Support for non-MCQ UFSHCI 4.0 controllers is broken.
> * The RTC update code is activated too early.
> 
> Is the patch below sufficient to fix both issues?

Yes it does!

Please add my:
Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # SM8650-QRD

Thanks,
Neil

> 
> Thanks,
> 
> Bart.
> 
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 0787387b7ce1..0b6b0cd4af33 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -8620,6 +8620,13 @@ static int ufshcd_add_lus(struct ufs_hba *hba)
>           ufshcd_init_clk_scaling_sysfs(hba);
>       }
> 
> +    /*
> +     * The RTC update code accesses the hba->ufs_device_wlun->sdev_gendev
> +     * pointer.
> +     */
> +    schedule_delayed_work(&hba->ufs_rtc_update_work,
> +                  msecs_to_jiffies(UFS_RTC_UPDATE_INTERVAL_MS));
> +
>       ufs_bsg_probe(hba);
>       scsi_scan_host(hba->host);
> 
> @@ -8714,8 +8721,6 @@ static int ufshcd_post_device_init(struct ufs_hba *hba)
>       ufshcd_force_reset_auto_bkops(hba);
> 
>       ufshcd_set_timestamp_attr(hba);
> -    schedule_delayed_work(&hba->ufs_rtc_update_work,
> -                  msecs_to_jiffies(UFS_RTC_UPDATE_INTERVAL_MS));
> 
>       if (!hba->max_pwr_info.is_valid)
>           return 0;
> @@ -10345,8 +10350,7 @@ static int ufshcd_add_scsi_host(struct ufs_hba *hba)
>               dev_err(hba->dev, "MCQ mode is disabled, err=%d\n",
>                   err);
>           }
> -    }
> -    if (!is_mcq_supported(hba) && !hba->lsdb_sup) {
> +    } else if (!hba->lsdb_sup) {
>           dev_err(hba->dev,
>               "%s: failed to initialize (legacy doorbell mode not supported)\n",
>               __func__);
>