> +static int exynos7_ufs_pre_link(struct exynos_ufs *ufs) > +{ > + struct ufs_hba *hba = ufs->hba; > + u32 val = ufs->drv_data->uic_attr->pa_dbg_option_suite; Can pa_dbg_option_suite be replaced by a macro? > + exynos_ufs_disable_ov_tm(hba); > + > + ufshcd_dme_set(hba, UIC_ARG_MIB(PA_DBG_OPTION_SUITE_DYN), > 0xf); > + ufshcd_dme_set(hba, UIC_ARG_MIB(PA_DBG_OPTION_SUITE_DYN), > 0xf); A typo? Set PA_DBG_OPTION_SUITE_DYN twice? > +#define PWR_MODE_STR_LEN 64 > +static int exynos_ufs_post_pwr_mode(struct ufs_hba *hba, > + struct ufs_pa_layer_attr *pwr_max, > + struct ufs_pa_layer_attr *pwr_req) > +{ > + struct exynos_ufs *ufs = ufshcd_get_variant(hba); > + struct phy *generic_phy = ufs->phy; > + struct uic_pwr_mode *pwr = &ufs->pwr_act; > + char pwr_str[PWR_MODE_STR_LEN] = ""; Un-needed complication IMO - all those snprintf that is. > + > +static void exynos_ufs_fit_aggr_timeout(struct exynos_ufs *ufs) > +{ > + const u8 cntr_div = 40; Can be replaced by a macro? > +struct exynos_ufs_drv_data exynos_ufs_drvs = { > + > + .compatible = "samsung,exynos7-ufs", > + .uic_attr = &exynos7_uic_attr, > + .quirks = UFSHCD_QUIRK_PRDT_BYTE_GRAN | > + UFSHCI_QUIRK_BROKEN_REQ_LIST_CLR | > + UFSHCI_QUIRK_BROKEN_HCE | > + UFSHCI_QUIRK_SKIP_RESET_INTR_AGGR, > + .opts = EXYNOS_UFS_OPT_HAS_APB_CLK_CTRL | > + EXYNOS_UFS_OPT_BROKEN_AUTO_CLK_CTRL | > + EXYNOS_UFS_OPT_BROKEN_RX_SEL_IDX, In what way opts are different from quirks? Thanks, Avri