Re: [PATCH v6 5/9] scsi: ufs: UFS crypto API

This line should be moved up to just below the previous statement adding
and object to fshcd-core-.

+static bool ufshcd_cap_idx_valid(struct ufs_hba *hba, unsigned int cap_idx)
+{
+	return cap_idx < hba->crypto_capabilities.num_crypto_cap;
+}
+
+static u8 get_data_unit_size_mask(unsigned int data_unit_size)
+{
+	if (data_unit_size < 512 || data_unit_size > 65536 ||
+	    !is_power_of_2(data_unit_size))
+		return 0;
+
+	return data_unit_size / 512;
+}
+
+static size_t get_keysize_bytes(enum ufs_crypto_key_size size)

Please add ufshcd_ prefixes to all the helpers.

+	pm_runtime_get_sync(hba->dev);
+	ufshcd_hold(hba, false);
+	/* Clear the dword 16 */
+	ufshcd_writel(hba, 0, slot_offset + 16 * sizeof(cfg->reg_val[0]));
+	/* Ensure that CFGE is cleared before programming the key */
+	wmb();
+	for (i = 0; i < 16; i++) {
+		ufshcd_writel(hba, le32_to_cpu(cfg->reg_val[i]),
+			      slot_offset + i * sizeof(cfg->reg_val[0]));
+		/* Spec says each dword in key must be written sequentially */
+		wmb();
+	}
+	/* Write dword 17 */
+	ufshcd_writel(hba, le32_to_cpu(cfg->reg_val[17]),
+		      slot_offset + 17 * sizeof(cfg->reg_val[0]));
+	/* Dword 16 must be written last */
+	wmb();
+	/* Write dword 16 */
+	ufshcd_writel(hba, le32_to_cpu(cfg->reg_val[16]),
+		      slot_offset + 16 * sizeof(cfg->reg_val[0]));
+	wmb();

All these wmb calls look bogus as writel itself orders mmio writes,
while wmb is not guaranteed to have any effect on mmio space.

+EXPORT_SYMBOL_GPL(ufshcd_crypto_enable);

+}
+EXPORT_SYMBOL_GPL(ufshcd_crypto_disable);

None of the exported symbols in this file is used outside
ufshcd-core.ko, so all the exports caan be dropped.

+	if (ufs_crypto_alg == UFS_CRYPTO_ALG_AES_XTS &&
+		key_size == UFS_CRYPTO_KEY_SIZE_256)
+		return BLK_ENCRYPTION_MODE_AES_256_XTS;

Please don't indent continuation lines of conditional with a single tab.

+static inline int ufshcd_num_keyslots(struct ufs_hba *hba)

+static inline bool ufshcd_keyslot_valid(struct ufs_hba *hba, unsigned int slot)

The two functions are only used in ufshcd-crypto.c and can be moved
there.

+static inline bool ufshcd_is_crypto_enabled(struct ufs_hba *hba)
+{
+	return hba->caps & UFSHCD_CAP_CRYPTO;
+}

I think this one would be clearer to just open code in the three callers.