> +static size_t get_keysize_bytes(enum ufs_crypto_key_size size)
> +{
> + switch (size) {
> + case UFS_CRYPTO_KEY_SIZE_128: return 16;
> + case UFS_CRYPTO_KEY_SIZE_192: return 24;
> + case UFS_CRYPTO_KEY_SIZE_256: return 32;
> + case UFS_CRYPTO_KEY_SIZE_512: return 64;
> + default: return 0;
> + }
> +}
Please fix the indentation and move all the returns to their own
lines. There are various more spots that will need to be fixed
like this as well later in the patch.
> +
> +static int ufshcd_crypto_cap_find(void *hba_p,
> + enum blk_crypto_mode_num crypto_mode,
> + unsigned int data_unit_size)
> +{
> + struct ufs_hba *hba = hba_p;
Please properly type the first argument.
> + case UFS_CRYPTO_ALG_BITLOCKER_AES_CBC: // fallthrough
Please don't use // comments.
> +static void program_key(struct ufs_hba *hba,
> + const union ufs_crypto_cfg_entry *cfg,
> + int slot)
The function name needs a ufshcd prefix.
> + 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();
wmb() has no meaning for MMIO operations, something looks very fishy
here.
> +static int ufshcd_crypto_keyslot_program(void *hba_p, const u8 *key,
> + enum blk_crypto_mode_num crypto_mode,
> + unsigned int data_unit_size,
> + unsigned int slot)
> +{
> + struct ufs_hba *hba = hba_p;
This is not a very type safe API. I think the proper thing to do
would be to allocte the struct keyslot_manager in the driver (ufshcd)
as part of the containing structure (ufs_hba) and then just have
a keyslot_manager_init that initializes the field. Then pass the
struct keyslot_manager to the methods, which can use container_of
to get the containing structure.
> +#define NUM_KEYSLOTS(hba) (hba->crypto_capabilities.config_count + 1)
Please make this an inline function.
