On 8/14/24 10:16 PM, Manivannan Sadhasivam via B4 Relay wrote:
/*
* The UFSHCI 3.0 specification does not define MCQ_SUPPORT and
- * LSDB_SUPPORT, but [31:29] as reserved bits with reset value 0s, which
+ * LSDBS_SUPPORT, but [31:29] as reserved bits with reset value 0s, which
* means we can simply read values regardless of version.
*/
Hmm ... neither MCQ_SUPPORT nor LSDBS_SUPPORT occurs in the UFSHCI 4.0
specification. I found the acronyms "MCQS" and "LSDBS" in that
specification. I propose either not to modify the above comment or to
use the acronyms used in the UFSHCI 4.0 standard.
hba->mcq_sup = FIELD_GET(MASK_MCQ_SUPPORT, hba->capabilities);
@@ -2426,7 +2426,7 @@ static inline int ufshcd_hba_capabilities(struct ufs_hba *hba)
* 0h: legacy single doorbell support is available
* 1h: indicate that legacy single doorbell support has been removed
*/
- hba->lsdb_sup = !FIELD_GET(MASK_LSDB_SUPPORT, hba->capabilities);
+ hba->lsdbs_sup = !FIELD_GET(MASK_LSDBS_SUPPORT, hba->capabilities);
if (!hba->mcq_sup)
return 0;
The final "s" in "lsdbs" stands for "support" so there are now two
references to the word "support" in the "lsdbs_sup" member name. Isn't
the original structure member name ("lsdb_sup") better because it
doesn't have that redundancy?
MASK_CRYPTO_SUPPORT = 0x10000000,
- MASK_LSDB_SUPPORT = 0x20000000,
+ MASK_LSDBS_SUPPORT = 0x20000000,
MASK_MCQ_SUPPORT = 0x40000000,
Same comment here: in the constant name "MASK_LSDBS_SUPPORT" there are
two references to the word "support". Isn't the original name better?
Additionally, this change introduces an inconsistency between the
constant names "MASK_LSDBS_SUPPORT" and "MASK_MCQ_SUPPORT". The former
name includes the acronym from the spec (LSDBS) but the latter name not
(MCQS). Wouldn't it be better to leave this change out?
Thanks,
Bart.
