* [PATCH v3] ufs: introduce UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR quirk
@ 2016-11-16 5:03 Kiwoong Kim
2016-11-16 18:52 ` Subhash Jadavani
2016-11-17 1:35 ` Martin K. Petersen
0 siblings, 2 replies; 5+ messages in thread
From: Kiwoong Kim @ 2016-11-16 5:03 UTC (permalink / raw)
To: linux-scsi; +Cc: cpgs, HeonGwang Chu
Some UFS host controllers may clear a transfer request slot
by setting an associated bit in UTRLCLR/UTMRLCLR to 1, not 0.
That's opposite to what UFS spec describes.
Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
---
drivers/scsi/ufs/ufshcd.c | 28 ++++++++++++++++++++++++++--
drivers/scsi/ufs/ufshcd.h | 7 +++++++
2 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index d6e3112..c9cf011 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -392,7 +392,31 @@ static inline void ufshcd_put_tm_slot(struct ufs_hba *hba, int slot)
*/
static inline void ufshcd_utrl_clear(struct ufs_hba *hba, u32 pos)
{
- ufshcd_writel(hba, ~(1 << pos), REG_UTP_TRANSFER_REQ_LIST_CLEAR);
+ u32 clear;
+
+ if (hba->quirks & UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR)
+ clear = (1 << pos);
+ else
+ clear = ~(1 << pos);
+
+ ufshcd_writel(hba, clear, REG_UTP_TRANSFER_REQ_LIST_CLEAR);
+}
+
+/**
+ * ufshcd_utmrl_clear - Clear a bit in UTRMLCLR register
+ * @hba: per adapter instance
+ * @pos: position of the bit to be cleared
+ */
+static inline void ufshcd_utmrl_clear(struct ufs_hba *hba, u32 pos)
+{
+ u32 clear;
+
+ if (hba->quirks & UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR)
+ clear = (1 << pos);
+ else
+ clear = ~(1 << pos);
+
+ ufshcd_writel(hba, clear, REG_UTP_TASK_REQ_LIST_CLEAR);
}
/**
@@ -4312,7 +4336,7 @@ static int ufshcd_clear_tm_cmd(struct ufs_hba *hba, int tag)
goto out;
spin_lock_irqsave(hba->host->host_lock, flags);
- ufshcd_writel(hba, ~(1 << tag), REG_UTP_TASK_REQ_LIST_CLEAR);
+ ufshcd_utmrl_clear(hba, tag);
spin_unlock_irqrestore(hba->host->host_lock, flags);
/* poll for max. 1 sec to clear door bell register by h/w */
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 7d9ff22..9838598 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -491,6 +491,13 @@ struct ufs_hba {
*/
#define UFSHCD_QUIRK_PRDT_BYTE_GRAN UFS_BIT(7)
+ /*
+ * This quirk needs to be enabled if the host contoller has to set
+ * the bit corresponding the slot to be cleared to 1, not 0 as
+ * described in UFS spec.
+ */
+ #define UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR UFS_BIT(8)
+
unsigned int quirks; /* Deviations from standard UFSHCI spec. */
/* Device deviations from standard UFS device spec. */
--
2.1.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v3] ufs: introduce UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR quirk
2016-11-16 5:03 [PATCH v3] ufs: introduce UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR quirk Kiwoong Kim
@ 2016-11-16 18:52 ` Subhash Jadavani
2016-11-17 13:52 ` Kiwoong Kim
2016-11-17 1:35 ` Martin K. Petersen
1 sibling, 1 reply; 5+ messages in thread
From: Subhash Jadavani @ 2016-11-16 18:52 UTC (permalink / raw)
To: Kiwoong Kim; +Cc: linux-scsi, cpgs, HeonGwang Chu, linux-scsi-owner
On 2016-11-15 21:03, Kiwoong Kim wrote:
> Some UFS host controllers may clear a transfer request slot
> by setting an associated bit in UTRLCLR/UTMRLCLR to 1, not 0.
> That's opposite to what UFS spec describes.
>
This was the comment on v2: "As Martin mentioned in other email, please
separate this version history from commit text with line having "----"
before the start of version history."
So i would have expected the patch version to be still v2 and just add
the version history separator added before version history. But you
posted v3 and removed the version history altogether. As far as patch
contents are concerned, it looks good to me hence i am adding by
reviewed-by. But please make sure to keep the version history intact for
fewer patches.
Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
> Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
> ---
> drivers/scsi/ufs/ufshcd.c | 28 ++++++++++++++++++++++++++--
> drivers/scsi/ufs/ufshcd.h | 7 +++++++
> 2 files changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index d6e3112..c9cf011 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -392,7 +392,31 @@ static inline void ufshcd_put_tm_slot(struct
> ufs_hba *hba, int slot)
> */
> static inline void ufshcd_utrl_clear(struct ufs_hba *hba, u32 pos)
> {
> - ufshcd_writel(hba, ~(1 << pos), REG_UTP_TRANSFER_REQ_LIST_CLEAR);
> + u32 clear;
> +
> + if (hba->quirks & UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR)
> + clear = (1 << pos);
> + else
> + clear = ~(1 << pos);
> +
> + ufshcd_writel(hba, clear, REG_UTP_TRANSFER_REQ_LIST_CLEAR);
> +}
> +
> +/**
> + * ufshcd_utmrl_clear - Clear a bit in UTRMLCLR register
> + * @hba: per adapter instance
> + * @pos: position of the bit to be cleared
> + */
> +static inline void ufshcd_utmrl_clear(struct ufs_hba *hba, u32 pos)
> +{
> + u32 clear;
> +
> + if (hba->quirks & UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR)
> + clear = (1 << pos);
> + else
> + clear = ~(1 << pos);
> +
> + ufshcd_writel(hba, clear, REG_UTP_TASK_REQ_LIST_CLEAR);
> }
>
> /**
> @@ -4312,7 +4336,7 @@ static int ufshcd_clear_tm_cmd(struct ufs_hba
> *hba, int tag)
> goto out;
>
> spin_lock_irqsave(hba->host->host_lock, flags);
> - ufshcd_writel(hba, ~(1 << tag), REG_UTP_TASK_REQ_LIST_CLEAR);
> + ufshcd_utmrl_clear(hba, tag);
> spin_unlock_irqrestore(hba->host->host_lock, flags);
>
> /* poll for max. 1 sec to clear door bell register by h/w */
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index 7d9ff22..9838598 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -491,6 +491,13 @@ struct ufs_hba {
> */
> #define UFSHCD_QUIRK_PRDT_BYTE_GRAN UFS_BIT(7)
>
> + /*
> + * This quirk needs to be enabled if the host contoller has to set
> + * the bit corresponding the slot to be cleared to 1, not 0 as
> + * described in UFS spec.
> + */
> + #define UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR UFS_BIT(8)
> +
> unsigned int quirks; /* Deviations from standard UFSHCI spec. */
>
> /* Device deviations from standard UFS device spec. */
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 5+ messages in thread* RE: [PATCH v3] ufs: introduce UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR quirk
2016-11-16 18:52 ` Subhash Jadavani
@ 2016-11-17 13:52 ` Kiwoong Kim
2016-11-17 19:29 ` Subhash Jadavani
0 siblings, 1 reply; 5+ messages in thread
From: Kiwoong Kim @ 2016-11-17 13:52 UTC (permalink / raw)
To: 'Subhash Jadavani'
Cc: linux-scsi, cpgs, 'HeonGwang Chu', linux-scsi-owner
> On 2016-11-15 21:03, Kiwoong Kim wrote:
> > Some UFS host controllers may clear a transfer request slot
> > by setting an associated bit in UTRLCLR/UTMRLCLR to 1, not 0.
> > That's opposite to what UFS spec describes.
> >
>
> This was the comment on v2: "As Martin mentioned in other email, please
> separate this version history from commit text with line having "----"
> before the start of version history."
> So i would have expected the patch version to be still v2 and just add
> the version history separator added before version history. But you
Okay. You mean re-sending it with moving the separator to another line,
don't you? I knew that I should update version even if there is a change
of commit message and version history. Thank you for your information.
> posted v3 and removed the version history altogether. As far as patch
> contents are concerned, it looks good to me hence i am adding by
> reviewed-by. But please make sure to keep the version history intact for
> fewer patches.
I have one question.
If I would update v4 of this patch as Martin asked,
which one should I choose?
1) adding changes from v1~v4
2) adding just something like 'version update'
Because there is no difference between v3 and v4.
>
> Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
>
>
> > Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
> > ---
> > drivers/scsi/ufs/ufshcd.c | 28 ++++++++++++++++++++++++++--
> > drivers/scsi/ufs/ufshcd.h | 7 +++++++
> > 2 files changed, 33 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index d6e3112..c9cf011 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -392,7 +392,31 @@ static inline void ufshcd_put_tm_slot(struct
> > ufs_hba *hba, int slot)
> > */
> > static inline void ufshcd_utrl_clear(struct ufs_hba *hba, u32 pos)
> > {
> > - ufshcd_writel(hba, ~(1 << pos), REG_UTP_TRANSFER_REQ_LIST_CLEAR);
> > + u32 clear;
> > +
> > + if (hba->quirks & UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR)
> > + clear = (1 << pos);
> > + else
> > + clear = ~(1 << pos);
> > +
> > + ufshcd_writel(hba, clear, REG_UTP_TRANSFER_REQ_LIST_CLEAR);
> > +}
> > +
> > +/**
> > + * ufshcd_utmrl_clear - Clear a bit in UTRMLCLR register
> > + * @hba: per adapter instance
> > + * @pos: position of the bit to be cleared
> > + */
> > +static inline void ufshcd_utmrl_clear(struct ufs_hba *hba, u32 pos)
> > +{
> > + u32 clear;
> > +
> > + if (hba->quirks & UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR)
> > + clear = (1 << pos);
> > + else
> > + clear = ~(1 << pos);
> > +
> > + ufshcd_writel(hba, clear, REG_UTP_TASK_REQ_LIST_CLEAR);
> > }
> >
> > /**
> > @@ -4312,7 +4336,7 @@ static int ufshcd_clear_tm_cmd(struct ufs_hba
> > *hba, int tag)
> > goto out;
> >
> > spin_lock_irqsave(hba->host->host_lock, flags);
> > - ufshcd_writel(hba, ~(1 << tag), REG_UTP_TASK_REQ_LIST_CLEAR);
> > + ufshcd_utmrl_clear(hba, tag);
> > spin_unlock_irqrestore(hba->host->host_lock, flags);
> >
> > /* poll for max. 1 sec to clear door bell register by h/w */
> > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> > index 7d9ff22..9838598 100644
> > --- a/drivers/scsi/ufs/ufshcd.h
> > +++ b/drivers/scsi/ufs/ufshcd.h
> > @@ -491,6 +491,13 @@ struct ufs_hba {
> > */
> > #define UFSHCD_QUIRK_PRDT_BYTE_GRAN UFS_BIT(7)
> >
> > + /*
> > + * This quirk needs to be enabled if the host contoller has to set
> > + * the bit corresponding the slot to be cleared to 1, not 0 as
> > + * described in UFS spec.
> > + */
> > + #define UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR UFS_BIT(8)
> > +
> > unsigned int quirks; /* Deviations from standard UFSHCI spec.
> */
> >
> > /* Device deviations from standard UFS device 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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v3] ufs: introduce UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR quirk
2016-11-17 13:52 ` Kiwoong Kim
@ 2016-11-17 19:29 ` Subhash Jadavani
0 siblings, 0 replies; 5+ messages in thread
From: Subhash Jadavani @ 2016-11-17 19:29 UTC (permalink / raw)
To: Kiwoong Kim; +Cc: linux-scsi, cpgs, 'HeonGwang Chu', linux-scsi-owner
On 2016-11-17 05:52, Kiwoong Kim wrote:
>> On 2016-11-15 21:03, Kiwoong Kim wrote:
>> > Some UFS host controllers may clear a transfer request slot
>> > by setting an associated bit in UTRLCLR/UTMRLCLR to 1, not 0.
>> > That's opposite to what UFS spec describes.
>> >
>>
>> This was the comment on v2: "As Martin mentioned in other email,
>> please
>> separate this version history from commit text with line having "----"
>> before the start of version history."
>> So i would have expected the patch version to be still v2 and just add
>> the version history separator added before version history. But you
>
> Okay. You mean re-sending it with moving the separator to another line,
> don't you? I knew that I should update version even if there is a
> change
> of commit message and version history. Thank you for your information.
>
>> posted v3 and removed the version history altogether. As far as patch
>> contents are concerned, it looks good to me hence i am adding by
>> reviewed-by. But please make sure to keep the version history intact
>> for
>> fewer patches.
>
> I have one question.
> If I would update v4 of this patch as Martin asked,
> which one should I choose?
> 1) adding changes from v1~v4
> 2) adding just something like 'version update'
> Because there is no difference between v3 and v4.
You should keep the all the previous version history intact and then add
v4 history saying "rebased to 4.10/scsi-queue tip".
>
>>
>> Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
>>
>>
>> > Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
>> > ---
>> > drivers/scsi/ufs/ufshcd.c | 28 ++++++++++++++++++++++++++--
>> > drivers/scsi/ufs/ufshcd.h | 7 +++++++
>> > 2 files changed, 33 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> > index d6e3112..c9cf011 100644
>> > --- a/drivers/scsi/ufs/ufshcd.c
>> > +++ b/drivers/scsi/ufs/ufshcd.c
>> > @@ -392,7 +392,31 @@ static inline void ufshcd_put_tm_slot(struct
>> > ufs_hba *hba, int slot)
>> > */
>> > static inline void ufshcd_utrl_clear(struct ufs_hba *hba, u32 pos)
>> > {
>> > - ufshcd_writel(hba, ~(1 << pos), REG_UTP_TRANSFER_REQ_LIST_CLEAR);
>> > + u32 clear;
>> > +
>> > + if (hba->quirks & UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR)
>> > + clear = (1 << pos);
>> > + else
>> > + clear = ~(1 << pos);
>> > +
>> > + ufshcd_writel(hba, clear, REG_UTP_TRANSFER_REQ_LIST_CLEAR);
>> > +}
>> > +
>> > +/**
>> > + * ufshcd_utmrl_clear - Clear a bit in UTRMLCLR register
>> > + * @hba: per adapter instance
>> > + * @pos: position of the bit to be cleared
>> > + */
>> > +static inline void ufshcd_utmrl_clear(struct ufs_hba *hba, u32 pos)
>> > +{
>> > + u32 clear;
>> > +
>> > + if (hba->quirks & UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR)
>> > + clear = (1 << pos);
>> > + else
>> > + clear = ~(1 << pos);
>> > +
>> > + ufshcd_writel(hba, clear, REG_UTP_TASK_REQ_LIST_CLEAR);
>> > }
>> >
>> > /**
>> > @@ -4312,7 +4336,7 @@ static int ufshcd_clear_tm_cmd(struct ufs_hba
>> > *hba, int tag)
>> > goto out;
>> >
>> > spin_lock_irqsave(hba->host->host_lock, flags);
>> > - ufshcd_writel(hba, ~(1 << tag), REG_UTP_TASK_REQ_LIST_CLEAR);
>> > + ufshcd_utmrl_clear(hba, tag);
>> > spin_unlock_irqrestore(hba->host->host_lock, flags);
>> >
>> > /* poll for max. 1 sec to clear door bell register by h/w */
>> > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
>> > index 7d9ff22..9838598 100644
>> > --- a/drivers/scsi/ufs/ufshcd.h
>> > +++ b/drivers/scsi/ufs/ufshcd.h
>> > @@ -491,6 +491,13 @@ struct ufs_hba {
>> > */
>> > #define UFSHCD_QUIRK_PRDT_BYTE_GRAN UFS_BIT(7)
>> >
>> > + /*
>> > + * This quirk needs to be enabled if the host contoller has to set
>> > + * the bit corresponding the slot to be cleared to 1, not 0 as
>> > + * described in UFS spec.
>> > + */
>> > + #define UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR UFS_BIT(8)
>> > +
>> > unsigned int quirks; /* Deviations from standard UFSHCI spec.
>> */
>> >
>> > /* Device deviations from standard UFS device 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@vger.kernel.org
>> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] ufs: introduce UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR quirk
2016-11-16 5:03 [PATCH v3] ufs: introduce UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR quirk Kiwoong Kim
2016-11-16 18:52 ` Subhash Jadavani
@ 2016-11-17 1:35 ` Martin K. Petersen
1 sibling, 0 replies; 5+ messages in thread
From: Martin K. Petersen @ 2016-11-17 1:35 UTC (permalink / raw)
To: Kiwoong Kim; +Cc: linux-scsi, cpgs, HeonGwang Chu
>>>>> "Kiwoong" == Kiwoong Kim <kwmad.kim@samsung.com> writes:
Hi Kiwoong,
Kiwoong> Some UFS host controllers may clear a transfer request slot by
Kiwoong> setting an associated bit in UTRLCLR/UTMRLCLR to 1, not 0.
Kiwoong> That's opposite to what UFS spec describes.
These don't apply cleanly to 4.10/scsi-queue. Please send a v4, ideally
as a series.
Thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-11-17 19:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-16 5:03 [PATCH v3] ufs: introduce UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR quirk Kiwoong Kim
2016-11-16 18:52 ` Subhash Jadavani
2016-11-17 13:52 ` Kiwoong Kim
2016-11-17 19:29 ` Subhash Jadavani
2016-11-17 1:35 ` Martin K. Petersen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).