On 2016-11-08 21:43, Kiwoong Kim wrote:
> If UFS driver resets interrupt aggregation timer and counter
> when there is a pending doorbell, an interrupt of IO completion
> of a corresponding task may be missed.
> That would cause a command timeout.
>
> If UFS driver resets interrupt aggregation timer and counter
> when there is a pending doorbell, a competion interrupt
> of a corresponing task may be issued.
> That would casue a command timeout.
Please add how are you fixing the issue in commit text.
>
> Signed-off-by: Kiwoong Kim <kwmad.kim@xxxxxxxxxxx>
> ---
> drivers/scsi/ufs/ufshcd.c | 4 +++-
> drivers/scsi/ufs/ufshcd.h | 1 +
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index c904854..65bbf59 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -3730,7 +3730,9 @@ static void ufshcd_transfer_req_compl(struct
> ufs_hba *hba)
> * false interrupt if device completes another request after
> resetting
> * aggregation and before reading the DB.
> */
> - if (ufshcd_is_intr_aggr_allowed(hba))
> + if ((ufshcd_is_intr_aggr_allowed(hba))
> + && !(hba->quirks & UFSHCI_QUIRK_SKIP_INTR_AGGR))
Why do we need this new quirk? If controller has the issue with
interrupt aggregation, you can remove this UFSHCD_CAP_INTR_AGGR to
disable the aggregation altogether.
This fragment doesn't mean disabling interrupt aggregation.
It just means that interrupt aggregation logic would not be reset in
ISR.
ok.
But I think that the name of the quirk doesn't represent the function
of it
expectedly.
I'll change the name with another one, such as
UFSHCI_QUIRK_SKIP_RESET_INTR_AGGR.
May be UFSHCD_QUIRK_BROKEN_INTR_AGGR_RESET ?
> + ufshcd_reset_intr_aggr(hba);
> ufshcd_reset_intr_aggr(hba);
>
> tr_doorbell = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index 6a96f24..6a9c6e9 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -497,6 +497,7 @@ struct ufs_hba {
> #define UFSHCD_QUIRK_BROKEN_DWORD_UTRD UFS_BIT(7)
> #define UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR UFS_BIT(8)
> #define UFSHCD_QUIRK_USE_OF_HCE UFS_BIT(9)
> + #define UFSHCI_QUIRK_SKIP_INTR_AGGR UFS_BIT(10)
Please also add code comments explaining what this quirk means and when
should we use it? Please add same for other quirks you introduced in
different patches.
>
>
> unsigned int quirks; /* Deviations from standard UFSHCI spec.
*/
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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
