[PATCH v1 00/10] *** introduce quirks for UFS host

[PATCH v1 00/10] *** introduce quirks for UFS host

[Yaniv Gardi]

This series of patches introduce additional quirks and also,
their enabling in QUALCOMM Technologies specific code.

Yaniv Gardi (10):
  scsi: ufs: introduce the capability and quirk for interrupt
    aggregation
  scsi: ufs-qcom: don't enable interrupt aggregation
  scsi: ufs: provide a quirk to disable the LCC
  scsi: ufs-qcom: enable UFSHCD_QUIRK_BROKEN_LCC
  scsi: ufs: introduce a broken PA_RXHSUNTERMCAP quirk
  scsi: ufs-qcom: enable quirk to fix gear change to HS
  scsi: ufs: introduce UFSHCD_QUIRK_DME_PEER_ACCESS_AUTO_MODE quirk
  scsi: ufs-qcom: enable UFSHCD_QUIRK_DME_PEER_ACCESS_AUTO_MODE quirk
  scsi: ufs: add quirk to handle broken UFS HCI version
  scsi: ufs-qcom: enable UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION

 drivers/scsi/ufs/ufs-qcom.c |  39 ++++++++++++++--
 drivers/scsi/ufs/ufshcd.c   | 108 ++++++++++++++++++++++++++++++++++++++++++--
 drivers/scsi/ufs/ufshcd.h   |  53 +++++++++++++++++++++-
 drivers/scsi/ufs/ufshci.h   |   8 +++-
 4 files changed, 199 insertions(+), 9 deletions(-)

-- 
1.8.5.2

-- 
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH v1 01/10] scsi: ufs: introduce the capability and quirk for interrupt aggregation

[Yaniv Gardi]

UFS HCI (Host Controller Interface) allows the transfer requests
interrupts to be aggregated to generate the single interrupt but
this can impact the performance. Hence introduce the capability which
gives choice to use the interrupt aggregation capability or not.
By default interrupt aggregation capability is kept disabled.

This change also introduces a quirk for broken interrupt aggregation
feature, as in some UFS controllers, this feature may not work.

Signed-off-by: Yaniv Gardi <ygardi@codeaurora.org>

---
 drivers/scsi/ufs/ufshcd.c | 19 ++++++++++++++++---
 drivers/scsi/ufs/ufshcd.h | 21 ++++++++++++++++++++-
 2 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 648a446..9641bcb 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -481,6 +481,15 @@ ufshcd_config_intr_aggr(struct ufs_hba *hba, u8 cnt, u8 tmout)
 }
 
 /**
+ * ufshcd_disable_intr_aggr - Disables interrupt aggregation.
+ * @hba: per adapter instance
+ */
+static inline void ufshcd_disable_intr_aggr(struct ufs_hba *hba)
+{
+	ufshcd_writel(hba, 0, REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
+}
+
+/**
  * ufshcd_enable_run_stop_reg - Enable run-stop registers,
  *			When run-stop registers are set to 1, it indicates the
  *			host controller that it can process the requests
@@ -1326,7 +1335,7 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
 	lrbp->sense_buffer = cmd->sense_buffer;
 	lrbp->task_tag = tag;
 	lrbp->lun = ufshcd_scsi_to_upiu_lun(cmd->device->lun);
-	lrbp->intr_cmd = false;
+	lrbp->intr_cmd = !ufshcd_is_intr_aggr_allowed(hba) ? true : false;
 	lrbp->command_type = UTP_CMD_TYPE_SCSI;
 
 	/* form UPIU before issuing the command */
@@ -2522,7 +2531,10 @@ static int ufshcd_make_hba_operational(struct ufs_hba *hba)
 	ufshcd_enable_intr(hba, UFSHCD_ENABLE_INTRS);
 
 	/* Configure interrupt aggregation */
-	ufshcd_config_intr_aggr(hba, hba->nutrs - 1, INT_AGGR_DEF_TO);
+	if (ufshcd_is_intr_aggr_allowed(hba))
+		ufshcd_config_intr_aggr(hba, hba->nutrs - 1, INT_AGGR_DEF_TO);
+	else
+		ufshcd_disable_intr_aggr(hba);
 
 	/* Configure UTRL and UTMRL base address registers */
 	ufshcd_writel(hba, lower_32_bits(hba->utrdl_dma_addr),
@@ -3073,7 +3085,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.
 	 */
-	ufshcd_reset_intr_aggr(hba);
+	if (ufshcd_is_intr_aggr_allowed(hba))
+		ufshcd_reset_intr_aggr(hba);
 
 	tr_doorbell = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
 	completed_reqs = tr_doorbell ^ hba->outstanding_reqs;
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index b47ff07..fc8bec9 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -417,11 +417,15 @@ struct ufs_hba {
 	unsigned int irq;
 	bool is_irq_enabled;
 
+	/* Interrupt aggregation support is broken */
+	#define UFSHCD_QUIRK_BROKEN_INTR_AGGR			UFS_BIT(0)
+
 	/*
 	 * delay before each dme command is required as the unipro
 	 * layer has shown instabilities
 	 */
-	#define UFSHCD_QUIRK_DELAY_BEFORE_DME_CMDS		UFS_BIT(0)
+	#define UFSHCD_QUIRK_DELAY_BEFORE_DME_CMDS		UFS_BIT(1)
+
 
 	unsigned int quirks;	/* Deviations from standard UFSHCI spec. */
 
@@ -478,6 +482,12 @@ struct ufs_hba {
 #define UFSHCD_CAP_CLK_SCALING	(1 << 2)
 	/* Allow auto bkops to enabled during runtime suspend */
 #define UFSHCD_CAP_AUTO_BKOPS_SUSPEND (1 << 3)
+	/*
+	 * This capability allows host controller driver to use the UFS HCI's
+	 * interrupt aggregation capability.
+	 * CAUTION: Enabling this might reduce overall UFS throughput.
+	 */
+#define UFSHCD_CAP_INTR_AGGR (1 << 4)
 
 	struct devfreq *devfreq;
 	struct ufs_clk_scaling clk_scaling;
@@ -502,6 +512,15 @@ static inline bool ufshcd_can_autobkops_during_suspend(struct ufs_hba *hba)
 	return hba->caps & UFSHCD_CAP_AUTO_BKOPS_SUSPEND;
 }
 
+static inline bool ufshcd_is_intr_aggr_allowed(struct ufs_hba *hba)
+{
+	if ((hba->caps & UFSHCD_CAP_INTR_AGGR) &&
+	    !(hba->quirks & UFSHCD_QUIRK_BROKEN_INTR_AGGR))
+		return true;
+	else
+		return false;
+}
+
 #define ufshcd_writel(hba, val, reg)	\
 	writel((val), (hba)->mmio_base + (reg))
 #define ufshcd_readl(hba, reg)	\
-- 
1.8.5.2

-- 
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH v1 02/10] scsi: ufs-qcom: don't enable interrupt aggregation

[Yaniv Gardi]

Current versions of UFS host controllers on QUALCOMM Technologies
have interrupt aggregation logic broken.
Interrupt aggregation may not work if both threshold count and timeout
is enabled. Hence disable interrupt aggregation by enabling
UFSHCD_QUIRK_BROKEN_INTR_AGGR quirk until its fixed in the newer
UFS host controller revisions.

Signed-off-by: Yaniv Gardi <ygardi@codeaurora.org>

---
 drivers/scsi/ufs/ufs-qcom.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index 6652a81..e8dc050 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -307,6 +307,7 @@ static int ufs_qcom_hce_enable_notify(struct ufs_hba *hba, bool status)
 static unsigned long
 ufs_qcom_cfg_timers(struct ufs_hba *hba, u32 gear, u32 hs, u32 rate)
 {
+	struct ufs_qcom_host *host = hba->priv;
 	struct ufs_clk_info *clki;
 	u32 core_clk_period_in_ns;
 	u32 tx_clk_cycles_per_us = 0;
@@ -330,6 +331,16 @@ ufs_qcom_cfg_timers(struct ufs_hba *hba, u32 gear, u32 hs, u32 rate)
 		{UFS_HS_G2, 0x49},
 	};
 
+	/*
+	 * The Qunipro controller does not use following registers:
+	 * SYS1CLK_1US_REG, TX_SYMBOL_CLK_1US_REG, CLK_NS_REG &
+	 * UFS_REG_PA_LINK_STARTUP_TIMER
+	 * But UTP controller uses SYS1CLK_1US_REG register for Interrupt
+	 * Aggregation logic.
+	*/
+	if (ufs_qcom_cap_qunipro(host) && !ufshcd_is_intr_aggr_allowed(hba))
+		goto out;
+
 	if (gear == 0) {
 		dev_err(hba->dev, "%s: invalid gear = %d\n", __func__, gear);
 		goto out_error;
@@ -696,9 +707,13 @@ static void ufs_qcom_advertise_quirks(struct ufs_hba *hba)
 {
 	struct ufs_qcom_host *host = hba->priv;
 
-	if (host->hw_ver.major == 0x1)
+	if (host->hw_ver.major == 0x01) {
 		hba->quirks |= UFSHCD_QUIRK_DELAY_BEFORE_DME_CMDS;
 
+		if (host->hw_ver.minor == 0x0001 && host->hw_ver.step == 0x0001)
+			hba->quirks |= UFSHCD_QUIRK_BROKEN_INTR_AGGR;
+	}
+
 	if (host->hw_ver.major >= 0x2) {
 		if (!ufs_qcom_cap_qunipro(host))
 			/* Legacy UniPro mode still need following quirks */
-- 
1.8.5.2

-- 
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH v1 03/10] scsi: ufs: provide a quirk to disable the LCC

[Yaniv Gardi]

LCC (Line Control Command) are being used for communication between
UFS host and UFS device.
New commercial UFS devices don't have the issues with LCC processing
but UFS host controller might still have the issue with LCC processing,
hence, added a routine to disable TX LCC on the device.

Signed-off-by: Yaniv Gardi <ygardi@codeaurora.org>

---
 drivers/scsi/ufs/ufshcd.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 drivers/scsi/ufs/ufshcd.h |  8 ++++++++
 drivers/scsi/ufs/ufshci.h |  3 +++
 3 files changed, 53 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 9641bcb..3e57cca 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -2640,6 +2640,42 @@ static int ufshcd_hba_enable(struct ufs_hba *hba)
 	return 0;
 }
 
+static int ufshcd_disable_tx_lcc(struct ufs_hba *hba, bool peer)
+{
+	int tx_lanes, i, err = 0;
+
+	if (!peer)
+		ufshcd_dme_get(hba, UIC_ARG_MIB(PA_CONNECTEDTXDATALANES),
+			       &tx_lanes);
+	else
+		ufshcd_dme_peer_get(hba, UIC_ARG_MIB(PA_CONNECTEDTXDATALANES),
+				    &tx_lanes);
+	for (i = 0; i < tx_lanes; i++) {
+		if (!peer)
+			err = ufshcd_dme_set(hba,
+				UIC_ARG_MIB_SEL(TX_LCC_ENABLE,
+					UIC_ARG_MPHY_TX_GEN_SEL_INDEX(i)),
+					0);
+		else
+			err = ufshcd_dme_peer_set(hba,
+				UIC_ARG_MIB_SEL(TX_LCC_ENABLE,
+					UIC_ARG_MPHY_TX_GEN_SEL_INDEX(i)),
+					0);
+		if (err) {
+			dev_err(hba->dev, "%s: TX LCC Disable failed, peer = %d, lane = %d, err = %d",
+				__func__, peer, i, err);
+			break;
+		}
+	}
+
+	return err;
+}
+
+static inline int ufshcd_disable_device_tx_lcc(struct ufs_hba *hba)
+{
+	return ufshcd_disable_tx_lcc(hba, true);
+}
+
 /**
  * ufshcd_link_startup - Initialize unipro link startup
  * @hba: per adapter instance
@@ -2677,6 +2713,12 @@ static int ufshcd_link_startup(struct ufs_hba *hba)
 		/* failed to get the link up... retire */
 		goto out;
 
+	if (hba->quirks & UFSHCD_QUIRK_BROKEN_LCC) {
+		ret = ufshcd_disable_device_tx_lcc(hba);
+		if (ret)
+			goto out;
+	}
+
 	/* Include any host controller configuration via UIC commands */
 	if (hba->vops && hba->vops->link_startup_notify) {
 		ret = hba->vops->link_startup_notify(hba, POST_CHANGE);
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index fc8bec9..b845f15 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -426,6 +426,14 @@ struct ufs_hba {
 	 */
 	#define UFSHCD_QUIRK_DELAY_BEFORE_DME_CMDS		UFS_BIT(1)
 
+	/*
+	 * If UFS host controller is having issue in processing LCC (Line
+	 * Control Command) coming from device then enable this quirk.
+	 * When this quirk is enabled, host controller driver should disable
+	 * the LCC transmission on UFS device (by clearing TX_LCC_ENABLE
+	 * attribute of device to 0).
+	 */
+	#define UFSHCD_QUIRK_BROKEN_LCC				UFS_BIT(2)
 
 	unsigned int quirks;	/* Deviations from standard UFSHCI spec. */
 
diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
index d572119..f8909ec 100644
--- a/drivers/scsi/ufs/ufshci.h
+++ b/drivers/scsi/ufs/ufshci.h
@@ -206,6 +206,9 @@ enum {
 #define CONFIG_RESULT_CODE_MASK		0xFF
 #define GENERIC_ERROR_CODE_MASK		0xFF
 
+/* GenSelectorIndex calculation macros for M-PHY attributes */
+#define UIC_ARG_MPHY_TX_GEN_SEL_INDEX(lane) (lane)
+
 #define UIC_ARG_MIB_SEL(attr, sel)	((((attr) & 0xFFFF) << 16) |\
 					 ((sel) & 0xFFFF))
 #define UIC_ARG_MIB(attr)		UIC_ARG_MIB_SEL(attr, 0)
-- 
1.8.5.2

-- 
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH v1 04/10] scsi: ufs-qcom: enable UFSHCD_QUIRK_BROKEN_LCC

[Yaniv Gardi]

LCC (Line Control Command) are being used for communication between
UFS host and UFS device. But UFS host controller on QUALCOMM
Technologies have an issue with issuing the LCC commands to
UFS device and hence this quirk is enabled in order to to disable
LCC from the host side.

Signed-off-by: Yaniv Gardi <ygardi@codeaurora.org>

---
 drivers/scsi/ufs/ufs-qcom.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index e8dc050..76d6e9d 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -715,6 +715,8 @@ static void ufs_qcom_advertise_quirks(struct ufs_hba *hba)
 	}
 
 	if (host->hw_ver.major >= 0x2) {
+		hba->quirks |= UFSHCD_QUIRK_BROKEN_LCC;
+
 		if (!ufs_qcom_cap_qunipro(host))
 			/* Legacy UniPro mode still need following quirks */
 			hba->quirks |= UFSHCD_QUIRK_DELAY_BEFORE_DME_CMDS;
-- 
1.8.5.2

-- 
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH v1 05/10] scsi: ufs: introduce a broken PA_RXHSUNTERMCAP quirk

[Yaniv Gardi]

The attribute PA_RXHSUNTERMCAP specifies whether or not the
inbound Link supports unterminated line in HS mode. enabling this
attribute to 1 fixes moving to HS gear.

Signed-off-by: Yaniv Gardi <ygardi@codeaurora.org>

---
 drivers/scsi/ufs/ufshcd.c | 11 +++++++++++
 drivers/scsi/ufs/ufshcd.h |  7 +++++++
 2 files changed, 18 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 3e57cca..a274df9 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -2258,6 +2258,16 @@ static int ufshcd_uic_change_pwr_mode(struct ufs_hba *hba, u8 mode)
 	struct uic_command uic_cmd = {0};
 	int ret;
 
+	if (hba->quirks & UFSHCD_QUIRK_BROKEN_PA_RXHSUNTERMCAP) {
+		ret = ufshcd_dme_set(hba,
+				UIC_ARG_MIB_SEL(PA_RXHSUNTERMCAP, 0), 1);
+		if (ret) {
+			dev_err(hba->dev, "%s: failed to enable PA_RXHSUNTERMCAP ret %d\n",
+						__func__, ret);
+			goto out;
+		}
+	}
+
 	uic_cmd.command = UIC_CMD_DME_SET;
 	uic_cmd.argument1 = UIC_ARG_MIB(PA_PWRMODE);
 	uic_cmd.argument3 = mode;
@@ -2265,6 +2275,7 @@ static int ufshcd_uic_change_pwr_mode(struct ufs_hba *hba, u8 mode)
 	ret = ufshcd_uic_pwr_ctrl(hba, &uic_cmd);
 	ufshcd_release(hba);
 
+out:
 	return ret;
 }
 
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index b845f15..8636ec9 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -435,6 +435,13 @@ struct ufs_hba {
 	 */
 	#define UFSHCD_QUIRK_BROKEN_LCC				UFS_BIT(2)
 
+	/*
+	 * The attribute PA_RXHSUNTERMCAP specifies whether or not the
+	 * inbound Link supports unterminated line in HS mode. Setting this
+	 * attribute to 1 fixes moving to HS gear.
+	 */
+	#define UFSHCD_QUIRK_BROKEN_PA_RXHSUNTERMCAP		UFS_BIT(3)
+
 	unsigned int quirks;	/* Deviations from standard UFSHCI spec. */
 
 	wait_queue_head_t tm_wq;
-- 
1.8.5.2

-- 
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH v1 06/10] scsi: ufs-qcom: enable quirk to fix gear change to HS

[Yaniv Gardi]

With the G3 UFS devices, changing gear into HS is failing in UFS host
controllers of version 0x2.
The quirk solves the problem of changing gear into HS by enabling
the attribute that specifies whether or not the inbound Link supports
unterminated line in HS mode.

Signed-off-by: Yaniv Gardi <ygardi@codeaurora.org>

---
 drivers/scsi/ufs/ufs-qcom.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index 76d6e9d..6e5b183 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -708,7 +708,8 @@ static void ufs_qcom_advertise_quirks(struct ufs_hba *hba)
 	struct ufs_qcom_host *host = hba->priv;
 
 	if (host->hw_ver.major == 0x01) {
-		hba->quirks |= UFSHCD_QUIRK_DELAY_BEFORE_DME_CMDS;
+		hba->quirks |= UFSHCD_QUIRK_DELAY_BEFORE_DME_CMDS
+			    | UFSHCD_QUIRK_BROKEN_PA_RXHSUNTERMCAP;
 
 		if (host->hw_ver.minor == 0x0001 && host->hw_ver.step == 0x0001)
 			hba->quirks |= UFSHCD_QUIRK_BROKEN_INTR_AGGR;
@@ -719,7 +720,8 @@ static void ufs_qcom_advertise_quirks(struct ufs_hba *hba)
 
 		if (!ufs_qcom_cap_qunipro(host))
 			/* Legacy UniPro mode still need following quirks */
-			hba->quirks |= UFSHCD_QUIRK_DELAY_BEFORE_DME_CMDS;
+			hba->quirks |= (UFSHCD_QUIRK_DELAY_BEFORE_DME_CMDS
+				| UFSHCD_QUIRK_BROKEN_PA_RXHSUNTERMCAP);
 	}
 }
 
-- 
1.8.5.2

-- 
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH v1 07/10] scsi: ufs: introduce UFSHCD_QUIRK_DME_PEER_ACCESS_AUTO_MODE quirk

[Yaniv Gardi]

Some UFS host controllers may only allow accessing the peer DME attribute
in AUTO mode (FAST AUTO or SLOW AUTO) hence we had added a quirk for
switching to AUTO power mode before accessing the peer DME attribute.

Signed-off-by: Yaniv Gardi <ygardi@codeaurora.org>

---
 drivers/scsi/ufs/ufshcd.c | 31 +++++++++++++++++++++++++++++++
 drivers/scsi/ufs/ufshcd.h |  7 +++++++
 2 files changed, 38 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index a274df9..014f1c0 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -188,6 +188,8 @@ static int ufshcd_host_reset_and_restore(struct ufs_hba *hba);
 static irqreturn_t ufshcd_intr(int irq, void *__hba);
 static int ufshcd_config_pwr_mode(struct ufs_hba *hba,
 		struct ufs_pa_layer_attr *desired_pwr_mode);
+static int ufshcd_change_power_mode(struct ufs_hba *hba,
+			     struct ufs_pa_layer_attr *pwr_mode);
 
 static inline int ufshcd_enable_irq(struct ufs_hba *hba)
 {
@@ -2156,6 +2158,31 @@ int ufshcd_dme_get_attr(struct ufs_hba *hba, u32 attr_sel,
 	};
 	const char *get = action[!!peer];
 	int ret;
+	struct ufs_pa_layer_attr orig_pwr_info;
+	struct ufs_pa_layer_attr temp_pwr_info;
+	bool pwr_mode_change = false;
+
+	if (peer && (hba->quirks & UFSHCD_QUIRK_DME_PEER_ACCESS_AUTO_MODE)) {
+		orig_pwr_info = hba->pwr_info;
+		temp_pwr_info = orig_pwr_info;
+
+		if (orig_pwr_info.pwr_tx == FAST_MODE ||
+		    orig_pwr_info.pwr_rx == FAST_MODE) {
+			temp_pwr_info.pwr_tx = FASTAUTO_MODE;
+			temp_pwr_info.pwr_rx = FASTAUTO_MODE;
+			pwr_mode_change = true;
+		} else if (orig_pwr_info.pwr_tx == SLOW_MODE ||
+		    orig_pwr_info.pwr_rx == SLOW_MODE) {
+			temp_pwr_info.pwr_tx = SLOWAUTO_MODE;
+			temp_pwr_info.pwr_rx = SLOWAUTO_MODE;
+			pwr_mode_change = true;
+		}
+		if (pwr_mode_change) {
+			ret = ufshcd_change_power_mode(hba, &temp_pwr_info);
+			if (ret)
+				goto out;
+		}
+	}
 
 	uic_cmd.command = peer ?
 		UIC_CMD_DME_PEER_GET : UIC_CMD_DME_GET;
@@ -2170,6 +2197,10 @@ int ufshcd_dme_get_attr(struct ufs_hba *hba, u32 attr_sel,
 
 	if (mib_val)
 		*mib_val = uic_cmd.argument3;
+
+	if (peer && (hba->quirks & UFSHCD_QUIRK_DME_PEER_ACCESS_AUTO_MODE)
+	    && pwr_mode_change)
+		ufshcd_change_power_mode(hba, &orig_pwr_info);
 out:
 	return ret;
 }
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 8636ec9..eb6831d 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -442,6 +442,13 @@ struct ufs_hba {
 	 */
 	#define UFSHCD_QUIRK_BROKEN_PA_RXHSUNTERMCAP		UFS_BIT(3)
 
+	/*
+	 * This quirk needs to be enabled if the host contoller only allows
+	 * accessing the peer dme attributes in AUTO mode (FAST AUTO or
+	 * SLOW AUTO).
+	 */
+	#define UFSHCD_QUIRK_DME_PEER_ACCESS_AUTO_MODE		UFS_BIT(4)
+
 	unsigned int quirks;	/* Deviations from standard UFSHCI spec. */
 
 	wait_queue_head_t tm_wq;
-- 
1.8.5.2

-- 
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH v1 08/10] scsi: ufs-qcom: enable UFSHCD_QUIRK_DME_PEER_ACCESS_AUTO_MODE quirk

[Yaniv Gardi]

Current version of host controller on QUALCOMM Technologies requires
this quirk to be enabled, as DME commands to device must be sent
only in AUTO mode (SLOW AUTO or FAST AUTO).

Signed-off-by: Yaniv Gardi <ygardi@codeaurora.org>

---
 drivers/scsi/ufs/ufs-qcom.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index 6e5b183..de9cfb0 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -709,7 +709,8 @@ static void ufs_qcom_advertise_quirks(struct ufs_hba *hba)
 
 	if (host->hw_ver.major == 0x01) {
 		hba->quirks |= UFSHCD_QUIRK_DELAY_BEFORE_DME_CMDS
-			    | UFSHCD_QUIRK_BROKEN_PA_RXHSUNTERMCAP;
+			    | UFSHCD_QUIRK_BROKEN_PA_RXHSUNTERMCAP
+			    | UFSHCD_QUIRK_DME_PEER_ACCESS_AUTO_MODE;
 
 		if (host->hw_ver.minor == 0x0001 && host->hw_ver.step == 0x0001)
 			hba->quirks |= UFSHCD_QUIRK_BROKEN_INTR_AGGR;
@@ -721,6 +722,7 @@ static void ufs_qcom_advertise_quirks(struct ufs_hba *hba)
 		if (!ufs_qcom_cap_qunipro(host))
 			/* Legacy UniPro mode still need following quirks */
 			hba->quirks |= (UFSHCD_QUIRK_DELAY_BEFORE_DME_CMDS
+				| UFSHCD_QUIRK_DME_PEER_ACCESS_AUTO_MODE
 				| UFSHCD_QUIRK_BROKEN_PA_RXHSUNTERMCAP);
 	}
 }
-- 
1.8.5.2

-- 
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH v1 09/10] scsi: ufs: add quirk to handle broken UFS HCI version

[Yaniv Gardi]

Some host controller hardware controllers may not advertise correct
version in UFS HCI VER register. To workaround this, add new quirk
and call the host controller hardware vendor specific callback to
get the correct UFS HCI version register value.

Signed-off-by: Yaniv Gardi <ygardi@codeaurora.org>

---
 drivers/scsi/ufs/ufshcd.c |  5 +++++
 drivers/scsi/ufs/ufshcd.h | 10 ++++++++++
 drivers/scsi/ufs/ufshci.h |  5 +++--
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 014f1c0..b0ade73 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -271,6 +271,11 @@ static inline u32 ufshcd_get_intr_mask(struct ufs_hba *hba)
  */
 static inline u32 ufshcd_get_ufs_version(struct ufs_hba *hba)
 {
+	if (hba->quirks & UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION) {
+		if (hba->vops && hba->vops->get_ufs_hci_version)
+			return hba->vops->get_ufs_hci_version(hba);
+	}
+
 	return ufshcd_readl(hba, REG_UFS_VERSION);
 }
 
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index eb6831d..c40a0e7 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -246,6 +246,7 @@ struct ufs_pwr_mode_info {
  * @name: variant name
  * @init: called when the driver is initialized
  * @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
  * @setup_clocks: called before touching any of the controller registers
  * @setup_regulators: called before accessing the host controller
@@ -263,6 +264,7 @@ struct ufs_hba_variant_ops {
 	const char *name;
 	int	(*init)(struct ufs_hba *);
 	void    (*exit)(struct ufs_hba *);
+	u32	(*get_ufs_hci_version)(struct ufs_hba *);
 	void    (*clk_scale_notify)(struct ufs_hba *);
 	int     (*setup_clocks)(struct ufs_hba *, bool);
 	int     (*setup_regulators)(struct ufs_hba *, bool);
@@ -449,6 +451,14 @@ struct ufs_hba {
 	 */
 	#define UFSHCD_QUIRK_DME_PEER_ACCESS_AUTO_MODE		UFS_BIT(4)
 
+	/*
+	 * This quirk needs to be enabled if the host contoller doesn't
+	 * advertise the correct version in UFS_VER register. If this quirk
+	 * is enabled, standard UFS host driver will call the vendor specific
+	 * ops (get_ufs_hci_version) to get the correct version.
+	 */
+	#define UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION		UFS_BIT(5)
+
 	unsigned int quirks;	/* Deviations from standard UFSHCI spec. */
 
 	wait_queue_head_t tm_wq;
diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
index f8909ec..0ae0967 100644
--- a/drivers/scsi/ufs/ufshci.h
+++ b/drivers/scsi/ufs/ufshci.h
@@ -89,8 +89,9 @@ enum {
 
 /* Controller UFSHCI version */
 enum {
-	UFSHCI_VERSION_10 = 0x00010000,
-	UFSHCI_VERSION_11 = 0x00010100,
+	UFSHCI_VERSION_10 = 0x00010000, /* 1.0 */
+	UFSHCI_VERSION_11 = 0x00010100, /* 1.1 */
+	UFSHCI_VERSION_20 = 0x00000200, /* 2.0 */
 };
 
 /*
-- 
1.8.5.2

-- 
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH v1 10/10] scsi: ufs-qcom: enable UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION

[Yaniv Gardi]

Newer revisions of QUALCOMM Technologies UFS host controller may not
advertise the correct version information in UFS HCI VER register.
To handle this, enable UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION to let
UFS standard host controller driver call into vendor specific
operation to get right UFS HCI VER register value.

Signed-off-by: Yaniv Gardi <ygardi@codeaurora.org>

---
 drivers/scsi/ufs/ufs-qcom.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index de9cfb0..4cdffa4 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -694,6 +694,16 @@ out:
 	return ret;
 }
 
+static u32 ufs_qcom_get_ufs_hci_version(struct ufs_hba *hba)
+{
+	struct ufs_qcom_host *host = hba->priv;
+
+	if (host->hw_ver.major == 0x1)
+		return UFSHCI_VERSION_11;
+	else
+		return UFSHCI_VERSION_20;
+}
+
 /**
  * ufs_qcom_advertise_quirks - advertise the known QCOM UFS controller quirks
  * @hba: host controller instance
@@ -718,6 +728,7 @@ static void ufs_qcom_advertise_quirks(struct ufs_hba *hba)
 
 	if (host->hw_ver.major >= 0x2) {
 		hba->quirks |= UFSHCD_QUIRK_BROKEN_LCC;
+		hba->quirks |= UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION;
 
 		if (!ufs_qcom_cap_qunipro(host))
 			/* Legacy UniPro mode still need following quirks */
@@ -1026,6 +1037,7 @@ static const struct ufs_hba_variant_ops ufs_hba_qcom_vops = {
 	.name                   = "qcom",
 	.init                   = ufs_qcom_init,
 	.exit                   = ufs_qcom_exit,
+	.get_ufs_hci_version	= ufs_qcom_get_ufs_hci_version,
 	.clk_scale_notify	= ufs_qcom_clk_scale_notify,
 	.setup_clocks           = ufs_qcom_setup_clocks,
 	.hce_enable_notify      = ufs_qcom_hce_enable_notify,
-- 
1.8.5.2

-- 
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH v1 00/10] *** introduce quirks for UFS host

[Akinobu Mita]

2015-05-18 0:54 GMT+09:00 Yaniv Gardi <ygardi@codeaurora.org>:
> This series of patches introduce additional quirks and also,
> their enabling in QUALCOMM Technologies specific code.

I've reviewed this series of patches.  I'll leave a few comments on
the corresponding emails, but I didn't see serious outstanding issues.
So please feel free to add:

Reviewed-by: Akinobu Mita <akinobu.mita@gmail.com>

(Since I don't have the platform with QUALCOMM UFS host controller and
any knowledge for it, we need appropriate reviewers for those parts.)

Re: [PATCH v1 03/10] scsi: ufs: provide a quirk to disable the LCC

[Akinobu Mita]

2015-05-18 0:54 GMT+09:00 Yaniv Gardi <ygardi@codeaurora.org>:
> LCC (Line Control Command) are being used for communication between
> UFS host and UFS device.
> New commercial UFS devices don't have the issues with LCC processing
> but UFS host controller might still have the issue with LCC processing,
> hence, added a routine to disable TX LCC on the device.
>
> Signed-off-by: Yaniv Gardi <ygardi@codeaurora.org>
>
> ---
>  drivers/scsi/ufs/ufshcd.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/scsi/ufs/ufshcd.h |  8 ++++++++
>  drivers/scsi/ufs/ufshci.h |  3 +++
>  3 files changed, 53 insertions(+)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 9641bcb..3e57cca 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -2640,6 +2640,42 @@ static int ufshcd_hba_enable(struct ufs_hba *hba)
>         return 0;
>  }
>
> +static int ufshcd_disable_tx_lcc(struct ufs_hba *hba, bool peer)
> +{
> +       int tx_lanes, i, err = 0;
> +
> +       if (!peer)
> +               ufshcd_dme_get(hba, UIC_ARG_MIB(PA_CONNECTEDTXDATALANES),
> +                              &tx_lanes);
> +       else
> +               ufshcd_dme_peer_get(hba, UIC_ARG_MIB(PA_CONNECTEDTXDATALANES),
> +                                   &tx_lanes);
> +       for (i = 0; i < tx_lanes; i++) {
> +               if (!peer)
> +                       err = ufshcd_dme_set(hba,
> +                               UIC_ARG_MIB_SEL(TX_LCC_ENABLE,
> +                                       UIC_ARG_MPHY_TX_GEN_SEL_INDEX(i)),
> +                                       0);
> +               else
> +                       err = ufshcd_dme_peer_set(hba,
> +                               UIC_ARG_MIB_SEL(TX_LCC_ENABLE,
> +                                       UIC_ARG_MPHY_TX_GEN_SEL_INDEX(i)),
> +                                       0);

You can reduce duplication by:

        err = ufshcd_dme_set_attr(hba, UIC_ARG_MIB_SEL(TX_LCC_ENABLE,
                                        UIC_ARG_MPHY_TX_GEN_SEL_INDEX(i)),
                                ATTR_SET_NOR, 0, peer ? DME_PEER : DME_LOCAL);

> +               if (err) {
> +                       dev_err(hba->dev, "%s: TX LCC Disable failed, peer = %d, lane = %d, err = %d",
> +                               __func__, peer, i, err);

'\n' is missing in format string.

Re: [PATCH v1 07/10] scsi: ufs: introduce UFSHCD_QUIRK_DME_PEER_ACCESS_AUTO_MODE quirk

[Akinobu Mita]

2015-05-18 0:55 GMT+09:00 Yaniv Gardi <ygardi@codeaurora.org>:
> Some UFS host controllers may only allow accessing the peer DME attribute
> in AUTO mode (FAST AUTO or SLOW AUTO) hence we had added a quirk for
> switching to AUTO power mode before accessing the peer DME attribute.
>
> Signed-off-by: Yaniv Gardi <ygardi@codeaurora.org>
>
> ---
>  drivers/scsi/ufs/ufshcd.c | 31 +++++++++++++++++++++++++++++++
>  drivers/scsi/ufs/ufshcd.h |  7 +++++++
>  2 files changed, 38 insertions(+)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index a274df9..014f1c0 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -188,6 +188,8 @@ static int ufshcd_host_reset_and_restore(struct ufs_hba *hba);
>  static irqreturn_t ufshcd_intr(int irq, void *__hba);
>  static int ufshcd_config_pwr_mode(struct ufs_hba *hba,
>                 struct ufs_pa_layer_attr *desired_pwr_mode);
> +static int ufshcd_change_power_mode(struct ufs_hba *hba,
> +                            struct ufs_pa_layer_attr *pwr_mode);
>
>  static inline int ufshcd_enable_irq(struct ufs_hba *hba)
>  {
> @@ -2156,6 +2158,31 @@ int ufshcd_dme_get_attr(struct ufs_hba *hba, u32 attr_sel,
>         };
>         const char *get = action[!!peer];
>         int ret;
> +       struct ufs_pa_layer_attr orig_pwr_info;
> +       struct ufs_pa_layer_attr temp_pwr_info;
> +       bool pwr_mode_change = false;
> +
> +       if (peer && (hba->quirks & UFSHCD_QUIRK_DME_PEER_ACCESS_AUTO_MODE)) {
> +               orig_pwr_info = hba->pwr_info;
> +               temp_pwr_info = orig_pwr_info;
> +
> +               if (orig_pwr_info.pwr_tx == FAST_MODE ||
> +                   orig_pwr_info.pwr_rx == FAST_MODE) {
> +                       temp_pwr_info.pwr_tx = FASTAUTO_MODE;
> +                       temp_pwr_info.pwr_rx = FASTAUTO_MODE;
> +                       pwr_mode_change = true;
> +               } else if (orig_pwr_info.pwr_tx == SLOW_MODE ||
> +                   orig_pwr_info.pwr_rx == SLOW_MODE) {
> +                       temp_pwr_info.pwr_tx = SLOWAUTO_MODE;
> +                       temp_pwr_info.pwr_rx = SLOWAUTO_MODE;
> +                       pwr_mode_change = true;
> +               }

What happens if tx and rx have different power mode (although it is
strange setting) ?

For example, if the current power mode is pwr_tx=FAST_MODE, gear_tx=3
and pwr_rx=SLOW_MODE, gear_rx=4, the temporary power mode will be
pwr_tx=FASTAUTO_MODE, gear_tx=3 and pwr_rx=FASTAUTO_MODE, gear_rx=4.
But HS-G4 is invalid.

> +               if (pwr_mode_change) {
> +                       ret = ufshcd_change_power_mode(hba, &temp_pwr_info);
> +                       if (ret)
> +                               goto out;
> +               }
> +       }

Re: [PATCH v1 10/10] scsi: ufs-qcom: enable UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION

[Akinobu Mita]

2015-05-18 0:55 GMT+09:00 Yaniv Gardi <ygardi@codeaurora.org>:
> Newer revisions of QUALCOMM Technologies UFS host controller may not
> advertise the correct version information in UFS HCI VER register.
> To handle this, enable UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION to let
> UFS standard host controller driver call into vendor specific
> operation to get right UFS HCI VER register value.
>
> Signed-off-by: Yaniv Gardi <ygardi@codeaurora.org>
>
> ---
>  drivers/scsi/ufs/ufs-qcom.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
> index de9cfb0..4cdffa4 100644
> --- a/drivers/scsi/ufs/ufs-qcom.c
> +++ b/drivers/scsi/ufs/ufs-qcom.c
> @@ -694,6 +694,16 @@ out:
>         return ret;
>  }
>
> +static u32 ufs_qcom_get_ufs_hci_version(struct ufs_hba *hba)
> +{
> +       struct ufs_qcom_host *host = hba->priv;
> +
> +       if (host->hw_ver.major == 0x1)
> +               return UFSHCI_VERSION_11;
> +       else
> +               return UFSHCI_VERSION_20;

Btw, UFS driver currently lacks UFSHCI 2.0 support.  Shouldn't we also
need a change like the patch submitted by Chuanxiao Dong:

http://marc.info/?l=linux-scsi&m=140688020815020&w=2

Re: [PATCH v1 07/10] scsi: ufs: introduce UFSHCD_QUIRK_DME_PEER_ACCESS_AUTO_MODE quirk

[Akinobu Mita]

2015-05-18 0:55 GMT+09:00 Yaniv Gardi <ygardi@codeaurora.org>:
> Some UFS host controllers may only allow accessing the peer DME attribute
> in AUTO mode (FAST AUTO or SLOW AUTO) hence we had added a quirk for
> switching to AUTO power mode before accessing the peer DME attribute.
>
> Signed-off-by: Yaniv Gardi <ygardi@codeaurora.org>
>
> ---
>  drivers/scsi/ufs/ufshcd.c | 31 +++++++++++++++++++++++++++++++
>  drivers/scsi/ufs/ufshcd.h |  7 +++++++
>  2 files changed, 38 insertions(+)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index a274df9..014f1c0 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -188,6 +188,8 @@ static int ufshcd_host_reset_and_restore(struct ufs_hba *hba);
>  static irqreturn_t ufshcd_intr(int irq, void *__hba);
>  static int ufshcd_config_pwr_mode(struct ufs_hba *hba,
>                 struct ufs_pa_layer_attr *desired_pwr_mode);
> +static int ufshcd_change_power_mode(struct ufs_hba *hba,
> +                            struct ufs_pa_layer_attr *pwr_mode);
>
>  static inline int ufshcd_enable_irq(struct ufs_hba *hba)
>  {
> @@ -2156,6 +2158,31 @@ int ufshcd_dme_get_attr(struct ufs_hba *hba, u32 attr_sel,
>         };
>         const char *get = action[!!peer];
>         int ret;
> +       struct ufs_pa_layer_attr orig_pwr_info;
> +       struct ufs_pa_layer_attr temp_pwr_info;
> +       bool pwr_mode_change = false;
> +
> +       if (peer && (hba->quirks & UFSHCD_QUIRK_DME_PEER_ACCESS_AUTO_MODE)) {
> +               orig_pwr_info = hba->pwr_info;
> +               temp_pwr_info = orig_pwr_info;
> +
> +               if (orig_pwr_info.pwr_tx == FAST_MODE ||
> +                   orig_pwr_info.pwr_rx == FAST_MODE) {
> +                       temp_pwr_info.pwr_tx = FASTAUTO_MODE;
> +                       temp_pwr_info.pwr_rx = FASTAUTO_MODE;
> +                       pwr_mode_change = true;
> +               } else if (orig_pwr_info.pwr_tx == SLOW_MODE ||
> +                   orig_pwr_info.pwr_rx == SLOW_MODE) {
> +                       temp_pwr_info.pwr_tx = SLOWAUTO_MODE;
> +                       temp_pwr_info.pwr_rx = SLOWAUTO_MODE;
> +                       pwr_mode_change = true;
> +               }
> +               if (pwr_mode_change) {
> +                       ret = ufshcd_change_power_mode(hba, &temp_pwr_info);

I'm a bit worried that this could cause an infinite recursion if
someone adds ufshcd_dme_peer_get() in ufshcd_change_power_mode()
for debugging or investigation.

So isn't it better to add note in code comment, or detect infinite
recursion?