[PATCH v4] ufs: introduce UFSHCD_QUIRK_PRDT_BYTE_GRAN quirk

[Kiwoong Kim]

Some UFS host controllers may think
granularities of PRDT length and offset as bytes, not double words.

Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
---
V2: change the name of the quirk
(UFSHCD_QUIRK_BROKEN_DWORD_UTRD -> UFSHCD_QUIRK_PRDT_BYTE_GRAN)
V3: (rebased to for-next tip)
V4: (rebased to 4.10/scsi-queue tip)

Sorry for repeating submitting the same patch, everyone.
And I truly appreciate that all of you always give me some advice
about the rules of mainline.

---
 drivers/scsi/ufs/ufshcd.c | 28 +---
 drivers/scsi/ufs/ufshcd.h |  6 ++
 2 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 4214881..f91e50b 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1129,7 +1129,7 @@ ufshcd_send_uic_cmd(struct ufs_hba *hba, struct 
uic_command *uic_cmd)
  *
  * Returns 0 in case of success, non-zero value in case of failure
  */
-static int ufshcd_map_sg(struct ufshcd_lrb *lrbp)
+static int ufshcd_map_sg(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 {
struct ufshcd_sg_entry *prd_table;
struct scatterlist *sg;
@@ -1143,8 +1143,13 @@ static int ufshcd_map_sg(struct ufshcd_lrb *lrbp)
return sg_segments;
 
if (sg_segments) {
-   lrbp->utr_descriptor_ptr->prd_table_length =
-   cpu_to_le16((u16) (sg_segments));
+   if (hba->quirks & UFSHCD_QUIRK_PRDT_BYTE_GRAN)
+   lrbp->utr_descriptor_ptr->prd_table_length =
+   cpu_to_le16((u16)(sg_segments *
+   sizeof(struct ufshcd_sg_entry)));
+   else
+   lrbp->utr_descriptor_ptr->prd_table_length =
+   cpu_to_le16((u16) (sg_segments));
 
prd_table = (struct ufshcd_sg_entry *)lrbp->ucd_prdt_ptr;
 
@@ -1507,7 +1512,7 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, 
struct scsi_cmnd *cmd)
 
ufshcd_comp_scsi_upiu(hba, lrbp);
 
-   err = ufshcd_map_sg(lrbp);
+   err = ufshcd_map_sg(hba, lrbp);
if (err) {
lrbp->cmd = NULL;
clear_bit_unlock(tag, >lrb_in_use); 
@@ -2368,12 +2373,21 @@ static void ufshcd_host_memory_configure(struct ufs_hba 
*hba)

cpu_to_le32(upper_32_bits(cmd_desc_element_addr));
 
/* Response upiu and prdt offset should be in double words */
-   utrdlp[i].response_upiu_offset =
+   if (hba->quirks & UFSHCD_QUIRK_PRDT_BYTE_GRAN) {
+   utrdlp[i].response_upiu_offset =
+   cpu_to_le16(response_offset);
+   utrdlp[i].prd_table_offset =
+   cpu_to_le16(prdt_offset);
+   utrdlp[i].response_upiu_length =
+   cpu_to_le16(ALIGNED_UPIU_SIZE);
+   } else {
+   utrdlp[i].response_upiu_offset =
cpu_to_le16((response_offset >> 2));
-   utrdlp[i].prd_table_offset =
+   utrdlp[i].prd_table_offset =
cpu_to_le16((prdt_offset >> 2));
-   utrdlp[i].response_upiu_length =
+   utrdlp[i].response_upiu_length =
cpu_to_le16(ALIGNED_UPIU_SIZE >> 2);
+   }
 
hba->lrb[i].utr_descriptor_ptr = (utrdlp + i);
hba->lrb[i].ucd_req_ptr =
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 8e76501..7d9ff22 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -485,6 +485,12 @@ struct ufs_hba {
 */
#define UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION UFS_BIT(5)
 
+   /*
+* This quirk needs to be enabled if the host contoller regards
+* resolution of the values of PRDTO and PRDTL in UTRD as byte.
+*/
+   #define UFSHCD_QUIRK_PRDT_BYTE_GRAN UFS_BIT(7)
+
unsigned int quirks;/* Deviations from standard UFSHCI spec. */
 
/* Device deviations from standard UFS device spec. */
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3] ufs: introduce UFSHCD_QUIRK_PRDT_BYTE_GRAN quirk

[Kiwoong Kim]

Some UFS host controllers may think
granularities of PRDT length and offset as bytes, not double words.

Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
---
V2: change the name of the quirk
(UFSHCD_QUIRK_BROKEN_DWORD_UTRD -> UFSHCD_QUIRK_PRDT_BYTE_GRAN)
V3: (rebased to 4.10/scsi-queue tip)
--- 
 drivers/scsi/ufs/ufshcd.c | 28 +---
 drivers/scsi/ufs/ufshcd.h |  6 ++
 2 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index e75fbb3..d6e3112 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1128,7 +1128,7 @@ ufshcd_send_uic_cmd(struct ufs_hba *hba, struct 
uic_command *uic_cmd)
  *
  * Returns 0 in case of success, non-zero value in case of failure
  */
-static int ufshcd_map_sg(struct ufshcd_lrb *lrbp)
+static int ufshcd_map_sg(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 {
struct ufshcd_sg_entry *prd_table;
struct scatterlist *sg;
@@ -1142,8 +1142,13 @@ static int ufshcd_map_sg(struct ufshcd_lrb *lrbp)
return sg_segments;
 
if (sg_segments) {
-   lrbp->utr_descriptor_ptr->prd_table_length =
-   cpu_to_le16((u16) (sg_segments));
+   if (hba->quirks & UFSHCD_QUIRK_PRDT_BYTE_GRAN)
+   lrbp->utr_descriptor_ptr->prd_table_length =
+   cpu_to_le16((u16)(sg_segments *
+   sizeof(struct ufshcd_sg_entry)));
+   else
+   lrbp->utr_descriptor_ptr->prd_table_length =
+   cpu_to_le16((u16) (sg_segments));
 
prd_table = (struct ufshcd_sg_entry *)lrbp->ucd_prdt_ptr;
 
@@ -1505,7 +1510,7 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, 
struct scsi_cmnd *cmd)
 
ufshcd_comp_scsi_upiu(hba, lrbp);
 
-   err = ufshcd_map_sg(lrbp);
+   err = ufshcd_map_sg(hba, lrbp);
if (err) {
lrbp->cmd = NULL;
clear_bit_unlock(tag, >lrb_in_use); 
@@ -2366,12 +2371,21 @@ static void ufshcd_host_memory_configure(struct ufs_hba 
*hba)

cpu_to_le32(upper_32_bits(cmd_desc_element_addr));
 
/* Response upiu and prdt offset should be in double words */
-   utrdlp[i].response_upiu_offset =
+   if (hba->quirks & UFSHCD_QUIRK_PRDT_BYTE_GRAN) {
+   utrdlp[i].response_upiu_offset =
+   cpu_to_le16(response_offset);
+   utrdlp[i].prd_table_offset =
+   cpu_to_le16(prdt_offset);
+   utrdlp[i].response_upiu_length =
+   cpu_to_le16(ALIGNED_UPIU_SIZE);
+   } else {
+   utrdlp[i].response_upiu_offset =
cpu_to_le16((response_offset >> 2));
-   utrdlp[i].prd_table_offset =
+   utrdlp[i].prd_table_offset =
cpu_to_le16((prdt_offset >> 2));
-   utrdlp[i].response_upiu_length =
+   utrdlp[i].response_upiu_length =
cpu_to_le16(ALIGNED_UPIU_SIZE >> 2);
+   }
 
hba->lrb[i].utr_descriptor_ptr = (utrdlp + i);
hba->lrb[i].ucd_req_ptr =
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 8e76501..7d9ff22 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h 
@@ -485,6 +485,12 @@ struct ufs_hba {
 */
#define UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION UFS_BIT(5)
 
+   /*
+* This quirk needs to be enabled if the host contoller regards
+* resolution of the values of PRDTO and PRDTL in UTRD as byte.
+*/
+   #define UFSHCD_QUIRK_PRDT_BYTE_GRAN UFS_BIT(7)
+
unsigned int quirks;/* Deviations from standard UFSHCI spec. */
 
/* Device deviations from standard UFS device spec. */
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v3] ufs: introduce UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR quirk

[Kiwoong Kim]

> 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 <subha...@codeaurora.org>
> 
> 
> > Signed-off-by: Kiwoong Kim <kwmad@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_CLRUFS_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 majord...@vger.kernel.org
> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3] ufs: add a variety of definitions decribed in UFS spec

[Kiwoong Kim]

These things are defined to be used by some UFS Host controllers.
And a new file for some declarations of mphy standard is added

Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
---
V3: add new macros of all bits of UECDL
--- 
 drivers/scsi/ufs/mphy.h   | 38 ++
 drivers/scsi/ufs/ufshci.h | 28 +---
 drivers/scsi/ufs/unipro.h | 26 ++
 3 files changed, 89 insertions(+), 3 deletions(-)
 create mode 100644 drivers/scsi/ufs/mphy.h

diff --git a/drivers/scsi/ufs/mphy.h b/drivers/scsi/ufs/mphy.h
new file mode 100644
index 000..c431f49
--- /dev/null
+++ b/drivers/scsi/ufs/mphy.h
@@ -0,0 +1,38 @@
+/*
+ * drivers/scsi/ufs/mphy.h
+ *
+ * Copyright (C) 2014 Samsung Electronics Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef _MPHY_H_
+#define _MPHY_H_
+
+#define TX_HIBERN8TIME_CAP 0x0f
+#define TX_MIN_ACTIVATE_TIME   0x33
+
+#define RX_HS_G1_SYNC_LEN_CAP  0x8b
+#define RX_HS_G1_PREP_LEN_CAP  0x8c
+#define RX_HS_G2_SYNC_LEN_CAP  0x94
+#define RX_HS_G3_SYNC_LEN_CAP  0x95
+#define RX_HS_G2_PREP_LEN_CAP  0x96
+#define RX_HS_G3_PREP_LEN_CAP  0x97
+ #define SYNC_RANGE_FINE   (0 << 6)
+ #define SYNC_RANGE_COARSE (1 << 6)
+ #define SYNC_LEN(x)   ((x) & 0x3f)
+ #define PREP_LEN(x)   ((x) & 0xf)
+#define RX_ADV_GRANULARITY_CAP 0x98
+ #define RX_ADV_GRAN_STEP(x)   x) & 0x3) << 1) | 0x1)
+#define TX_ADV_GRANULARITY_CAP 0x10
+ #define TX_ADV_GRAN_STEP(x)   x) & 0x3) << 1) | 0x1)
+#define RX_MIN_ACTIVATETIME_CAP0x8f
+#define RX_HIBERN8TIME_CAP 0x92
+#define RX_ADV_HIBERN8TIME_CAP 0x99
+#define RX_ADV_MIN_ACTIVATETIME_CAP0x9a
+
+#endif /* _MPHY_H_ */
+ 
diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
index 9599741..799cb26 100644
--- a/drivers/scsi/ufs/ufshci.h
+++ b/drivers/scsi/ufs/ufshci.h
@@ -170,17 +170,39 @@ enum {
 /* UECDL - Host UIC Error Code Data Link Layer 3Ch */
 #define UIC_DATA_LINK_LAYER_ERROR  UFS_BIT(31)
 #define UIC_DATA_LINK_LAYER_ERROR_CODE_MASK0x7FFF
-#define UIC_DATA_LINK_LAYER_ERROR_PA_INIT  0x2000
-#define UIC_DATA_LINK_LAYER_ERROR_NAC_RECEIVED 0x0001
-#define UIC_DATA_LINK_LAYER_ERROR_TCx_REPLAY_TIMEOUT 0x0002
+#define UIC_DATA_LINK_LAYER_ERROR_NAC_RECEIVED UFS_BIT(0)
+#define UIC_DATA_LINK_LAYER_ERROR_TCx_REPLAY_TIMEOUT   UFS_BIT(1)
+#define UIC_DATA_LINK_LAYER_ERROR_AFCx_REQUEST_TIMEOUT UFS_BIT(2)
+#define UIC_DATA_LINK_LAYER_ERROR_FCx_PROTECTION_TIMEOUT   UFS_BIT(3)
+#define UIC_DATA_LINK_LAYER_ERROR_CRC_ERRORUFS_BIT(4)
+#define UIC_DATA_LINK_LAYER_ERROR_RX_BUF_OFUFS_BIT(5)
+#define UIC_DATA_LINK_LAYER_ERROR_MAX_FRAME_LENGTH_EXCEEDEDUFS_BIT(6)
+#define UIC_DATA_LINK_LAYER_ERROR_WRONG_SEQUENCE_NUMBER
UFS_BIT(7)
+#define UIC_DATA_LINK_LAYER_ERROR_AFC_FRAME_SYNTAX_ERROR   UFS_BIT(8)
+#define UIC_DATA_LINK_LAYER_ERROR_NAC_FRAME_SYNTAX_ERROR   UFS_BIT(9)
+#define UIC_DATA_LINK_LAYER_ERROR_EOF_SYNTAX_ERROR UFS_BIT(10)
+#define UIC_DATA_LINK_LAYER_ERROR_FRAME_SYNTAX_ERROR   UFS_BIT(11)
+#define UIC_DATA_LINK_LAYER_ERROR_BAD_CTRL_SYMBOL_TYPE UFS_BIT(12)
+#define UIC_DATA_LINK_LAYER_ERROR_PA_INIT  UFS_BIT(13)
+#define UIC_DATA_LINK_LAYER_ERROR_PA_ERROR_IND_RECEIVED
UFS_BIT(14)
+
 
 /* UECN - Host UIC Error Code Network Layer 40h */
 #define UIC_NETWORK_LAYER_ERRORUFS_BIT(31)
 #define UIC_NETWORK_LAYER_ERROR_CODE_MASK  0x7
+#define UIC_NETWORK_UNSUPPORTED_HEADER_TYPEBIT(0)
+#define UIC_NETWORK_BAD_DEVICEID_ENC   BIT(1)
+#define UIC_NETWORK_LHDR_TRAP_PACKET_DROPPING  BIT(2)
 
 /* UECT - Host UIC Error Code Transport Layer 44h */
 #define UIC_TRANSPORT_LAYER_ERROR  UFS_BIT(31)
 #define UIC_TRANSPORT_LAYER_ERROR_CODE_MASK0x7F
+#define UIC_TRANSPORT_UNSUPPORTED_HEADER_TYPE  BIT(0)
+#define UIC_TRANSPORT_UNKNOWN_CPORTID  BIT(1)
+#define UIC_TRANSPORT_NO_CONNECTION_RX BIT(2)
+#define UIC_TRANSPORT_BAD_TC   BIT(4)
+#define UIC_TRANSPORT_E2E_CREDIT_OVERFLOW  BIT(5)
+#define UIC_TRANSPORT_SAFETY_VALVE_DROPPINGBIT(6)
 
 /* UECDME - Host UIC Error Code DME 48h */
 #define UIC_DME_ERROR  UFS_BIT(31) 
diff --git a/drivers/scsi/ufs/unipro.h b/drivers/scsi/ufs/unipro.h
index eff8b56..490d867 100644
--- a/drivers/scsi/ufs/unipro.h
+++ b/drivers/scsi/ufs/unipro.h
@@ -127,6 +127,7 @@
 #define PA_PACPREQEOBTIMEOUT   0x1591
 #define PA_HIBERN8TIME 0x15A7
 #define PA_LOCALVERINFO0x15A9
+#define PA_GRANULARITY 0x15AA
 

RE: [PATCH v2] ufs: add a variety of definitions decribed in UFS spec

[Kiwoong Kim]

> On 2016-11-15 03:24, Kiwoong Kim wrote:
> > These things are defined to be used by some UFS Host controllers.
> > And a new file for some declarations of mphy standard is added
> >
> > V2
> > - modify the commit message
> > - add a new macro for UECDL
> > - add two definitions about UECDL
> > - change the names of two macros
> > (s/IS_PWR_MODE_HS/IS_HS_PWR_MODE, s/IS_PWR_MODE_PWM/IS_PWM_PWR_MODE)
> >
> > Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
> > ---
> >  drivers/scsi/ufs/mphy.h   | 38 ++
> >  drivers/scsi/ufs/ufshci.h | 17 ++---
> > drivers/scsi/ufs/unipro.h | 26 ++
> >  3 files changed, 78 insertions(+), 3 deletions(-)  create mode 100644
> > drivers/scsi/ufs/mphy.h
> >
> > diff --git a/drivers/scsi/ufs/mphy.h b/drivers/scsi/ufs/mphy.h new
> > file mode 100644 index 000..c431f49
> > --- /dev/null
> > +++ b/drivers/scsi/ufs/mphy.h
> > @@ -0,0 +1,38 @@
> > +/*
> > + * drivers/scsi/ufs/mphy.h
> > + *
> > + * Copyright (C) 2014 Samsung Electronics Co., Ltd.
> > + *
> > + * This program is free software; you can redistribute it and/or
> > modify
> > + * it under the terms of the GNU General Public License as published
> > by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + */
> > +
> > +#ifndef _MPHY_H_
> > +#define _MPHY_H_
> > +
> > +#define TX_HIBERN8TIME_CAP 0x0f
> > +#define TX_MIN_ACTIVATE_TIME   0x33
> > +
> > +#define RX_HS_G1_SYNC_LEN_CAP  0x8b
> > +#define RX_HS_G1_PREP_LEN_CAP  0x8c
> > +#define RX_HS_G2_SYNC_LEN_CAP  0x94
> > +#define RX_HS_G3_SYNC_LEN_CAP  0x95
> > +#define RX_HS_G2_PREP_LEN_CAP  0x96
> > +#define RX_HS_G3_PREP_LEN_CAP  0x97
> > + #define SYNC_RANGE_FINE   (0 << 6)
> > + #define SYNC_RANGE_COARSE (1 << 6)
> > + #define SYNC_LEN(x)   ((x) & 0x3f)
> > + #define PREP_LEN(x)   ((x) & 0xf)
> > +#define RX_ADV_GRANULARITY_CAP 0x98
> > + #define RX_ADV_GRAN_STEP(x)   x) & 0x3) << 1) | 0x1)
> > +#define TX_ADV_GRANULARITY_CAP 0x10
> > + #define TX_ADV_GRAN_STEP(x)   x) & 0x3) << 1) | 0x1)
> > +#define RX_MIN_ACTIVATETIME_CAP0x8f
> > +#define RX_HIBERN8TIME_CAP 0x92
> > +#define RX_ADV_HIBERN8TIME_CAP 0x99
> > +#define RX_ADV_MIN_ACTIVATETIME_CAP0x9a
> > +
> > +#endif /* _MPHY_H_ */
> > +
> > diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
> > index 9599741..a1a06ac9 100644
> > --- a/drivers/scsi/ufs/ufshci.h
> > +++ b/drivers/scsi/ufs/ufshci.h
> > @@ -170,17 +170,28 @@ enum {
> >  /* UECDL - Host UIC Error Code Data Link Layer 3Ch */
> >  #define UIC_DATA_LINK_LAYER_ERROR  UFS_BIT(31)
> >  #define UIC_DATA_LINK_LAYER_ERROR_CODE_MASK0x7FFF
> > -#define UIC_DATA_LINK_LAYER_ERROR_PA_INIT  0x2000
> > -#define UIC_DATA_LINK_LAYER_ERROR_NAC_RECEIVED 0x0001
> > -#define UIC_DATA_LINK_LAYER_ERROR_TCx_REPLAY_TIMEOUT 0x0002
> > +#define UIC_DATA_LINK_LAYER_ERROR_NAC_RECEIVED UFS_BIT(0)
> > +#define UIC_DATA_LINK_LAYER_ERROR_TCx_REPLAY_TIMEOUT   UFS_BIT(1)
> > +#define UIC_DATA_LINK_LAYER_ERROR_RX_BUF_OFUFS_BIT(5)
> > +#define UIC_DATA_LINK_LAYER_ERROR_PA_INIT  UFS_BIT(13)
> > +#define UIC_DATA_LINK_LAYER_ERROR_CASE(r, x)   (r & x)
> 
> 
> I has this comment on Patch set #1 : "why don't we just add macros for all
> the bits in UECDL ? This makes it easy in future." and you had replied
> "Okay. I'll think about it and apply new macros on new version of this
> patch." but i don't see these macros added. Do you want to add them? Also,
> i am not sure why we are adding
> UIC_DATA_LINK_LAYER_ERROR_CASE() macro?

I misunderstood what you said. Sorry for bothering you.
I'll update this patch.

> 
> One more thing,
> As Martin mentioned in other email, please separate this version history
> from commit text with line having "" before the start of version
> history.
> Rest all looks good but i will wait for updated patch fixing above before
> giving Reviewed-By.
> 
> 
> >
> >  /* UECN - Host UIC Error Code Network Layer 40h */
> >  #define UIC_NETWORK_LAYER_ERRORUFS_BIT(31)
> >  #define UIC_NETWORK_LAYER_ERROR_CODE_MASK  0x7
> > +#define UIC_NETWORK_UNSUPPO

[PATCH v3] ufs: introduce UFSHCI_QUIRK_SKIP_RESET_INTR_AGGR quirk

[Kiwoong Kim]

If UFS driver resets interrupt aggregation timer and counter
when there are some pended tasks, an IO competion interrupt
of any corresponing task may be issued.
That would casue a command timeout.

One thing you should mind to use interrupt aggreation
with this quirk is that the host controller should be
able to refresh interrupt aggreation counter or timer
in other way, such as doing it automatically when receiving
any response.

Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
---
 drivers/scsi/ufs/ufshcd.c | 3 ++-
 drivers/scsi/ufs/ufshcd.h | 7 +++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 8aac98f..7b62d8b 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -3713,7 +3713,8 @@ 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_RESET_INTR_AGGR))
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 dfa17ac..d6861ed 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -505,6 +505,13 @@ struct ufs_hba {
 */
#define UFSHCD_QUIRK_BROKEN_HCE UFS_BIT(9)
 
+   /*
+* This quirk is only not to reset interrupt aggregation logic
+* in ISR. The reset can make the host controller miss an event
+* of previously completed IO.
+*/
+   #define UFSHCI_QUIRK_SKIP_RESET_INTR_AGGR   UFS_BIT(10)
+
unsigned int quirks;/* Deviations from standard UFSHCI spec. */
 
/* Device deviations from standard UFS device spec. */
-- 
2.1.4


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3] ufs: introduce UFSHCD_QUIRK_BROKEN_HCE quirk

[Kiwoong Kim]

Some UFS host controllers might not be able to
reset UIC by setting HCE to 1.
Those controllers should invoke 'DME reset' and 'DME enable'
in order instead.

Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
---
 drivers/scsi/ufs/ufshcd.c | 44 +++-
 drivers/scsi/ufs/ufshcd.h |  7 +++
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index c9cf011..8aac98f 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -2477,6 +2477,37 @@ static inline void 
ufshcd_add_delay_before_dme_cmd(struct ufs_hba *hba)
usleep_range(min_sleep_time_us, min_sleep_time_us + 50);
 }
 
+static int ufshcd_dme_reset(struct ufs_hba *hba)
+{
+   struct uic_command uic_cmd = {0};
+   int ret;
+
+   uic_cmd.command = UIC_CMD_DME_RESET;
+   uic_cmd.argument1 = 0x1;
+
+   ret = ufshcd_send_uic_cmd(hba, _cmd);
+   if (ret)
+   dev_err(hba->dev,
+   "dme-reset: error code %d\n", ret);
+
+   return ret;
+}
+
+static int ufshcd_dme_enable(struct ufs_hba *hba)
+{
+   struct uic_command uic_cmd = {0};
+   int ret;
+
+   uic_cmd.command = UIC_CMD_DME_ENABLE;
+
+   ret = ufshcd_send_uic_cmd(hba, _cmd);
+   if (ret)
+   dev_err(hba->dev,
+   "dme-enable: error code %d\n", ret);
+
+   return ret;
+}
+
 /**
  * ufshcd_dme_set_attr - UIC command for DME_SET, DME_PEER_SET
  * @hba: per adapter instance
@@ -3084,6 +3115,7 @@ static inline void ufshcd_hba_stop(struct ufs_hba *hba, 
bool can_sleep)
 static int ufshcd_hba_enable(struct ufs_hba *hba)
 {
int retry;
+   int ret = 0;
 
/*
 * msleep of 1 and 5 used in this function might result in msleep(20),
@@ -3100,6 +3132,9 @@ static int ufshcd_hba_enable(struct ufs_hba *hba)
 
ufshcd_vops_hce_enable_notify(hba, PRE_CHANGE);
 
+   if (hba->quirks & UFSHCD_QUIRK_BROKEN_HCE)
+   goto use_dme;
+
/* start controller initialization sequence */
ufshcd_hba_start(hba);
 
@@ -3128,12 +3163,19 @@ static int ufshcd_hba_enable(struct ufs_hba *hba)
msleep(5);
}
 
+use_dme:
/* enable UIC related interrupts */
ufshcd_enable_intr(hba, UFSHCD_UIC_MASK);
 
+   if (hba->quirks & UFSHCD_QUIRK_BROKEN_HCE) {
+   ret = ufshcd_dme_reset(hba);
+   if (!ret)
+   ret = ufshcd_dme_enable(hba);
+   }
+
ufshcd_vops_hce_enable_notify(hba, POST_CHANGE);
 
-   return 0;
+   return ret;
 }
 
 static int ufshcd_disable_tx_lcc(struct ufs_hba *hba, bool peer)
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 9838598..dfa17ac 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -498,6 +498,13 @@ struct ufs_hba {
 */
#define UFSHCD_QUIRK_BROKEN_REQ_LIST_CLRUFS_BIT(8)
 
+   /*
+* This quirk needs to be enabled if the host contoller can't reset
+* UIC by setting HCE to 1. Those controllers should invoke
+* DME reset and DME enable in order.
+*/
+   #define UFSHCD_QUIRK_BROKEN_HCE UFS_BIT(9)
+
unsigned int quirks;/* Deviations from standard UFSHCI spec. */
 
/* Device deviations from standard UFS device spec. */
-- 
2.1.4


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3] ufs: introduce UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR quirk

[Kiwoong Kim]

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@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_CLRUFS_BIT(8)
+
unsigned int quirks;/* Deviations from standard UFSHCI spec. */
 
/* Device deviations from standard UFS device spec. */
-- 
2.1.4


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v2] ufs: introduce UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR quirk

[Kiwoong Kim]

>> > 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.
>> >
>> > v2: modify the commit message, remove unrelated changes
>> 
>> As Martin mentioned in other email, please separate this version history
>> from commit text with line having "" before the start of version
>> history.

I didn't understand clearly what Martin said.
Thank you for your comment.

>> Rest all looks good but i will wait for updated patch fixing above before
>> giving Reviewed-By.
>> 
>> >
>> > Signed-off-by: Kiwoong Kim <kwmad@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_CLRUFS_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 majord...@vger.kernel.org
>> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] ufs: add a variety of definitions decribed in UFS spec

[Kiwoong Kim]

These things are defined to be used by some UFS Host controllers.
And a new file for some declarations of mphy standard is added

V2
- modify the commit message
- add a new macro for UECDL
- add two definitions about UECDL
- change the names of two macros
(s/IS_PWR_MODE_HS/IS_HS_PWR_MODE, s/IS_PWR_MODE_PWM/IS_PWM_PWR_MODE)

Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
--- 
 drivers/scsi/ufs/mphy.h   | 38 ++
 drivers/scsi/ufs/ufshci.h | 17 ++---
 drivers/scsi/ufs/unipro.h | 26 ++
 3 files changed, 78 insertions(+), 3 deletions(-)
 create mode 100644 drivers/scsi/ufs/mphy.h

diff --git a/drivers/scsi/ufs/mphy.h b/drivers/scsi/ufs/mphy.h
new file mode 100644
index 000..c431f49
--- /dev/null
+++ b/drivers/scsi/ufs/mphy.h
@@ -0,0 +1,38 @@
+/*
+ * drivers/scsi/ufs/mphy.h
+ *
+ * Copyright (C) 2014 Samsung Electronics Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef _MPHY_H_
+#define _MPHY_H_
+
+#define TX_HIBERN8TIME_CAP 0x0f
+#define TX_MIN_ACTIVATE_TIME   0x33
+
+#define RX_HS_G1_SYNC_LEN_CAP  0x8b
+#define RX_HS_G1_PREP_LEN_CAP  0x8c
+#define RX_HS_G2_SYNC_LEN_CAP  0x94
+#define RX_HS_G3_SYNC_LEN_CAP  0x95
+#define RX_HS_G2_PREP_LEN_CAP  0x96
+#define RX_HS_G3_PREP_LEN_CAP  0x97
+ #define SYNC_RANGE_FINE   (0 << 6)
+ #define SYNC_RANGE_COARSE (1 << 6)
+ #define SYNC_LEN(x)   ((x) & 0x3f)
+ #define PREP_LEN(x)   ((x) & 0xf)
+#define RX_ADV_GRANULARITY_CAP 0x98
+ #define RX_ADV_GRAN_STEP(x)   x) & 0x3) << 1) | 0x1)
+#define TX_ADV_GRANULARITY_CAP 0x10
+ #define TX_ADV_GRAN_STEP(x)   x) & 0x3) << 1) | 0x1)
+#define RX_MIN_ACTIVATETIME_CAP0x8f
+#define RX_HIBERN8TIME_CAP 0x92
+#define RX_ADV_HIBERN8TIME_CAP 0x99
+#define RX_ADV_MIN_ACTIVATETIME_CAP0x9a
+
+#endif /* _MPHY_H_ */
+
diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
index 9599741..a1a06ac9 100644
--- a/drivers/scsi/ufs/ufshci.h
+++ b/drivers/scsi/ufs/ufshci.h
@@ -170,17 +170,28 @@ enum {
 /* UECDL - Host UIC Error Code Data Link Layer 3Ch */
 #define UIC_DATA_LINK_LAYER_ERROR  UFS_BIT(31)
 #define UIC_DATA_LINK_LAYER_ERROR_CODE_MASK0x7FFF
-#define UIC_DATA_LINK_LAYER_ERROR_PA_INIT  0x2000
-#define UIC_DATA_LINK_LAYER_ERROR_NAC_RECEIVED 0x0001
-#define UIC_DATA_LINK_LAYER_ERROR_TCx_REPLAY_TIMEOUT 0x0002
+#define UIC_DATA_LINK_LAYER_ERROR_NAC_RECEIVED UFS_BIT(0)
+#define UIC_DATA_LINK_LAYER_ERROR_TCx_REPLAY_TIMEOUT   UFS_BIT(1)
+#define UIC_DATA_LINK_LAYER_ERROR_RX_BUF_OFUFS_BIT(5)
+#define UIC_DATA_LINK_LAYER_ERROR_PA_INIT  UFS_BIT(13)
+#define UIC_DATA_LINK_LAYER_ERROR_CASE(r, x)   (r & x)
 
 /* UECN - Host UIC Error Code Network Layer 40h */
 #define UIC_NETWORK_LAYER_ERRORUFS_BIT(31)
 #define UIC_NETWORK_LAYER_ERROR_CODE_MASK  0x7
+#define UIC_NETWORK_UNSUPPORTED_HEADER_TYPEBIT(0)
+#define UIC_NETWORK_BAD_DEVICEID_ENC   BIT(1)
+#define UIC_NETWORK_LHDR_TRAP_PACKET_DROPPING  BIT(2)
 
 /* UECT - Host UIC Error Code Transport Layer 44h */
 #define UIC_TRANSPORT_LAYER_ERROR  UFS_BIT(31)
 #define UIC_TRANSPORT_LAYER_ERROR_CODE_MASK0x7F
+#define UIC_TRANSPORT_UNSUPPORTED_HEADER_TYPE  BIT(0)
+#define UIC_TRANSPORT_UNKNOWN_CPORTID  BIT(1)
+#define UIC_TRANSPORT_NO_CONNECTION_RX BIT(2)
+#define UIC_TRANSPORT_BAD_TC   BIT(4)
+#define UIC_TRANSPORT_E2E_CREDIT_OVERFLOW  BIT(5)
+#define UIC_TRANSPORT_SAFETY_VALVE_DROPPINGBIT(6)
 
 /* UECDME - Host UIC Error Code DME 48h */
 #define UIC_DME_ERROR  UFS_BIT(31)
diff --git a/drivers/scsi/ufs/unipro.h b/drivers/scsi/ufs/unipro.h
index eff8b56..490d867 100644
--- a/drivers/scsi/ufs/unipro.h
+++ b/drivers/scsi/ufs/unipro.h
@@ -127,6 +127,7 @@
 #define PA_PACPREQEOBTIMEOUT   0x1591
 #define PA_HIBERN8TIME 0x15A7
 #define PA_LOCALVERINFO0x15A9
+#define PA_GRANULARITY 0x15AA
 #define PA_TACTIVATE   0x15A8
 #define PA_PACPFRAMECOUNT  0x15C0
 #define PA_PACPERRORCOUNT  0x15C1
@@ -170,6 +171,9 @@ enum {
UNCHANGED   = 7,
 };
 
+#define IS_HS_PWR_MODE(m)(((m) == FAST_MODE) || ((m) == FASTAUTO_MODE))
+#define IS_PWM_PWR_MODE(m)   (((m) == SLOW_MODE) || ((m) == SLOWAUTO_MODE))
+
 /* PA TX/RX Frequency Series */
 enum {
PA_HS_MODE_A= 1,
@@ -231,6 +235,11 @@ enum ufs_unipro_ver {
 #define DL_PEERTC1PRESENT  0x2066
 #define DL_PEERTC1RXINITCREVAL 0x2067
 
+/* Default value of L2 Timer */
+#define FC0PROTTIMEOUTVAL  8191
+#define TC0REPLAYTIMEOUTVAL65535
+#define AFC0REQTIMEOU

[PATCH v2] ufs: introduce UFSHCI_QUIRK_SKIP_RESET_INTR_AGGR quirk

[Kiwoong Kim]

If UFS driver resets interrupt aggregation timer and counter
when there are some pended tasks, an IO competion interrupt
of any corresponing task may be issued.
That would casue a command timeout.

One thing you should mind to use interrupt aggreation
with this quirk is that the host controller should be
able to refresh interrupt aggreation counter or timer
in other way, such as doing it automatically when receiving
any response. 

V2
- modify the commit message
- change the name of the quirk
(s/ UFSHCI_QUIRK_SKIP_INTR_AGGR/UFSHCI_QUIRK_SKIP_RESET_INTR_AGGR)

Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
---
 drivers/scsi/ufs/ufshcd.c | 3 ++-
 drivers/scsi/ufs/ufshcd.h | 7 +++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 8aac98f..7b62d8b 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -3713,7 +3713,8 @@ 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_RESET_INTR_AGGR))
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 dfa17ac..d6861ed 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -505,6 +505,13 @@ struct ufs_hba {
 */
#define UFSHCD_QUIRK_BROKEN_HCE UFS_BIT(9)
 
+   /*
+* This quirk is only not to reset interrupt aggregation logic
+* in ISR. The reset can make the host controller miss an event
+* of previously completed IO.
+*/
+   #define UFSHCI_QUIRK_SKIP_RESET_INTR_AGGR   UFS_BIT(10)
+
unsigned int quirks;/* Deviations from standard UFSHCI spec. */
 
/* Device deviations from standard UFS device spec. */
-- 
2.1.4


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] ufs: introduce UFSHCD_QUIRK_PRDT_BYTE_GRAN quirk

[Kiwoong Kim]

Some UFS host controllers may think
granularitys of PRDT length and offset as bytes, not double words.

Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
---
 drivers/scsi/ufs/ufshcd.c | 28 +---
 drivers/scsi/ufs/ufshcd.h |  6 ++
 2 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index e75fbb3..d6e3112 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1128,7 +1128,7 @@ ufshcd_send_uic_cmd(struct ufs_hba *hba, struct 
uic_command *uic_cmd)
  *
  * Returns 0 in case of success, non-zero value in case of failure
  */
-static int ufshcd_map_sg(struct ufshcd_lrb *lrbp)
+static int ufshcd_map_sg(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 {
struct ufshcd_sg_entry *prd_table;
struct scatterlist *sg;
@@ -1142,8 +1142,13 @@ static int ufshcd_map_sg(struct ufshcd_lrb *lrbp)
return sg_segments;
 
if (sg_segments) {
-   lrbp->utr_descriptor_ptr->prd_table_length =
-   cpu_to_le16((u16) (sg_segments));
+   if (hba->quirks & UFSHCD_QUIRK_PRDT_BYTE_GRAN)
+   lrbp->utr_descriptor_ptr->prd_table_length =
+   cpu_to_le16((u16)(sg_segments *
+   sizeof(struct ufshcd_sg_entry)));
+   else
+   lrbp->utr_descriptor_ptr->prd_table_length =
+   cpu_to_le16((u16) (sg_segments));
 
prd_table = (struct ufshcd_sg_entry *)lrbp->ucd_prdt_ptr;
 
@@ -1505,7 +1510,7 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, 
struct scsi_cmnd *cmd)
 
ufshcd_comp_scsi_upiu(hba, lrbp);
 
-   err = ufshcd_map_sg(lrbp);
+   err = ufshcd_map_sg(hba, lrbp);
if (err) {
lrbp->cmd = NULL;
clear_bit_unlock(tag, >lrb_in_use);
@@ -2366,12 +2371,21 @@ static void ufshcd_host_memory_configure(struct ufs_hba 
*hba)

cpu_to_le32(upper_32_bits(cmd_desc_element_addr));
 
/* Response upiu and prdt offset should be in double words */
-   utrdlp[i].response_upiu_offset =
+   if (hba->quirks & UFSHCD_QUIRK_PRDT_BYTE_GRAN) {
+   utrdlp[i].response_upiu_offset =
+   cpu_to_le16(response_offset);
+   utrdlp[i].prd_table_offset =
+   cpu_to_le16(prdt_offset);
+   utrdlp[i].response_upiu_length =
+   cpu_to_le16(ALIGNED_UPIU_SIZE);
+   } else {
+   utrdlp[i].response_upiu_offset =
cpu_to_le16((response_offset >> 2));
-   utrdlp[i].prd_table_offset =
+   utrdlp[i].prd_table_offset =
cpu_to_le16((prdt_offset >> 2));
-   utrdlp[i].response_upiu_length =
+   utrdlp[i].response_upiu_length =
cpu_to_le16(ALIGNED_UPIU_SIZE >> 2);
+   }
 
hba->lrb[i].utr_descriptor_ptr = (utrdlp + i);
hba->lrb[i].ucd_req_ptr =
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 8e76501..7d9ff22 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -485,6 +485,12 @@ struct ufs_hba {
 */
#define UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION UFS_BIT(5)
 
+   /*
+* This quirk needs to be enabled if the host contoller regards
+* resolution of the values of PRDTO and PRDTL in UTRD as byte.
+*/
+   #define UFSHCD_QUIRK_PRDT_BYTE_GRAN UFS_BIT(7)
+
unsigned int quirks;/* Deviations from standard UFSHCI spec. */
 
/* Device deviations from standard UFS device spec. */
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] ufs: introduce hibern8_notify callback

[Kiwoong Kim]

Some UFS host controller may need to configure some things
around hibern8 enter/exit

v2: change the callback name and data type of 2nd argument

Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
---
 drivers/scsi/ufs/ufshcd.c | 12 ++--
 drivers/scsi/ufs/ufshcd.h | 12 
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 301b592..e75fbb3 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -2697,6 +2697,8 @@ static int __ufshcd_uic_hibern8_enter(struct ufs_hba *hba)
int ret;
struct uic_command uic_cmd = {0};
 
+   ufshcd_vops_hibern8_notify(hba, UIC_CMD_DME_HIBER_ENTER, PRE_CHANGE);
+
uic_cmd.command = UIC_CMD_DME_HIBER_ENTER;
ret = ufshcd_uic_pwr_ctrl(hba, _cmd);
 
@@ -2710,7 +2712,9 @@ static int __ufshcd_uic_hibern8_enter(struct ufs_hba *hba)
 */
if (ufshcd_link_recovery(hba))
ret = -ENOLINK;
-   }
+   } else
+   ufshcd_vops_hibern8_notify(hba, UIC_CMD_DME_HIBER_ENTER,
+   POST_CHANGE);
 
return ret;
 }
@@ -2733,13 +2737,17 @@ static int ufshcd_uic_hibern8_exit(struct ufs_hba *hba)
struct uic_command uic_cmd = {0};
int ret;
 
+   ufshcd_vops_hibern8_notify(hba, UIC_CMD_DME_HIBER_EXIT, PRE_CHANGE);
+
uic_cmd.command = UIC_CMD_DME_HIBER_EXIT;
ret = ufshcd_uic_pwr_ctrl(hba, _cmd);
if (ret) {
dev_err(hba->dev, "%s: hibern8 exit failed. ret = %d\n",
__func__, ret);
ret = ufshcd_link_recovery(hba);
-   }
+   } else
+   ufshcd_vops_hibern8_notify(hba, UIC_CMD_DME_HIBER_EXIT,
+   POST_CHANGE);
 
return ret;
 }
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 05d8384..8e76501 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -265,6 +265,8 @@ struct ufs_pwr_mode_info {
  *  to set some things
  * @setup_task_mgmt: called before any task management request is issued
  *  to set some things
+ * @hibern8_notify: called around hibern8 enter/exit
+ * to configure some things
  * @suspend: called during host controller PM callback
  * @resume: called during host controller PM callback
  * @dbg_register_dump: used to dump controller debug information
@@ -290,6 +292,8 @@ struct ufs_hba_variant_ops {
struct ufs_pa_layer_attr *);
void(*setup_xfer_req)(struct ufs_hba *, int, bool);
void(*setup_task_mgmt)(struct ufs_hba *, int, u8);
+   void(*hibern8_notify)(struct ufs_hba *, enum uic_cmd_dme,
+  enum ufs_notify_change_status);
int (*suspend)(struct ufs_hba *, enum ufs_pm_op);
int (*resume)(struct ufs_hba *, enum ufs_pm_op);
void(*dbg_register_dump)(struct ufs_hba *hba);
@@ -821,6 +825,14 @@ static inline void ufshcd_vops_setup_task_mgmt(struct 
ufs_hba *hba,
return hba->vops->setup_task_mgmt(hba, tag, tm_function);
 }
 
+static inline void ufshcd_vops_hibern8_notify(struct ufs_hba *hba,
+   enum uic_cmd_dme cmd,
+   enum ufs_notify_change_status status)
+{
+   if (hba->vops && hba->vops->hibern8_notify)
+   return hba->vops->hibern8_notify(hba, cmd, status);
+}
+
 static inline int ufshcd_vops_suspend(struct ufs_hba *hba, enum ufs_pm_op op)
 {
if (hba->vops && hba->vops->suspend)
-- 
2.1.4


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] ufs: introduce setup_task_mgmt

[Kiwoong Kim]

Some UFS host controller may need to configure some things
before any task management request is issued

v2: move a comment to another place

Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
---
 drivers/scsi/ufs/ufshcd.c |  2 ++
 drivers/scsi/ufs/ufshcd.h | 10 ++
 2 files changed, 12 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index bf78321..301b592 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -4357,6 +4357,8 @@ static int ufshcd_issue_tm_cmd(struct ufs_hba *hba, int 
lun_id, int task_id,
task_req_upiup->input_param1 = cpu_to_be32(lun_id);
task_req_upiup->input_param2 = cpu_to_be32(task_id);
 
+   ufshcd_vops_setup_task_mgmt(hba, free_slot, tm_function);
+
/* send command to the controller */
__set_bit(free_slot, >outstanding_tasks);
 
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index e935fd1..05d8384 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -263,6 +263,8 @@ struct ufs_pwr_mode_info {
  * to be set.
  * @setup_xfer_req: called before any transfer request is issued
  *  to set some things
+ * @setup_task_mgmt: called before any task management request is issued
+ *  to set some things
  * @suspend: called during host controller PM callback
  * @resume: called during host controller PM callback
  * @dbg_register_dump: used to dump controller debug information
@@ -287,6 +289,7 @@ struct ufs_hba_variant_ops {
struct ufs_pa_layer_attr *,
struct ufs_pa_layer_attr *);
void(*setup_xfer_req)(struct ufs_hba *, int, bool);
+   void(*setup_task_mgmt)(struct ufs_hba *, int, u8);
int (*suspend)(struct ufs_hba *, enum ufs_pm_op);
int (*resume)(struct ufs_hba *, enum ufs_pm_op);
void(*dbg_register_dump)(struct ufs_hba *hba);
@@ -811,6 +814,13 @@ static inline void ufshcd_vops_setup_xfer_req(struct 
ufs_hba *hba, int tag,
return hba->vops->setup_xfer_req(hba, tag, is_scsi_cmd);
 }
 
+static inline void ufshcd_vops_setup_task_mgmt(struct ufs_hba *hba,
+   int tag, u8 tm_function)
+{
+   if (hba->vops && hba->vops->setup_task_mgmt)
+   return hba->vops->setup_task_mgmt(hba, tag, tm_function);
+}
+
 static inline int ufshcd_vops_suspend(struct ufs_hba *hba, enum ufs_pm_op op)
 {
if (hba->vops && hba->vops->suspend)
-- 
2.1.4


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] ufs: introduce setup_xfer_req callback

[Kiwoong Kim]

Some UFS host controller may need to configure some things
before any transfer request is issued.

V2: change data type of 2nd argument

Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
---
 drivers/scsi/ufs/ufshcd.c |  2 ++
 drivers/scsi/ufs/ufshcd.h | 10 ++
 2 files changed, 12 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 8cf5d8f..bf78321 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1516,6 +1516,7 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, 
struct scsi_cmnd *cmd)
 
/* issue command to the controller */
spin_lock_irqsave(hba->host->host_lock, flags);
+   ufshcd_vops_setup_xfer_req(hba, tag, (lrbp->cmd ? true : false));
ufshcd_send_command(hba, tag);
 out_unlock:
spin_unlock_irqrestore(hba->host->host_lock, flags);
@@ -1727,6 +1728,7 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba *hba,
/* Make sure descriptors are ready before ringing the doorbell */
wmb();
spin_lock_irqsave(hba->host->host_lock, flags);
+   ufshcd_vops_setup_xfer_req(hba, tag, (lrbp->cmd ? true : false));
ufshcd_send_command(hba, tag);
spin_unlock_irqrestore(hba->host->host_lock, flags);
 
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index afff7f4..e935fd1 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -261,6 +261,8 @@ struct ufs_pwr_mode_info {
  * @pwr_change_notify: called before and after a power mode change
  * is carried out to allow vendor spesific capabilities
  * to be set.
+ * @setup_xfer_req: called before any transfer request is issued
+ *  to set some things
  * @suspend: called during host controller PM callback
  * @resume: called during host controller PM callback
  * @dbg_register_dump: used to dump controller debug information
@@ -284,6 +286,7 @@ struct ufs_hba_variant_ops {
enum ufs_notify_change_status status,
struct ufs_pa_layer_attr *,
struct ufs_pa_layer_attr *);
+   void(*setup_xfer_req)(struct ufs_hba *, int, bool);
int (*suspend)(struct ufs_hba *, enum ufs_pm_op);
int (*resume)(struct ufs_hba *, enum ufs_pm_op);
void(*dbg_register_dump)(struct ufs_hba *hba);
@@ -801,6 +804,13 @@ static inline int ufshcd_vops_pwr_change_notify(struct 
ufs_hba *hba,
return -ENOTSUPP;
 }
 
+static inline void ufshcd_vops_setup_xfer_req(struct ufs_hba *hba, int tag,
+   bool is_scsi_cmd)
+{
+   if (hba->vops && hba->vops->setup_xfer_req)
+   return hba->vops->setup_xfer_req(hba, tag, is_scsi_cmd);
+}
+
 static inline int ufshcd_vops_suspend(struct ufs_hba *hba, enum ufs_pm_op op)
 {
if (hba->vops && hba->vops->suspend)
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v1] ufs: add a variety of definitions decribed in UFS spec

[Kiwoong Kim]

Hi, Subhash

I'll update this patch referring what you said below except for creating of
"mphy.h".

> >> > These things are defined to be used by some UFS Host controllers.
> >> >
> >> > Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
> >> > ---
> >> >  drivers/scsi/ufs/mphy.h   | 38
++
> >> >  drivers/scsi/ufs/ufshci.h | 14 +++---
> >> > drivers/scsi/ufs/unipro.h | 24 
> >> >  3 files changed, 73 insertions(+), 3 deletions(-)  create mode
> >> > 100644 drivers/scsi/ufs/mphy.h
> >> >
> >> > diff --git a/drivers/scsi/ufs/mphy.h b/drivers/scsi/ufs/mphy.h new
> >> > file mode 100644 index 000..c431f49
> >> > --- /dev/null
> >> > +++ b/drivers/scsi/ufs/mphy.h
> >> > @@ -0,0 +1,38 @@
> >> > +/*
> >> > + * drivers/scsi/ufs/mphy.h
> >> > + *
> >> > + * Copyright (C) 2014 Samsung Electronics Co., Ltd.
> >> > + *
> >> > + * This program is free software; you can redistribute it and/or
> >> > modify
> >> > + * it under the terms of the GNU General Public License as
> >> > + published
> >> > by
> >> > + * the Free Software Foundation; either version 2 of the License,
> >> > + or
> >> > + * (at your option) any later version.
> >> > + */
> >> > +
> >> > +#ifndef _MPHY_H_
> >> > +#define _MPHY_H_
> >>
> >> Do we really need separate file for MPHY? May be we can have these
> >> accomodated in unipro.h itself?
> >
> > I think it's needed. MPHY spec isn't related with Unipro spec directly.
> 
> Yes but for UFS host controller, Unipro is the communication channel
> between host and device, includes M-PHY as well.
> 
> > Is there any reason why we say that this file is deprecated?
> > Please let me know if you have.
> 
> These are only few declarations and i guess it would be appopriate to put
> them under UniPro header file. Anyways it is your choice.

As you said, for now, there are not many declarations of mphy.
But I expect adding more in the future.
I think that all other UFS driver including coming new ones, if any,
aren't separated from mphy like "ufs-qcom.c", especially Exynos driver.

Anyway, thank you for your opinion.

> 
> >
> >>
> >> > +
> >> > +#define TX_HIBERN8TIME_CAP  0x0f
> >> > +#define TX_MIN_ACTIVATE_TIME0x33
> >> > +
> >> > +#define RX_HS_G1_SYNC_LEN_CAP   0x8b
> >> > +#define RX_HS_G1_PREP_LEN_CAP   0x8c
> >> > +#define RX_HS_G2_SYNC_LEN_CAP   0x94
> >> > +#define RX_HS_G3_SYNC_LEN_CAP   0x95
> >> > +#define RX_HS_G2_PREP_LEN_CAP   0x96
> >> > +#define RX_HS_G3_PREP_LEN_CAP   0x97
> >> > + #define SYNC_RANGE_FINE(0 << 6)
> >> > + #define SYNC_RANGE_COARSE  (1 << 6)
> >> > + #define SYNC_LEN(x)((x) & 0x3f)
> >> > + #define PREP_LEN(x)((x) & 0xf)
> >> > +#define RX_ADV_GRANULARITY_CAP  0x98
> >> > + #define RX_ADV_GRAN_STEP(x)x) & 0x3) << 1) | 0x1)
> >> > +#define TX_ADV_GRANULARITY_CAP  0x10
> >> > + #define TX_ADV_GRAN_STEP(x)x) & 0x3) << 1) | 0x1)
> >> > +#define RX_MIN_ACTIVATETIME_CAP 0x8f
> >> > +#define RX_HIBERN8TIME_CAP  0x92
> >> > +#define RX_ADV_HIBERN8TIME_CAP  0x99
> >> > +#define RX_ADV_MIN_ACTIVATETIME_CAP 0x9a
> >> > +
> >> > +#endif /* _MPHY_H_ */
> >> > +
> >> > diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
> >> > index 9599741..26dc340 100644
> >> > --- a/drivers/scsi/ufs/ufshci.h
> >> > +++ b/drivers/scsi/ufs/ufshci.h
> >> > @@ -170,17 +170,25 @@ enum {
> >> >  /* UECDL - Host UIC Error Code Data Link Layer 3Ch */
> >> >  #define UIC_DATA_LINK_LAYER_ERROR   UFS_BIT(31)
> >> >  #define UIC_DATA_LINK_LAYER_ERROR_CODE_MASK 0x7FFF
> >> > -#define UIC_DATA_LINK_LAYER_ERROR_PA_INIT   0x2000
> >> > -#define UIC_DATA_LINK_LAYER_ERROR_NAC_RECEIVED  0x0001
> >> > -#define UIC_DATA_LINK_LAYER_ERROR_TCx_REPLAY_TIMEOUT 0x0002
> >> > +#define UIC_DATA_LINK_LAYER_ERROR_NAC_RECEIVED  UFS_BIT(0)
> >> > +#define UIC_DATA_LINK_LAYER_ERROR_TCx_REPLAY_TIMEOUTUFS_BIT

RE: [PATCH v1] ufs: introduce UFSHCI_QUIRK_SKIP_INTR_AGGR quirk

[Kiwoong Kim]

Hi, Subhash

I'll update this patch referring what you said (including adding comments of
other quirks)

> > 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.
> >
> > Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
> > ---
> >  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.
> 
> > +   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_CLRUFS_BIT(8)
> > #define UFSHCD_QUIRK_USE_OF_HCE UFS_BIT(9)
> > +   #define UFSHCI_QUIRK_SKIP_INTR_AGGR UFS_BIT(10)
> >
> >
> > 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 majord...@vger.kernel.org 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v1] ufs: introduce UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR quirk

[Kiwoong Kim]

> > Some UFS host controllers may clear a transfer request slot by setting
> > an associated bit in UTLRCLR/UTMLRCLR to 1, not 0.
> 
> s/UTLRCLR/UTRLCLR
> s/UTMLRCLR/UTMRLCLR
> 

Okay

> 
> > That's opposite to what UFS spec decribes.
> >
> > Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
> > ---
> >  drivers/scsi/ufs/ufshcd.c | 32 
> > drivers/scsi/ufs/ufshcd.h |  1 +
> >  2 files changed, 29 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index 549e3e8..24d6ea7 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);
> >  }
> >
> >  /**
> > @@ -1147,7 +1171,7 @@ ufshcd_send_uic_cmd(struct ufs_hba *hba, struct
> > uic_command *uic_cmd)
> >   *
> >   * Returns 0 in case of success, non-zero value in case of failure
> >   */
> > -static int ufshcd_map_sg(struct ufshcd_lrb *lrbp)
> > +static int ufshcd_map_sg(struct ufs_hba *hba, struct ufshcd_lrb
> > +*lrbp)
> 
> unrelated change. Please remove it.
> 

Okay

> >  {
> > struct ufshcd_sg_entry *prd_table;
> > struct scatterlist *sg;
> > @@ -1529,7 +1553,7 @@ static int ufshcd_queuecommand(struct Scsi_Host
> > *host, struct scsi_cmnd *cmd)
> >
> > ufshcd_comp_scsi_upiu(hba, lrbp);
> >
> > -   err = ufshcd_map_sg(lrbp);
> > +   err = ufshcd_map_sg(hba, lrbp);
> 
> unrelated change.  Please remove it.

Okay

> 
> 
> > if (err) {
> > lrbp->cmd = NULL;
> > clear_bit_unlock(tag, >lrb_in_use); @@ -4329,7 +4353,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
> > 565f005..c4abd76 100644
> > --- a/drivers/scsi/ufs/ufshcd.h
> > +++ b/drivers/scsi/ufs/ufshcd.h
> > @@ -495,6 +495,7 @@ struct ufs_hba {
> >
> > #define UFSHCD_QUIRK_GET_VS_RESULT  UFS_BIT(6)
> > #define UFSHCD_QUIRK_BROKEN_DWORD_UTRD  UFS_BIT(7)
> > +   #define UFSHCD_QUIRK_BROKEN_REQ_LIST_CLRUFS_BIT(8)
> >
> >
> > 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 majord...@vger.kernel.org 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v1] ufs: introcude UFSHCD_QUIRK_GET_VS_RESULT quirk

[Kiwoong Kim]

> > Some UFS host controllers may not report a result of UIC command
> > completion properly.
> > In such cases, they should get the result from UIC directly if their
> > architectures support that.
> >
> > Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
> > ---
> >  drivers/scsi/ufs/ufshcd.c | 27 +++
> > drivers/scsi/ufs/ufshcd.h | 20 
> >  2 files changed, 43 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index d4a5a9c..8e19631 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -1011,7 +1011,10 @@ static inline bool
> > ufshcd_ready_for_uic_cmd(struct ufs_hba *hba)
> >   */
> >  static inline u8 ufshcd_get_upmcrs(struct ufs_hba *hba)  {
> > -   return (ufshcd_readl(hba, REG_CONTROLLER_STATUS) >> 8) & 0x7;
> > +   if (hba->quirks & UFSHCD_QUIRK_GET_VS_RESULT)
> > +   return (u8)ufshcd_vops_get_vs_info(hba, VS_OP_03);
> > +   else
> > +   return (ufshcd_readl(hba, REG_CONTROLLER_STATUS) >> 8) &
0x7;
> >  }
> >
> >  /**
> > @@ -1038,6 +1041,17 @@ ufshcd_dispatch_uic_cmd(struct ufs_hba *hba,
> > struct uic_command *uic_cmd)
> >   REG_UIC_COMMAND);
> >  }
> >
> > +static inline enum vs_opcode ufshcd_get_vs_opcode(struct uic_command
> > *uic_cmd)
> > +{
> > +   if (uic_cmd->command == UIC_CMD_DME_LINK_STARTUP)
> > +   return VS_OP_00;
> > +   else if ((uic_cmd->command == UIC_CMD_DME_HIBER_ENTER))
> > +   return VS_OP_01;
> > +   else if ((uic_cmd->command == UIC_CMD_DME_HIBER_EXIT))
> > +   return VS_OP_02;
> > +   else
> > +   return VS_OP_INVALID;
> > +}
> >  /**
> >   * ufshcd_wait_for_uic_cmd - Wait complectioin of UIC command
> >   * @hba: per adapter instance
> > @@ -1051,11 +1065,16 @@ ufshcd_wait_for_uic_cmd(struct ufs_hba *hba,
> > struct uic_command *uic_cmd)  {
> > int ret;
> > unsigned long flags;
> > +   enum vs_opcode vs_op = ufshcd_get_vs_opcode(uic_cmd);
> 
> Please move this after we check the quirk below.

Okay, I'll apply what you said.

> 
> >
> > if (wait_for_completion_timeout(_cmd->done,
> > -   msecs_to_jiffies(UIC_CMD_TIMEOUT)))
> > -   ret = uic_cmd->argument2 & MASK_UIC_COMMAND_RESULT;
> > -   else
> > +   msecs_to_jiffies(UIC_CMD_TIMEOUT)))
{
> > +   if (hba->quirks & UFSHCD_QUIRK_GET_VS_RESULT &&
> > +   vs_op != VS_OP_INVALID)
> > +   ret = ufshcd_vops_get_vs_info(hba, vs_op);
> > +   else
> > +   ret = uic_cmd->argument2 & MASK_UIC_COMMAND_RESULT;
> > +   } else
> > ret = -ETIMEDOUT;
> >
> > spin_lock_irqsave(hba->host->host_lock, flags);
> > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> > index 13504b4..8cf3991 100644
> > --- a/drivers/scsi/ufs/ufshcd.h
> > +++ b/drivers/scsi/ufs/ufshcd.h
> > @@ -245,6 +245,14 @@ struct ufs_pwr_mode_info {
> > struct ufs_pa_layer_attr info;
> >  };
> >
> > +enum vs_opcode {
> > +   VS_OP_00 = 0x00,
> > +   VS_OP_01,
> > +   VS_OP_02,
> > +   VS_OP_03,
> > +   VS_OP_INVALID = 0xFF,
> > +};
> > +
> 
> How do we interpret these codes? and how is this useful for non-exynos
> UFS host controller? Please make it generic which can be used by other
> controllers in future (if required).

As you guessed, this patch is to get some information
by using vendor specific ways directly, which was got from standard
UFSHCI SFRs for some hardware problems.

If other controllers have similar problems, but the problems are acutally
another cases, you should add the function call - ufshcd_vops_get_vs_info
- wherever you want to use this interface.

This patch is intended to cover all results of link startup, hibern8 and
power mode change. Because I don't want to add an additional interface
whenever a new controller has a new problem similar with what I mentioned
above.

Anyway, could you give me your opinion about it?
Or, if you don't agree applying something just to fix some simple problems,
such as reading SFRs, I won't update this patch any more.

Thank you.

> 
> >  /**
> >   * struct ufs_hba_variant_ops - variant specific callbacks
> >   * @name: variant name
> > @@ -297,6 +305,7 @@ struct ufs_hba_variant_ops {
> > int (*re

RE: [PATCH v1] ufs: introduce setup_hibern8 callback

[Kiwoong Kim]

> > Some UFS host controller may need to configure some things around
> > hibern8 enter/exit
> >
> > Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
> > ---
> >  drivers/scsi/ufs/ufshcd.c | 10 --  drivers/scsi/ufs/ufshcd.h
> > | 10 ++
> >  2 files changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index fdb0502..d4a5a9c 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -2697,6 +2697,8 @@ static int __ufshcd_uic_hibern8_enter(struct
> > ufs_hba *hba)
> > int ret;
> > struct uic_command uic_cmd = {0};
> >
> > +   ufshcd_vops_setup_hibern8(hba, true, PRE_CHANGE);
> > +
> > uic_cmd.command = UIC_CMD_DME_HIBER_ENTER;
> > ret = ufshcd_uic_pwr_ctrl(hba, _cmd);
> >
> > @@ -2710,7 +2712,8 @@ static int __ufshcd_uic_hibern8_enter(struct
> > ufs_hba *hba)
> >  */
> > if (ufshcd_link_recovery(hba))
> > ret = -ENOLINK;
> > -   }
> > +   } else
> > +   ufshcd_vops_setup_hibern8(hba, true, POST_CHANGE);
> >
> > return ret;
> >  }
> > @@ -2733,13 +2736,16 @@ static int ufshcd_uic_hibern8_exit(struct
> > ufs_hba *hba)
> > struct uic_command uic_cmd = {0};
> > int ret;
> >
> > +   ufshcd_vops_setup_hibern8(hba, false, PRE_CHANGE);
> > +
> > uic_cmd.command = UIC_CMD_DME_HIBER_EXIT;
> > ret = ufshcd_uic_pwr_ctrl(hba, _cmd);
> > if (ret) {
> > dev_err(hba->dev, "%s: hibern8 exit failed. ret = %d\n",
> > __func__, ret);
> > ret = ufshcd_link_recovery(hba);
> > -   }
> > +   } else
> > +   ufshcd_vops_setup_hibern8(hba, false, POST_CHANGE);
> >
> > return ret;
> >  }
> > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> > index b084d89..13504b4 100644
> > --- a/drivers/scsi/ufs/ufshcd.h
> > +++ b/drivers/scsi/ufs/ufshcd.h
> > @@ -265,6 +265,8 @@ struct ufs_pwr_mode_info {
> >   *  to set some things
> >   * @setup_task_mgmt: called before any task management request is
> > issued
> >   *  to set some things
> > + * @setup_hibern8: called around hibern8 enter/exit
> > + * to configure some things
> >   * @suspend: called during host controller PM callback
> >   * @resume: called during host controller PM callback
> >   * @dbg_register_dump: used to dump controller debug information @@
> > -290,6 +292,7 @@ struct ufs_hba_variant_ops {
> > struct ufs_pa_layer_attr *);
> > void(*setup_xfer_req)(struct ufs_hba *, int, bool);
> > void(*setup_task_mgmt)(struct ufs_hba *, int, u8);
> > +   void(*setup_hibern8)(struct ufs_hba *, bool, bool);
> 
> Can we change the name to "hibern8_notify" ? You may check other
> ufs_hba_variant_ops for reference.

Okay, I'll apply what you said.

> 
> > int (*suspend)(struct ufs_hba *, enum ufs_pm_op);
> > int (*resume)(struct ufs_hba *, enum ufs_pm_op);
> > void(*dbg_register_dump)(struct ufs_hba *hba);
> > @@ -821,6 +824,13 @@ static inline void
> > ufshcd_vops_setup_task_mgmt(struct ufs_hba *hba,
> > return hba->vops->setup_task_mgmt(hba, tag, tm_function);  }
> >
> > +static inline void ufshcd_vops_setup_hibern8(struct ufs_hba *hba,
> > +   bool enter, bool notify)
> 
> Using bool to specify whether it is hibern8 enter or hibern8 exit doesn't
> seem to be readable. May be you can pass the UIC_CMD_DME_HIBER_ENTER or
> UIC_CMD_DME_HIBER_EXIT (in other words use "enum uic_cmd_dme" type).
> 
> also "notify" type should be changed from "bool" to "enum
> ufs_notify_change_status".

Okay, I'll apply what you said and use "enum uic_cmd_dme"
as 2nd argument date type.

> 
> > +{
> > +   if (hba->vops && hba->vops->setup_hibern8)
> > +   return hba->vops->setup_hibern8(hba, enter, notify); }
> > +
> >  static inline int ufshcd_vops_suspend(struct ufs_hba *hba, enum
> > ufs_pm_op op)  {
> > if (hba->vops && hba->vops->suspend)
> 
> --
> 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 majord...@vger.kernel.org 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v1] ufs: introduce setup_xfer_req callback

[Kiwoong Kim]

> > Some UFS host controller may need to configure some things before any
> > transfer request is issued.
> >
> > Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
> > ---
> >  drivers/scsi/ufs/ufshcd.c |  2 ++
> >  drivers/scsi/ufs/ufshcd.h | 10 ++
> >  2 files changed, 12 insertions(+)
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index 8cf5d8f..bf78321 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -1516,6 +1516,7 @@ static int ufshcd_queuecommand(struct Scsi_Host
> > *host, struct scsi_cmnd *cmd)
> >
> > /* issue command to the controller */
> > spin_lock_irqsave(hba->host->host_lock, flags);
> > +   ufshcd_vops_setup_xfer_req(hba, tag, (lrbp->cmd ? true : false));
> > ufshcd_send_command(hba, tag);
> >  out_unlock:
> > spin_unlock_irqrestore(hba->host->host_lock, flags); @@ -1727,6
> > +1728,7 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba *hba,
> > /* Make sure descriptors are ready before ringing the doorbell */
> > wmb();
> > spin_lock_irqsave(hba->host->host_lock, flags);
> > +   ufshcd_vops_setup_xfer_req(hba, tag, (lrbp->cmd ? true : false));
> > ufshcd_send_command(hba, tag);
> > spin_unlock_irqrestore(hba->host->host_lock, flags);
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> > index afff7f4..f2a69c0 100644
> > --- a/drivers/scsi/ufs/ufshcd.h
> > +++ b/drivers/scsi/ufs/ufshcd.h
> > @@ -261,6 +261,8 @@ struct ufs_pwr_mode_info {
> >   * @pwr_change_notify: called before and after a power mode change
> >   * is carried out to allow vendor spesific capabilities
> >   * to be set.
> > + * @setup_xfer_req: called before any transfer request is issued
> > + *  to set some things
> >   * @suspend: called during host controller PM callback
> >   * @resume: called during host controller PM callback
> >   * @dbg_register_dump: used to dump controller debug information @@
> > -284,6 +286,7 @@ struct ufs_hba_variant_ops {
> > enum ufs_notify_change_status
status,
> > struct ufs_pa_layer_attr *,
> > struct ufs_pa_layer_attr *);
> > +   void(*setup_xfer_req)(struct ufs_hba *, int, bool);
> > int (*suspend)(struct ufs_hba *, enum ufs_pm_op);
> > int (*resume)(struct ufs_hba *, enum ufs_pm_op);
> > void(*dbg_register_dump)(struct ufs_hba *hba);
> > @@ -801,6 +804,13 @@ static inline int
> > ufshcd_vops_pwr_change_notify(struct ufs_hba *hba,
> > return -ENOTSUPP;
> >  }
> >
> > +static inline void ufshcd_vops_setup_xfer_req(struct ufs_hba *hba,
> > +int
> > tag,
> > +   bool is_cmd_not_null)
> 
> This might be more readable: s/is_cmd_not_null/is_scsi_cmd , rest looks
> good in this patch.

Okay. I'll apply what you said.

> 
> 
> > +{
> > +   if (hba->vops && hba->vops->setup_xfer_req)
> > +   return hba->vops->setup_xfer_req(hba, tag, is_cmd_not_null);
> > +}
> > +
> >  static inline int ufshcd_vops_suspend(struct ufs_hba *hba, enum
> > ufs_pm_op op)
> >  {
> > if (hba->vops && hba->vops->suspend)
> 
> --
> 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 majord...@vger.kernel.org
> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v1] ufs: introduce setup_task_mgmt

[Kiwoong Kim]

> > Some UFS host controller may need to configure some things before any
> > task management request is issued
> >
> > Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
> > ---
> >  drivers/scsi/ufs/ufshcd.c |  1 +
> >  drivers/scsi/ufs/ufshcd.h | 10 ++
> >  2 files changed, 11 insertions(+)
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index bf78321..fdb0502 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -4358,6 +4358,7 @@ static int ufshcd_issue_tm_cmd(struct ufs_hba
> > *hba, int lun_id, int task_id,
> > task_req_upiup->input_param2 = cpu_to_be32(task_id);
> >
> > /* send command to the controller */
> 
> May be you need to move this comment just above __set_bit() call. Rest all
> looks good in this patch.

Okay. I'll apply what you said on new version of this patch..

> 
> > +   ufshcd_vops_setup_task_mgmt(hba, free_slot, tm_function);
> > __set_bit(free_slot, >outstanding_tasks);
> >
> > /* Make sure descriptors are ready before ringing the task doorbell
> > */ diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> > index f2a69c0..b084d89 100644
> > --- a/drivers/scsi/ufs/ufshcd.h
> > +++ b/drivers/scsi/ufs/ufshcd.h
> > @@ -263,6 +263,8 @@ struct ufs_pwr_mode_info {
> >   * to be set.
> >   * @setup_xfer_req: called before any transfer request is issued
> >   *  to set some things
> > + * @setup_task_mgmt: called before any task management request is
> > issued
> > + *  to set some things
> >   * @suspend: called during host controller PM callback
> >   * @resume: called during host controller PM callback
> >   * @dbg_register_dump: used to dump controller debug information @@
> > -287,6 +289,7 @@ struct ufs_hba_variant_ops {
> > struct ufs_pa_layer_attr *,
> > struct ufs_pa_layer_attr *);
> > void(*setup_xfer_req)(struct ufs_hba *, int, bool);
> > +   void(*setup_task_mgmt)(struct ufs_hba *, int, u8);
> > int (*suspend)(struct ufs_hba *, enum ufs_pm_op);
> > int (*resume)(struct ufs_hba *, enum ufs_pm_op);
> > void(*dbg_register_dump)(struct ufs_hba *hba);
> > @@ -811,6 +814,13 @@ static inline void
> > ufshcd_vops_setup_xfer_req(struct ufs_hba *hba, int tag,
> > return hba->vops->setup_xfer_req(hba, tag,
> is_cmd_not_null);  }
> >
> > +static inline void ufshcd_vops_setup_task_mgmt(struct ufs_hba *hba,
> > +   int tag, u8 tm_function)
> > +{
> > +   if (hba->vops && hba->vops->setup_task_mgmt)
> > +   return hba->vops->setup_task_mgmt(hba, tag, tm_function); }
> > +
> >  static inline int ufshcd_vops_suspend(struct ufs_hba *hba, enum
> > ufs_pm_op op)  {
> > if (hba->vops && hba->vops->suspend)
> 
> --
> 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 majord...@vger.kernel.org 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v1] ufs: introduce UFSHCI_QUIRK_SKIP_INTR_AGGR quirk

[Kiwoong Kim]

> > 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.
> >
> > Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
> > ---
> >  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.

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.


> 
> > +   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_CLRUFS_BIT(8)
> > #define UFSHCD_QUIRK_USE_OF_HCE UFS_BIT(9)
> > +   #define UFSHCI_QUIRK_SKIP_INTR_AGGR UFS_BIT(10)
> >
> >
> > 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 majord...@vger.kernel.org
> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v1] ufs: add a variety of definitions decribed in UFS spec

[Kiwoong Kim]

> > These things are defined to be used by some UFS Host controllers.
> >
> > Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
> > ---
> >  drivers/scsi/ufs/mphy.h   | 38 ++
> >  drivers/scsi/ufs/ufshci.h | 14 +++---
> > drivers/scsi/ufs/unipro.h | 24 
> >  3 files changed, 73 insertions(+), 3 deletions(-)  create mode 100644
> > drivers/scsi/ufs/mphy.h
> >
> > diff --git a/drivers/scsi/ufs/mphy.h b/drivers/scsi/ufs/mphy.h new
> > file mode 100644 index 000..c431f49
> > --- /dev/null
> > +++ b/drivers/scsi/ufs/mphy.h
> > @@ -0,0 +1,38 @@
> > +/*
> > + * drivers/scsi/ufs/mphy.h
> > + *
> > + * Copyright (C) 2014 Samsung Electronics Co., Ltd.
> > + *
> > + * This program is free software; you can redistribute it and/or
> > modify
> > + * it under the terms of the GNU General Public License as published
> > by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + */
> > +
> > +#ifndef _MPHY_H_
> > +#define _MPHY_H_
> 
> Do we really need separate file for MPHY? May be we can have these
> accomodated in unipro.h itself?

I think it's needed. MPHY spec isn't related with Unipro spec directly.
Is there any reason why we say that this file is deprecated?
Please let me know if you have.

> 
> > +
> > +#define TX_HIBERN8TIME_CAP 0x0f
> > +#define TX_MIN_ACTIVATE_TIME   0x33
> > +
> > +#define RX_HS_G1_SYNC_LEN_CAP  0x8b
> > +#define RX_HS_G1_PREP_LEN_CAP  0x8c
> > +#define RX_HS_G2_SYNC_LEN_CAP  0x94
> > +#define RX_HS_G3_SYNC_LEN_CAP  0x95
> > +#define RX_HS_G2_PREP_LEN_CAP  0x96
> > +#define RX_HS_G3_PREP_LEN_CAP  0x97
> > + #define SYNC_RANGE_FINE   (0 << 6)
> > + #define SYNC_RANGE_COARSE (1 << 6)
> > + #define SYNC_LEN(x)   ((x) & 0x3f)
> > + #define PREP_LEN(x)   ((x) & 0xf)
> > +#define RX_ADV_GRANULARITY_CAP 0x98
> > + #define RX_ADV_GRAN_STEP(x)   x) & 0x3) << 1) | 0x1)
> > +#define TX_ADV_GRANULARITY_CAP 0x10
> > + #define TX_ADV_GRAN_STEP(x)   x) & 0x3) << 1) | 0x1)
> > +#define RX_MIN_ACTIVATETIME_CAP0x8f
> > +#define RX_HIBERN8TIME_CAP 0x92
> > +#define RX_ADV_HIBERN8TIME_CAP 0x99
> > +#define RX_ADV_MIN_ACTIVATETIME_CAP0x9a
> > +
> > +#endif /* _MPHY_H_ */
> > +
> > diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
> > index 9599741..26dc340 100644
> > --- a/drivers/scsi/ufs/ufshci.h
> > +++ b/drivers/scsi/ufs/ufshci.h
> > @@ -170,17 +170,25 @@ enum {
> >  /* UECDL - Host UIC Error Code Data Link Layer 3Ch */
> >  #define UIC_DATA_LINK_LAYER_ERROR  UFS_BIT(31)
> >  #define UIC_DATA_LINK_LAYER_ERROR_CODE_MASK0x7FFF
> > -#define UIC_DATA_LINK_LAYER_ERROR_PA_INIT  0x2000
> > -#define UIC_DATA_LINK_LAYER_ERROR_NAC_RECEIVED 0x0001
> > -#define UIC_DATA_LINK_LAYER_ERROR_TCx_REPLAY_TIMEOUT 0x0002
> > +#define UIC_DATA_LINK_LAYER_ERROR_NAC_RECEIVED UFS_BIT(0)
> > +#define UIC_DATA_LINK_LAYER_ERROR_TCx_REPLAY_TIMEOUT   UFS_BIT(1)
> > +#define UIC_DATA_LINK_LAYER_ERROR_RX_BUF_OFUFS_BIT(5)
> 
> why don't we just add macros for all the bits in UECDL ? This makes it
> easy in future.

Okay. I'll think about it and apply new macros on new version of this patch.

> 
> > +#define UIC_DATA_LINK_LAYER_ERROR_PA_INIT  UFS_BIT(13)
> >
> >  /* UECN - Host UIC Error Code Network Layer 40h */
> >  #define UIC_NETWORK_LAYER_ERRORUFS_BIT(31)
> >  #define UIC_NETWORK_LAYER_ERROR_CODE_MASK  0x7
> > +#define UIC_NETWORK_UNSUPPORTED_HEADER_TYPEBIT(0)
> > +#define UIC_NETWORK_BAD_DEVICEID_ENC   BIT(1)
> > +#define UIC_NETWORK_LHDR_TRAP_PACKET_DROPPING  BIT(2)
> >
> >  /* UECT - Host UIC Error Code Transport Layer 44h */
> >  #define UIC_TRANSPORT_LAYER_ERROR  UFS_BIT(31)
> >  #define UIC_TRANSPORT_LAYER_ERROR_CODE_MASK0x7F
> > +#define UIC_TRANSPORT_UNSUPPORTED_HEADER_TYPE  BIT(0)
> > +#define UIC_TRANSPORT_UNKNOWN_CPORTID  BIT(1)
> > +#define UIC_TRANSPORT_NO_CONNECTION_RX BIT(2)
> > +#define UIC_TRANSPORT_BAD_TC   BIT(4)
> 
> May be add definition for bit-5 and 6 as well.

Okay. I'll add those things on new version of this patch.

> 
> >
> >  /* UECDME - Host UIC Error Co

RE: [PATCH v1] ufs: introduce UFSHCD_QUIRK_BROKEN_DWORD_UTRD quirk

[Kiwoong Kim]

> > Some UFS host controllers may think
> > granularitys of PRDT length and offset as bytes, not double words.
> >
> > Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
> > ---
> >  drivers/scsi/ufs/ufshcd.c | 24 +++-
> > drivers/scsi/ufs/ufshcd.h |  2 ++
> >  2 files changed, 21 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index 8e19631..549e3e8 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -1161,8 +1161,13 @@ static int ufshcd_map_sg(struct ufshcd_lrb
> > *lrbp)
> > return sg_segments;
> >
> > if (sg_segments) {
> > -   lrbp->utr_descriptor_ptr->prd_table_length =
> > -   cpu_to_le16((u16) (sg_segments));
> > +   if (hba->quirks & UFSHCD_QUIRK_BROKEN_DWORD_UTRD)
> 
> This might sound more specific:
> s/UFSHCD_QUIRK_BROKEN_DWORD_UTRD/UFSHCD_QUIRK_PRDT_BYTE_GRAN .

That sounds good. I'll apply what you said.

> 
> > +   lrbp->utr_descriptor_ptr->prd_table_length =
> > +   cpu_to_le16((u16)(sg_segments *
> > +   sizeof(struct ufshcd_sg_entry)));
> > +   else
> > +   lrbp->utr_descriptor_ptr->prd_table_length =
> > +   cpu_to_le16((u16) (sg_segments));
> >
> > prd_table = (struct ufshcd_sg_entry *)lrbp->ucd_prdt_ptr;
> >
> > @@ -2385,12 +2390,21 @@ static void
> > ufshcd_host_memory_configure(struct ufs_hba *hba)
> >
>   cpu_to_le32(upper_32_bits(cmd_desc_element_addr));
> >
> > /* Response upiu and prdt offset should be in double words
> */
> > -   utrdlp[i].response_upiu_offset =
> > +   if (hba->quirks & UFSHCD_QUIRK_BROKEN_DWORD_UTRD) {
> > +   utrdlp[i].response_upiu_offset =
> > +   cpu_to_le16(response_offset);
> > +   utrdlp[i].prd_table_offset =
> > +   cpu_to_le16(prdt_offset);
> > +   utrdlp[i].response_upiu_length =
> > +   cpu_to_le16(ALIGNED_UPIU_SIZE);
> > +   } else {
> > +   utrdlp[i].response_upiu_offset =
> > cpu_to_le16((response_offset >> 2));
> > -   utrdlp[i].prd_table_offset =
> > +   utrdlp[i].prd_table_offset =
> > cpu_to_le16((prdt_offset >> 2));
> > -   utrdlp[i].response_upiu_length =
> > +   utrdlp[i].response_upiu_length =
> > cpu_to_le16(ALIGNED_UPIU_SIZE >> 2);
> > +   }
> >
> > hba->lrb[i].utr_descriptor_ptr = (utrdlp + i);
> > hba->lrb[i].ucd_req_ptr =
> > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> > index 8cf3991..565f005 100644
> > --- a/drivers/scsi/ufs/ufshcd.h
> > +++ b/drivers/scsi/ufs/ufshcd.h
> > @@ -494,6 +494,8 @@ struct ufs_hba {
> > #define UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION UFS_BIT(5)
> >
> > #define UFSHCD_QUIRK_GET_VS_RESULT  UFS_BIT(6)
> > +   #define UFSHCD_QUIRK_BROKEN_DWORD_UTRD  UFS_BIT(7)
> > +
> 
> This extra line space isn't needed.

Okay.

> 
> >
> > 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 majord...@vger.kernel.org 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v1] ufs: introduce UFSHCD_QUIRK_USE_OF_HCE quirk

[Kiwoong Kim]

> On 2016-11-07 23:57, Kiwoong Kim wrote:
> > Some host controller might not initialize itself by setting "Host
> > Controller Enable" to 1.
> 
> > They should do this to reset UIC.
> 
> I am not sure if i understood this statment. can you give more details?

UFSHCI spec says as follows:
- When HCE is ‘0’ and software writes ‘1’, the host 
controller hardware shall execute the step 2 described in section 7.1.1
of this standard, including reset of the host UTP and UIC layers.

I found that a specific controller didn't execute reset of UIC layers
Properly for an internal problem.
At that time, DME reset and DME enable could make UIC be reset properly
instead of using "Host Controller Enable".

> 
> > In such cases, 'DME reset' and 'DME enable' are required for normal
> > subsequent operations.
> 
> This means HCE implementation is broken, you should name the quirk as
> UFSHCD_QUIRK_BROKEN_HCE .

Okay.

> 
> >
> > Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
> > ---
> >  drivers/scsi/ufs/ufshcd.c | 44
> > +++-
> >  drivers/scsi/ufs/ufshcd.h |  1 +
> >  2 files changed, 44 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index 24d6ea7..c904854 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -2496,6 +2496,37 @@ static inline void
> > ufshcd_add_delay_before_dme_cmd(struct ufs_hba *hba)
> > usleep_range(min_sleep_time_us, min_sleep_time_us + 50);  }
> >
> > +static int ufshcd_dme_reset(struct ufs_hba *hba) {
> > +   struct uic_command uic_cmd = {0};
> > +   int ret;
> > +
> > +   uic_cmd.command = UIC_CMD_DME_RESET;
> > +   uic_cmd.argument1 = 0x1;
> > +
> > +   ret = ufshcd_send_uic_cmd(hba, _cmd);
> > +   if (ret)
> > +   dev_err(hba->dev,
> > +   "dme-reset: error code %d\n", ret);
> 
> This error message doesn't say which DME command failed, do you want to
> add that?

I think that the string "dme-reset" is useful when clarifying some problems.
But if you don't agree merging it to mainline, I'll remove it in next
version.

> 
> > +
> > +   return ret;
> > +}
> > +
> > +static int ufshcd_dme_enable(struct ufs_hba *hba) {
> > +   struct uic_command uic_cmd = {0};
> > +   int ret;
> > +
> > +   uic_cmd.command = UIC_CMD_DME_ENABLE;
> > +
> > +   ret = ufshcd_send_uic_cmd(hba, _cmd);
> > +   if (ret)
> > +   dev_err(hba->dev,
> > +   "dme-enable: error code %d\n", ret);
> 
> This error message doesn't say which DME command failed, do you want to
> add that?

Same with above.

> 
> > +
> > +   return ret;
> > +}
> > +
> >  /**
> >   * ufshcd_dme_set_attr - UIC command for DME_SET, DME_PEER_SET
> >   * @hba: per adapter instance
> > @@ -3101,6 +3132,7 @@ static inline void ufshcd_hba_stop(struct
> > ufs_hba *hba, bool can_sleep)  static int ufshcd_hba_enable(struct
> > ufs_hba *hba)  {
> > int retry;
> > +   int ret = 0;
> >
> > /*
> >  * msleep of 1 and 5 used in this function might result in
> > msleep(20), @@ -3117,6 +3149,9 @@ static int ufshcd_hba_enable(struct
> > ufs_hba *hba)
> >
> > ufshcd_vops_hce_enable_notify(hba, PRE_CHANGE);
> >
> > +   if (hba->quirks & UFSHCD_QUIRK_USE_OF_HCE)
> > +   goto use_dme;
> > +
> > /* start controller initialization sequence */
> > ufshcd_hba_start(hba);
> >
> > @@ -3145,12 +3180,19 @@ static int ufshcd_hba_enable(struct ufs_hba
> > *hba)
> > msleep(5);
> > }
> >
> > +use_dme:
> > /* enable UIC related interrupts */
> > ufshcd_enable_intr(hba, UFSHCD_UIC_MASK);
> >
> > +   if (hba->quirks & UFSHCD_QUIRK_USE_OF_HCE) {
> > +   ret = ufshcd_dme_reset(hba);
> > +   if (!ret)
> > +   ret = ufshcd_dme_enable(hba);
> > +   }
> > +
> > ufshcd_vops_hce_enable_notify(hba, POST_CHANGE);
> >
> > -   return 0;
> > +   return ret;
> >  }
> >
> >  static int ufshcd_disable_tx_lcc(struct ufs_hba *hba, bool peer) diff
> > --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h index
> > c4abd76..6a96f24 100644
> > --- a/drivers/scsi/ufs/ufshcd.h
> > +++ b/drivers/scsi/ufs/ufshcd.h
> > @@ -496,6 +496,7 @@ struct ufs_hba {
> > #define UFSHCD_QUIRK_GET_VS_RESULT  UFS_BIT(6)
> > #define UFSHCD_QUIRK_BROKEN_DWORD_UTRD  UFS_BIT(7)
> > #define UFSHCD_QUIRK_BROKEN_REQ_LIST_CLRUFS_BIT(8)
> > +   #define UFSHCD_QUIRK_USE_OF_HCE UFS_BIT(9)
> >
> >
> > 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RESEND][PATCH v1] ufs: introduce setup_task_mgmt

[Kiwoong Kim]

Some UFS host controller may need to configure some things
before any task management request is issued.

Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
---
 drivers/scsi/ufs/ufshcd.c |  1 +
 drivers/scsi/ufs/ufshcd.h | 10 ++
 2 files changed, 11 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index bf78321..fdb0502 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -4358,6 +4358,7 @@ static int ufshcd_issue_tm_cmd(struct ufs_hba *hba, int 
lun_id, int task_id,
task_req_upiup->input_param2 = cpu_to_be32(task_id);
 
/* send command to the controller */
+   ufshcd_vops_setup_task_mgmt(hba, free_slot, tm_function);
__set_bit(free_slot, >outstanding_tasks);
 
/* Make sure descriptors are ready before ringing the task doorbell */
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index f2a69c0..b084d89 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -263,6 +263,8 @@ struct ufs_pwr_mode_info {
  * to be set.
  * @setup_xfer_req: called before any transfer request is issued
  *  to set some things
+ * @setup_task_mgmt: called before any task management request is issued
+ *  to set some things
  * @suspend: called during host controller PM callback
  * @resume: called during host controller PM callback
  * @dbg_register_dump: used to dump controller debug information
@@ -287,6 +289,7 @@ struct ufs_hba_variant_ops {
struct ufs_pa_layer_attr *,
struct ufs_pa_layer_attr *);
void(*setup_xfer_req)(struct ufs_hba *, int, bool);
+   void(*setup_task_mgmt)(struct ufs_hba *, int, u8);
int (*suspend)(struct ufs_hba *, enum ufs_pm_op);
int (*resume)(struct ufs_hba *, enum ufs_pm_op);
void(*dbg_register_dump)(struct ufs_hba *hba);
@@ -811,6 +814,13 @@ static inline void ufshcd_vops_setup_xfer_req(struct 
ufs_hba *hba, int tag,
return hba->vops->setup_xfer_req(hba, tag, is_cmd_not_null);
 }
 
+static inline void ufshcd_vops_setup_task_mgmt(struct ufs_hba *hba,
+   int tag, u8 tm_function)
+{
+   if (hba->vops && hba->vops->setup_task_mgmt)
+   return hba->vops->setup_task_mgmt(hba, tag, tm_function);
+}
+
 static inline int ufshcd_vops_suspend(struct ufs_hba *hba, enum ufs_pm_op op)
 {
if (hba->vops && hba->vops->suspend)
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1] ufs: add a variety of definitions decribed in UFS spec

[Kiwoong Kim]

These things are defined to be used by some UFS Host controllers.

Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
---
 drivers/scsi/ufs/mphy.h   | 38 ++
 drivers/scsi/ufs/ufshci.h | 14 +++---
 drivers/scsi/ufs/unipro.h | 24 
 3 files changed, 73 insertions(+), 3 deletions(-)
 create mode 100644 drivers/scsi/ufs/mphy.h

diff --git a/drivers/scsi/ufs/mphy.h b/drivers/scsi/ufs/mphy.h
new file mode 100644
index 000..c431f49
--- /dev/null
+++ b/drivers/scsi/ufs/mphy.h
@@ -0,0 +1,38 @@
+/*
+ * drivers/scsi/ufs/mphy.h
+ *
+ * Copyright (C) 2014 Samsung Electronics Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef _MPHY_H_
+#define _MPHY_H_
+
+#define TX_HIBERN8TIME_CAP 0x0f
+#define TX_MIN_ACTIVATE_TIME   0x33
+
+#define RX_HS_G1_SYNC_LEN_CAP  0x8b
+#define RX_HS_G1_PREP_LEN_CAP  0x8c
+#define RX_HS_G2_SYNC_LEN_CAP  0x94
+#define RX_HS_G3_SYNC_LEN_CAP  0x95
+#define RX_HS_G2_PREP_LEN_CAP  0x96
+#define RX_HS_G3_PREP_LEN_CAP  0x97
+ #define SYNC_RANGE_FINE   (0 << 6)
+ #define SYNC_RANGE_COARSE (1 << 6)
+ #define SYNC_LEN(x)   ((x) & 0x3f)
+ #define PREP_LEN(x)   ((x) & 0xf)
+#define RX_ADV_GRANULARITY_CAP 0x98
+ #define RX_ADV_GRAN_STEP(x)   x) & 0x3) << 1) | 0x1)
+#define TX_ADV_GRANULARITY_CAP 0x10
+ #define TX_ADV_GRAN_STEP(x)   x) & 0x3) << 1) | 0x1)
+#define RX_MIN_ACTIVATETIME_CAP0x8f
+#define RX_HIBERN8TIME_CAP 0x92
+#define RX_ADV_HIBERN8TIME_CAP 0x99
+#define RX_ADV_MIN_ACTIVATETIME_CAP0x9a
+
+#endif /* _MPHY_H_ */
+
diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
index 9599741..26dc340 100644
--- a/drivers/scsi/ufs/ufshci.h
+++ b/drivers/scsi/ufs/ufshci.h
@@ -170,17 +170,25 @@ enum {
 /* UECDL - Host UIC Error Code Data Link Layer 3Ch */
 #define UIC_DATA_LINK_LAYER_ERROR  UFS_BIT(31)
 #define UIC_DATA_LINK_LAYER_ERROR_CODE_MASK0x7FFF
-#define UIC_DATA_LINK_LAYER_ERROR_PA_INIT  0x2000
-#define UIC_DATA_LINK_LAYER_ERROR_NAC_RECEIVED 0x0001
-#define UIC_DATA_LINK_LAYER_ERROR_TCx_REPLAY_TIMEOUT 0x0002
+#define UIC_DATA_LINK_LAYER_ERROR_NAC_RECEIVED UFS_BIT(0)
+#define UIC_DATA_LINK_LAYER_ERROR_TCx_REPLAY_TIMEOUT   UFS_BIT(1)
+#define UIC_DATA_LINK_LAYER_ERROR_RX_BUF_OFUFS_BIT(5)
+#define UIC_DATA_LINK_LAYER_ERROR_PA_INIT  UFS_BIT(13)
 
 /* UECN - Host UIC Error Code Network Layer 40h */
 #define UIC_NETWORK_LAYER_ERRORUFS_BIT(31)
 #define UIC_NETWORK_LAYER_ERROR_CODE_MASK  0x7
+#define UIC_NETWORK_UNSUPPORTED_HEADER_TYPEBIT(0)
+#define UIC_NETWORK_BAD_DEVICEID_ENC   BIT(1)
+#define UIC_NETWORK_LHDR_TRAP_PACKET_DROPPING  BIT(2)
 
 /* UECT - Host UIC Error Code Transport Layer 44h */
 #define UIC_TRANSPORT_LAYER_ERROR  UFS_BIT(31)
 #define UIC_TRANSPORT_LAYER_ERROR_CODE_MASK0x7F
+#define UIC_TRANSPORT_UNSUPPORTED_HEADER_TYPE  BIT(0)
+#define UIC_TRANSPORT_UNKNOWN_CPORTID  BIT(1)
+#define UIC_TRANSPORT_NO_CONNECTION_RX BIT(2)
+#define UIC_TRANSPORT_BAD_TC   BIT(4)
 
 /* UECDME - Host UIC Error Code DME 48h */
 #define UIC_DME_ERROR  UFS_BIT(31)
diff --git a/drivers/scsi/ufs/unipro.h b/drivers/scsi/ufs/unipro.h
index eff8b56..e47e2c2 100644
--- a/drivers/scsi/ufs/unipro.h
+++ b/drivers/scsi/ufs/unipro.h
@@ -127,6 +127,7 @@
 #define PA_PACPREQEOBTIMEOUT   0x1591
 #define PA_HIBERN8TIME 0x15A7
 #define PA_LOCALVERINFO0x15A9
+#define PA_GRANULARITY 0x15AA
 #define PA_TACTIVATE   0x15A8
 #define PA_PACPFRAMECOUNT  0x15C0
 #define PA_PACPERRORCOUNT  0x15C1
@@ -170,6 +171,9 @@ enum {
UNCHANGED   = 7,
 };
 
+#define IS_PWR_MODE_HS(m)(((m) == FAST_MODE) || ((m) == FASTAUTO_MODE))
+#define IS_PWR_MODE_PWM(m)   (((m) == SLOW_MODE) || ((m) == SLOWAUTO_MODE))
+
 /* PA TX/RX Frequency Series */
 enum {
PA_HS_MODE_A= 1,
@@ -231,6 +235,11 @@ enum ufs_unipro_ver {
 #define DL_PEERTC1PRESENT  0x2066
 #define DL_PEERTC1RXINITCREVAL 0x2067
 
+/* Default value of L2 Timer */
+#define FC0PROTTIMEOUTVAL  8191
+#define TC0REPLAYTIMEOUTVAL65535
+#define AFC0REQTIMEOUTVAL  32767
+
 /*
  * Network Layer Attributes
  */
@@ -259,6 +268,21 @@ enum ufs_unipro_ver {
 #define T_TC0TXMAXSDUSIZE  0x4060
 #define T_TC1TXMAXSDUSIZE  0x4061
 
+/* CPort setting */
+#define E2EFC_ON   (1 << 0)
+#define E2EFC_OFF  (0 << 0)
+#define CSD_N_ON   (0 << 1)
+#define CSD_N_OFF  (1 << 1)
+#define CSV_N_ON   (0 << 2)
+#define CSV_N_OFF  (1 << 2)
+#define CPORT

[PATCH v1] ufs: introduce UFSHCI_QUIRK_SKIP_INTR_AGGR quirk

[Kiwoong Kim]

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.

Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
---
 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))
+   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_CLRUFS_BIT(8)
#define UFSHCD_QUIRK_USE_OF_HCE UFS_BIT(9)
+   #define UFSHCI_QUIRK_SKIP_INTR_AGGR UFS_BIT(10)
 
 
unsigned int quirks;/* Deviations from standard UFSHCI spec. */
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1] ufs: introduce setup_hibern8 callback

[Kiwoong Kim]

Some UFS host controller may need to configure some things
around hibern8 enter/exit

Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
---
 drivers/scsi/ufs/ufshcd.c | 10 --
 drivers/scsi/ufs/ufshcd.h | 10 ++
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index fdb0502..d4a5a9c 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -2697,6 +2697,8 @@ static int __ufshcd_uic_hibern8_enter(struct ufs_hba *hba)
int ret;
struct uic_command uic_cmd = {0};
 
+   ufshcd_vops_setup_hibern8(hba, true, PRE_CHANGE);
+
uic_cmd.command = UIC_CMD_DME_HIBER_ENTER;
ret = ufshcd_uic_pwr_ctrl(hba, _cmd);
 
@@ -2710,7 +2712,8 @@ static int __ufshcd_uic_hibern8_enter(struct ufs_hba *hba)
 */
if (ufshcd_link_recovery(hba))
ret = -ENOLINK;
-   }
+   } else
+   ufshcd_vops_setup_hibern8(hba, true, POST_CHANGE);
 
return ret;
 }
@@ -2733,13 +2736,16 @@ static int ufshcd_uic_hibern8_exit(struct ufs_hba *hba)
struct uic_command uic_cmd = {0};
int ret;
 
+   ufshcd_vops_setup_hibern8(hba, false, PRE_CHANGE);
+
uic_cmd.command = UIC_CMD_DME_HIBER_EXIT;
ret = ufshcd_uic_pwr_ctrl(hba, _cmd);
if (ret) {
dev_err(hba->dev, "%s: hibern8 exit failed. ret = %d\n",
__func__, ret);
ret = ufshcd_link_recovery(hba);
-   }
+   } else
+   ufshcd_vops_setup_hibern8(hba, false, POST_CHANGE);
 
return ret;
 }
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index b084d89..13504b4 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -265,6 +265,8 @@ struct ufs_pwr_mode_info {
  *  to set some things
  * @setup_task_mgmt: called before any task management request is issued
  *  to set some things
+ * @setup_hibern8: called around hibern8 enter/exit
+ * to configure some things
  * @suspend: called during host controller PM callback
  * @resume: called during host controller PM callback
  * @dbg_register_dump: used to dump controller debug information
@@ -290,6 +292,7 @@ struct ufs_hba_variant_ops {
struct ufs_pa_layer_attr *);
void(*setup_xfer_req)(struct ufs_hba *, int, bool);
void(*setup_task_mgmt)(struct ufs_hba *, int, u8);
+   void(*setup_hibern8)(struct ufs_hba *, bool, bool);
int (*suspend)(struct ufs_hba *, enum ufs_pm_op);
int (*resume)(struct ufs_hba *, enum ufs_pm_op);
void(*dbg_register_dump)(struct ufs_hba *hba);
@@ -821,6 +824,13 @@ static inline void ufshcd_vops_setup_task_mgmt(struct 
ufs_hba *hba,
return hba->vops->setup_task_mgmt(hba, tag, tm_function);
 }
 
+static inline void ufshcd_vops_setup_hibern8(struct ufs_hba *hba,
+   bool enter, bool notify)
+{
+   if (hba->vops && hba->vops->setup_hibern8)
+   return hba->vops->setup_hibern8(hba, enter, notify);
+}
+
 static inline int ufshcd_vops_suspend(struct ufs_hba *hba, enum ufs_pm_op op)
 {
if (hba->vops && hba->vops->suspend)
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1] ufs: introduce setup_task_mgmt

[Kiwoong Kim]

Some UFS host controller may need to configure some things
before any task management request is issued

Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
---
 drivers/scsi/ufs/ufshcd.c |  1 +
 drivers/scsi/ufs/ufshcd.h | 10 ++
 2 files changed, 11 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index bf78321..fdb0502 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -4358,6 +4358,7 @@ static int ufshcd_issue_tm_cmd(struct ufs_hba *hba, int 
lun_id, int task_id,
task_req_upiup->input_param2 = cpu_to_be32(task_id);
 
/* send command to the controller */
+   ufshcd_vops_setup_task_mgmt(hba, free_slot, tm_function);
__set_bit(free_slot, >outstanding_tasks);
 
/* Make sure descriptors are ready before ringing the task doorbell */
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index f2a69c0..b084d89 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -263,6 +263,8 @@ struct ufs_pwr_mode_info {
  * to be set.
  * @setup_xfer_req: called before any transfer request is issued
  *  to set some things
+ * @setup_task_mgmt: called before any task management request is issued
+ *  to set some things
  * @suspend: called during host controller PM callback
  * @resume: called during host controller PM callback
  * @dbg_register_dump: used to dump controller debug information
@@ -287,6 +289,7 @@ struct ufs_hba_variant_ops {
struct ufs_pa_layer_attr *,
struct ufs_pa_layer_attr *);
void(*setup_xfer_req)(struct ufs_hba *, int, bool);
+   void(*setup_task_mgmt)(struct ufs_hba *, int, u8);
int (*suspend)(struct ufs_hba *, enum ufs_pm_op);
int (*resume)(struct ufs_hba *, enum ufs_pm_op);
void(*dbg_register_dump)(struct ufs_hba *hba);
@@ -811,6 +814,13 @@ static inline void ufshcd_vops_setup_xfer_req(struct 
ufs_hba *hba, int tag,
return hba->vops->setup_xfer_req(hba, tag, is_cmd_not_null);
 }
 
+static inline void ufshcd_vops_setup_task_mgmt(struct ufs_hba *hba,
+   int tag, u8 tm_function)
+{
+   if (hba->vops && hba->vops->setup_task_mgmt)
+   return hba->vops->setup_task_mgmt(hba, tag, tm_function);
+}
+
 static inline int ufshcd_vops_suspend(struct ufs_hba *hba, enum ufs_pm_op op)
 {
if (hba->vops && hba->vops->suspend)
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1] ufs: introduce UFSHCD_QUIRK_USE_OF_HCE quirk

[Kiwoong Kim]

Some host controller might not initialize itself
by setting "Host Controller Enable" to 1.
They should do this to reset UIC.
In such cases, 'DME reset' and 'DME enable' are required
for normal subsequent operations.

Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
---
 drivers/scsi/ufs/ufshcd.c | 44 +++-
 drivers/scsi/ufs/ufshcd.h |  1 +
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 24d6ea7..c904854 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -2496,6 +2496,37 @@ static inline void 
ufshcd_add_delay_before_dme_cmd(struct ufs_hba *hba)
usleep_range(min_sleep_time_us, min_sleep_time_us + 50);
 }
 
+static int ufshcd_dme_reset(struct ufs_hba *hba)
+{
+   struct uic_command uic_cmd = {0};
+   int ret;
+
+   uic_cmd.command = UIC_CMD_DME_RESET;
+   uic_cmd.argument1 = 0x1;
+
+   ret = ufshcd_send_uic_cmd(hba, _cmd);
+   if (ret)
+   dev_err(hba->dev,
+   "dme-reset: error code %d\n", ret);
+
+   return ret;
+}
+
+static int ufshcd_dme_enable(struct ufs_hba *hba)
+{
+   struct uic_command uic_cmd = {0};
+   int ret;
+
+   uic_cmd.command = UIC_CMD_DME_ENABLE;
+
+   ret = ufshcd_send_uic_cmd(hba, _cmd);
+   if (ret)
+   dev_err(hba->dev,
+   "dme-enable: error code %d\n", ret);
+
+   return ret;
+}
+
 /**
  * ufshcd_dme_set_attr - UIC command for DME_SET, DME_PEER_SET
  * @hba: per adapter instance
@@ -3101,6 +3132,7 @@ static inline void ufshcd_hba_stop(struct ufs_hba *hba, 
bool can_sleep)
 static int ufshcd_hba_enable(struct ufs_hba *hba)
 {
int retry;
+   int ret = 0;
 
/*
 * msleep of 1 and 5 used in this function might result in msleep(20),
@@ -3117,6 +3149,9 @@ static int ufshcd_hba_enable(struct ufs_hba *hba)
 
ufshcd_vops_hce_enable_notify(hba, PRE_CHANGE);
 
+   if (hba->quirks & UFSHCD_QUIRK_USE_OF_HCE)
+   goto use_dme;
+
/* start controller initialization sequence */
ufshcd_hba_start(hba);
 
@@ -3145,12 +3180,19 @@ static int ufshcd_hba_enable(struct ufs_hba *hba)
msleep(5);
}
 
+use_dme:
/* enable UIC related interrupts */
ufshcd_enable_intr(hba, UFSHCD_UIC_MASK);
 
+   if (hba->quirks & UFSHCD_QUIRK_USE_OF_HCE) {
+   ret = ufshcd_dme_reset(hba);
+   if (!ret)
+   ret = ufshcd_dme_enable(hba);
+   }
+
ufshcd_vops_hce_enable_notify(hba, POST_CHANGE);
 
-   return 0;
+   return ret;
 }
 
 static int ufshcd_disable_tx_lcc(struct ufs_hba *hba, bool peer)
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index c4abd76..6a96f24 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -496,6 +496,7 @@ struct ufs_hba {
#define UFSHCD_QUIRK_GET_VS_RESULT  UFS_BIT(6)
#define UFSHCD_QUIRK_BROKEN_DWORD_UTRD  UFS_BIT(7)
#define UFSHCD_QUIRK_BROKEN_REQ_LIST_CLRUFS_BIT(8)
+   #define UFSHCD_QUIRK_USE_OF_HCE UFS_BIT(9)
 
 
unsigned int quirks;/* Deviations from standard UFSHCI spec. */
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1] ufs: introduce UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR quirk

[Kiwoong Kim]

Some UFS host controllers may clear a transfer request slot
by setting an associated bit in UTLRCLR/UTMLRCLR to 1, not 0.
That's opposite to what UFS spec decribes.

Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
---
 drivers/scsi/ufs/ufshcd.c | 32 
 drivers/scsi/ufs/ufshcd.h |  1 +
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 549e3e8..24d6ea7 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);
 }
 
 /**
@@ -1147,7 +1171,7 @@ ufshcd_send_uic_cmd(struct ufs_hba *hba, struct 
uic_command *uic_cmd)
  *
  * Returns 0 in case of success, non-zero value in case of failure
  */
-static int ufshcd_map_sg(struct ufshcd_lrb *lrbp)
+static int ufshcd_map_sg(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 {
struct ufshcd_sg_entry *prd_table;
struct scatterlist *sg;
@@ -1529,7 +1553,7 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, 
struct scsi_cmnd *cmd)
 
ufshcd_comp_scsi_upiu(hba, lrbp);
 
-   err = ufshcd_map_sg(lrbp);
+   err = ufshcd_map_sg(hba, lrbp);
if (err) {
lrbp->cmd = NULL;
clear_bit_unlock(tag, >lrb_in_use);
@@ -4329,7 +4353,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 565f005..c4abd76 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -495,6 +495,7 @@ struct ufs_hba {
 
#define UFSHCD_QUIRK_GET_VS_RESULT  UFS_BIT(6)
#define UFSHCD_QUIRK_BROKEN_DWORD_UTRD  UFS_BIT(7)
+   #define UFSHCD_QUIRK_BROKEN_REQ_LIST_CLRUFS_BIT(8)
 
 
unsigned int quirks;/* Deviations from standard UFSHCI spec. */
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1] ufs: introduce UFSHCD_QUIRK_BROKEN_DWORD_UTRD quirk

[Kiwoong Kim]

Some UFS host controllers may think
granularitys of PRDT length and offset as bytes, not double words.

Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
---
 drivers/scsi/ufs/ufshcd.c | 24 +++-
 drivers/scsi/ufs/ufshcd.h |  2 ++
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 8e19631..549e3e8 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1161,8 +1161,13 @@ static int ufshcd_map_sg(struct ufshcd_lrb *lrbp)
return sg_segments;
 
if (sg_segments) {
-   lrbp->utr_descriptor_ptr->prd_table_length =
-   cpu_to_le16((u16) (sg_segments));
+   if (hba->quirks & UFSHCD_QUIRK_BROKEN_DWORD_UTRD)
+   lrbp->utr_descriptor_ptr->prd_table_length =
+   cpu_to_le16((u16)(sg_segments *
+   sizeof(struct ufshcd_sg_entry)));
+   else
+   lrbp->utr_descriptor_ptr->prd_table_length =
+   cpu_to_le16((u16) (sg_segments));
 
prd_table = (struct ufshcd_sg_entry *)lrbp->ucd_prdt_ptr;
 
@@ -2385,12 +2390,21 @@ static void ufshcd_host_memory_configure(struct ufs_hba 
*hba)

cpu_to_le32(upper_32_bits(cmd_desc_element_addr));
 
/* Response upiu and prdt offset should be in double words */
-   utrdlp[i].response_upiu_offset =
+   if (hba->quirks & UFSHCD_QUIRK_BROKEN_DWORD_UTRD) {
+   utrdlp[i].response_upiu_offset =
+   cpu_to_le16(response_offset);
+   utrdlp[i].prd_table_offset =
+   cpu_to_le16(prdt_offset);
+   utrdlp[i].response_upiu_length =
+   cpu_to_le16(ALIGNED_UPIU_SIZE);
+   } else {
+   utrdlp[i].response_upiu_offset =
cpu_to_le16((response_offset >> 2));
-   utrdlp[i].prd_table_offset =
+   utrdlp[i].prd_table_offset =
cpu_to_le16((prdt_offset >> 2));
-   utrdlp[i].response_upiu_length =
+   utrdlp[i].response_upiu_length =
cpu_to_le16(ALIGNED_UPIU_SIZE >> 2);
+   }
 
hba->lrb[i].utr_descriptor_ptr = (utrdlp + i);
hba->lrb[i].ucd_req_ptr =
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 8cf3991..565f005 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -494,6 +494,8 @@ struct ufs_hba {
#define UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION UFS_BIT(5)
 
#define UFSHCD_QUIRK_GET_VS_RESULT  UFS_BIT(6)
+   #define UFSHCD_QUIRK_BROKEN_DWORD_UTRD  UFS_BIT(7)
+
 
unsigned int quirks;/* Deviations from standard UFSHCI spec. */
 
-- 
2.1.4


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1] ufs: introcude UFSHCD_QUIRK_GET_VS_RESULT quirk

[Kiwoong Kim]

Some UFS host controllers may not report a result of
UIC command completion properly.
In such cases, they should get the result from UIC directly
if their architectures support that.

Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
---
 drivers/scsi/ufs/ufshcd.c | 27 +++
 drivers/scsi/ufs/ufshcd.h | 20 
 2 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index d4a5a9c..8e19631 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1011,7 +1011,10 @@ static inline bool ufshcd_ready_for_uic_cmd(struct 
ufs_hba *hba)
  */
 static inline u8 ufshcd_get_upmcrs(struct ufs_hba *hba)
 {
-   return (ufshcd_readl(hba, REG_CONTROLLER_STATUS) >> 8) & 0x7;
+   if (hba->quirks & UFSHCD_QUIRK_GET_VS_RESULT)
+   return (u8)ufshcd_vops_get_vs_info(hba, VS_OP_03);
+   else
+   return (ufshcd_readl(hba, REG_CONTROLLER_STATUS) >> 8) & 0x7;
 }
 
 /**
@@ -1038,6 +1041,17 @@ ufshcd_dispatch_uic_cmd(struct ufs_hba *hba, struct 
uic_command *uic_cmd)
  REG_UIC_COMMAND);
 }
 
+static inline enum vs_opcode ufshcd_get_vs_opcode(struct uic_command *uic_cmd)
+{
+   if (uic_cmd->command == UIC_CMD_DME_LINK_STARTUP)
+   return VS_OP_00;
+   else if ((uic_cmd->command == UIC_CMD_DME_HIBER_ENTER))
+   return VS_OP_01;
+   else if ((uic_cmd->command == UIC_CMD_DME_HIBER_EXIT))
+   return VS_OP_02;
+   else
+   return VS_OP_INVALID;
+}
 /**
  * ufshcd_wait_for_uic_cmd - Wait complectioin of UIC command
  * @hba: per adapter instance
@@ -1051,11 +1065,16 @@ ufshcd_wait_for_uic_cmd(struct ufs_hba *hba, struct 
uic_command *uic_cmd)
 {
int ret;
unsigned long flags;
+   enum vs_opcode vs_op = ufshcd_get_vs_opcode(uic_cmd);
 
if (wait_for_completion_timeout(_cmd->done,
-   msecs_to_jiffies(UIC_CMD_TIMEOUT)))
-   ret = uic_cmd->argument2 & MASK_UIC_COMMAND_RESULT;
-   else
+   msecs_to_jiffies(UIC_CMD_TIMEOUT))) {
+   if (hba->quirks & UFSHCD_QUIRK_GET_VS_RESULT &&
+   vs_op != VS_OP_INVALID)
+   ret = ufshcd_vops_get_vs_info(hba, vs_op);
+   else
+   ret = uic_cmd->argument2 & MASK_UIC_COMMAND_RESULT;
+   } else
ret = -ETIMEDOUT;
 
spin_lock_irqsave(hba->host->host_lock, flags);
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 13504b4..8cf3991 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -245,6 +245,14 @@ struct ufs_pwr_mode_info {
struct ufs_pa_layer_attr info;
 };
 
+enum vs_opcode {
+   VS_OP_00 = 0x00,
+   VS_OP_01,
+   VS_OP_02,
+   VS_OP_03,
+   VS_OP_INVALID = 0xFF,
+};
+
 /**
  * struct ufs_hba_variant_ops - variant specific callbacks
  * @name: variant name
@@ -297,6 +305,7 @@ struct ufs_hba_variant_ops {
int (*resume)(struct ufs_hba *, enum ufs_pm_op);
void(*dbg_register_dump)(struct ufs_hba *hba);
int (*phy_initialization)(struct ufs_hba *);
+   int (*get_vs_info)(struct ufs_hba *hba, enum vs_opcode);
 };
 
 /* clock gating state  */
@@ -484,6 +493,8 @@ struct ufs_hba {
 */
#define UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION UFS_BIT(5)
 
+   #define UFSHCD_QUIRK_GET_VS_RESULT  UFS_BIT(6)
+
unsigned int quirks;/* Deviations from standard UFSHCI spec. */
 
/* Device deviations from standard UFS device spec. */
@@ -853,4 +864,13 @@ static inline void ufshcd_vops_dbg_register_dump(struct 
ufs_hba *hba)
hba->vops->dbg_register_dump(hba);
 }
 
+static inline int ufshcd_vops_get_vs_info(struct ufs_hba *hba,
+   enum vs_opcode op)
+{
+   if (hba->vops && hba->vops->get_vs_info)
+   return hba->vops->get_vs_info(hba, op);
+
+   return -ENOTSUPP;
+}
+
 #endif /* End of Header */
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1] ufs: introduce setup_xfer_req callback

[Kiwoong Kim]

Some UFS host controller may need to configure some things
before any transfer request is issued.

Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
---
 drivers/scsi/ufs/ufshcd.c |  2 ++
 drivers/scsi/ufs/ufshcd.h | 10 ++
 2 files changed, 12 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 8cf5d8f..bf78321 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1516,6 +1516,7 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, 
struct scsi_cmnd *cmd)
 
/* issue command to the controller */
spin_lock_irqsave(hba->host->host_lock, flags);
+   ufshcd_vops_setup_xfer_req(hba, tag, (lrbp->cmd ? true : false));
ufshcd_send_command(hba, tag);
 out_unlock:
spin_unlock_irqrestore(hba->host->host_lock, flags);
@@ -1727,6 +1728,7 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba *hba,
/* Make sure descriptors are ready before ringing the doorbell */
wmb();
spin_lock_irqsave(hba->host->host_lock, flags);
+   ufshcd_vops_setup_xfer_req(hba, tag, (lrbp->cmd ? true : false));
ufshcd_send_command(hba, tag);
spin_unlock_irqrestore(hba->host->host_lock, flags);
 
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index afff7f4..f2a69c0 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -261,6 +261,8 @@ struct ufs_pwr_mode_info {
  * @pwr_change_notify: called before and after a power mode change
  * is carried out to allow vendor spesific capabilities
  * to be set.
+ * @setup_xfer_req: called before any transfer request is issued
+ *  to set some things
  * @suspend: called during host controller PM callback
  * @resume: called during host controller PM callback
  * @dbg_register_dump: used to dump controller debug information
@@ -284,6 +286,7 @@ struct ufs_hba_variant_ops {
enum ufs_notify_change_status status,
struct ufs_pa_layer_attr *,
struct ufs_pa_layer_attr *);
+   void(*setup_xfer_req)(struct ufs_hba *, int, bool);
int (*suspend)(struct ufs_hba *, enum ufs_pm_op);
int (*resume)(struct ufs_hba *, enum ufs_pm_op);
void(*dbg_register_dump)(struct ufs_hba *hba);
@@ -801,6 +804,13 @@ static inline int ufshcd_vops_pwr_change_notify(struct 
ufs_hba *hba,
return -ENOTSUPP;
 }
 
+static inline void ufshcd_vops_setup_xfer_req(struct ufs_hba *hba, int tag,
+   bool is_cmd_not_null)
+{
+   if (hba->vops && hba->vops->setup_xfer_req)
+   return hba->vops->setup_xfer_req(hba, tag, is_cmd_not_null);
+}
+
 static inline int ufshcd_vops_suspend(struct ufs_hba *hba, enum ufs_pm_op op)
 {
if (hba->vops && hba->vops->suspend)
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [RESEND PATCH v3] scsi: ufshcd: fix possible unclocked register access

[Kiwoong Kim]

Reviewed-by: Kiwoong Kim <kwmad@samsung.com>

> Vendor specific setup_clocks callback may require the clocks managed
> by ufshcd driver to be ON. So if the vendor specific setup_clocks callback
> is called while the required clocks are turned off, it could result into
> unclocked register access.
> 
> To prevent possible unclock register access, this change adds one more
> argument to setup_clocks callback to let it know whether it is called
> pre/post the clock changes by core driver.
> 
> Signed-off-by: Subhash Jadavani <subha...@codeaurora.org>
> ---
> Changes from v2:
> * Added one more argument to setup_clocks callback, this should address
>   Kiwoong Kim's comments on v2.
> 
> Changes from v1:
> * Don't call ufshcd_vops_setup_clocks() again for clock off
> ---
>  drivers/scsi/ufs/ufs-qcom.c | 10 ++
>  drivers/scsi/ufs/ufshcd.c   | 17 -
>  drivers/scsi/ufs/ufshcd.h   |  8 +---
>  3 files changed, 19 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
> index 3aedf73..3c4f602 100644
> --- a/drivers/scsi/ufs/ufs-qcom.c
> +++ b/drivers/scsi/ufs/ufs-qcom.c
> @@ -1094,10 +1094,12 @@ static void ufs_qcom_set_caps(struct ufs_hba *hba)
>   * ufs_qcom_setup_clocks - enables/disable clocks
>   * @hba: host controller instance
>   * @on: If true, enable clocks else disable them.
> + * @status: PRE_CHANGE or POST_CHANGE notify
>   *
>   * Returns 0 on success, non-zero on failure.
>   */
> -static int ufs_qcom_setup_clocks(struct ufs_hba *hba, bool on)
> +static int ufs_qcom_setup_clocks(struct ufs_hba *hba, bool on,
> +  enum ufs_notify_change_status status)
>  {
>   struct ufs_qcom_host *host = ufshcd_get_variant(hba);
>   int err;
> @@ -,7 +1113,7 @@ static int ufs_qcom_setup_clocks(struct ufs_hba
*hba,
> bool on)
>   if (!host)
>   return 0;
> 
> - if (on) {
> + if (on && (status == POST_CHANGE)) {
>   err = ufs_qcom_phy_enable_iface_clk(host->generic_phy);
>   if (err)
>   goto out;
> @@ -1130,7 +1132,7 @@ static int ufs_qcom_setup_clocks(struct ufs_hba
*hba,
> bool on)
>   if (vote == host->bus_vote.min_bw_vote)
>   ufs_qcom_update_bus_bw_vote(host);
> 
> - } else {
> + } else if (!on && (status == PRE_CHANGE)) {
> 
>   /* M-PHY RMMI interface clocks can be turned off */
>   ufs_qcom_phy_disable_iface_clk(host->generic_phy);
> @@ -1254,7 +1256,7 @@ static int ufs_qcom_init(struct ufs_hba *hba)
>   ufs_qcom_set_caps(hba);
>   ufs_qcom_advertise_quirks(hba);
> 
> - ufs_qcom_setup_clocks(hba, true);
> + ufs_qcom_setup_clocks(hba, true, POST_CHANGE);
> 
>   if (hba->dev->id < MAX_UFS_QCOM_HOSTS)
>   ufs_qcom_hosts[hba->dev->id] = host;
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 05c7456..571a2f6 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -5389,6 +5389,10 @@ static int __ufshcd_setup_clocks(struct ufs_hba
> *hba, bool on,
>   if (!head || list_empty(head))
>   goto out;
> 
> + ret = ufshcd_vops_setup_clocks(hba, on, PRE_CHANGE);
> + if (ret)
> + return ret;
> +
>   list_for_each_entry(clki, head, list) {
>   if (!IS_ERR_OR_NULL(clki->clk)) {
>   if (skip_ref_clk && !strcmp(clki->name, "ref_clk"))
> @@ -5410,7 +5414,10 @@ static int __ufshcd_setup_clocks(struct ufs_hba
> *hba, bool on,
>   }
>   }
> 
> - ret = ufshcd_vops_setup_clocks(hba, on);
> + ret = ufshcd_vops_setup_clocks(hba, on, POST_CHANGE);
> + if (ret)
> + return ret;
> +
>  out:
>   if (ret) {
>   list_for_each_entry(clki, head, list) {
> @@ -5500,8 +5507,6 @@ static void ufshcd_variant_hba_exit(struct ufs_hba
> *hba)
>   if (!hba->vops)
>   return;
> 
> - ufshcd_vops_setup_clocks(hba, false);
> -
>   ufshcd_vops_setup_regulators(hba, false);
> 
>   ufshcd_vops_exit(hba);
> @@ -5905,10 +5910,6 @@ disable_clks:
>   if (ret)
>   goto set_link_active;
> 
> - ret = ufshcd_vops_setup_clocks(hba, false);
> - if (ret)
> - goto vops_resume;
> -
>   if (!ufshcd_is_link_active(hba))
>   ufshcd_setup_clocks(hba, false);
>   else
> @@ -5925,8 +5926,6 @@ disable_clks:
>   ufshcd_hba_vreg_set_lpm(hba);
>   goto out;
> 
> -vops_resume:
>

[PATCH 01/10] ufs: introduce setup_xfer_req callback

[Kiwoong Kim]

Some UFS host controller may need to configure some things
before any transfer request is issued.

Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
---
 drivers/scsi/ufs/ufshcd.c |2 ++
 drivers/scsi/ufs/ufshcd.h |   10 ++
 2 files changed, 12 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 05c7456..bb537db 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1474,6 +1474,7 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, 
struct scsi_cmnd *cmd)
 
/* issue command to the controller */
spin_lock_irqsave(hba->host->host_lock, flags);
+   ufshcd_vops_setup_xfer_req(hba, tag, (lrbp->cmd ? true : false));
ufshcd_send_command(hba, tag);
 out_unlock:
spin_unlock_irqrestore(hba->host->host_lock, flags);
@@ -1683,6 +1684,7 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba *hba,
/* Make sure descriptors are ready before ringing the doorbell */
wmb();
spin_lock_irqsave(hba->host->host_lock, flags);
+   ufshcd_vops_setup_xfer_req(hba, tag, (lrbp->cmd ? true : false));
ufshcd_send_command(hba, tag);
spin_unlock_irqrestore(hba->host->host_lock, flags);
 
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 430bef1..f37e5a4 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -261,6 +261,8 @@ struct ufs_pwr_mode_info {
  * @pwr_change_notify: called before and after a power mode change
  * is carried out to allow vendor spesific capabilities
  * to be set.
+ * @setup_xfer_req: called before any transfer request is issued
+ *  to set some things
  * @suspend: called during host controller PM callback
  * @resume: called during host controller PM callback
  * @dbg_register_dump: used to dump controller debug information
@@ -283,6 +285,7 @@ struct ufs_hba_variant_ops {
enum ufs_notify_change_status status,
struct ufs_pa_layer_attr *,
struct ufs_pa_layer_attr *);
+   void(*setup_xfer_req)(struct ufs_hba *, int, bool);
int (*suspend)(struct ufs_hba *, enum ufs_pm_op);
int (*resume)(struct ufs_hba *, enum ufs_pm_op);
void(*dbg_register_dump)(struct ufs_hba *hba);
@@ -799,6 +802,13 @@ static inline int ufshcd_vops_pwr_change_notify(struct 
ufs_hba *hba,
return -ENOTSUPP;
 }
 
+static inline void ufshcd_vops_setup_xfer_req(struct ufs_hba *hba, int tag,
+   bool is_cmd_not_null)
+{
+   if (hba->vops && hba->vops->setup_xfer_req)
+   return hba->vops->setup_xfer_req(hba, tag, is_cmd_not_null);
+}
+
 static inline int ufshcd_vops_suspend(struct ufs_hba *hba, enum ufs_pm_op op)
 {
if (hba->vops && hba->vops->suspend)
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RESEND PATCH v2 0/10] Support for Exynos specific UFS driver

[Kiwoong Kim]

This series of patches introduce support for Exynos specific UFS driver.
An original version of this has been applied on mass production of commercial 
mobile devices.

I checked compiling with Subhash's patch
- [PATCH v3] scsi: ufshcd: fix possible unclocked register access

---
Changes from v1:
* Separate a set of newer callbacks to be added
* Make Exynos specific UFS driver compatible with a following patch
 [PATCH v3] scsi: ufshcd: fix possible unclocked register access
 (http://www.spinics.net/lists/linux-scsi/msg100290.html)
* Rename three of the callbacks as follows:
   set_nexus_t_xfer_req -> setup_xfer_req
   set_nexus_t_task_mgmt -> setup_task_mgmt
   hibern8_notify -> setup_hibern8
---
 Documentation/devicetree/bindings/ufs/ufs-exynos.txt |   93 +
 drivers/scsi/ufs/Kconfig   |   10 +
 drivers/scsi/ufs/Makefile |1 +
 drivers/scsi/ufs/mphy.h|   38 +
 drivers/scsi/ufs/ufs-exynos.c| 2099 

 drivers/scsi/ufs/ufs-exynos.h|  621 ++
 drivers/scsi/ufs/ufshcd.c |  144 +-
 drivers/scsi/ufs/ufshcd.h |   55 +
 drivers/scsi/ufs/ufshci.h |   14 +-
 drivers/scsi/ufs/unipro.h |   24 +
 10 files changed, 3079 insertions(+), 20 deletions(-)  create mode 100644 
Documentation/devicetree/bindings/ufs/ufs-exynos.txt
 create mode 100644 drivers/scsi/ufs/mphy.h  create mode 100644 
drivers/scsi/ufs/ufs-exynos.c  create mode 100644 drivers/scsi/ufs/ufs-exynos.h

--
1.7.9.5

--

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 0/8] Support for Exynos specific UFS driver

[Kiwoong Kim]

This series of patches introduce support for Exynos specific UFS driver.
An original version of this has been applied on mass production
of commercial mobile devices.

I checked compiling with Subhash's patch
- [PATCH v3] scsi: ufshcd: fix possible unclocked register access

---
Changes from v1:
* Separate a set of newer callbacks to be added
* Make Exynos specific UFS driver compatible with a following patch
 [PATCH v3] scsi: ufshcd: fix possible unclocked register access
 (http://www.spinics.net/lists/linux-scsi/msg100290.html)
* Rename three of the callbacks as follows:
   set_nexus_t_xfer_req -> setup_xfer_req
   set_nexus_t_task_mgmt -> setup_task_mgmt
   hibern8_notify -> setup_hibern8
---
 Documentation/devicetree/bindings/ufs/ufs-exynos.txt |   93 +
 drivers/scsi/ufs/Kconfig   |   10 +
 drivers/scsi/ufs/Makefile |1 +
 drivers/scsi/ufs/mphy.h|   38 +
 drivers/scsi/ufs/ufs-exynos.c| 2099 

 drivers/scsi/ufs/ufs-exynos.h|  621 ++
 drivers/scsi/ufs/ufshcd.c |  144 +-
 drivers/scsi/ufs/ufshcd.h |   55 +
 drivers/scsi/ufs/ufshci.h |   14 +-
 drivers/scsi/ufs/unipro.h |   24 +
 10 files changed, 3079 insertions(+), 20 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/ufs/ufs-exynos.txt
 create mode 100644 drivers/scsi/ufs/mphy.h
 create mode 100644 drivers/scsi/ufs/ufs-exynos.c
 create mode 100644 drivers/scsi/ufs/ufs-exynos.h

-- 
1.7.9.5

--

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v1 1/8] ufs: add some vendor specific operations

[Kiwoong Kim]

Hi, Subhash.

> Hi Kim,
> 
> On 2016-10-06 18:38, Kiwoong Kim wrote:
> > Hi, Subhash.
> >
> > Thank for your comments.
> > I added my opinions below.
> >
> > Regards
> >
> >> Hi Kim,
> >>
> >>
> >> On 2016-10-06 01:17, Kiwoong Kim wrote:
> >> > Some UFS host controller may need to require some sequences before
> >> > used clocks are turned on or off.
> >> > e.g) control internal gates in UFS host
> >>
> >>
> >> I believe we are doing couple of other things as well other than
> >> clock control in this patch, such as hibern8 notify, *nexus_t*
> >> notify. You might want to separate each in different patches. Also,
> >> please find more
> 
> are you going to divide this patch?

Yes.

> 
> >> additional below.
> >>
> >> >
> >> > Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
> >> > ---
> >> >  drivers/scsi/ufs/ufshcd.c |   15 +--
> >> >  drivers/scsi/ufs/ufshcd.h |   37
> +
> >> >  2 files changed, 50 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> >> > index 05c7456..82c9a40 100644
> >> > --- a/drivers/scsi/ufs/ufshcd.c
> >> > +++ b/drivers/scsi/ufs/ufshcd.c
> >> > @@ -1474,6 +1474,7 @@ static int ufshcd_queuecommand(struct
> >> > Scsi_Host *host, struct scsi_cmnd *cmd)
> >> >
> >> >  /* issue command to the controller */
> >> >  spin_lock_irqsave(hba->host->host_lock, flags);
> >> > +ufshcd_vops_set_nexus_t_xfer_req(hba, tag, lrbp->cmd);
> >> >  ufshcd_send_command(hba, tag);
> >> >  out_unlock:
> >> >  spin_unlock_irqrestore(hba->host->host_lock, flags); @@
-1683,6
> >> > +1684,7 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba *hba,
> >> >  /* Make sure descriptors are ready before ringing the
doorbell */
> >> >  wmb();
> >> >  spin_lock_irqsave(hba->host->host_lock, flags);
> >> > +ufshcd_vops_set_nexus_t_xfer_req(hba, tag, lrbp->cmd);
> >>
> >> what does "next_t_xfer_req" means? why do we need this? and do you
> >> think
> >> it may be useful/needed for non-exynos UFS host controllers as well?
> >
> > Exynos UFS controller has  a logic to determine which slot owns
> > received
> > UPIU,
> > to handle it properly.
> > This only run when software set a exynos-specific SFR to bitwise-1.
> > If the controller sends non-dependent request, such as NOP,
> > software would set the SFR to 0 because it don't need to determine.
> 
> Ok, this is very specific to your controller. But i guess still the
> callback names should be generic enough so that it may be used by other
> host controller drivers if required in future.
> Can we think of different name? something like notify_cmd_issue?

Okay, that's good.

> Also, why do we need to pass whole pointer to pass scsi command pointer?
> what all scsi cmnd parameters will be used by exynos driver? may be your
> full patch series might give more idea?

I get it. How about using (void *) type pointer as an additional argument
To pass anything away to platform-specific driver ?

Thank you
Regards

> 
> 
> >
> > An answer of your question depends on whether other controllers
> > has a logic to function that.
> >
> >>
> >>
> >>
> >> >  ufshcd_send_command(hba, tag);
> >> >  spin_unlock_irqrestore(hba->host->host_lock, flags);
> >> >
> >> > @@ -2651,6 +2653,8 @@ static int __ufshcd_uic_hibern8_enter(struct
> >> > ufs_hba *hba)
> >> >  int ret;
> >> >  struct uic_command uic_cmd = {0};
> >> >
> >> > +ufshcd_vops_hibern8_notify(hba, true, PRE_CHANGE);
> >> > +
> >> >  uic_cmd.command = UIC_CMD_DME_HIBER_ENTER;
> >> >  ret = ufshcd_uic_pwr_ctrl(hba, _cmd);
> >> >
> >> > @@ -2664,7 +2668,8 @@ static int __ufshcd_uic_hibern8_enter(struct
> >> > ufs_hba *hba)
> >> >   */
> >> >  if (ufshcd_link_recovery(hba))
> >> >  ret = -ENOLINK;
> >> > -}
> >> > +} else
> >> > +ufshcd_vops_hibern8_notif

RE: [PATCH v1 1/8] ufs: add some vendor specific operations

[Kiwoong Kim]

Hi, Subhash.

Thank for your comments.
I added my opinions below.

Regards

> Hi Kim,
> 
> 
> On 2016-10-06 01:17, Kiwoong Kim wrote:
> > Some UFS host controller may need to require some sequences before
> > used clocks are turned on or off.
> > e.g) control internal gates in UFS host
> 
> 
> I believe we are doing couple of other things as well other than clock
> control in this patch, such as hibern8 notify, *nexus_t* notify. You might
> want to separate each in different patches. Also, please find more
> additional below.
> 
> >
> > Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
> > ---
> >  drivers/scsi/ufs/ufshcd.c |   15 +--
> >  drivers/scsi/ufs/ufshcd.h |   37 +
> >  2 files changed, 50 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index 05c7456..82c9a40 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -1474,6 +1474,7 @@ static int ufshcd_queuecommand(struct Scsi_Host
> > *host, struct scsi_cmnd *cmd)
> >
> > /* issue command to the controller */
> > spin_lock_irqsave(hba->host->host_lock, flags);
> > +   ufshcd_vops_set_nexus_t_xfer_req(hba, tag, lrbp->cmd);
> > ufshcd_send_command(hba, tag);
> >  out_unlock:
> > spin_unlock_irqrestore(hba->host->host_lock, flags);
> > @@ -1683,6 +1684,7 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba
> > *hba,
> > /* Make sure descriptors are ready before ringing the doorbell */
> > wmb();
> > spin_lock_irqsave(hba->host->host_lock, flags);
> > +   ufshcd_vops_set_nexus_t_xfer_req(hba, tag, lrbp->cmd);
> 
> what does "next_t_xfer_req" means? why do we need this? and do you think
> it may be useful/needed for non-exynos UFS host controllers as well?

Exynos UFS controller has  a logic to determine which slot owns received
UPIU,
to handle it properly.
This only run when software set a exynos-specific SFR to bitwise-1.
If the controller sends non-dependent request, such as NOP,
software would set the SFR to 0 because it don't need to determine.

An answer of your question depends on whether other controllers
has a logic to function that.

> 
> 
> 
> > ufshcd_send_command(hba, tag);
> > spin_unlock_irqrestore(hba->host->host_lock, flags);
> >
> > @@ -2651,6 +2653,8 @@ static int __ufshcd_uic_hibern8_enter(struct
> > ufs_hba *hba)
> > int ret;
> > struct uic_command uic_cmd = {0};
> >
> > +   ufshcd_vops_hibern8_notify(hba, true, PRE_CHANGE);
> > +
> > uic_cmd.command = UIC_CMD_DME_HIBER_ENTER;
> > ret = ufshcd_uic_pwr_ctrl(hba, _cmd);
> >
> > @@ -2664,7 +2668,8 @@ static int __ufshcd_uic_hibern8_enter(struct
> > ufs_hba *hba)
> >  */
> > if (ufshcd_link_recovery(hba))
> > ret = -ENOLINK;
> > -   }
> > +   } else
> > +   ufshcd_vops_hibern8_notify(hba, true, POST_CHANGE);
> >
> > return ret;
> >  }
> > @@ -2687,13 +2692,16 @@ static int ufshcd_uic_hibern8_exit(struct
> > ufs_hba *hba)
> > struct uic_command uic_cmd = {0};
> > int ret;
> >
> > +   ufshcd_vops_hibern8_notify(hba, false, PRE_CHANGE);
> > +
> > uic_cmd.command = UIC_CMD_DME_HIBER_EXIT;
> > ret = ufshcd_uic_pwr_ctrl(hba, _cmd);
> > if (ret) {
> > dev_err(hba->dev, "%s: hibern8 exit failed. ret = %d\n",
> > __func__, ret);
> > ret = ufshcd_link_recovery(hba);
> > -   }
> > +   } else
> > +   ufshcd_vops_hibern8_notify(hba, false, POST_CHANGE);
> >
> > return ret;
> >  }
> > @@ -4312,6 +4320,7 @@ static int ufshcd_issue_tm_cmd(struct ufs_hba
> > *hba, int lun_id, int task_id,
> > task_req_upiup->input_param2 = cpu_to_be32(task_id);
> >
> > /* send command to the controller */
> > +   ufshcd_vops_set_nexus_t_task_mgmt(hba, free_slot, tm_function);
> 
> 
> Same question as "next_t_xfer_req". what does "next_t_task_mgmt" means?
> why do we need this? and do you think it may be useful/needed for
> non-exynos UFS host controllers as well?

Please refer to what I mentioned above.

> 
> 
> > __set_bit(free_slot, >outstanding_tasks);
> >
> > /* Make sure descriptors are ready before ringing the task doorbell
> > */
> > @@ -5389,6 +5398,8 @@ static int __ufshcd_setup_clocks(struct ufs_hba
> > *hba, bool on,
> >

RE: [PATCH v1 0/8] Support for Exynos specific UFS driver

[Kiwoong Kim]

Hi, Subhash

You're right.
I submitted one of  patches first to link to the previous summary mail
thread,
but it failed. I'll try it again.

Thank you
Regards

> Hi Kim,
> 
> 
> On 2016-10-06 01:12, Kiwoong Kim wrote:
> > This series of patches introduce support for Exynos specific UFS
> > driver.
> > An original version of this has been applied on mass production of
> > commercial mobile devices.
> >
> >  Documentation/devicetree/bindings/ufs/ufs-exynos.txt |   93 +
> >  drivers/scsi/ufs/Kconfig  |   10 +
> >  drivers/scsi/ufs/Makefile |1 +
> >  drivers/scsi/ufs/mphy.h|   38
> > +
> >  drivers/scsi/ufs/ufs-exynos.c| 2084
> > 
> >  drivers/scsi/ufs/ufs-exynos.h|  621
> > ++
> >  drivers/scsi/ufs/ufshcd.c |  146
> > +-
> >  drivers/scsi/ufs/ufshcd.h |   62 +
> >  drivers/scsi/ufs/ufshci.h |   14
> > +-
> >  drivers/scsi/ufs/unipro.h |   24 +
> >  10 files changed, 3073 insertions(+), 20 deletions(-)  create mode
> > 100644 Documentation/devicetree/bindings/ufs/ufs-exynos.txt
> >  create mode 100644 drivers/scsi/ufs/mphy.h  create mode 100644
> > drivers/scsi/ufs/ufs-exynos.c  create mode 100644
> > drivers/scsi/ufs/ufs-exynos.h
> 
> This supposed to be 8-patch series but i only patch #1 (subject: [PATCH
> v1 1/8] ufs: add some vendor specific operations) on mailing list. Can you
> please send out the full patch series?
> 
> --
> 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 majord...@vger.kernel.org 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v2] scsi: ufshcd: fix possible unclocked register access

[Kiwoong Kim]

Hi, Subhash.

> Thanks Kim for the response.
> 
> On 2016-10-06 03:28, Kiwoong Kim wrote:
> > Hi, Subhash.
> >
> > Some UFS host controllers may need to call the vendor specific
> > callback before and after controlling by clock control framework,
> > regardless of whether available clocks are turned on or off.
> 
> Are you suggesting to call ufshcd_vops_setup_clocks() 2 times, one before
> the on/off by ufshcd core driver and one after the on/off? If yes, then we
> also have add 3rd argument clarifying if this is PRE_CHANGE or
POST_CHANGE.
> 
> >
> > Is there any special reason to limit to invoke the callback
> > only when the clocks are turned on or not?
> >
> > Besides, the callback is acknowledged from core driver
> > because 2nd argument is whether the clocks are turned on or not.
> >
> > If you have any other idea, please let me know.
> 
> This is my suggestion:
> 1. Add 3rd argument to setup_clocks ops to let the vendor callback
> function know if this is called PRE_CHANGE or POST_CHANGE.
> 2. If #1 is in place, call setup_clocks 2 times, one with PRE_CHANGE
> argument before making any clock changes in core driver, 2nd with
> POST_CHANGE argument after making the clock changes to core driver.

Yes, that's exactly the same what I mean.

Thanks


> 
> Let me know if this would work or not.
> 
> Thanks,
> Subhash
> 
> >
> > Thanks
> > Regards
> >
> >> Vendor specific setup_clocks callback may require the clocks managed
> >> by ufshcd driver to be ON. So if the vendor specific setup_clocks
> >> callback
> >> is called while the required clocks are turned off, it could result
> >> into
> >> unclocked register access.
> >>
> >> To prevent possible unclock register access, this change makes sure
> >> that
> >> required clocks remain enabled before calling into vendor specific
> >> setup_clocks callback.
> >>
> >> Signed-off-by: Subhash Jadavani <subha...@codeaurora.org>
> >> ---
> >> Changes from v2:
> >> * Don't call ufshcd_vops_setup_clocks() again for clock off
> >> ---
> >>  drivers/scsi/ufs/ufshcd.c | 22 +-
> >>  1 file changed, 21 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> >> index 05c7456..c1a77d3 100644
> >> --- a/drivers/scsi/ufs/ufshcd.c
> >> +++ b/drivers/scsi/ufs/ufshcd.c
> >> @@ -5389,6 +5389,17 @@ static int __ufshcd_setup_clocks(struct ufs_hba
> >> *hba, bool on,
> >>if (!head || list_empty(head))
> >>goto out;
> >>
> >> +  /*
> >> +   * vendor specific setup_clocks ops may depend on clocks managed by
> >> +   * this standard driver hence call the vendor specific setup_clocks
> >> +   * before disabling the clocks managed here.
> >> +   */
> >> +  if (!on) {
> >> +  ret = ufshcd_vops_setup_clocks(hba, on);
> >> +  if (ret)
> >> +  return ret;
> >> +  }
> >> +
> >>list_for_each_entry(clki, head, list) {
> >>if (!IS_ERR_OR_NULL(clki->clk)) {
> >>if (skip_ref_clk && !strcmp(clki->name, "ref_clk"))
> >> @@ -5410,7 +5421,16 @@ static int __ufshcd_setup_clocks(struct ufs_hba
> >> *hba, bool on,
> >>}
> >>}
> >>
> >> -  ret = ufshcd_vops_setup_clocks(hba, on);
> >> +  /*
> >> +   * vendor specific setup_clocks ops may depend on clocks managed by
> >> +   * this standard driver hence call the vendor specific setup_clocks
> >> +   * after enabling the clocks managed here.
> >> +   */
> >> +  if (on) {
> >> +  ret = ufshcd_vops_setup_clocks(hba, on);
> >> +  if (ret)
> >> +  return ret;
> >> +  }
> >>  out:
> >>if (ret) {
> >>list_for_each_entry(clki, head, list) {
> >> --
> >> The Qualcomm Innovation Center, Inc. is a member of the 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 majord...@vger.kernel.org
> >> 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 majord...@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
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org
> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v2] scsi: ufshcd: fix possible unclocked register access

[Kiwoong Kim]

Hi, Subhash.

Some UFS host controllers may need to call the vendor specific callback
before and after controlling by clock control framework,
regardless of whether available clocks are turned on or off.

Is there any special reason to limit to invoke the callback
only when the clocks are turned on or not?
Besides, the callback is acknowledged from core driver
because 2nd argument is whether the clocks are turned on or not.

If you have any other idea, please let me know.

Thanks
Regards

> Vendor specific setup_clocks callback may require the clocks managed
> by ufshcd driver to be ON. So if the vendor specific setup_clocks callback
> is called while the required clocks are turned off, it could result into
> unclocked register access.
> 
> To prevent possible unclock register access, this change makes sure that
> required clocks remain enabled before calling into vendor specific
> setup_clocks callback.
> 
> Signed-off-by: Subhash Jadavani 
> ---
> Changes from v2:
> * Don't call ufshcd_vops_setup_clocks() again for clock off
> ---
>  drivers/scsi/ufs/ufshcd.c | 22 +-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 05c7456..c1a77d3 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -5389,6 +5389,17 @@ static int __ufshcd_setup_clocks(struct ufs_hba
> *hba, bool on,
>   if (!head || list_empty(head))
>   goto out;
> 
> + /*
> +  * vendor specific setup_clocks ops may depend on clocks managed by
> +  * this standard driver hence call the vendor specific setup_clocks
> +  * before disabling the clocks managed here.
> +  */
> + if (!on) {
> + ret = ufshcd_vops_setup_clocks(hba, on);
> + if (ret)
> + return ret;
> + }
> +
>   list_for_each_entry(clki, head, list) {
>   if (!IS_ERR_OR_NULL(clki->clk)) {
>   if (skip_ref_clk && !strcmp(clki->name, "ref_clk"))
> @@ -5410,7 +5421,16 @@ static int __ufshcd_setup_clocks(struct ufs_hba
> *hba, bool on,
>   }
>   }
> 
> - ret = ufshcd_vops_setup_clocks(hba, on);
> + /*
> +  * vendor specific setup_clocks ops may depend on clocks managed by
> +  * this standard driver hence call the vendor specific setup_clocks
> +  * after enabling the clocks managed here.
> +  */
> + if (on) {
> + ret = ufshcd_vops_setup_clocks(hba, on);
> + if (ret)
> + return ret;
> + }
>  out:
>   if (ret) {
>   list_for_each_entry(clki, head, list) {
> --
> The Qualcomm Innovation Center, Inc. is a member of the 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 majord...@vger.kernel.org
> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 0/8] Support for Exynos specific UFS driver

[Kiwoong Kim]

This series of patches introduce support for Exynos specific UFS driver.
An original version of this has been applied on mass production
of commercial mobile devices.

 Documentation/devicetree/bindings/ufs/ufs-exynos.txt |   93 +
 drivers/scsi/ufs/Kconfig  |   10 +
 drivers/scsi/ufs/Makefile |1 +
 drivers/scsi/ufs/mphy.h|   38 +
 drivers/scsi/ufs/ufs-exynos.c| 2084 

 drivers/scsi/ufs/ufs-exynos.h|  621 ++
 drivers/scsi/ufs/ufshcd.c |  146 +-
 drivers/scsi/ufs/ufshcd.h |   62 +
 drivers/scsi/ufs/ufshci.h |   14 +-
 drivers/scsi/ufs/unipro.h |   24 +
 10 files changed, 3073 insertions(+), 20 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/ufs/ufs-exynos.txt
 create mode 100644 drivers/scsi/ufs/mphy.h
 create mode 100644 drivers/scsi/ufs/ufs-exynos.c
 create mode 100644 drivers/scsi/ufs/ufs-exynos.h

-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 1/8] ufs: add some vendor specific operations

[Kiwoong Kim]

Some UFS host controller may need to require some sequences
before used clocks are turned on or off.
e.g) control internal gates in UFS host

Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
---
 drivers/scsi/ufs/ufshcd.c |   15 +--
 drivers/scsi/ufs/ufshcd.h |   37 +
 2 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 05c7456..82c9a40 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1474,6 +1474,7 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, 
struct scsi_cmnd *cmd)
 
/* issue command to the controller */
spin_lock_irqsave(hba->host->host_lock, flags);
+   ufshcd_vops_set_nexus_t_xfer_req(hba, tag, lrbp->cmd);
ufshcd_send_command(hba, tag);
 out_unlock:
spin_unlock_irqrestore(hba->host->host_lock, flags);
@@ -1683,6 +1684,7 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba *hba,
/* Make sure descriptors are ready before ringing the doorbell */
wmb();
spin_lock_irqsave(hba->host->host_lock, flags);
+   ufshcd_vops_set_nexus_t_xfer_req(hba, tag, lrbp->cmd);
ufshcd_send_command(hba, tag);
spin_unlock_irqrestore(hba->host->host_lock, flags);
 
@@ -2651,6 +2653,8 @@ static int __ufshcd_uic_hibern8_enter(struct ufs_hba *hba)
int ret;
struct uic_command uic_cmd = {0};
 
+   ufshcd_vops_hibern8_notify(hba, true, PRE_CHANGE);
+
uic_cmd.command = UIC_CMD_DME_HIBER_ENTER;
ret = ufshcd_uic_pwr_ctrl(hba, _cmd);
 
@@ -2664,7 +2668,8 @@ static int __ufshcd_uic_hibern8_enter(struct ufs_hba *hba)
 */
if (ufshcd_link_recovery(hba))
ret = -ENOLINK;
-   }
+   } else
+   ufshcd_vops_hibern8_notify(hba, true, POST_CHANGE);
 
return ret;
 }
@@ -2687,13 +2692,16 @@ static int ufshcd_uic_hibern8_exit(struct ufs_hba *hba)
struct uic_command uic_cmd = {0};
int ret;
 
+   ufshcd_vops_hibern8_notify(hba, false, PRE_CHANGE);
+
uic_cmd.command = UIC_CMD_DME_HIBER_EXIT;
ret = ufshcd_uic_pwr_ctrl(hba, _cmd);
if (ret) {
dev_err(hba->dev, "%s: hibern8 exit failed. ret = %d\n",
__func__, ret);
ret = ufshcd_link_recovery(hba);
-   }
+   } else
+   ufshcd_vops_hibern8_notify(hba, false, POST_CHANGE);
 
return ret;
 }
@@ -4312,6 +4320,7 @@ static int ufshcd_issue_tm_cmd(struct ufs_hba *hba, int 
lun_id, int task_id,
task_req_upiup->input_param2 = cpu_to_be32(task_id);
 
/* send command to the controller */
+   ufshcd_vops_set_nexus_t_task_mgmt(hba, free_slot, tm_function);
__set_bit(free_slot, >outstanding_tasks);
 
/* Make sure descriptors are ready before ringing the task doorbell */
@@ -5389,6 +5398,8 @@ static int __ufshcd_setup_clocks(struct ufs_hba *hba, 
bool on,
if (!head || list_empty(head))
goto out;
 
+   ufshcd_vops_pre_setup_clocks(hba, on);
+
list_for_each_entry(clki, head, list) {
if (!IS_ERR_OR_NULL(clki->clk)) {
if (skip_ref_clk && !strcmp(clki->name, "ref_clk"))
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 430bef1..b97f7c2 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -252,6 +252,7 @@ struct ufs_pwr_mode_info {
  * @exit: called to cleanup everything done in init
  * @get_ufs_hci_version: called to get UFS HCI version
  * @clk_scale_notify: notifies that clks are scaled up/down
+ * @pre_setup_clocks: called before controlling used clocks
  * @setup_clocks: called before touching any of the controller registers
  * @setup_regulators: called before accessing the host controller
  * @hce_enable_notify: called before and after HCE enable bit is set to allow
@@ -261,6 +262,9 @@ struct ufs_pwr_mode_info {
  * @pwr_change_notify: called before and after a power mode change
  * is carried out to allow vendor spesific capabilities
  * to be set.
+ * @set_nexus_t_xfer_req:
+ * @set_nexus_t_task_mgmt:
+ * @hibern8_notify:
  * @suspend: called during host controller PM callback
  * @resume: called during host controller PM callback
  * @dbg_register_dump: used to dump controller debug information
@@ -273,6 +277,7 @@ struct ufs_hba_variant_ops {
u32 (*get_ufs_hci_version)(struct ufs_hba *);
int (*clk_scale_notify)(struct ufs_hba *, bool,
enum ufs_notify_change_status);
+   int (*pre_setup_clocks)(struct ufs_hba *, bool);
int (*setup_clocks)(struct ufs_hba *, bool);
int (*setup_regulators)(struct ufs_hba *, bool);
int (*hce_enable_notify)(struct ufs_hba *,
@@ -283,6 +288,

RE: [PATCH -next] scsi: ufs: fix error return code in ufshcd_init()

[Kiwoong Kim]

Reviewed-by: Kiwoong Kim <kwmad@samsung.com>

> -Original Message-
> From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
> ow...@vger.kernel.org] On Behalf Of subha...@codeaurora.org
> Sent: Thursday, September 29, 2016 7:50 AM
> To: Wei Yongjun
> Cc: Vinayak Holikatti; James E.J. Bottomley; Martin K. Petersen; Wei
> Yongjun; linux-scsi@vger.kernel.org; linux-scsi-ow...@vger.kernel.org
> Subject: Re: [PATCH -next] scsi: ufs: fix error return code in
> ufshcd_init()
> 
> Looks good to me.
> Reviewed-by: Subhash Jadavani <subha...@codeaurora.org>
> 
> On 2016-09-28 07:49, Wei Yongjun wrote:
> > From: Wei Yongjun <weiyongj...@huawei.com>
> >
> > Fix to return a negative error code from the error handling case
> > instead of 0, as done elsewhere in this function.
> >
> > Signed-off-by: Wei Yongjun <weiyongj...@huawei.com>
> > ---
> >  drivers/scsi/ufs/ufshcd.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index 37f3c51..6aebb7e 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -6500,6 +6500,7 @@ int ufshcd_init(struct ufs_hba *hba, void
> > __iomem *mmio_base, unsigned int irq)
> > if (IS_ERR(hba->devfreq)) {
> > dev_err(hba->dev, "Unable to register with
> devfreq %ld\n",
> > PTR_ERR(hba->devfreq));
> > +   err = PTR_ERR(hba->devfreq);
> > goto out_remove_scsi_host;
> > }
> > /* Suspend devfreq until the UFS device is detected */
> >
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-scsi"
> > in
> > the body of a message to majord...@vger.kernel.org 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 majord...@vger.kernel.org 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v2] UFS: Date Segment only need for WRITE DESCRIPTOR

[Kiwoong Kim]

Hi, Martin.

I think that the patch is correct.
UFS spec says "The Data Segment area is empty" for Read Descriptor.
I have been using similar code with it and it works.
That have been already applied in Android kernel.

Reviewed-by: Kiwoong Kim <kwmad@samsung.com>

Regards.

> -Original Message-
> From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
> ow...@vger.kernel.org] On Behalf Of Martin K. Petersen
> Sent: Wednesday, September 28, 2016 2:14 PM
> To: subha...@codeaurora.org
> Cc: Zang Leigang; vinholika...@gmail.com; j...@linux.vnet.ibm.com;
> martin.peter...@oracle.com; linux-scsi@vger.kernel.org; linux-
> ker...@vger.kernel.org; linux-scsi-ow...@vger.kernel.org
> Subject: Re: [PATCH v2] UFS: Date Segment only need for WRITE DESCRIPTOR
> 
> >>>>> "Subhash" == subhashj  <subha...@codeaurora.org> writes:
> 
> Subhash> Looks good to me.
> 
> > -   /* Data segment length */
> > -   ucd_req_ptr->header.dword_2 = UPIU_HEADER_DWORD(
> > -   0, 0, len >> 8, (u8)len);
> > +   /* Data segment length only need for WRITE_DESC */
> > +   if (query->request.upiu_req.opcode == UPIU_QUERY_OPCODE_WRITE_DESC)
> > +   ucd_req_ptr->header.dword_2 =
> > +   UPIU_HEADER_DWORD(0, 0, (len >> 8), (u8)len);
> > +   else
> > +   ucd_req_ptr->header.dword_2 = 0;
> 
> What about READ_DESC?
> 
> --
> Martin K. PetersenOracle Linux Engineering
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: UFS API in the kernel

[Kiwoong Kim]

Hi.

If you want to declare some things for user interface,
is it be better to put those thing include/uapi/linux/ than include/linux?

Agreed with Mr. Pinto's opinion with respect to implementing additional ioctls.

Regards.

> -Original Message-
> From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
> ow...@vger.kernel.org] On Behalf Of Shaun Tancheff
> Sent: Tuesday, September 27, 2016 4:23 AM
> To: Joao Pinto
> Cc: linux-scsi@vger.kernel.org; linux-ker...@vger.kernel.org
> Subject: Re: UFS API in the kernel
> 
> On Thu, Sep 22, 2016 at 10:21 AM, Joao Pinto 
> wrote:
> > Hi!
> >
> > I am designing an application that has the goal to be an utility for
> > Unipro and UFS testing purposes. This application is going to run on
> > top of a recent Linux Kernel containing the new UFS stack (including the
> new DWC drivers).
> >
> > I am considering doing the following:
> > a) Create a new config item called CONFIG_UFS_CHARDEV which is going
> > to create a char device responsible to make some IOCTL available for
> > user-space applications
> > b) Create a linux/ufs.h header file that contains data structures
> > declarations that will be needed in user-space applications
> 
> I am not very familiar with UFS devices, that said you should have an sgX
> chardev being created already so you can handle SG_IO requests.
> There also appear to be some sysfs entries being created.
> 
> So between sg and sysfs you should be able to handle any user-space out of
> band requests without resorting to making a new chardev.
> 
> Adding more sysfs entries, if you need them, should be fine.
> 
> You may find it easier to expand on the existing interfaces than to get
> consensus on a new driver and ioctls.
> 
> Hope this helps,
> Shaun
> 
> > Could you please advise me about what the correct approach should be
> > to make it as standard as possible and usable in the future?
> >
> > Thank you very much for your help!
> >
> > regards,
> > Joao
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-scsi"
> > in the body of a message to majord...@vger.kernel.org More majordomo
> > info at
> > https://urldefense.proofpoint.com/v2/url?u=http-3A__vger.kernel.org_ma
> > jordomo-2Dinfo.html=DQICaQ=IGDlg0lD0b-nebmJJ0Kp8A=Wg5NqlNlVTT7Ug
> > l8V50qIHLe856QW0qfG3WVYGOrWzA=vJFB6pCywWtdvkgHz9Vc0jQz0xzeyZlr-7eCWY
> > u88nM=yiQLPFpqmMrbqLZz1Jb3aNqOje2dRMLJHEzUDobwcXc=
> 
> 
> 
> --
> Shaun Tancheff
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH][SCSI] scsi: ufs: get a TM service response from the correct offset

[Kiwoong Kim]

From: Kiwoong Kim <kwmad@samsung.com>

When any UFS host controller receives a TM(Task Management)
response from a UFS device,
UFS driver has been recognize like receiving a message of
"Task Management Function Complete"(00h) in all cases, so far.
That means there is no pending task for a tag of the TM request
sent before in the UFS device.
That's because the byte offset 6 in TM response which has been used
to get a TM service response so far
represents just whether or not a TM transmission passes.

Regarding UFS spec, the correct byte offset to
get TM service response is 15, not 6.

I tested that UFS driver responds properly for the TM response
>From a UFS device with an reference board with exynos8890, as follow:
No pending task -> Task Management Function Complete (00h)
Pending task -> Task Management Function Succeeded (08h)

Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
Signed-off-by: HeonGwang Chu <hg@samsung.com>
Tested-by: : Kiwoong Kim <kwmad@samsung.com>
---
 drivers/scsi/ufs/ufs.h|1 +
 drivers/scsi/ufs/ufshcd.c |4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 42c459a..89c121e 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -295,6 +295,7 @@ enum {
MASK_QUERY_DATA_SEG_LEN = 0x,
MASK_RSP_UPIU_DATA_SEG_LEN  = 0x,
MASK_RSP_EXCEPTION_EVENT= 0x1,
+   MASK_TM_SERVICE_RESP= 0xFF,
 };

 /* Task management service response */
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index e8a706b..c641cd3 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -3013,8 +3013,8 @@ static int ufshcd_task_req_compl(struct ufs_hba *hba, u32 
index, u8 *resp)
if (ocs_value == OCS_SUCCESS) {
task_rsp_upiup = (struct utp_upiu_task_rsp *)
task_req_descp[index].task_rsp_upiu;
-   task_result = be32_to_cpu(task_rsp_upiup->header.dword_1);
-   task_result = ((task_result & MASK_TASK_RESPONSE) >> 8);
+   task_result = be32_to_cpu(task_rsp_upiup->output_param1);
+   task_result = task_result & MASK_TM_SERVICE_RESP;
if (resp)
*resp = (u8)task_result;
} else {
--
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH][SCSI] ufs: fix a wrong string in power mode change

[Kiwoong Kim]

From: Kiwoong Kim <kwmad@samsung.com>

I modified a string as described in UFS spec as follow:
umpcrs -> upmcrs 

Signed-off-by: Kiwoong Kim <kwmad@samsung.com>
---
 drivers/scsi/ufs/ufshcd.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index b0ade73..e8a706b 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -2268,7 +2268,7 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba *hba, 
struct uic_command *cmd)
status = ufshcd_get_upmcrs(hba);
if (status != PWR_LOCAL) {
dev_err(hba->dev,
-   "pwr ctrl cmd 0x%0x failed, host umpcrs:0x%x\n",
+   "pwr ctrl cmd 0x%0x failed, host upmcrs:0x%x\n",
cmd->command, status);
ret = (status != PWR_OK) ? status : -1;
}
--
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html