Tested-by: Yaniv Gardi <ygardi@xxxxxxxxxxxxxx> QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation = > -----Original Message----- = > From: linux-scsi-owner@xxxxxxxxxxxxxxx [mailto:linux-scsi- = > owner@xxxxxxxxxxxxxxx] On Behalf Of Subhash Jadavani = > Sent: Tuesday, August 27, 2013 12:02 PM = > To: Seungwon Jeon = > Cc: linux-scsi@xxxxxxxxxxxxxxx; 'Vinayak Holikatti'; 'Santosh Y'; 'James E.J. = > Bottomley' = > Subject: Re: [PATCH v3 2/6] scsi: ufs: fix the setting interrupt aggregation = > counter = > = > = > Looks good to me. = > Reviewed-by: Subhash Jadavani <subhashj@xxxxxxxxxxxxxx> = > = > On 8/26/2013 8:10 PM, Seungwon Jeon wrote: = > > IACTH(Interrupt aggregation counter threshold) value is allowed up to = > > 0x1F and current setting value is the maximum. = > > This value is related with NUTRS(max:0x20) of HCI's capability. = > > Considering HCI controller doesn't support the maximum, IACTH setting = > > should be adjusted with possible value. = > > For that, existing 'ufshcd_config_int_aggr' is split into two part = > > [reset, configure]. = > > = > > Signed-off-by: Seungwon Jeon <tgih.jun@xxxxxxxxxxx> = > > Reviewed-by: Subhash Jadavani <subhashj@xxxxxxxxxxxxxx> = > > --- = > > drivers/scsi/ufs/ufshcd.c | 53 +++++++++++++++++++++------------------ = > ----- = > > drivers/scsi/ufs/ufshci.h | 4 +- = > > 2 files changed, 27 insertions(+), 30 deletions(-) = > > = > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c = > > index 6ff16c9..c90b88a 100644 = > > --- a/drivers/scsi/ufs/ufshcd.c = > > +++ b/drivers/scsi/ufs/ufshcd.c = > > @@ -59,6 +59,9 @@ = > > /* Expose the flag value from utp_upiu_query.value */ = > > #define MASK_QUERY_UPIU_FLAG_LOC 0xFF = > > = > > +/* Interrupt aggregation default timeout, unit: 40us */ = > > +#define INT_AGGR_DEF_TO 0x02 = > > + = > > enum { = > > UFSHCD_MAX_CHANNEL = 0, = > > UFSHCD_MAX_ID = 1, = > > @@ -94,12 +97,6 @@ enum { = > > UFSHCD_INT_CLEAR, = > > }; = > > = > > -/* Interrupt aggregation options */ = > > -enum { = > > - INT_AGGR_RESET, = > > - INT_AGGR_CONFIG, = > > -}; = > > - = > > #define ufshcd_set_eh_in_progress(h) \ = > > (h->eh_flags |= UFSHCD_EH_IN_PROGRESS) = > > #define ufshcd_eh_in_progress(h) \ = > > @@ -340,30 +337,30 @@ static inline bool = > ufshcd_is_exception_event(struct utp_upiu_rsp *ucd_rsp_ptr) = > > } = > > = > > /** = > > - * ufshcd_config_int_aggr - Configure interrupt aggregation values. = > > - * Currently there is no use case where we want to configure = > > - * interrupt aggregation dynamically. So to configure interrupt = > > - * aggregation, #define = > INT_AGGR_COUNTER_THRESHOLD_VALUE and = > > - * INT_AGGR_TIMEOUT_VALUE are used. = > > + * ufshcd_reset_intr_aggr - Reset interrupt aggregation values. = > > * @hba: per adapter instance = > > - * @option: Interrupt aggregation option = > > */ = > > static inline void = > > -ufshcd_config_int_aggr(struct ufs_hba *hba, int option) = > > +ufshcd_reset_intr_aggr(struct ufs_hba *hba) = > > { = > > - switch (option) { = > > - case INT_AGGR_RESET: = > > - ufshcd_writel(hba, INT_AGGR_ENABLE | = > > - INT_AGGR_COUNTER_AND_TIMER_RESET, = > > - = > REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL); = > > - break; = > > - case INT_AGGR_CONFIG: = > > - ufshcd_writel(hba, INT_AGGR_ENABLE | = > INT_AGGR_PARAM_WRITE | = > > - INT_AGGR_COUNTER_THRESHOLD_VALUE | = > > - INT_AGGR_TIMEOUT_VALUE, = > > - = > REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL); = > > - break; = > > - } = > > + ufshcd_writel(hba, INT_AGGR_ENABLE | = > > + INT_AGGR_COUNTER_AND_TIMER_RESET, = > > + REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL); = > > +} = > > + = > > +/** = > > + * ufshcd_config_intr_aggr - Configure interrupt aggregation values. = > > + * @hba: per adapter instance = > > + * @cnt: Interrupt aggregation counter threshold = > > + * @tmout: Interrupt aggregation timeout value */ static inline void = > > +ufshcd_config_intr_aggr(struct ufs_hba *hba, u8 cnt, u8 tmout) { = > > + ufshcd_writel(hba, INT_AGGR_ENABLE | = > INT_AGGR_PARAM_WRITE | = > > + INT_AGGR_COUNTER_THLD_VAL(cnt) | = > > + INT_AGGR_TIMEOUT_VAL(tmout), = > > + REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL); = > > } = > > = > > /** = > > @@ -1523,7 +1520,7 @@ static int ufshcd_make_hba_operational(struct = > ufs_hba *hba) = > > ufshcd_enable_intr(hba, UFSHCD_ENABLE_INTRS); = > > = > > /* Configure interrupt aggregation */ = > > - ufshcd_config_int_aggr(hba, INT_AGGR_CONFIG); = > > + ufshcd_config_intr_aggr(hba, hba->nutrs - 1, INT_AGGR_DEF_TO); = > > = > > /* Configure UTRL and UTMRL base address registers */ = > > ufshcd_writel(hba, lower_32_bits(hba->utrdl_dma_addr), = > > @@ -1971,7 +1968,7 @@ static void ufshcd_transfer_req_compl(struct = > > ufs_hba *hba) = > > = > > /* Reset interrupt aggregation counters */ = > > if (int_aggr_reset) = > > - ufshcd_config_int_aggr(hba, INT_AGGR_RESET); = > > + ufshcd_reset_intr_aggr(hba); = > > } = > > = > > /** = > > diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h = > > index f1e1b74..739ae3a 100644 = > > --- a/drivers/scsi/ufs/ufshci.h = > > +++ b/drivers/scsi/ufs/ufshci.h = > > @@ -226,8 +226,8 @@ enum { = > > = > > #define MASK_UIC_COMMAND_RESULT 0xFF = > > = > > -#define INT_AGGR_COUNTER_THRESHOLD_VALUE (0x1F << 8) = > > -#define INT_AGGR_TIMEOUT_VALUE (0x02) = > > +#define INT_AGGR_COUNTER_THLD_VAL(c) (((c) & 0x1F) << 8) = > > +#define INT_AGGR_TIMEOUT_VAL(t) (((t) & 0xFF) << 0) = > > = > > /* Interrupt disable masks */ = > > enum { = > = > -- = > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the = > body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info = > at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html