On 7/8/24 3:44 AM, Manivannan Sadhasivam wrote:
On Sat, Jul 06, 2024 at 08:24:06PM -0700, Bart Van Assche wrote:
On 7/6/24 9:33 AM, Manivannan Sadhasivam wrote:
On Wed, Jul 03, 2024 at 01:36:46PM -0700, Bart Van Assche wrote:
If an UFSHCI controller is reset, the controller is reset from MCQ mode
to SDB mode and it is derived from the hba->mcq_enabled structure member
that MCQ was enabled before the reset. In other words, moving all
hba->mcq_enabled assignments into ufshcd_mcq_{enable/disable}() would
break the code that resets the UFSHCI controller.
Hmm, could you please point me to the code that does this? I tried looking for
it but couldn't spot.
* There is no "hba->mcq_enabled = false;" statement anywhere in the UFS
driver core. This shows that the reset code does not reset
hba->mcq_enabled.
Right. So this flag is used to reconfigure the MCQ mode upon HCI reset.
Previously we never disabled MCQ mode as well. But that is being changed by this
patch.
Only in an error path.
* From the UFSHCI specification, offset 300h, global config register:
in the "reset" column I see 0h for the queue type (QT) bit. In other
words, if a UFS host controller is reset, if MCQ is enabled (QT=1),
it must be disabled (QT=0) until the application processor enables it
again.
So this means, once the HCI is reset, the QT field will become '0' by default.
So if MCQ mode is supposed to be enabled, then driver has to explicitly enable
it.
But only place where you disable MCQ is when ufshcd_alloc_mcq() fails (in this
patch). Then you also set 'hba->mcq_enabled = false' afterwards. I fail to
understand why you cannot move the assignment to the enable/disable API itself.
If you do not set the flag after calling the ufshcd_mcq_disable() API, your
argument makes sense. But that's not the case. Perhaps I'm missing anything
obvious?
What do you want? That I move the "hba->mcq_enabled = false;" statement
into ufshcd_mcq_disable()? That would be wrong because (a) it would
introduce a confusing behavior difference between ufshcd_mcq_enable()
(does not modify hba->mcq_enabled) and ufshcd_mcq_disable() (would
modify hba->mcq_enabled if I implement your proposal) and (b) wouldn't
reduce the code size because ufshcd_mcq_disable() only has one caller.
Bart.
