>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? --- 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) >>>> >>> >