diff mbox series

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

Message ID 20241016201249.2256266-12-bvanassche@acm.org (mailing list archive)
State Accepted
Headers show
Series Combine the two UFS driver scsi_add_host() calls | expand

Commit Message

Bart Van Assche Oct. 16, 2024, 8:12 p.m. UTC 
The previous patch in this series introduced identical code in both
branches of an if-statement. Move that code outside the if-statement.

Reviewed-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 32 +++++++++++++-------------------
 1 file changed, 13 insertions(+), 19 deletions(-)

Comments

Neil Armstrong Oct. 31, 2024, 2:46 p.m. UTC | #1 
Hi,

On 16/10/2024 22:12, Bart Van Assche wrote:
> The previous patch in this series introduced identical code in both
> branches of an if-statement. Move that code outside the if-statement.
> 
> Reviewed-by: Bao D. Nguyen <quic_nguyenb@quicinc.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>   drivers/ufs/core/ufshcd.c | 32 +++++++++++++-------------------
>   1 file changed, 13 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index f34563e3a51d..70d89e154c4f 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -10400,26 +10400,20 @@ static int ufshcd_add_scsi_host(struct ufs_hba *hba)
>   			dev_err(hba->dev, "MCQ mode is disabled, err=%d\n",
>   				err);
>   		}
> -		err = scsi_add_host(hba->host, hba->dev);
> -		if (err) {
> -			dev_err(hba->dev, "scsi_add_host failed\n");
> -			return err;
> -		}
> -		hba->scsi_host_added = true;
> -	} else {
> -		if (!hba->lsdb_sup) {
> -			dev_err(hba->dev,
> -				"%s: failed to initialize (legacy doorbell mode not supported)\n",
> -				__func__);
> -			return -EINVAL;
> -		}
> -		err = scsi_add_host(hba->host, hba->dev);
> -		if (err) {
> -			dev_err(hba->dev, "scsi_add_host failed\n");
> -			return err;
> -		}
> -		hba->scsi_host_added = true;
>   	}
> +	if (!is_mcq_supported(hba) && !hba->lsdb_sup) {
> +		dev_err(hba->dev,
> +			"%s: failed to initialize (legacy doorbell mode not supported)\n",
> +			__func__);
> +		return -EINVAL;
> +	}
> +
> +	err = scsi_add_host(hba->host, hba->dev);
> +	if (err) {
> +		dev_err(hba->dev, "scsi_add_host failed\n");
> +		return err;
> +	}
> +	hba->scsi_host_added = true;
>   
>   	hba->tmf_tag_set = (struct blk_mq_tag_set) {
>   		.nr_hw_queues	= 1,
> 

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
<snip>
[   15.544573] CPU: 1 UID: 0 PID: 87 Comm: kworker/1:1 Tainted: G S                 6.12.0-rc5-next-20241030 #1
[   15.554938] Tainted: [S]=CPU_OUT_OF_SPEC
[   15.559098] Hardware name: Qualcomm Technologies, Inc. SM8650 QRD (DT)
[   15.565984] Workqueue: events ufshcd_rtc_work
[   15.570628] pstate: 234000c5 (nzCv daIF +PAN -UAO +TCO +DIT -SSBS BTYPE=--)
[   15.577986] pc : _raw_spin_lock_irqsave+0x34/0x8c
[   15.582981] lr : pm_runtime_get_if_active+0x24/0x9c
[   15.588159] sp : ffff800080df3d10
[   15.591679] x29: ffff800080df3d10 x28: 0000000000000000 x27: 0000000000000000
[   15.599235] x26: 0000000000000000 x25: ffff713a408fa1c0 x24: ffff713a40020205
[   15.606788] x23: 0000000000000000 x22: 0000000000000001 x21: ffff713a427ca848
[   15.614344] x20: 00000000000002a4 x19: 0000000000000000 x18: 0000000000000000
[   15.621896] x17: 0000000000000000 x16: 0000000000000000 x15: 00003a474f4572ec
[   15.629449] x14: 0000476e937c7726 x13: ffff713a40b64580 x12: 0000000000000001
[   15.637005] x11: 000000038cd4b97b x10: 3fb907ccb9c2fb76 x9 : 1eb87ffdfb6b34c8
[   15.644559] x8 : ffff713a40b655e8 x7 : ffff713a408fa200 x6 : 0000000000000020
[   15.652113] x5 : 000073746e657665 x4 : 0000000000000000 x3 : 000000003b9ac9ff
[   15.659665] x2 : 0000000000000001 x1 : 0000000000000000 x0 : 00000000000002a4
[   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 ]---

Since next-20241028, bisect log on next-20241030:
# bad: [86e3904dcdc7e70e3257fc1de294a1b75f3d8d04] Add linux-next specific files for 20241030
# good: [81983758430957d9a5cb3333fe324fd70cf63e7e] Linux 6.12-rc5
git bisect start 'HEAD' 'v6.12-rc5'
# good: [9bebee868a9f662d3829e7529b99d63f14245673] Merge branch 'nand/next' of git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git
git bisect good 9bebee868a9f662d3829e7529b99d63f14245673
# good: [64f1d5c3ad7542ea8f979988d2af75fd4e18148e] Merge branch 'for-backlight-next' of git://git.kernel.org/pub/scm/linux/kernel/git/lee/backlight.git
git bisect good 64f1d5c3ad7542ea8f979988d2af75fd4e18148e
# good: [38754c326c1fa76b673652b406d06d10004bf372] Merge branch 'char-misc-next' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
git bisect good 38754c326c1fa76b673652b406d06d10004bf372
# bad: [08b6666d1a597b0ac3753ed4228707e5067db303] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git
git bisect bad 08b6666d1a597b0ac3753ed4228707e5067db303
# good: [27709a220e05ec666c65b2266162bdd51a1f9b58] Merge branch 'spmi-next' of git://git.kernel.org/pub/scm/linux/kernel/git/sboyd/spmi.git
git bisect good 27709a220e05ec666c65b2266162bdd51a1f9b58
# good: [0edaa545afbbcd7e8ff162f9fd8852c3589d2fa6] staging: gpib: fmh_gpib: Fix typo
git bisect good 0edaa545afbbcd7e8ff162f9fd8852c3589d2fa6
# bad: [b92e5937e3523b0b7d41373681256bec78d7e134] scsi: ufs: core: Move code out of an if-statement
git bisect bad b92e5937e3523b0b7d41373681256bec78d7e134
# good: [a3517717c3c0dbad771f5e491191b4b7b69808fb] Merge patch series "scsi: hisi_sas: Some fixes for hisi_sas"
git bisect good a3517717c3c0dbad771f5e491191b4b7b69808fb
# good: [5824e18b3db468e6eb5e9ef226eed80db26f581a] scsi: ufs: core: Remove redundant host_lock calls around UTMRLCLR
git bisect good 5824e18b3db468e6eb5e9ef226eed80db26f581a
# good: [0936001322646a15d7091f61232e5ded9bf1883f] scsi: ufs: core: Convert a comment into an explicit check
git bisect good 0936001322646a15d7091f61232e5ded9bf1883f
# good: [a390e6677f4119e3b9e6364ac2c5cbe3ef1321a2] scsi: ufs: core: Expand the ufshcd_device_init(hba, true) call
git bisect good a390e6677f4119e3b9e6364ac2c5cbe3ef1321a2
# good: [72e979225ed2e9427396e317d33050bcf50ad899] scsi: ufs: core: Move the MCQ scsi_add_host() call
git bisect good 72e979225ed2e9427396e317d33050bcf50ad899
# first bad commit: [b92e5937e3523b0b7d41373681256bec78d7e134] scsi: ufs: core: Move code out of an if-statement

#regzbot introduced b92e5937e3523b0b7d41373681256bec78d7e134

Thanks,
Neil
Bart Van Assche Oct. 31, 2024, 5:51 p.m. UTC | #2 
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?

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__);
Neil Armstrong Oct. 31, 2024, 7:55 p.m. UTC | #3 
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__);
>
Bart Van Assche Oct. 31, 2024, 9:15 p.m. UTC | #4 
On 10/31/24 12:55 PM, Neil Armstrong wrote:
> Le 31/10/2024 à 18:51, Bart Van Assche a écrit :
>> Is the patch below sufficient to fix both issues?
> 
> Yes it does!

Thank you for having tested this patch quickly. Would it be possible
to test whether the patch below also fixes the reported boot failure?
I think the patch below is a better fix.

Thanks,

Bart.


diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index a5a0646bb80a..3b592492e152 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -874,7 +874,8 @@ static void ufs_qcom_advertise_quirks(struct ufs_hba 
*hba)
  	if (host->hw_ver.major > 0x3)
  		hba->quirks |= UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH;

-	if (of_device_is_compatible(hba->dev->of_node, "qcom,sm8550-ufshc"))
+	if (of_device_is_compatible(hba->dev->of_node, "qcom,sm8550-ufshc") ||
+	    of_device_is_compatible(hba->dev->of_node, "qcom,sm8650-ufshc"))
  		hba->quirks |= UFSHCD_QUIRK_BROKEN_LSDBS_CAP;
  }
Bart Van Assche Nov. 5, 2024, 10:01 p.m. UTC | #5 
On 10/31/24 2:15 PM, Bart Van Assche wrote:
> On 10/31/24 12:55 PM, Neil Armstrong wrote:
>> Le 31/10/2024 à 18:51, Bart Van Assche a écrit :
>>> Is the patch below sufficient to fix both issues?
>>
>> Yes it does!
> 
> Thank you for having tested this patch quickly. Would it be possible
> to test whether the patch below also fixes the reported boot failure?
> I think the patch below is a better fix.
> 
> Thanks,
> 
> Bart.
> 
> 
> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> index a5a0646bb80a..3b592492e152 100644
> --- a/drivers/ufs/host/ufs-qcom.c
> +++ b/drivers/ufs/host/ufs-qcom.c
> @@ -874,7 +874,8 @@ static void ufs_qcom_advertise_quirks(struct ufs_hba 
> *hba)
>       if (host->hw_ver.major > 0x3)
>           hba->quirks |= UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH;
> 
> -    if (of_device_is_compatible(hba->dev->of_node, "qcom,sm8550-ufshc"))
> +    if (of_device_is_compatible(hba->dev->of_node, "qcom,sm8550-ufshc") ||
> +        of_device_is_compatible(hba->dev->of_node, "qcom,sm8650-ufshc"))
>           hba->quirks |= UFSHCD_QUIRK_BROKEN_LSDBS_CAP;
>   }

(replying to my own email)

Can anyone who has access to a Qualcomm SM8650 Platform please help with
testing the above patch on top of linux-next?

Thanks,

Bart.
Neil Armstrong Nov. 6, 2024, 8:48 a.m. UTC | #6 
Hi,

On 05/11/2024 23:01, Bart Van Assche wrote:
> On 10/31/24 2:15 PM, Bart Van Assche wrote:
>> On 10/31/24 12:55 PM, Neil Armstrong wrote:
>>> Le 31/10/2024 à 18:51, Bart Van Assche a écrit :
>>>> Is the patch below sufficient to fix both issues?
>>>
>>> Yes it does!
>>
>> Thank you for having tested this patch quickly. Would it be possible
>> to test whether the patch below also fixes the reported boot failure?
>> I think the patch below is a better fix.
>>
>> Thanks,
>>
>> Bart.
>>
>>
>> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
>> index a5a0646bb80a..3b592492e152 100644
>> --- a/drivers/ufs/host/ufs-qcom.c
>> +++ b/drivers/ufs/host/ufs-qcom.c
>> @@ -874,7 +874,8 @@ static void ufs_qcom_advertise_quirks(struct ufs_hba *hba)
>>       if (host->hw_ver.major > 0x3)
>>           hba->quirks |= UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH;
>>
>> -    if (of_device_is_compatible(hba->dev->of_node, "qcom,sm8550-ufshc"))
>> +    if (of_device_is_compatible(hba->dev->of_node, "qcom,sm8550-ufshc") ||
>> +        of_device_is_compatible(hba->dev->of_node, "qcom,sm8650-ufshc"))
>>           hba->quirks |= UFSHCD_QUIRK_BROKEN_LSDBS_CAP;
>>   }
> 
> (replying to my own email)
> 
> Can anyone who has access to a Qualcomm SM8650 Platform please help with
> testing the above patch on top of linux-next?

Sorry I was traveling and I forgot about it, I'll try to test this today.

Neil

> 
> Thanks,
> 
> Bart.
Neil Armstrong Nov. 6, 2024, 9:57 a.m. UTC | #7 
+ Mani

On 05/11/2024 23:01, Bart Van Assche wrote:
> On 10/31/24 2:15 PM, Bart Van Assche wrote:
>> On 10/31/24 12:55 PM, Neil Armstrong wrote:
>>> Le 31/10/2024 à 18:51, Bart Van Assche a écrit :
>>>> Is the patch below sufficient to fix both issues?
>>>
>>> Yes it does!
>>
>> Thank you for having tested this patch quickly. Would it be possible
>> to test whether the patch below also fixes the reported boot failure?
>> I think the patch below is a better fix.
>>
>> Thanks,
>>
>> Bart.
>>
>>
>> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
>> index a5a0646bb80a..3b592492e152 100644
>> --- a/drivers/ufs/host/ufs-qcom.c
>> +++ b/drivers/ufs/host/ufs-qcom.c
>> @@ -874,7 +874,8 @@ static void ufs_qcom_advertise_quirks(struct ufs_hba *hba)
>>       if (host->hw_ver.major > 0x3)
>>           hba->quirks |= UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH;
>>
>> -    if (of_device_is_compatible(hba->dev->of_node, "qcom,sm8550-ufshc"))
>> +    if (of_device_is_compatible(hba->dev->of_node, "qcom,sm8550-ufshc") ||
>> +        of_device_is_compatible(hba->dev->of_node, "qcom,sm8650-ufshc"))
>>           hba->quirks |= UFSHCD_QUIRK_BROKEN_LSDBS_CAP;
>>   }
> 
> (replying to my own email)
> 
> Can anyone who has access to a Qualcomm SM8650 Platform please help with
> testing the above patch on top of linux-next?
> 
> Thanks,
> 
> Bart.

Thanks, it does fix the issue. But it won't scale since the next platforms will
probably need the same tweak in the future.

Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8650-HDK
Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8650-QRD

Thanks,
Neil
Bart Van Assche Nov. 6, 2024, 5:51 p.m. UTC | #8 
On 11/6/24 1:57 AM, Neil Armstrong wrote:
> Thanks, it does fix the issue. But it won't scale since the next 
> platforms will probably need the same tweak in the future.
> 
> Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8650-HDK
> Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8650-QRD

Hi Neil,

Thanks for testing!

The comment about future platforms is not clear to me. This patch fixes
support for a non-standard controller (reports UFSHCI version 4 but
supports UFSHCI 3 register set). Shouldn't all future platforms support
the UFSHCI 4 register set?

Thanks,

Bart.
Manivannan Sadhasivam Nov. 7, 2024, 10:49 a.m. UTC | #9 
On Wed, Nov 06, 2024 at 09:51:06AM -0800, Bart Van Assche wrote:
> On 11/6/24 1:57 AM, Neil Armstrong wrote:
> > Thanks, it does fix the issue. But it won't scale since the next
> > platforms will probably need the same tweak in the future.
> > 
> > Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8650-HDK
> > Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8650-QRD
> 
> Hi Neil,
> 
> Thanks for testing!
> 
> The comment about future platforms is not clear to me. This patch fixes
> support for a non-standard controller (reports UFSHCI version 4 but
> supports UFSHCI 3 register set). Shouldn't all future platforms support
> the UFSHCI 4 register set?
> 

Yeah, the future platforms should (hopefully) no longer have this issue.

- Mani
diff mbox series

Patch

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index f34563e3a51d..70d89e154c4f 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -10400,26 +10400,20 @@  static int ufshcd_add_scsi_host(struct ufs_hba *hba)
 			dev_err(hba->dev, "MCQ mode is disabled, err=%d\n",
 				err);
 		}
-		err = scsi_add_host(hba->host, hba->dev);
-		if (err) {
-			dev_err(hba->dev, "scsi_add_host failed\n");
-			return err;
-		}
-		hba->scsi_host_added = true;
-	} else {
-		if (!hba->lsdb_sup) {
-			dev_err(hba->dev,
-				"%s: failed to initialize (legacy doorbell mode not supported)\n",
-				__func__);
-			return -EINVAL;
-		}
-		err = scsi_add_host(hba->host, hba->dev);
-		if (err) {
-			dev_err(hba->dev, "scsi_add_host failed\n");
-			return err;
-		}
-		hba->scsi_host_added = true;
 	}
+	if (!is_mcq_supported(hba) && !hba->lsdb_sup) {
+		dev_err(hba->dev,
+			"%s: failed to initialize (legacy doorbell mode not supported)\n",
+			__func__);
+		return -EINVAL;
+	}
+
+	err = scsi_add_host(hba->host, hba->dev);
+	if (err) {
+		dev_err(hba->dev, "scsi_add_host failed\n");
+		return err;
+	}
+	hba->scsi_host_added = true;
 
 	hba->tmf_tag_set = (struct blk_mq_tag_set) {
 		.nr_hw_queues	= 1,