Re: [PATCH v2 05/17] ufs: core: mcq: Introduce Multi Circular Queue

Asutosh Das
2022-10-18T16:00:58Z

On Tue, Oct 18 2022 at 22:29 -0700, Eddie Huang wrote:
[...]
>> ---
>> drivers/ufs/core/Makefile | 2 +-
>> drivers/ufs/core/ufs-mcq.c | 113
>> +++++++++++++++++++++++++++++++++++++++++
[...]
>> create mode 100644 drivers/ufs/core/ufs-mcq.c
>>
>
>[...]
>
>> /**
>> * ufshcd_probe_hba - probe hba to detect device and initialize it
>> * @hba: per-adapter instance
>> @@ -8224,6 +8233,9 @@ static int ufshcd_probe_hba(struct ufs_hba
>> *hba, bool init_dev_params)
>> goto out;
>>
>> if (is_mcq_supported(hba)) {
>> + ret = ufshcd_config_mcq(hba);
[...]

>
>ufshcd_probe_hba() may be called multiple times (from
>ufshcd_async_scan() and ufshcd_host_reset_and_restore()). It is not a
>good idea to allocate memory in ufshcd_config_mcq(). Although use
>parameter init_dev_params to decide call ufshcd_config_mcq() or not, it
>may cause ufshcd_host_reset_and_restore() not to configure MCQ (init
>SQ/CQ ptr...) again.
>
I don't think the memory allocation can be moved prior to reading the device
descriptor since the bQueueDepth is necessary.
But I agree to your point that ufshcd_host_reset_and_restore() wouldn't
reconfigure MCQ now. Thanks.

>Suggest to separate configure MCQ (set hardware register) and allocate
>memory to different function
>
How about I keep the memory allocation in ufshcd_probe_hba() within the
init_dev_params check and separate out the initialization outside the check.
That'd ensure that the configuration is done for each call to
ufshcd_probe_hba(). I'm open to any other idea that you may have, plmk.
>
-asd