>On 24/06/21 1:44 pm, Keoseong Park wrote: >>> On 24/06/21 9:41 am, Keoseong Park wrote: >>>>> On 21/06/21 11:51 am, Keoseong Park wrote: >>>>>> Change conditional compilation to IS_ENABLED macro, >>>>>> and simplify if else statement to return statement. >>>>>> No functional change. >>>>>> >>>>>> Signed-off-by: Keoseong Park <keosung.park@xxxxxxxxxxx> >>>>>> --- >>>>>> drivers/scsi/ufs/ufshcd.h | 17 ++++++++--------- >>>>>> 1 file changed, 8 insertions(+), 9 deletions(-) >>>>>> >>>>>> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h >>>>>> index c98d540ac044..6d239a855753 100644 >>>>>> --- a/drivers/scsi/ufs/ufshcd.h >>>>>> +++ b/drivers/scsi/ufs/ufshcd.h >>>>>> @@ -893,16 +893,15 @@ static inline bool ufshcd_is_rpm_autosuspend_allowed(struct ufs_hba *hba) >>>>>> >>>>>> static inline bool ufshcd_is_intr_aggr_allowed(struct ufs_hba *hba) >>>>>> { >>>>>> -/* DWC UFS Core has the Interrupt aggregation feature but is not detectable*/ >>>>>> -#ifndef CONFIG_SCSI_UFS_DWC >>>>>> - if ((hba->caps & UFSHCD_CAP_INTR_AGGR) && >>>>>> - !(hba->quirks & UFSHCD_QUIRK_BROKEN_INTR_AGGR)) >>>>>> + /* >>>>>> + * DWC UFS Core has the Interrupt aggregation feature >>>>>> + * but is not detectable. >>>>>> + */ >>>>>> + if (IS_ENABLED(CONFIG_SCSI_UFS_DWC)) >>>>> >>>>> Why is this needed? It seems like you could just set UFSHCD_CAP_INTR_AGGR >>>>> and clear UFSHCD_QUIRK_BROKEN_INTR_AGGR instead? >>>> >>>> Hello Adrian, >>>> Sorry for late reply. >>>> >>>> The code that returns true when CONFIG_SCSI_UFS_DWC is set in the original code >>>> is only changed using the IS_ENABLED macro. >>>> (Linux kernel coding style, 21) Conditional Compilation) >>>> >>>> When CONFIG_SCSI_UFS_DWC is not defined, the code for checking quirk >>>> and caps has been moved to the newly added return statement below. >>> >>> Looking closer I cannot find CONFIG_SCSI_UFS_DWC at all. It seems like it >>> never existed. >>> >>> Why should we not remove the code related to CONFIG_SCSI_UFS_DWC entirely? >> >> You're right. What do you think of deleting the code related to CONFIG_SCSI_UFS_DWC >> and changing it to the patch below? > >Yes, but cc Joao Pinto <jpinto@xxxxxxxxxxxx> who introduced the code Thanks for your advice. I will upload next version patch by adding cc. Thanks, Keoseong > >> >> --- >> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h >> index c98d540ac044..c9faca237290 100644 >> --- a/drivers/scsi/ufs/ufshcd.h >> +++ b/drivers/scsi/ufs/ufshcd.h >> @@ -893,16 +893,8 @@ static inline bool ufshcd_is_rpm_autosuspend_allowed(struct ufs_hba *hba) >> >> static inline bool ufshcd_is_intr_aggr_allowed(struct ufs_hba *hba) >> { >> -/* DWC UFS Core has the Interrupt aggregation feature but is not detectable*/ >> -#ifndef CONFIG_SCSI_UFS_DWC >> - if ((hba->caps & UFSHCD_CAP_INTR_AGGR) && >> - !(hba->quirks & UFSHCD_QUIRK_BROKEN_INTR_AGGR)) >> - return true; >> - else >> - return false; >> -#else >> -return true; >> -#endif >> + return (hba->caps & UFSHCD_CAP_INTR_AGGR) && >> + !(hba->quirks & UFSHCD_QUIRK_BROKEN_INTR_AGGR); >> } >> >>> >>> >>>> >>>> Thanks, >>>> Keoseong >>>> >>>>> >>>>>> return true; >>>>>> - else >>>>>> - return false; >>>>>> -#else >>>>>> -return true; >>>>>> -#endif >>>>>> + >>>>>> + return (hba->caps & UFSHCD_CAP_INTR_AGGR) && >>>>>> + !(hba->quirks & UFSHCD_QUIRK_BROKEN_INTR_AGGR); >>>>>> } >>>>>> >>>>>> static inline bool ufshcd_can_aggressive_pc(struct ufs_hba *hba) >>>>>> >>>>> >>> >