linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] Support sdio feature
@ 2016-11-08  6:08 Yong Mao
  2016-11-08  6:08 ` [PATCH 1/4] mmc: mediatek: Fix CMD6 timeout issue Yong Mao
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Yong Mao @ 2016-11-08  6:08 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
	Matthias Brugger, YH Huang, Mathias Nyman, Chunfeng Yun,
	Eddie Huang, Philipp Zabel, yong mao, Chaotian Jing,
	Nicolas Boichat, Douglas Anderson, Geert Uytterhoeven,
	devicetree, linux-arm-kernel, linux-kernel, linux-mmc,
	linux-mediatek, srv_heupstream

Fix CMD6 timeout issue
Add irqlock to protect accessing the shared register
Modify the implementation of msdc_card_busy
Add msdc_recheck_sdio_irq mechanism
Support sdr104_clk_delay in sdio
Add description of mmc3 for supporting sdio feature

yong mao (4):
  mmc: mediatek: Fix CMD6 timeout issue
  sdio: mediatek: Support sdio feature
  sdio: mediatek: support sdr104_clk_delay in sdio
  dts: arm64: enable mmc3 for supporting sdio feature

 arch/arm64/boot/dts/mediatek/mt8173-evb.dts |  82 +++++++++
 drivers/mmc/host/mtk-sd.c                   | 254 +++++++++++++++++++++-------
 2 files changed, 271 insertions(+), 65 deletions(-)

-- 
1.8.1.1.dirty

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/4] mmc: mediatek: Fix CMD6 timeout issue
  2016-11-08  6:08 [PATCH 0/4] Support sdio feature Yong Mao
@ 2016-11-08  6:08 ` Yong Mao
  2016-12-01  9:51   ` Ulf Hansson
  2016-11-08  6:08 ` [PATCH 2/4] sdio: mediatek: Support sdio feature Yong Mao
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Yong Mao @ 2016-11-08  6:08 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
	Matthias Brugger, YH Huang, Mathias Nyman, Chunfeng Yun,
	Eddie Huang, Philipp Zabel, yong mao, Chaotian Jing,
	Nicolas Boichat, Douglas Anderson, Geert Uytterhoeven,
	devicetree, linux-arm-kernel, linux-kernel, linux-mmc,
	linux-mediatek, srv_heupstream

From: yong mao <yong.mao@mediatek.com>

When initializing EMMC, after switch to HS400,
it will issue CMD6 to change ext_csd, if first CMD6 got CRC
error, the repeat CMD6 may get timeout, that's
because SDCBSY was cleared by msdc_reset_hw()

Signed-off-by: Yong Mao <yong.mao@mediatek.com>
Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
---
 drivers/mmc/host/mtk-sd.c |   77 ++++++++++++++++++++++++++++++---------------
 1 file changed, 51 insertions(+), 26 deletions(-)

diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
index 84e9afc..b29683b 100644
--- a/drivers/mmc/host/mtk-sd.c
+++ b/drivers/mmc/host/mtk-sd.c
@@ -826,6 +826,15 @@ static bool msdc_cmd_done(struct msdc_host *host, int events,
 	return true;
 }
 
+static int msdc_card_busy(struct mmc_host *mmc)
+{
+	struct msdc_host *host = mmc_priv(mmc);
+	u32 status = readl(host->base + MSDC_PS);
+
+	/* check if data0 is low */
+	return !(status & BIT(16));
+}
+
 /* It is the core layer's responsibility to ensure card status
  * is correct before issue a request. but host design do below
  * checks recommended.
@@ -835,10 +844,20 @@ static inline bool msdc_cmd_is_ready(struct msdc_host *host,
 {
 	/* The max busy time we can endure is 20ms */
 	unsigned long tmo = jiffies + msecs_to_jiffies(20);
+	u32 count = 0;
+
+	if (in_interrupt()) {
+		while ((readl(host->base + SDC_STS) & SDC_STS_CMDBUSY) &&
+		       (count < 1000)) {
+			udelay(1);
+			count++;
+		}
+	} else {
+		while ((readl(host->base + SDC_STS) & SDC_STS_CMDBUSY) &&
+		       time_before(jiffies, tmo))
+			cpu_relax();
+	}
 
-	while ((readl(host->base + SDC_STS) & SDC_STS_CMDBUSY) &&
-			time_before(jiffies, tmo))
-		cpu_relax();
 	if (readl(host->base + SDC_STS) & SDC_STS_CMDBUSY) {
 		dev_err(host->dev, "CMD bus busy detected\n");
 		host->error |= REQ_CMD_BUSY;
@@ -846,17 +865,35 @@ static inline bool msdc_cmd_is_ready(struct msdc_host *host,
 		return false;
 	}
 
-	if (mmc_resp_type(cmd) == MMC_RSP_R1B || cmd->data) {
-		tmo = jiffies + msecs_to_jiffies(20);
-		/* R1B or with data, should check SDCBUSY */
-		while ((readl(host->base + SDC_STS) & SDC_STS_SDCBUSY) &&
-				time_before(jiffies, tmo))
-			cpu_relax();
-		if (readl(host->base + SDC_STS) & SDC_STS_SDCBUSY) {
-			dev_err(host->dev, "Controller busy detected\n");
-			host->error |= REQ_CMD_BUSY;
-			msdc_cmd_done(host, MSDC_INT_CMDTMO, mrq, cmd);
-			return false;
+	if (cmd->opcode != MMC_SEND_STATUS) {
+		count = 0;
+		/* Consider that CMD6 crc error before card was init done,
+		 * mmc_retune() will return directly as host->card is null.
+		 * and CMD6 will retry 3 times, must ensure card is in transfer
+		 * state when retry.
+		 */
+		tmo = jiffies + msecs_to_jiffies(60 * 1000);
+		while (1) {
+			if (msdc_card_busy(host->mmc)) {
+				if (in_interrupt()) {
+					udelay(1);
+					count++;
+				} else {
+					msleep_interruptible(10);
+				}
+			} else {
+				break;
+			}
+			/* Timeout if the device never
+			 * leaves the program state.
+			 */
+			if (count > 1000 || time_after(jiffies, tmo)) {
+				pr_err("%s: Card stuck in programming state! %s\n",
+				       mmc_hostname(host->mmc), __func__);
+				host->error |= REQ_CMD_BUSY;
+				msdc_cmd_done(host, MSDC_INT_CMDTMO, mrq, cmd);
+				return false;
+			}
 		}
 	}
 	return true;
@@ -1070,18 +1107,6 @@ static int msdc_ops_switch_volt(struct mmc_host *mmc, struct mmc_ios *ios)
 	return ret;
 }
 
-static int msdc_card_busy(struct mmc_host *mmc)
-{
-	struct msdc_host *host = mmc_priv(mmc);
-	u32 status = readl(host->base + MSDC_PS);
-
-	/* check if any pin between dat[0:3] is low */
-	if (((status >> 16) & 0xf) != 0xf)
-		return 1;
-
-	return 0;
-}
-
 static void msdc_request_timeout(struct work_struct *work)
 {
 	struct msdc_host *host = container_of(work, struct msdc_host,
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/4] sdio: mediatek: Support sdio feature
  2016-11-08  6:08 [PATCH 0/4] Support sdio feature Yong Mao
  2016-11-08  6:08 ` [PATCH 1/4] mmc: mediatek: Fix CMD6 timeout issue Yong Mao
@ 2016-11-08  6:08 ` Yong Mao
  2017-01-23 11:24   ` Wei-Ning Huang
  2016-11-08  6:09 ` [PATCH 3/4] sdio: mediatek: support sdr104_clk_delay in sdio Yong Mao
  2016-11-08  6:09 ` [PATCH 4/4] dts: arm64: enable mmc3 for supporting sdio feature Yong Mao
  3 siblings, 1 reply; 10+ messages in thread
From: Yong Mao @ 2016-11-08  6:08 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
	Matthias Brugger, YH Huang, Mathias Nyman, Chunfeng Yun,
	Eddie Huang, Philipp Zabel, yong mao, Chaotian Jing,
	Nicolas Boichat, Douglas Anderson, Geert Uytterhoeven,
	devicetree, linux-arm-kernel, linux-kernel, linux-mmc,
	linux-mediatek, srv_heupstream

From: yong mao <yong.mao@mediatek.com>

1. Add irqlock to protect accessing the shared register
2. Modify the implementation of msdc_card_busy due to SDIO
3. Implement enable_sdio_irq
4. Add msdc_recheck_sdio_irq mechanism to make sure all
   interrupts can be processed immediately

Signed-off-by: Yong Mao <yong.mao@mediatek.com>
Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
---
 drivers/mmc/host/mtk-sd.c |  167 ++++++++++++++++++++++++++++++++++-----------
 1 file changed, 129 insertions(+), 38 deletions(-)

diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
index b29683b..37edf30 100644
--- a/drivers/mmc/host/mtk-sd.c
+++ b/drivers/mmc/host/mtk-sd.c
@@ -117,6 +117,7 @@
 #define MSDC_PS_CDSTS           (0x1 << 1)	/* R  */
 #define MSDC_PS_CDDEBOUNCE      (0xf << 12)	/* RW */
 #define MSDC_PS_DAT             (0xff << 16)	/* R  */
+#define MSDC_PS_DATA1           (0x1 << 17)	/* R  */
 #define MSDC_PS_CMD             (0x1 << 24)	/* R  */
 #define MSDC_PS_WP              (0x1 << 31)	/* R  */
 
@@ -304,6 +305,7 @@ struct msdc_host {
 	int cmd_rsp;
 
 	spinlock_t lock;
+	spinlock_t irqlock;	/* sdio irq lock */
 	struct mmc_request *mrq;
 	struct mmc_command *cmd;
 	struct mmc_data *data;
@@ -322,12 +324,14 @@ struct msdc_host {
 	struct pinctrl_state *pins_uhs;
 	struct delayed_work req_timeout;
 	int irq;		/* host interrupt */
+	bool irq_thread_alive;
 
 	struct clk *src_clk;	/* msdc source clock */
 	struct clk *h_clk;      /* msdc h_clk */
 	u32 mclk;		/* mmc subsystem clock frequency */
 	u32 src_clk_freq;	/* source clock frequency */
 	u32 sclk;		/* SD/MS bus clock frequency */
+	bool clock_on;
 	unsigned char timing;
 	bool vqmmc_enabled;
 	u32 hs400_ds_delay;
@@ -387,6 +391,7 @@ static void msdc_reset_hw(struct msdc_host *host)
 
 static void msdc_cmd_next(struct msdc_host *host,
 		struct mmc_request *mrq, struct mmc_command *cmd);
+static void msdc_recheck_sdio_irq(struct msdc_host *host);
 
 static const u32 cmd_ints_mask = MSDC_INTEN_CMDRDY | MSDC_INTEN_RSPCRCERR |
 			MSDC_INTEN_CMDTMO | MSDC_INTEN_ACMDRDY |
@@ -513,6 +518,7 @@ static void msdc_gate_clock(struct msdc_host *host)
 {
 	clk_disable_unprepare(host->src_clk);
 	clk_disable_unprepare(host->h_clk);
+	host->clock_on = false;
 }
 
 static void msdc_ungate_clock(struct msdc_host *host)
@@ -521,6 +527,7 @@ static void msdc_ungate_clock(struct msdc_host *host)
 	clk_prepare_enable(host->src_clk);
 	while (!(readl(host->base + MSDC_CFG) & MSDC_CFG_CKSTB))
 		cpu_relax();
+	host->clock_on = true;
 }
 
 static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz)
@@ -529,6 +536,7 @@ static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz)
 	u32 flags;
 	u32 div;
 	u32 sclk;
+	unsigned long irq_flags;
 
 	if (!hz) {
 		dev_dbg(host->dev, "set mclk to 0\n");
@@ -537,8 +545,11 @@ static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz)
 		return;
 	}
 
+	spin_lock_irqsave(&host->irqlock, irq_flags);
 	flags = readl(host->base + MSDC_INTEN);
 	sdr_clr_bits(host->base + MSDC_INTEN, flags);
+	spin_unlock_irqrestore(&host->irqlock, irq_flags);
+
 	sdr_clr_bits(host->base + MSDC_CFG, MSDC_CFG_HS400_CK_MODE);
 	if (timing == MMC_TIMING_UHS_DDR50 ||
 	    timing == MMC_TIMING_MMC_DDR52 ||
@@ -588,7 +599,10 @@ static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz)
 	host->timing = timing;
 	/* need because clk changed. */
 	msdc_set_timeout(host, host->timeout_ns, host->timeout_clks);
+
+	spin_lock_irqsave(&host->irqlock, irq_flags);
 	sdr_set_bits(host->base + MSDC_INTEN, flags);
+	spin_unlock_irqrestore(&host->irqlock, irq_flags);
 
 	/*
 	 * mmc_select_hs400() will drop to 50Mhz and High speed mode,
@@ -690,6 +704,7 @@ static inline u32 msdc_cmd_prepare_raw_cmd(struct msdc_host *host,
 static void msdc_start_data(struct msdc_host *host, struct mmc_request *mrq,
 			    struct mmc_command *cmd, struct mmc_data *data)
 {
+	unsigned long flags;
 	bool read;
 
 	WARN_ON(host->data);
@@ -698,8 +713,12 @@ static void msdc_start_data(struct msdc_host *host, struct mmc_request *mrq,
 
 	mod_delayed_work(system_wq, &host->req_timeout, DAT_TIMEOUT);
 	msdc_dma_setup(host, &host->dma, data);
+
+	spin_lock_irqsave(&host->irqlock, flags);
 	sdr_set_bits(host->base + MSDC_INTEN, data_ints_mask);
 	sdr_set_field(host->base + MSDC_DMA_CTRL, MSDC_DMA_CTRL_START, 1);
+	spin_unlock_irqrestore(&host->irqlock, flags);
+
 	dev_dbg(host->dev, "DMA start\n");
 	dev_dbg(host->dev, "%s: cmd=%d DMA data: %d blocks; read=%d\n",
 			__func__, cmd->opcode, data->blocks, read);
@@ -756,6 +775,7 @@ static void msdc_request_done(struct msdc_host *host, struct mmc_request *mrq)
 	if (mrq->data)
 		msdc_unprepare_data(host, mrq);
 	mmc_request_done(host->mmc, mrq);
+	msdc_recheck_sdio_irq(host);
 }
 
 /* returns true if command is fully handled; returns false otherwise */
@@ -779,15 +799,17 @@ static bool msdc_cmd_done(struct msdc_host *host, int events,
 					| MSDC_INT_CMDTMO)))
 		return done;
 
-	spin_lock_irqsave(&host->lock, flags);
 	done = !host->cmd;
+	spin_lock_irqsave(&host->lock, flags);
 	host->cmd = NULL;
 	spin_unlock_irqrestore(&host->lock, flags);
 
 	if (done)
 		return true;
 
+	spin_lock_irqsave(&host->irqlock, flags);
 	sdr_clr_bits(host->base + MSDC_INTEN, cmd_ints_mask);
+	spin_unlock_irqrestore(&host->irqlock, flags);
 
 	if (cmd->flags & MMC_RSP_PRESENT) {
 		if (cmd->flags & MMC_RSP_136) {
@@ -902,6 +924,7 @@ static inline bool msdc_cmd_is_ready(struct msdc_host *host,
 static void msdc_start_command(struct msdc_host *host,
 		struct mmc_request *mrq, struct mmc_command *cmd)
 {
+	unsigned long flags;
 	u32 rawcmd;
 
 	WARN_ON(host->cmd);
@@ -920,7 +943,10 @@ static void msdc_start_command(struct msdc_host *host,
 	rawcmd = msdc_cmd_prepare_raw_cmd(host, mrq, cmd);
 	mod_delayed_work(system_wq, &host->req_timeout, DAT_TIMEOUT);
 
+	spin_lock_irqsave(&host->irqlock, flags);
 	sdr_set_bits(host->base + MSDC_INTEN, cmd_ints_mask);
+	spin_unlock_irqrestore(&host->irqlock, flags);
+
 	writel(cmd->arg, host->base + SDC_ARG);
 	writel(rawcmd, host->base + SDC_CMD);
 }
@@ -1013,8 +1039,8 @@ static bool msdc_data_xfer_done(struct msdc_host *host, u32 events,
 	     | MSDC_INT_DMA_BDCSERR | MSDC_INT_DMA_GPDCSERR
 	     | MSDC_INT_DMA_PROTECT);
 
-	spin_lock_irqsave(&host->lock, flags);
 	done = !host->data;
+	spin_lock_irqsave(&host->lock, flags);
 	if (check_data)
 		host->data = NULL;
 	spin_unlock_irqrestore(&host->lock, flags);
@@ -1029,7 +1055,11 @@ static bool msdc_data_xfer_done(struct msdc_host *host, u32 events,
 				1);
 		while (readl(host->base + MSDC_DMA_CFG) & MSDC_DMA_CFG_STS)
 			cpu_relax();
+
+		spin_lock_irqsave(&host->irqlock, flags);
 		sdr_clr_bits(host->base + MSDC_INTEN, data_ints_mask);
+		spin_unlock_irqrestore(&host->irqlock, flags);
+
 		dev_dbg(host->dev, "DMA stop\n");
 
 		if ((events & MSDC_INT_XFER_COMPL) && (!stop || !stop->error)) {
@@ -1134,44 +1164,47 @@ static void msdc_request_timeout(struct work_struct *work)
 
 static irqreturn_t msdc_irq(int irq, void *dev_id)
 {
+	unsigned long flags;
 	struct msdc_host *host = (struct msdc_host *) dev_id;
+	struct mmc_request *mrq;
+	struct mmc_command *cmd;
+	struct mmc_data *data;
+	u32 events, event_mask;
+
+	spin_lock_irqsave(&host->irqlock, flags);
+	events = readl(host->base + MSDC_INT);
+	event_mask = readl(host->base + MSDC_INTEN);
+	/* clear interrupts */
+	writel(events & event_mask, host->base + MSDC_INT);
+
+	mrq = host->mrq;
+	cmd = host->cmd;
+	data = host->data;
+	spin_unlock_irqrestore(&host->irqlock, flags);
+
+	if ((events & event_mask) & MSDC_INT_SDIOIRQ) {
+		mmc_signal_sdio_irq(host->mmc);
+		if (!mrq)
+			return IRQ_HANDLED;
+	}
 
-	while (true) {
-		unsigned long flags;
-		struct mmc_request *mrq;
-		struct mmc_command *cmd;
-		struct mmc_data *data;
-		u32 events, event_mask;
-
-		spin_lock_irqsave(&host->lock, flags);
-		events = readl(host->base + MSDC_INT);
-		event_mask = readl(host->base + MSDC_INTEN);
-		/* clear interrupts */
-		writel(events & event_mask, host->base + MSDC_INT);
-
-		mrq = host->mrq;
-		cmd = host->cmd;
-		data = host->data;
-		spin_unlock_irqrestore(&host->lock, flags);
-
-		if (!(events & event_mask))
-			break;
+	if (!(events & (event_mask & ~MSDC_INT_SDIOIRQ)))
+		return IRQ_HANDLED;
 
-		if (!mrq) {
-			dev_err(host->dev,
-				"%s: MRQ=NULL; events=%08X; event_mask=%08X\n",
-				__func__, events, event_mask);
-			WARN_ON(1);
-			break;
-		}
+	if (!mrq) {
+		dev_err(host->dev,
+			"%s: MRQ=NULL; events=%08X; event_mask=%08X\n",
+			__func__, events, event_mask);
+		WARN_ON(1);
+		return IRQ_HANDLED;
+	}
 
-		dev_dbg(host->dev, "%s: events=%08X\n", __func__, events);
+	dev_dbg(host->dev, "%s: events=%08X\n", __func__, events);
 
-		if (cmd)
-			msdc_cmd_done(host, events, mrq, cmd);
-		else if (data)
-			msdc_data_xfer_done(host, events, mrq, data);
-	}
+	if (cmd)
+		msdc_cmd_done(host, events, mrq, cmd);
+	else if (data)
+		msdc_data_xfer_done(host, events, mrq, data);
 
 	return IRQ_HANDLED;
 }
@@ -1179,6 +1212,7 @@ static irqreturn_t msdc_irq(int irq, void *dev_id)
 static void msdc_init_hw(struct msdc_host *host)
 {
 	u32 val;
+	unsigned long flags;
 
 	/* Configure to MMC/SD mode, clock free running */
 	sdr_set_bits(host->base + MSDC_CFG, MSDC_CFG_MODE | MSDC_CFG_CKPDN);
@@ -1190,9 +1224,11 @@ static void msdc_init_hw(struct msdc_host *host)
 	sdr_clr_bits(host->base + MSDC_PS, MSDC_PS_CDEN);
 
 	/* Disable and clear all interrupts */
+	spin_lock_irqsave(&host->irqlock, flags);
 	writel(0, host->base + MSDC_INTEN);
 	val = readl(host->base + MSDC_INT);
 	writel(val, host->base + MSDC_INT);
+	spin_unlock_irqrestore(&host->irqlock, flags);
 
 	writel(0, host->base + MSDC_PAD_TUNE);
 	writel(0, host->base + MSDC_IOCON);
@@ -1207,9 +1243,11 @@ static void msdc_init_hw(struct msdc_host *host)
 	 */
 	sdr_set_bits(host->base + SDC_CFG, SDC_CFG_SDIO);
 
-	/* disable detect SDIO device interrupt function */
-	sdr_clr_bits(host->base + SDC_CFG, SDC_CFG_SDIOIDE);
-
+	if (host->mmc->caps & MMC_CAP_SDIO_IRQ)
+		sdr_set_bits(host->base + SDC_CFG, SDC_CFG_SDIOIDE);
+	else
+		/* disable detect SDIO device interrupt function */
+		sdr_clr_bits(host->base + SDC_CFG, SDC_CFG_SDIOIDE);
 	/* Configure to default data timeout */
 	sdr_set_field(host->base + SDC_CFG, SDC_CFG_DTOC, 3);
 
@@ -1221,11 +1259,15 @@ static void msdc_init_hw(struct msdc_host *host)
 static void msdc_deinit_hw(struct msdc_host *host)
 {
 	u32 val;
+	unsigned long flags;
+
 	/* Disable and clear all interrupts */
+	spin_lock_irqsave(&host->irqlock, flags);
 	writel(0, host->base + MSDC_INTEN);
 
 	val = readl(host->base + MSDC_INT);
 	writel(val, host->base + MSDC_INT);
+	spin_unlock_irqrestore(&host->irqlock, flags);
 }
 
 /* init gpd and bd list in msdc_drv_probe */
@@ -1493,6 +1535,52 @@ static void msdc_hw_reset(struct mmc_host *mmc)
 	sdr_clr_bits(host->base + EMMC_IOCON, 1);
 }
 
+/**
+ * msdc_recheck_sdio_irq - recheck whether the SDIO IRQ is lost
+ * @host: The host to check.
+ *
+ * Host controller may lost interrupt in some special case.
+ * Add sdio IRQ recheck mechanism to make sure all interrupts
+ * can be processed immediately
+ *
+ */
+static void msdc_recheck_sdio_irq(struct msdc_host *host)
+{
+	u32 reg_int, reg_ps;
+
+	if (host->clock_on &&
+	    (host->mmc->caps & MMC_CAP_SDIO_IRQ) &&
+	    host->irq_thread_alive) {
+		reg_int = readl(host->base + MSDC_INT);
+		reg_ps  = readl(host->base + MSDC_PS);
+		if (!((reg_int & MSDC_INT_SDIOIRQ) ||
+		      (reg_ps & MSDC_PS_DATA1))) {
+			mmc_signal_sdio_irq(host->mmc);
+		}
+	}
+}
+
+static void msdc_enable_sdio_irq(struct mmc_host *mmc, int enable)
+{
+	unsigned long flags;
+	struct msdc_host *host = mmc_priv(mmc);
+
+	host->irq_thread_alive = true;
+	if (enable) {
+		pm_runtime_get_sync(host->dev);
+		msdc_recheck_sdio_irq(host);
+
+		spin_lock_irqsave(&host->irqlock, flags);
+		sdr_set_bits(host->base + SDC_CFG, SDC_CFG_SDIOIDE);
+		sdr_set_bits(host->base + MSDC_INTEN, MSDC_INTEN_SDIOIRQ);
+		spin_unlock_irqrestore(&host->irqlock, flags);
+	} else {
+		spin_lock_irqsave(&host->irqlock, flags);
+		sdr_clr_bits(host->base + MSDC_INTEN, MSDC_INTEN_SDIOIRQ);
+		spin_unlock_irqrestore(&host->irqlock, flags);
+	}
+}
+
 static struct mmc_host_ops mt_msdc_ops = {
 	.post_req = msdc_post_req,
 	.pre_req = msdc_pre_req,
@@ -1504,6 +1592,7 @@ static void msdc_hw_reset(struct mmc_host *mmc)
 	.execute_tuning = msdc_execute_tuning,
 	.prepare_hs400_tuning = msdc_prepare_hs400_tuning,
 	.hw_reset = msdc_hw_reset,
+	.enable_sdio_irq = msdc_enable_sdio_irq,
 };
 
 static int msdc_drv_probe(struct platform_device *pdev)
@@ -1600,6 +1689,7 @@ static int msdc_drv_probe(struct platform_device *pdev)
 	mmc_dev(mmc)->dma_mask = &host->dma_mask;
 
 	host->timeout_clks = 3 * 1048576;
+	host->irq_thread_alive = false;
 	host->dma.gpd = dma_alloc_coherent(&pdev->dev,
 				2 * sizeof(struct mt_gpdma_desc),
 				&host->dma.gpd_addr, GFP_KERNEL);
@@ -1613,6 +1703,7 @@ static int msdc_drv_probe(struct platform_device *pdev)
 	msdc_init_gpd_bd(host, &host->dma);
 	INIT_DELAYED_WORK(&host->req_timeout, msdc_request_timeout);
 	spin_lock_init(&host->lock);
+	spin_lock_init(&host->irqlock);
 
 	platform_set_drvdata(pdev, mmc);
 	msdc_ungate_clock(host);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/4] sdio: mediatek: support sdr104_clk_delay in sdio
  2016-11-08  6:08 [PATCH 0/4] Support sdio feature Yong Mao
  2016-11-08  6:08 ` [PATCH 1/4] mmc: mediatek: Fix CMD6 timeout issue Yong Mao
  2016-11-08  6:08 ` [PATCH 2/4] sdio: mediatek: Support sdio feature Yong Mao
@ 2016-11-08  6:09 ` Yong Mao
  2016-11-08  6:09 ` [PATCH 4/4] dts: arm64: enable mmc3 for supporting sdio feature Yong Mao
  3 siblings, 0 replies; 10+ messages in thread
From: Yong Mao @ 2016-11-08  6:09 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
	Matthias Brugger, YH Huang, Mathias Nyman, Chunfeng Yun,
	Eddie Huang, Philipp Zabel, yong mao, Chaotian Jing,
	Nicolas Boichat, Douglas Anderson, Geert Uytterhoeven,
	devicetree, linux-arm-kernel, linux-kernel, linux-mmc,
	linux-mediatek, srv_heupstream

From: yong mao <yong.mao@mediatek.com>

In order to let sdio run stable with 200M clock,
we should setup the value of clock delay.

Signed-off-by: Yong Mao <yong.mao@mediatek.com>
Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
---
 drivers/mmc/host/mtk-sd.c |   10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
index 37edf30..fba28f2 100644
--- a/drivers/mmc/host/mtk-sd.c
+++ b/drivers/mmc/host/mtk-sd.c
@@ -213,6 +213,7 @@
 
 #define MSDC_PAD_TUNE_DATRRDLY	  (0x1f <<  8)	/* RW */
 #define MSDC_PAD_TUNE_CMDRDLY	  (0x1f << 16)  /* RW */
+#define MSDC_PAD_TUNE_CLKTDLY     (0x1f << 27)  /* RW */
 
 #define PAD_DS_TUNE_DLY1	  (0x1f << 2)   /* RW */
 #define PAD_DS_TUNE_DLY2	  (0x1f << 7)   /* RW */
@@ -335,6 +336,7 @@ struct msdc_host {
 	unsigned char timing;
 	bool vqmmc_enabled;
 	u32 hs400_ds_delay;
+	u32 sdr104_clk_delay;
 	bool hs400_mode;	/* current eMMC will run at hs400 mode */
 	struct msdc_save_para save_para; /* used when gate HCLK */
 	struct msdc_tune_para def_tune_para; /* default tune setting */
@@ -1230,7 +1232,8 @@ static void msdc_init_hw(struct msdc_host *host)
 	writel(val, host->base + MSDC_INT);
 	spin_unlock_irqrestore(&host->irqlock, flags);
 
-	writel(0, host->base + MSDC_PAD_TUNE);
+	sdr_set_field(host->base + MSDC_PAD_TUNE,
+		      MSDC_PAD_TUNE_CLKTDLY, host->sdr104_clk_delay);
 	writel(0, host->base + MSDC_IOCON);
 	sdr_set_field(host->base + MSDC_IOCON, MSDC_IOCON_DDLSEL, 0);
 	writel(0x403c0046, host->base + MSDC_PATCH_BIT);
@@ -1671,6 +1674,11 @@ static int msdc_drv_probe(struct platform_device *pdev)
 		dev_dbg(&pdev->dev, "hs400-ds-delay: %x\n",
 			host->hs400_ds_delay);
 
+	if (!of_property_read_u32(pdev->dev.of_node, "sdr104-clk-delay",
+				  &host->sdr104_clk_delay))
+		dev_dbg(&pdev->dev, "sdr104-clk-delay: %x\n",
+			host->sdr104_clk_delay);
+
 	host->dev = &pdev->dev;
 	host->mmc = mmc;
 	host->src_clk_freq = clk_get_rate(host->src_clk);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/4] dts: arm64: enable mmc3 for supporting sdio feature
  2016-11-08  6:08 [PATCH 0/4] Support sdio feature Yong Mao
                   ` (2 preceding siblings ...)
  2016-11-08  6:09 ` [PATCH 3/4] sdio: mediatek: support sdr104_clk_delay in sdio Yong Mao
@ 2016-11-08  6:09 ` Yong Mao
  2016-11-10  2:08   ` Yingjoe Chen
  3 siblings, 1 reply; 10+ messages in thread
From: Yong Mao @ 2016-11-08  6:09 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
	Matthias Brugger, YH Huang, Mathias Nyman, Chunfeng Yun,
	Eddie Huang, Philipp Zabel, yong mao, Chaotian Jing,
	Nicolas Boichat, Douglas Anderson, Geert Uytterhoeven,
	devicetree, linux-arm-kernel, linux-kernel, linux-mmc,
	linux-mediatek, srv_heupstream

From: yong mao <yong.mao@mediatek.com>

Add description of mmc3 for supporting sdio feature

Signed-off-by: Yong Mao <yong.mao@mediatek.com>
Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8173-evb.dts |   82 +++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173-evb.dts b/arch/arm64/boot/dts/mediatek/mt8173-evb.dts
index 2a7f731..4dbd299 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173-evb.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8173-evb.dts
@@ -43,6 +43,14 @@
 		enable-active-high;
 	};
 
+	sdio_fixed_3v3: fixedregulator@0 {
+		compatible = "regulator-fixed";
+		regulator-name = "3V3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		gpio = <&pio 85 GPIO_ACTIVE_HIGH>;
+	};
+
 	connector {
 		compatible = "hdmi-connector";
 		label = "hdmi";
@@ -139,6 +147,25 @@
 	vqmmc-supply = <&mt6397_vmc_reg>;
 };
 
+&mmc3 {
+	status = "okay";
+	pinctrl-names = "default", "state_uhs";
+	pinctrl-0 = <&mmc3_pins_default>;
+	pinctrl-1 = <&mmc3_pins_uhs>;
+	bus-width = <4>;
+	max-frequency = <200000000>;
+	cap-sd-highspeed;
+	sd-uhs-sdr50;
+	sd-uhs-sdr104;
+	sdr104-clk-delay = <5>;
+	keep-power-in-suspend;
+	enable-sdio-wakeup;
+	cap-sdio-irq;
+	vmmc-supply = <&sdio_fixed_3v3>;
+	vqmmc-supply = <&mt6397_vgp3_reg>;
+	non-removable;
+};
+
 &pio {
 	disp_pwm0_pins: disp_pwm0_pins {
 		pins1 {
@@ -197,6 +224,36 @@
 		};
 	};
 
+	mmc3_pins_default: mmc3default {
+		pins_dat {
+			pinmux = <MT8173_PIN_22_MSDC3_DAT0__FUNC_MSDC3_DAT0>,
+				 <MT8173_PIN_23_MSDC3_DAT1__FUNC_MSDC3_DAT1>,
+				 <MT8173_PIN_24_MSDC3_DAT2__FUNC_MSDC3_DAT2>,
+				 <MT8173_PIN_25_MSDC3_DAT3__FUNC_MSDC3_DAT3>;
+			input-enable;
+			drive-strength = <MTK_DRIVE_8mA>;
+			bias-pull-up = <MTK_PUPD_SET_R1R0_10>;
+		};
+
+		pins_cmd {
+			pinmux = <MT8173_PIN_27_MSDC3_CMD__FUNC_MSDC3_CMD>;
+			input-enable;
+			drive-strength = <MTK_DRIVE_8mA>;
+			bias-pull-up = <MTK_PUPD_SET_R1R0_10>;
+		};
+
+		pins_clk {
+			pinmux = <MT8173_PIN_26_MSDC3_CLK__FUNC_MSDC3_CLK>;
+			bias-pull-down;
+			drive-strength = <MTK_DRIVE_8mA>;
+		};
+
+		pins_pdn {
+			pinmux = <MT8173_PIN_85_AUD_DAT_MOSI__FUNC_GPIO85>;
+			output-low;
+		};
+	};
+
 	mmc0_pins_uhs: mmc0 {
 		pins_cmd_dat {
 			pinmux = <MT8173_PIN_57_MSDC0_DAT0__FUNC_MSDC0_DAT0>,
@@ -243,6 +300,31 @@
 			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
 		};
 	};
+
+	mmc3_pins_uhs: mmc3 {
+		pins_dat {
+			pinmux = <MT8173_PIN_22_MSDC3_DAT0__FUNC_MSDC3_DAT0>,
+				 <MT8173_PIN_23_MSDC3_DAT1__FUNC_MSDC3_DAT1>,
+				 <MT8173_PIN_24_MSDC3_DAT2__FUNC_MSDC3_DAT2>,
+				 <MT8173_PIN_25_MSDC3_DAT3__FUNC_MSDC3_DAT3>;
+			input-enable;
+			drive-strength = <MTK_DRIVE_8mA>;
+			bias-pull-up = <MTK_PUPD_SET_R1R0_10>;
+		};
+
+		pins_cmd {
+			pinmux = <MT8173_PIN_27_MSDC3_CMD__FUNC_MSDC3_CMD>;
+			input-enable;
+			drive-strength = <MTK_DRIVE_8mA>;
+			bias-pull-up = <MTK_PUPD_SET_R1R0_10>;
+		};
+
+		pins_clk {
+			pinmux = <MT8173_PIN_26_MSDC3_CLK__FUNC_MSDC3_CLK>;
+			drive-strength = <MTK_DRIVE_8mA>;
+			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
+		};
+	};
 };
 
 &pwm0 {
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/4] dts: arm64: enable mmc3 for supporting sdio feature
  2016-11-08  6:09 ` [PATCH 4/4] dts: arm64: enable mmc3 for supporting sdio feature Yong Mao
@ 2016-11-10  2:08   ` Yingjoe Chen
  0 siblings, 0 replies; 10+ messages in thread
From: Yingjoe Chen @ 2016-11-10  2:08 UTC (permalink / raw)
  To: Yong Mao
  Cc: Ulf Hansson, Mark Rutland, devicetree, YH Huang, Nicolas Boichat,
	Mathias Nyman, srv_heupstream, Catalin Marinas, linux-mediatek,
	Will Deacon, Douglas Anderson, linux-kernel, Chunfeng Yun,
	Rob Herring, Geert Uytterhoeven, linux-arm-kernel, Philipp Zabel,
	Matthias Brugger, linux-mmc, Eddie Huang, Chaotian Jing

On Tue, 2016-11-08 at 14:09 +0800, Yong Mao wrote:
> From: yong mao <yong.mao@mediatek.com>
> 
> Add description of mmc3 for supporting sdio feature
> 
> Signed-off-by: Yong Mao <yong.mao@mediatek.com>
> Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt8173-evb.dts |   82 +++++++++++++++++++++++++++
>  1 file changed, 82 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8173-evb.dts b/arch/arm64/boot/dts/mediatek/mt8173-evb.dts
> index 2a7f731..4dbd299 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8173-evb.dts
> +++ b/arch/arm64/boot/dts/mediatek/mt8173-evb.dts
> @@ -43,6 +43,14 @@
>  		enable-active-high;
>  	};
>  
> +	sdio_fixed_3v3: fixedregulator@0 {

This should be regulator@1 instead of fixedregulator.

> +		compatible = "regulator-fixed";
> +		regulator-name = "3V3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		gpio = <&pio 85 GPIO_ACTIVE_HIGH>;
> +	};
> +
>  	connector {
>  		compatible = "hdmi-connector";
>  		label = "hdmi";
> @@ -139,6 +147,25 @@
>  	vqmmc-supply = <&mt6397_vmc_reg>;
>  };
>  
> +&mmc3 {
> +	status = "okay";
> +	pinctrl-names = "default", "state_uhs";
> +	pinctrl-0 = <&mmc3_pins_default>;
> +	pinctrl-1 = <&mmc3_pins_uhs>;
> +	bus-width = <4>;
> +	max-frequency = <200000000>;
> +	cap-sd-highspeed;
> +	sd-uhs-sdr50;
> +	sd-uhs-sdr104;
> +	sdr104-clk-delay = <5>;
> +	keep-power-in-suspend;
> +	enable-sdio-wakeup;
> +	cap-sdio-irq;
> +	vmmc-supply = <&sdio_fixed_3v3>;
> +	vqmmc-supply = <&mt6397_vgp3_reg>;
> +	non-removable;
> +};
> +
>  &pio {
>  	disp_pwm0_pins: disp_pwm0_pins {
>  		pins1 {
> @@ -197,6 +224,36 @@
>  		};
>  	};
>  
> +	mmc3_pins_default: mmc3default {

Please keep nodes in &pio sorted, move this one after mmc1_pins_uhs.

> +		pins_dat {
> +			pinmux = <MT8173_PIN_22_MSDC3_DAT0__FUNC_MSDC3_DAT0>,
> +				 <MT8173_PIN_23_MSDC3_DAT1__FUNC_MSDC3_DAT1>,
> +				 <MT8173_PIN_24_MSDC3_DAT2__FUNC_MSDC3_DAT2>,
> +				 <MT8173_PIN_25_MSDC3_DAT3__FUNC_MSDC3_DAT3>;
> +			input-enable;
> +			drive-strength = <MTK_DRIVE_8mA>;
> +			bias-pull-up = <MTK_PUPD_SET_R1R0_10>;
> +		};
> +
> +		pins_cmd {
> +			pinmux = <MT8173_PIN_27_MSDC3_CMD__FUNC_MSDC3_CMD>;
> +			input-enable;
> +			drive-strength = <MTK_DRIVE_8mA>;
> +			bias-pull-up = <MTK_PUPD_SET_R1R0_10>;
> +		};
> +
> +		pins_clk {
> +			pinmux = <MT8173_PIN_26_MSDC3_CLK__FUNC_MSDC3_CLK>;
> +			bias-pull-down;
> +			drive-strength = <MTK_DRIVE_8mA>;
> +		};
> +
> +		pins_pdn {
> +			pinmux = <MT8173_PIN_85_AUD_DAT_MOSI__FUNC_GPIO85>;
> +			output-low;
> +		};

This one is used in regulator, not really an mmc pin.
Also, you don't need to add node for gpio usage, request_gpio will set
mode for you.
So please remove pins_pdn node.


> +	};
> +
>  	mmc0_pins_uhs: mmc0 {
>  		pins_cmd_dat {
>  			pinmux = <MT8173_PIN_57_MSDC0_DAT0__FUNC_MSDC0_DAT0>,
> @@ -243,6 +300,31 @@
>  			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
>  		};
>  	};
> +
> +	mmc3_pins_uhs: mmc3 {


Please keep nodes in &pio sorted, move this one after mmc1_pins_uhs.


Joe.C

> +		pins_dat {
> +			pinmux = <MT8173_PIN_22_MSDC3_DAT0__FUNC_MSDC3_DAT0>,
> +				 <MT8173_PIN_23_MSDC3_DAT1__FUNC_MSDC3_DAT1>,
> +				 <MT8173_PIN_24_MSDC3_DAT2__FUNC_MSDC3_DAT2>,
> +				 <MT8173_PIN_25_MSDC3_DAT3__FUNC_MSDC3_DAT3>;
> +			input-enable;
> +			drive-strength = <MTK_DRIVE_8mA>;
> +			bias-pull-up = <MTK_PUPD_SET_R1R0_10>;
> +		};
> +
> +		pins_cmd {
> +			pinmux = <MT8173_PIN_27_MSDC3_CMD__FUNC_MSDC3_CMD>;
> +			input-enable;
> +			drive-strength = <MTK_DRIVE_8mA>;
> +			bias-pull-up = <MTK_PUPD_SET_R1R0_10>;
> +		};
> +
> +		pins_clk {
> +			pinmux = <MT8173_PIN_26_MSDC3_CLK__FUNC_MSDC3_CLK>;
> +			drive-strength = <MTK_DRIVE_8mA>;
> +			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
> +		};
> +	};
>  };
>  
>  &pwm0 {

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/4] mmc: mediatek: Fix CMD6 timeout issue
  2016-11-08  6:08 ` [PATCH 1/4] mmc: mediatek: Fix CMD6 timeout issue Yong Mao
@ 2016-12-01  9:51   ` Ulf Hansson
  2017-01-03  9:14     ` Yong Mao
  0 siblings, 1 reply; 10+ messages in thread
From: Ulf Hansson @ 2016-12-01  9:51 UTC (permalink / raw)
  To: Yong Mao
  Cc: Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
	Matthias Brugger, YH Huang, Mathias Nyman, Chunfeng Yun,
	Eddie Huang, Philipp Zabel, Chaotian Jing, Nicolas Boichat,
	Douglas Anderson, Geert Uytterhoeven, devicetree,
	linux-arm-kernel, linux-kernel, linux-mmc, linux-mediatek,
	srv_heupstream

On 8 November 2016 at 07:08, Yong Mao <yong.mao@mediatek.com> wrote:
> From: yong mao <yong.mao@mediatek.com>
>
> When initializing EMMC, after switch to HS400,
> it will issue CMD6 to change ext_csd, if first CMD6 got CRC
> error, the repeat CMD6 may get timeout, that's
> because SDCBSY was cleared by msdc_reset_hw()

Sorry for the delay!

We have recently been re-working the sequence for how to deal more
properly with CMD6 in the mmc core.

The changes done so far should mostly concern switches to HS and HS
DDR, but I think you should run a re-test to make sure you still hit
the same problems.

>
> Signed-off-by: Yong Mao <yong.mao@mediatek.com>
> Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
> ---
>  drivers/mmc/host/mtk-sd.c |   77 ++++++++++++++++++++++++++++++---------------
>  1 file changed, 51 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
> index 84e9afc..b29683b 100644
> --- a/drivers/mmc/host/mtk-sd.c
> +++ b/drivers/mmc/host/mtk-sd.c
> @@ -826,6 +826,15 @@ static bool msdc_cmd_done(struct msdc_host *host, int events,
>         return true;
>  }
>
> +static int msdc_card_busy(struct mmc_host *mmc)
> +{
> +       struct msdc_host *host = mmc_priv(mmc);
> +       u32 status = readl(host->base + MSDC_PS);
> +
> +       /* check if data0 is low */
> +       return !(status & BIT(16));
> +}
> +
>  /* It is the core layer's responsibility to ensure card status
>   * is correct before issue a request. but host design do below
>   * checks recommended.

Hmm. Why?

I think you should rely on the mmc core to invoke the ->card_busy()
ops instead. The core knows better when it's needed.

Perhaps that may also resolve some of these issues for you!?

> @@ -835,10 +844,20 @@ static inline bool msdc_cmd_is_ready(struct msdc_host *host,
>  {
>         /* The max busy time we can endure is 20ms */
>         unsigned long tmo = jiffies + msecs_to_jiffies(20);
> +       u32 count = 0;
> +
> +       if (in_interrupt()) {
> +               while ((readl(host->base + SDC_STS) & SDC_STS_CMDBUSY) &&
> +                      (count < 1000)) {
> +                       udelay(1);
> +                       count++;

This seems like a bad idea, "busy-wait" in irq context is never a good idea.

> +               }
> +       } else {
> +               while ((readl(host->base + SDC_STS) & SDC_STS_CMDBUSY) &&
> +                      time_before(jiffies, tmo))
> +                       cpu_relax();
> +       }
>
> -       while ((readl(host->base + SDC_STS) & SDC_STS_CMDBUSY) &&
> -                       time_before(jiffies, tmo))
> -               cpu_relax();
>         if (readl(host->base + SDC_STS) & SDC_STS_CMDBUSY) {
>                 dev_err(host->dev, "CMD bus busy detected\n");
>                 host->error |= REQ_CMD_BUSY;
> @@ -846,17 +865,35 @@ static inline bool msdc_cmd_is_ready(struct msdc_host *host,
>                 return false;
>         }
>
> -       if (mmc_resp_type(cmd) == MMC_RSP_R1B || cmd->data) {
> -               tmo = jiffies + msecs_to_jiffies(20);
> -               /* R1B or with data, should check SDCBUSY */
> -               while ((readl(host->base + SDC_STS) & SDC_STS_SDCBUSY) &&
> -                               time_before(jiffies, tmo))
> -                       cpu_relax();
> -               if (readl(host->base + SDC_STS) & SDC_STS_SDCBUSY) {
> -                       dev_err(host->dev, "Controller busy detected\n");
> -                       host->error |= REQ_CMD_BUSY;
> -                       msdc_cmd_done(host, MSDC_INT_CMDTMO, mrq, cmd);
> -                       return false;
> +       if (cmd->opcode != MMC_SEND_STATUS) {
> +               count = 0;
> +               /* Consider that CMD6 crc error before card was init done,
> +                * mmc_retune() will return directly as host->card is null.
> +                * and CMD6 will retry 3 times, must ensure card is in transfer
> +                * state when retry.
> +                */
> +               tmo = jiffies + msecs_to_jiffies(60 * 1000);
> +               while (1) {
> +                       if (msdc_card_busy(host->mmc)) {
> +                               if (in_interrupt()) {
> +                                       udelay(1);
> +                                       count++;
> +                               } else {
> +                                       msleep_interruptible(10);
> +                               }
> +                       } else {
> +                               break;
> +                       }
> +                       /* Timeout if the device never
> +                        * leaves the program state.
> +                        */
> +                       if (count > 1000 || time_after(jiffies, tmo)) {
> +                               pr_err("%s: Card stuck in programming state! %s\n",
> +                                      mmc_hostname(host->mmc), __func__);
> +                               host->error |= REQ_CMD_BUSY;
> +                               msdc_cmd_done(host, MSDC_INT_CMDTMO, mrq, cmd);
> +                               return false;
> +                       }

This hole new code is a hack, that shouldn't be needed in the host driver.

I think we need to investigate and fix the issue in the mmc core
layer, to make this work for your host driver. That instead of doing a
work around in the host.

>                 }
>         }
>         return true;
> @@ -1070,18 +1107,6 @@ static int msdc_ops_switch_volt(struct mmc_host *mmc, struct mmc_ios *ios)
>         return ret;
>  }
>
> -static int msdc_card_busy(struct mmc_host *mmc)
> -{
> -       struct msdc_host *host = mmc_priv(mmc);
> -       u32 status = readl(host->base + MSDC_PS);
> -
> -       /* check if any pin between dat[0:3] is low */
> -       if (((status >> 16) & 0xf) != 0xf)
> -               return 1;
> -
> -       return 0;
> -}
> -
>  static void msdc_request_timeout(struct work_struct *work)
>  {
>         struct msdc_host *host = container_of(work, struct msdc_host,
> --
> 1.7.9.5
>

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/4] mmc: mediatek: Fix CMD6 timeout issue
  2016-12-01  9:51   ` Ulf Hansson
@ 2017-01-03  9:14     ` Yong Mao
  0 siblings, 0 replies; 10+ messages in thread
From: Yong Mao @ 2017-01-03  9:14 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
	Matthias Brugger, YH Huang, Mathias Nyman, Chunfeng Yun,
	Eddie Huang, Philipp Zabel, Chaotian Jing, Nicolas Boichat,
	Douglas Anderson, Geert Uytterhoeven, devicetree,
	linux-arm-kernel, linux-kernel, linux-mmc, linux-mediatek,
	srv_heupstream

On Thu, 2016-12-01 at 10:51 +0100, Ulf Hansson wrote:
> On 8 November 2016 at 07:08, Yong Mao <yong.mao@mediatek.com> wrote:
> > From: yong mao <yong.mao@mediatek.com>
> >
> > When initializing EMMC, after switch to HS400,
> > it will issue CMD6 to change ext_csd, if first CMD6 got CRC
> > error, the repeat CMD6 may get timeout, that's
> > because SDCBSY was cleared by msdc_reset_hw()
> 
> Sorry for the delay!
> 
> We have recently been re-working the sequence for how to deal more
> properly with CMD6 in the mmc core.
> 
> The changes done so far should mostly concern switches to HS and HS
> DDR, but I think you should run a re-test to make sure you still hit
> the same problems.
> 
Happy New Year!

The issue we met is not only just for CMD6, but also for other R1B CMD.
Your new changes does not cover all of these cases.
We would like to make error handle in the core layer to deal with these
issues.

I had submitted a new path ([PATCH v2] Fix CMD6 timeout issue) to you,
please help to review it.
Thanks.


> >
> > Signed-off-by: Yong Mao <yong.mao@mediatek.com>
> > Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
> > ---
> >  drivers/mmc/host/mtk-sd.c |   77 ++++++++++++++++++++++++++++++---------------
> >  1 file changed, 51 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
> > index 84e9afc..b29683b 100644
> > --- a/drivers/mmc/host/mtk-sd.c
> > +++ b/drivers/mmc/host/mtk-sd.c
> > @@ -826,6 +826,15 @@ static bool msdc_cmd_done(struct msdc_host *host, int events,
> >         return true;
> >  }
> >
> > +static int msdc_card_busy(struct mmc_host *mmc)
> > +{
> > +       struct msdc_host *host = mmc_priv(mmc);
> > +       u32 status = readl(host->base + MSDC_PS);
> > +
> > +       /* check if data0 is low */
> > +       return !(status & BIT(16));
> > +}
> > +
> >  /* It is the core layer's responsibility to ensure card status
> >   * is correct before issue a request. but host design do below
> >   * checks recommended.
> 
> Hmm. Why?
> 
> I think you should rely on the mmc core to invoke the ->card_busy()
> ops instead. The core knows better when it's needed.
> 
> Perhaps that may also resolve some of these issues for you!?
In my latest patch, msdc_card_busy will not be used in mtk-sd.c
directly. It only can be invoked by ->card_busy() in the mmc core.


> 
> > @@ -835,10 +844,20 @@ static inline bool msdc_cmd_is_ready(struct msdc_host *host,
> >  {
> >         /* The max busy time we can endure is 20ms */
> >         unsigned long tmo = jiffies + msecs_to_jiffies(20);
> > +       u32 count = 0;
> > +
> > +       if (in_interrupt()) {
> > +               while ((readl(host->base + SDC_STS) & SDC_STS_CMDBUSY) &&
> > +                      (count < 1000)) {
> > +                       udelay(1);
> > +                       count++;
> 
> This seems like a bad idea, "busy-wait" in irq context is never a good idea.
Thanks. The modification here is not for the current issue.
I will submit a new patch to discuss with you.


> > +               }
> > +       } else {
> > +               while ((readl(host->base + SDC_STS) & SDC_STS_CMDBUSY) &&
> > +                      time_before(jiffies, tmo))
> > +                       cpu_relax();
> > +       }
> >
> > -       while ((readl(host->base + SDC_STS) & SDC_STS_CMDBUSY) &&
> > -                       time_before(jiffies, tmo))
> > -               cpu_relax();
> >         if (readl(host->base + SDC_STS) & SDC_STS_CMDBUSY) {
> >                 dev_err(host->dev, "CMD bus busy detected\n");
> >                 host->error |= REQ_CMD_BUSY;
> > @@ -846,17 +865,35 @@ static inline bool msdc_cmd_is_ready(struct msdc_host *host,
> >                 return false;
> >         }
> >
> > -       if (mmc_resp_type(cmd) == MMC_RSP_R1B || cmd->data) {
> > -               tmo = jiffies + msecs_to_jiffies(20);
> > -               /* R1B or with data, should check SDCBUSY */
> > -               while ((readl(host->base + SDC_STS) & SDC_STS_SDCBUSY) &&
> > -                               time_before(jiffies, tmo))
> > -                       cpu_relax();
> > -               if (readl(host->base + SDC_STS) & SDC_STS_SDCBUSY) {
> > -                       dev_err(host->dev, "Controller busy detected\n");
> > -                       host->error |= REQ_CMD_BUSY;
> > -                       msdc_cmd_done(host, MSDC_INT_CMDTMO, mrq, cmd);
> > -                       return false;
> > +       if (cmd->opcode != MMC_SEND_STATUS) {
> > +               count = 0;
> > +               /* Consider that CMD6 crc error before card was init done,
> > +                * mmc_retune() will return directly as host->card is null.
> > +                * and CMD6 will retry 3 times, must ensure card is in transfer
> > +                * state when retry.
> > +                */
> > +               tmo = jiffies + msecs_to_jiffies(60 * 1000);
> > +               while (1) {
> > +                       if (msdc_card_busy(host->mmc)) {
> > +                               if (in_interrupt()) {
> > +                                       udelay(1);
> > +                                       count++;
> > +                               } else {
> > +                                       msleep_interruptible(10);
> > +                               }
> > +                       } else {
> > +                               break;
> > +                       }
> > +                       /* Timeout if the device never
> > +                        * leaves the program state.
> > +                        */
> > +                       if (count > 1000 || time_after(jiffies, tmo)) {
> > +                               pr_err("%s: Card stuck in programming state! %s\n",
> > +                                      mmc_hostname(host->mmc), __func__);
> > +                               host->error |= REQ_CMD_BUSY;
> > +                               msdc_cmd_done(host, MSDC_INT_CMDTMO, mrq, cmd);
> > +                               return false;
> > +                       }
> 
> This hole new code is a hack, that shouldn't be needed in the host driver.
> 
> I think we need to investigate and fix the issue in the mmc core
> layer, to make this work for your host driver. That instead of doing a
> work around in the host.
> 
I had already make some modification on the mmc core to resolve these
issues in the latest patch.

> >                 }
> >         }
> >         return true;
> > @@ -1070,18 +1107,6 @@ static int msdc_ops_switch_volt(struct mmc_host *mmc, struct mmc_ios *ios)
> >         return ret;
> >  }
> >
> > -static int msdc_card_busy(struct mmc_host *mmc)
> > -{
> > -       struct msdc_host *host = mmc_priv(mmc);
> > -       u32 status = readl(host->base + MSDC_PS);
> > -
> > -       /* check if any pin between dat[0:3] is low */
> > -       if (((status >> 16) & 0xf) != 0xf)
> > -               return 1;
> > -
> > -       return 0;
> > -}
> > -
> >  static void msdc_request_timeout(struct work_struct *work)
> >  {
> >         struct msdc_host *host = container_of(work, struct msdc_host,
> > --
> > 1.7.9.5
> >
> 
> Kind regards
> Uffe

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] sdio: mediatek: Support sdio feature
  2016-11-08  6:08 ` [PATCH 2/4] sdio: mediatek: Support sdio feature Yong Mao
@ 2017-01-23 11:24   ` Wei-Ning Huang
  2017-01-23 11:34     ` Wei-Ning Huang
  0 siblings, 1 reply; 10+ messages in thread
From: Wei-Ning Huang @ 2017-01-23 11:24 UTC (permalink / raw)
  To: Yong Mao
  Cc: Ulf Hansson, Rob Herring, Mark Rutland, Catalin Marinas,
	Will Deacon, Matthias Brugger, YH Huang, Mathias Nyman,
	Chunfeng Yun, Eddie Huang, Philipp Zabel, Chaotian Jing,
	Nicolas Boichat, Douglas Anderson, Geert Uytterhoeven,
	devicetree, linux-arm-kernel, LKML, linux-mmc, linux-mediatek,
	srv_heupstream

On Tue, Nov 8, 2016 at 2:08 PM, Yong Mao <yong.mao@mediatek.com> wrote:
> From: yong mao <yong.mao@mediatek.com>
>
> 1. Add irqlock to protect accessing the shared register
> 2. Modify the implementation of msdc_card_busy due to SDIO
> 3. Implement enable_sdio_irq
> 4. Add msdc_recheck_sdio_irq mechanism to make sure all
>    interrupts can be processed immediately
>
> Signed-off-by: Yong Mao <yong.mao@mediatek.com>
> Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
> ---
>  drivers/mmc/host/mtk-sd.c |  167 ++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 129 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
> index b29683b..37edf30 100644
> --- a/drivers/mmc/host/mtk-sd.c
> +++ b/drivers/mmc/host/mtk-sd.c
> @@ -117,6 +117,7 @@
>  #define MSDC_PS_CDSTS           (0x1 << 1)     /* R  */
>  #define MSDC_PS_CDDEBOUNCE      (0xf << 12)    /* RW */
>  #define MSDC_PS_DAT             (0xff << 16)   /* R  */
> +#define MSDC_PS_DATA1           (0x1 << 17)    /* R  */
>  #define MSDC_PS_CMD             (0x1 << 24)    /* R  */
>  #define MSDC_PS_WP              (0x1 << 31)    /* R  */
>
> @@ -304,6 +305,7 @@ struct msdc_host {
>         int cmd_rsp;
>
>         spinlock_t lock;
> +       spinlock_t irqlock;     /* sdio irq lock */
>         struct mmc_request *mrq;
>         struct mmc_command *cmd;
>         struct mmc_data *data;
> @@ -322,12 +324,14 @@ struct msdc_host {
>         struct pinctrl_state *pins_uhs;
>         struct delayed_work req_timeout;
>         int irq;                /* host interrupt */
> +       bool irq_thread_alive;
>
>         struct clk *src_clk;    /* msdc source clock */
>         struct clk *h_clk;      /* msdc h_clk */
>         u32 mclk;               /* mmc subsystem clock frequency */
>         u32 src_clk_freq;       /* source clock frequency */
>         u32 sclk;               /* SD/MS bus clock frequency */
> +       bool clock_on;
>         unsigned char timing;
>         bool vqmmc_enabled;
>         u32 hs400_ds_delay;
> @@ -387,6 +391,7 @@ static void msdc_reset_hw(struct msdc_host *host)
>
>  static void msdc_cmd_next(struct msdc_host *host,
>                 struct mmc_request *mrq, struct mmc_command *cmd);
> +static void msdc_recheck_sdio_irq(struct msdc_host *host);
>
>  static const u32 cmd_ints_mask = MSDC_INTEN_CMDRDY | MSDC_INTEN_RSPCRCERR |
>                         MSDC_INTEN_CMDTMO | MSDC_INTEN_ACMDRDY |
> @@ -513,6 +518,7 @@ static void msdc_gate_clock(struct msdc_host *host)
>  {
>         clk_disable_unprepare(host->src_clk);
>         clk_disable_unprepare(host->h_clk);
> +       host->clock_on = false;
>  }
>
>  static void msdc_ungate_clock(struct msdc_host *host)
> @@ -521,6 +527,7 @@ static void msdc_ungate_clock(struct msdc_host *host)
>         clk_prepare_enable(host->src_clk);
>         while (!(readl(host->base + MSDC_CFG) & MSDC_CFG_CKSTB))
>                 cpu_relax();
> +       host->clock_on = true;
>  }
>
>  static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz)
> @@ -529,6 +536,7 @@ static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz)
>         u32 flags;
>         u32 div;
>         u32 sclk;
> +       unsigned long irq_flags;
>
>         if (!hz) {
>                 dev_dbg(host->dev, "set mclk to 0\n");
> @@ -537,8 +545,11 @@ static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz)
>                 return;
>         }
>
> +       spin_lock_irqsave(&host->irqlock, irq_flags);
>         flags = readl(host->base + MSDC_INTEN);
>         sdr_clr_bits(host->base + MSDC_INTEN, flags);
> +       spin_unlock_irqrestore(&host->irqlock, irq_flags);
> +
>         sdr_clr_bits(host->base + MSDC_CFG, MSDC_CFG_HS400_CK_MODE);
>         if (timing == MMC_TIMING_UHS_DDR50 ||
>             timing == MMC_TIMING_MMC_DDR52 ||
> @@ -588,7 +599,10 @@ static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz)
>         host->timing = timing;
>         /* need because clk changed. */
>         msdc_set_timeout(host, host->timeout_ns, host->timeout_clks);
> +
> +       spin_lock_irqsave(&host->irqlock, irq_flags);
>         sdr_set_bits(host->base + MSDC_INTEN, flags);
> +       spin_unlock_irqrestore(&host->irqlock, irq_flags);
>
>         /*
>          * mmc_select_hs400() will drop to 50Mhz and High speed mode,
> @@ -690,6 +704,7 @@ static inline u32 msdc_cmd_prepare_raw_cmd(struct msdc_host *host,
>  static void msdc_start_data(struct msdc_host *host, struct mmc_request *mrq,
>                             struct mmc_command *cmd, struct mmc_data *data)
>  {
> +       unsigned long flags;
>         bool read;
>
>         WARN_ON(host->data);
> @@ -698,8 +713,12 @@ static void msdc_start_data(struct msdc_host *host, struct mmc_request *mrq,
>
>         mod_delayed_work(system_wq, &host->req_timeout, DAT_TIMEOUT);
>         msdc_dma_setup(host, &host->dma, data);
> +
> +       spin_lock_irqsave(&host->irqlock, flags);
>         sdr_set_bits(host->base + MSDC_INTEN, data_ints_mask);
>         sdr_set_field(host->base + MSDC_DMA_CTRL, MSDC_DMA_CTRL_START, 1);
> +       spin_unlock_irqrestore(&host->irqlock, flags);
> +
>         dev_dbg(host->dev, "DMA start\n");
>         dev_dbg(host->dev, "%s: cmd=%d DMA data: %d blocks; read=%d\n",
>                         __func__, cmd->opcode, data->blocks, read);
> @@ -756,6 +775,7 @@ static void msdc_request_done(struct msdc_host *host, struct mmc_request *mrq)
>         if (mrq->data)
>                 msdc_unprepare_data(host, mrq);
>         mmc_request_done(host->mmc, mrq);
> +       msdc_recheck_sdio_irq(host);
>  }
>
>  /* returns true if command is fully handled; returns false otherwise */
> @@ -779,15 +799,17 @@ static bool msdc_cmd_done(struct msdc_host *host, int events,
>                                         | MSDC_INT_CMDTMO)))
>                 return done;
>
> -       spin_lock_irqsave(&host->lock, flags);
>         done = !host->cmd;
> +       spin_lock_irqsave(&host->lock, flags);
>         host->cmd = NULL;
>         spin_unlock_irqrestore(&host->lock, flags);
>
>         if (done)
>                 return true;
>
> +       spin_lock_irqsave(&host->irqlock, flags);
>         sdr_clr_bits(host->base + MSDC_INTEN, cmd_ints_mask);
> +       spin_unlock_irqrestore(&host->irqlock, flags);
>
>         if (cmd->flags & MMC_RSP_PRESENT) {
>                 if (cmd->flags & MMC_RSP_136) {
> @@ -902,6 +924,7 @@ static inline bool msdc_cmd_is_ready(struct msdc_host *host,
>  static void msdc_start_command(struct msdc_host *host,
>                 struct mmc_request *mrq, struct mmc_command *cmd)
>  {
> +       unsigned long flags;
>         u32 rawcmd;
>
>         WARN_ON(host->cmd);
> @@ -920,7 +943,10 @@ static void msdc_start_command(struct msdc_host *host,
>         rawcmd = msdc_cmd_prepare_raw_cmd(host, mrq, cmd);
>         mod_delayed_work(system_wq, &host->req_timeout, DAT_TIMEOUT);
>
> +       spin_lock_irqsave(&host->irqlock, flags);
>         sdr_set_bits(host->base + MSDC_INTEN, cmd_ints_mask);
> +       spin_unlock_irqrestore(&host->irqlock, flags);
> +
>         writel(cmd->arg, host->base + SDC_ARG);
>         writel(rawcmd, host->base + SDC_CMD);
>  }
> @@ -1013,8 +1039,8 @@ static bool msdc_data_xfer_done(struct msdc_host *host, u32 events,
>              | MSDC_INT_DMA_BDCSERR | MSDC_INT_DMA_GPDCSERR
>              | MSDC_INT_DMA_PROTECT);
>
> -       spin_lock_irqsave(&host->lock, flags);
>         done = !host->data;
> +       spin_lock_irqsave(&host->lock, flags);
>         if (check_data)
>                 host->data = NULL;
>         spin_unlock_irqrestore(&host->lock, flags);
> @@ -1029,7 +1055,11 @@ static bool msdc_data_xfer_done(struct msdc_host *host, u32 events,
>                                 1);
>                 while (readl(host->base + MSDC_DMA_CFG) & MSDC_DMA_CFG_STS)
>                         cpu_relax();
> +
> +               spin_lock_irqsave(&host->irqlock, flags);
>                 sdr_clr_bits(host->base + MSDC_INTEN, data_ints_mask);
> +               spin_unlock_irqrestore(&host->irqlock, flags);
> +
>                 dev_dbg(host->dev, "DMA stop\n");
>
>                 if ((events & MSDC_INT_XFER_COMPL) && (!stop || !stop->error)) {
> @@ -1134,44 +1164,47 @@ static void msdc_request_timeout(struct work_struct *work)
>
>  static irqreturn_t msdc_irq(int irq, void *dev_id)
>  {
> +       unsigned long flags;
>         struct msdc_host *host = (struct msdc_host *) dev_id;
> +       struct mmc_request *mrq;
> +       struct mmc_command *cmd;
> +       struct mmc_data *data;
> +       u32 events, event_mask;
> +
> +       spin_lock_irqsave(&host->irqlock, flags);
> +       events = readl(host->base + MSDC_INT);
> +       event_mask = readl(host->base + MSDC_INTEN);
> +       /* clear interrupts */
> +       writel(events & event_mask, host->base + MSDC_INT);
> +
> +       mrq = host->mrq;
> +       cmd = host->cmd;
> +       data = host->data;
> +       spin_unlock_irqrestore(&host->irqlock, flags);
> +
> +       if ((events & event_mask) & MSDC_INT_SDIOIRQ) {
> +               mmc_signal_sdio_irq(host->mmc);
> +               if (!mrq)
> +                       return IRQ_HANDLED;
> +       }
>
> -       while (true) {
> -               unsigned long flags;
> -               struct mmc_request *mrq;
> -               struct mmc_command *cmd;
> -               struct mmc_data *data;
> -               u32 events, event_mask;
> -
> -               spin_lock_irqsave(&host->lock, flags);
> -               events = readl(host->base + MSDC_INT);
> -               event_mask = readl(host->base + MSDC_INTEN);
> -               /* clear interrupts */
> -               writel(events & event_mask, host->base + MSDC_INT);
> -
> -               mrq = host->mrq;
> -               cmd = host->cmd;
> -               data = host->data;
> -               spin_unlock_irqrestore(&host->lock, flags);
> -
> -               if (!(events & event_mask))
> -                       break;
> +       if (!(events & (event_mask & ~MSDC_INT_SDIOIRQ)))
> +               return IRQ_HANDLED;
>
> -               if (!mrq) {
> -                       dev_err(host->dev,
> -                               "%s: MRQ=NULL; events=%08X; event_mask=%08X\n",
> -                               __func__, events, event_mask);
> -                       WARN_ON(1);
> -                       break;
> -               }
> +       if (!mrq) {
> +               dev_err(host->dev,
> +                       "%s: MRQ=NULL; events=%08X; event_mask=%08X\n",
> +                       __func__, events, event_mask);
> +               WARN_ON(1);
> +               return IRQ_HANDLED;
> +       }
>
> -               dev_dbg(host->dev, "%s: events=%08X\n", __func__, events);
> +       dev_dbg(host->dev, "%s: events=%08X\n", __func__, events);
>
> -               if (cmd)
> -                       msdc_cmd_done(host, events, mrq, cmd);
> -               else if (data)
> -                       msdc_data_xfer_done(host, events, mrq, data);
> -       }
> +       if (cmd)
> +               msdc_cmd_done(host, events, mrq, cmd);
> +       else if (data)
> +               msdc_data_xfer_done(host, events, mrq, data);
>
>         return IRQ_HANDLED;
>  }
> @@ -1179,6 +1212,7 @@ static irqreturn_t msdc_irq(int irq, void *dev_id)
>  static void msdc_init_hw(struct msdc_host *host)
>  {
>         u32 val;
> +       unsigned long flags;
>
>         /* Configure to MMC/SD mode, clock free running */
>         sdr_set_bits(host->base + MSDC_CFG, MSDC_CFG_MODE | MSDC_CFG_CKPDN);
> @@ -1190,9 +1224,11 @@ static void msdc_init_hw(struct msdc_host *host)
>         sdr_clr_bits(host->base + MSDC_PS, MSDC_PS_CDEN);
>
>         /* Disable and clear all interrupts */
> +       spin_lock_irqsave(&host->irqlock, flags);
>         writel(0, host->base + MSDC_INTEN);
>         val = readl(host->base + MSDC_INT);
>         writel(val, host->base + MSDC_INT);
> +       spin_unlock_irqrestore(&host->irqlock, flags);
>
>         writel(0, host->base + MSDC_PAD_TUNE);
>         writel(0, host->base + MSDC_IOCON);
> @@ -1207,9 +1243,11 @@ static void msdc_init_hw(struct msdc_host *host)
>          */
>         sdr_set_bits(host->base + SDC_CFG, SDC_CFG_SDIO);
>
> -       /* disable detect SDIO device interrupt function */
> -       sdr_clr_bits(host->base + SDC_CFG, SDC_CFG_SDIOIDE);
> -
> +       if (host->mmc->caps & MMC_CAP_SDIO_IRQ)
> +               sdr_set_bits(host->base + SDC_CFG, SDC_CFG_SDIOIDE);
> +       else
> +               /* disable detect SDIO device interrupt function */
> +               sdr_clr_bits(host->base + SDC_CFG, SDC_CFG_SDIOIDE);
>         /* Configure to default data timeout */
>         sdr_set_field(host->base + SDC_CFG, SDC_CFG_DTOC, 3);
>
> @@ -1221,11 +1259,15 @@ static void msdc_init_hw(struct msdc_host *host)
>  static void msdc_deinit_hw(struct msdc_host *host)
>  {
>         u32 val;
> +       unsigned long flags;
> +
>         /* Disable and clear all interrupts */
> +       spin_lock_irqsave(&host->irqlock, flags);
>         writel(0, host->base + MSDC_INTEN);
>
>         val = readl(host->base + MSDC_INT);
>         writel(val, host->base + MSDC_INT);
> +       spin_unlock_irqrestore(&host->irqlock, flags);
>  }
>
>  /* init gpd and bd list in msdc_drv_probe */
> @@ -1493,6 +1535,52 @@ static void msdc_hw_reset(struct mmc_host *mmc)
>         sdr_clr_bits(host->base + EMMC_IOCON, 1);
>  }
>
> +/**
> + * msdc_recheck_sdio_irq - recheck whether the SDIO IRQ is lost
> + * @host: The host to check.
> + *
> + * Host controller may lost interrupt in some special case.
> + * Add sdio IRQ recheck mechanism to make sure all interrupts
> + * can be processed immediately
> + *
> + */
> +static void msdc_recheck_sdio_irq(struct msdc_host *host)
> +{
> +       u32 reg_int, reg_ps;
> +
> +       if (host->clock_on &&
> +           (host->mmc->caps & MMC_CAP_SDIO_IRQ) &&
> +           host->irq_thread_alive) {
> +               reg_int = readl(host->base + MSDC_INT);
> +               reg_ps  = readl(host->base + MSDC_PS);
> +               if (!((reg_int & MSDC_INT_SDIOIRQ) ||
> +                     (reg_ps & MSDC_PS_DATA1))) {
> +                       mmc_signal_sdio_irq(host->mmc);
> +               }
> +       }
> +}
> +
> +static void msdc_enable_sdio_irq(struct mmc_host *mmc, int enable)
> +{
> +       unsigned long flags;
> +       struct msdc_host *host = mmc_priv(mmc);
> +
> +       host->irq_thread_alive = true;
> +       if (enable) {
> +               pm_runtime_get_sync(host->dev);\
This cause problems in kernel >= 3.19. Since pm_runtime_get_sync calls
__might_sleep, and you are not suppose to sleep in a IRQ handler.

2017-01-20T00:32:49.855603-08:00 WARNING kernel: [   11.068860] do not
call blocking ops when !TASK_RUNNING; state=1 set at
[<ffffffc0007a350c>] sdio_irq_thread+0x11c/0x1dc
...
2017-01-20T00:32:49.856042-08:00 EMERG kernel: [   12.136156] Call
trace:
2017-01-20T00:32:49.856044-08:00 WARNING kernel: [   12.138584]
[<ffffffc000249d84>] __might_sleep+0x64/0x90
2017-01-20T00:32:49.856047-08:00 WARNING kernel: [   12.143855]
[<ffffffc000630f54>] __pm_runtime_resume+0x40/0x9c
2017-01-20T00:32:49.856049-08:00 WARNING kernel: [   12.149644]
[<ffffffc0007ae994>] msdc_enable_sdio_irq+0x44/0xd0
2017-01-20T00:32:49.856051-08:00 WARNING kernel: [   12.155516]
[<ffffffc0007a3544>] sdio_irq_thread+0x154/0x1dc
2017-01-20T00:32:49.856053-08:00 WARNING kernel: [   12.161131]
[<ffffffc00023f30c>] kthread+0x10c/0x114
2017-01-20T00:32:49.856055-08:00 WARNING kernel: [   12.166056]
[<ffffffc000203dd0>] ret_from_fork+0x10/0x40
2017-01-20T00:32:49.856057-08:00 WARNING kernel: [   12.171368] sched:
RT throttling activated for rt_rq ffffffc0fff5a5c0 (cpu 1)
2017-01-20T00:32:49.856059-08:00 WARNING kernel: [   12.171368]
potential CPU hogs:
2017-01-20T00:32:49.856061-08:00 WARNING kernel: [   12.171368]
 ksdioirqd/mmc2 (1772)

dw_mmc also enables autosuspend but it didn't call any pm_runtime_get*
function in it's enable_sdio_irq function, I don't really know how it
worked around this.
Ulf, any suggestions on how we can do this?

Wei-Ning

> +               msdc_recheck_sdio_irq(host);
> +
> +               spin_lock_irqsave(&host->irqlock, flags);
> +               sdr_set_bits(host->base + SDC_CFG, SDC_CFG_SDIOIDE);
> +               sdr_set_bits(host->base + MSDC_INTEN, MSDC_INTEN_SDIOIRQ);
> +               spin_unlock_irqrestore(&host->irqlock, flags);
> +       } else {
> +               spin_lock_irqsave(&host->irqlock, flags);
> +               sdr_clr_bits(host->base + MSDC_INTEN, MSDC_INTEN_SDIOIRQ);
> +               spin_unlock_irqrestore(&host->irqlock, flags);
> +       }
> +}
> +
>  static struct mmc_host_ops mt_msdc_ops = {
>         .post_req = msdc_post_req,
>         .pre_req = msdc_pre_req,
> @@ -1504,6 +1592,7 @@ static void msdc_hw_reset(struct mmc_host *mmc)
>         .execute_tuning = msdc_execute_tuning,
>         .prepare_hs400_tuning = msdc_prepare_hs400_tuning,
>         .hw_reset = msdc_hw_reset,
> +       .enable_sdio_irq = msdc_enable_sdio_irq,
>  };
>
>  static int msdc_drv_probe(struct platform_device *pdev)
> @@ -1600,6 +1689,7 @@ static int msdc_drv_probe(struct platform_device *pdev)
>         mmc_dev(mmc)->dma_mask = &host->dma_mask;
>
>         host->timeout_clks = 3 * 1048576;
> +       host->irq_thread_alive = false;
>         host->dma.gpd = dma_alloc_coherent(&pdev->dev,
>                                 2 * sizeof(struct mt_gpdma_desc),
>                                 &host->dma.gpd_addr, GFP_KERNEL);
> @@ -1613,6 +1703,7 @@ static int msdc_drv_probe(struct platform_device *pdev)
>         msdc_init_gpd_bd(host, &host->dma);
>         INIT_DELAYED_WORK(&host->req_timeout, msdc_request_timeout);
>         spin_lock_init(&host->lock);
> +       spin_lock_init(&host->irqlock);
>
>         platform_set_drvdata(pdev, mmc);
>         msdc_ungate_clock(host);
> --
> 1.7.9.5
>



-- 
Wei-Ning Huang, 黃偉寧 | Software Engineer, Google Inc., Taiwan |
wnhuang@google.com | Cell: +886 910-380678

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] sdio: mediatek: Support sdio feature
  2017-01-23 11:24   ` Wei-Ning Huang
@ 2017-01-23 11:34     ` Wei-Ning Huang
  0 siblings, 0 replies; 10+ messages in thread
From: Wei-Ning Huang @ 2017-01-23 11:34 UTC (permalink / raw)
  To: Yong Mao
  Cc: Ulf Hansson, Rob Herring, Mark Rutland, Catalin Marinas,
	Will Deacon, Matthias Brugger, YH Huang, Mathias Nyman,
	Chunfeng Yun, Eddie Huang, Philipp Zabel, Chaotian Jing,
	Nicolas Boichat, Douglas Anderson, Geert Uytterhoeven,
	devicetree, linux-arm-kernel, LKML, linux-mmc, linux-mediatek,
	srv_heupstream

On Mon, Jan 23, 2017 at 7:24 PM, Wei-Ning Huang <wnhuang@google.com> wrote:
> On Tue, Nov 8, 2016 at 2:08 PM, Yong Mao <yong.mao@mediatek.com> wrote:
>> From: yong mao <yong.mao@mediatek.com>
>>
>> 1. Add irqlock to protect accessing the shared register
>> 2. Modify the implementation of msdc_card_busy due to SDIO
>> 3. Implement enable_sdio_irq
>> 4. Add msdc_recheck_sdio_irq mechanism to make sure all
>>    interrupts can be processed immediately
>>
>> Signed-off-by: Yong Mao <yong.mao@mediatek.com>
>> Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
>> ---
>>  drivers/mmc/host/mtk-sd.c |  167 ++++++++++++++++++++++++++++++++++-----------
>>  1 file changed, 129 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
>> index b29683b..37edf30 100644
>> --- a/drivers/mmc/host/mtk-sd.c
>> +++ b/drivers/mmc/host/mtk-sd.c
>> @@ -117,6 +117,7 @@
>>  #define MSDC_PS_CDSTS           (0x1 << 1)     /* R  */
>>  #define MSDC_PS_CDDEBOUNCE      (0xf << 12)    /* RW */
>>  #define MSDC_PS_DAT             (0xff << 16)   /* R  */
>> +#define MSDC_PS_DATA1           (0x1 << 17)    /* R  */
>>  #define MSDC_PS_CMD             (0x1 << 24)    /* R  */
>>  #define MSDC_PS_WP              (0x1 << 31)    /* R  */
>>
>> @@ -304,6 +305,7 @@ struct msdc_host {
>>         int cmd_rsp;
>>
>>         spinlock_t lock;
>> +       spinlock_t irqlock;     /* sdio irq lock */
>>         struct mmc_request *mrq;
>>         struct mmc_command *cmd;
>>         struct mmc_data *data;
>> @@ -322,12 +324,14 @@ struct msdc_host {
>>         struct pinctrl_state *pins_uhs;
>>         struct delayed_work req_timeout;
>>         int irq;                /* host interrupt */
>> +       bool irq_thread_alive;
>>
>>         struct clk *src_clk;    /* msdc source clock */
>>         struct clk *h_clk;      /* msdc h_clk */
>>         u32 mclk;               /* mmc subsystem clock frequency */
>>         u32 src_clk_freq;       /* source clock frequency */
>>         u32 sclk;               /* SD/MS bus clock frequency */
>> +       bool clock_on;
>>         unsigned char timing;
>>         bool vqmmc_enabled;
>>         u32 hs400_ds_delay;
>> @@ -387,6 +391,7 @@ static void msdc_reset_hw(struct msdc_host *host)
>>
>>  static void msdc_cmd_next(struct msdc_host *host,
>>                 struct mmc_request *mrq, struct mmc_command *cmd);
>> +static void msdc_recheck_sdio_irq(struct msdc_host *host);
>>
>>  static const u32 cmd_ints_mask = MSDC_INTEN_CMDRDY | MSDC_INTEN_RSPCRCERR |
>>                         MSDC_INTEN_CMDTMO | MSDC_INTEN_ACMDRDY |
>> @@ -513,6 +518,7 @@ static void msdc_gate_clock(struct msdc_host *host)
>>  {
>>         clk_disable_unprepare(host->src_clk);
>>         clk_disable_unprepare(host->h_clk);
>> +       host->clock_on = false;
>>  }
>>
>>  static void msdc_ungate_clock(struct msdc_host *host)
>> @@ -521,6 +527,7 @@ static void msdc_ungate_clock(struct msdc_host *host)
>>         clk_prepare_enable(host->src_clk);
>>         while (!(readl(host->base + MSDC_CFG) & MSDC_CFG_CKSTB))
>>                 cpu_relax();
>> +       host->clock_on = true;
>>  }
>>
>>  static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz)
>> @@ -529,6 +536,7 @@ static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz)
>>         u32 flags;
>>         u32 div;
>>         u32 sclk;
>> +       unsigned long irq_flags;
>>
>>         if (!hz) {
>>                 dev_dbg(host->dev, "set mclk to 0\n");
>> @@ -537,8 +545,11 @@ static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz)
>>                 return;
>>         }
>>
>> +       spin_lock_irqsave(&host->irqlock, irq_flags);
>>         flags = readl(host->base + MSDC_INTEN);
>>         sdr_clr_bits(host->base + MSDC_INTEN, flags);
>> +       spin_unlock_irqrestore(&host->irqlock, irq_flags);
>> +
>>         sdr_clr_bits(host->base + MSDC_CFG, MSDC_CFG_HS400_CK_MODE);
>>         if (timing == MMC_TIMING_UHS_DDR50 ||
>>             timing == MMC_TIMING_MMC_DDR52 ||
>> @@ -588,7 +599,10 @@ static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz)
>>         host->timing = timing;
>>         /* need because clk changed. */
>>         msdc_set_timeout(host, host->timeout_ns, host->timeout_clks);
>> +
>> +       spin_lock_irqsave(&host->irqlock, irq_flags);
>>         sdr_set_bits(host->base + MSDC_INTEN, flags);
>> +       spin_unlock_irqrestore(&host->irqlock, irq_flags);
>>
>>         /*
>>          * mmc_select_hs400() will drop to 50Mhz and High speed mode,
>> @@ -690,6 +704,7 @@ static inline u32 msdc_cmd_prepare_raw_cmd(struct msdc_host *host,
>>  static void msdc_start_data(struct msdc_host *host, struct mmc_request *mrq,
>>                             struct mmc_command *cmd, struct mmc_data *data)
>>  {
>> +       unsigned long flags;
>>         bool read;
>>
>>         WARN_ON(host->data);
>> @@ -698,8 +713,12 @@ static void msdc_start_data(struct msdc_host *host, struct mmc_request *mrq,
>>
>>         mod_delayed_work(system_wq, &host->req_timeout, DAT_TIMEOUT);
>>         msdc_dma_setup(host, &host->dma, data);
>> +
>> +       spin_lock_irqsave(&host->irqlock, flags);
>>         sdr_set_bits(host->base + MSDC_INTEN, data_ints_mask);
>>         sdr_set_field(host->base + MSDC_DMA_CTRL, MSDC_DMA_CTRL_START, 1);
>> +       spin_unlock_irqrestore(&host->irqlock, flags);
>> +
>>         dev_dbg(host->dev, "DMA start\n");
>>         dev_dbg(host->dev, "%s: cmd=%d DMA data: %d blocks; read=%d\n",
>>                         __func__, cmd->opcode, data->blocks, read);
>> @@ -756,6 +775,7 @@ static void msdc_request_done(struct msdc_host *host, struct mmc_request *mrq)
>>         if (mrq->data)
>>                 msdc_unprepare_data(host, mrq);
>>         mmc_request_done(host->mmc, mrq);
>> +       msdc_recheck_sdio_irq(host);
>>  }
>>
>>  /* returns true if command is fully handled; returns false otherwise */
>> @@ -779,15 +799,17 @@ static bool msdc_cmd_done(struct msdc_host *host, int events,
>>                                         | MSDC_INT_CMDTMO)))
>>                 return done;
>>
>> -       spin_lock_irqsave(&host->lock, flags);
>>         done = !host->cmd;
>> +       spin_lock_irqsave(&host->lock, flags);
>>         host->cmd = NULL;
>>         spin_unlock_irqrestore(&host->lock, flags);
>>
>>         if (done)
>>                 return true;
>>
>> +       spin_lock_irqsave(&host->irqlock, flags);
>>         sdr_clr_bits(host->base + MSDC_INTEN, cmd_ints_mask);
>> +       spin_unlock_irqrestore(&host->irqlock, flags);
>>
>>         if (cmd->flags & MMC_RSP_PRESENT) {
>>                 if (cmd->flags & MMC_RSP_136) {
>> @@ -902,6 +924,7 @@ static inline bool msdc_cmd_is_ready(struct msdc_host *host,
>>  static void msdc_start_command(struct msdc_host *host,
>>                 struct mmc_request *mrq, struct mmc_command *cmd)
>>  {
>> +       unsigned long flags;
>>         u32 rawcmd;
>>
>>         WARN_ON(host->cmd);
>> @@ -920,7 +943,10 @@ static void msdc_start_command(struct msdc_host *host,
>>         rawcmd = msdc_cmd_prepare_raw_cmd(host, mrq, cmd);
>>         mod_delayed_work(system_wq, &host->req_timeout, DAT_TIMEOUT);
>>
>> +       spin_lock_irqsave(&host->irqlock, flags);
>>         sdr_set_bits(host->base + MSDC_INTEN, cmd_ints_mask);
>> +       spin_unlock_irqrestore(&host->irqlock, flags);
>> +
>>         writel(cmd->arg, host->base + SDC_ARG);
>>         writel(rawcmd, host->base + SDC_CMD);
>>  }
>> @@ -1013,8 +1039,8 @@ static bool msdc_data_xfer_done(struct msdc_host *host, u32 events,
>>              | MSDC_INT_DMA_BDCSERR | MSDC_INT_DMA_GPDCSERR
>>              | MSDC_INT_DMA_PROTECT);
>>
>> -       spin_lock_irqsave(&host->lock, flags);
>>         done = !host->data;
>> +       spin_lock_irqsave(&host->lock, flags);
>>         if (check_data)
>>                 host->data = NULL;
>>         spin_unlock_irqrestore(&host->lock, flags);
>> @@ -1029,7 +1055,11 @@ static bool msdc_data_xfer_done(struct msdc_host *host, u32 events,
>>                                 1);
>>                 while (readl(host->base + MSDC_DMA_CFG) & MSDC_DMA_CFG_STS)
>>                         cpu_relax();
>> +
>> +               spin_lock_irqsave(&host->irqlock, flags);
>>                 sdr_clr_bits(host->base + MSDC_INTEN, data_ints_mask);
>> +               spin_unlock_irqrestore(&host->irqlock, flags);
>> +
>>                 dev_dbg(host->dev, "DMA stop\n");
>>
>>                 if ((events & MSDC_INT_XFER_COMPL) && (!stop || !stop->error)) {
>> @@ -1134,44 +1164,47 @@ static void msdc_request_timeout(struct work_struct *work)
>>
>>  static irqreturn_t msdc_irq(int irq, void *dev_id)
>>  {
>> +       unsigned long flags;
>>         struct msdc_host *host = (struct msdc_host *) dev_id;
>> +       struct mmc_request *mrq;
>> +       struct mmc_command *cmd;
>> +       struct mmc_data *data;
>> +       u32 events, event_mask;
>> +
>> +       spin_lock_irqsave(&host->irqlock, flags);
>> +       events = readl(host->base + MSDC_INT);
>> +       event_mask = readl(host->base + MSDC_INTEN);
>> +       /* clear interrupts */
>> +       writel(events & event_mask, host->base + MSDC_INT);
>> +
>> +       mrq = host->mrq;
>> +       cmd = host->cmd;
>> +       data = host->data;
>> +       spin_unlock_irqrestore(&host->irqlock, flags);
>> +
>> +       if ((events & event_mask) & MSDC_INT_SDIOIRQ) {
>> +               mmc_signal_sdio_irq(host->mmc);
>> +               if (!mrq)
>> +                       return IRQ_HANDLED;
>> +       }
>>
>> -       while (true) {
>> -               unsigned long flags;
>> -               struct mmc_request *mrq;
>> -               struct mmc_command *cmd;
>> -               struct mmc_data *data;
>> -               u32 events, event_mask;
>> -
>> -               spin_lock_irqsave(&host->lock, flags);
>> -               events = readl(host->base + MSDC_INT);
>> -               event_mask = readl(host->base + MSDC_INTEN);
>> -               /* clear interrupts */
>> -               writel(events & event_mask, host->base + MSDC_INT);
>> -
>> -               mrq = host->mrq;
>> -               cmd = host->cmd;
>> -               data = host->data;
>> -               spin_unlock_irqrestore(&host->lock, flags);
>> -
>> -               if (!(events & event_mask))
>> -                       break;
>> +       if (!(events & (event_mask & ~MSDC_INT_SDIOIRQ)))
>> +               return IRQ_HANDLED;
>>
>> -               if (!mrq) {
>> -                       dev_err(host->dev,
>> -                               "%s: MRQ=NULL; events=%08X; event_mask=%08X\n",
>> -                               __func__, events, event_mask);
>> -                       WARN_ON(1);
>> -                       break;
>> -               }
>> +       if (!mrq) {
>> +               dev_err(host->dev,
>> +                       "%s: MRQ=NULL; events=%08X; event_mask=%08X\n",
>> +                       __func__, events, event_mask);
>> +               WARN_ON(1);
>> +               return IRQ_HANDLED;
>> +       }
>>
>> -               dev_dbg(host->dev, "%s: events=%08X\n", __func__, events);
>> +       dev_dbg(host->dev, "%s: events=%08X\n", __func__, events);
>>
>> -               if (cmd)
>> -                       msdc_cmd_done(host, events, mrq, cmd);
>> -               else if (data)
>> -                       msdc_data_xfer_done(host, events, mrq, data);
>> -       }
>> +       if (cmd)
>> +               msdc_cmd_done(host, events, mrq, cmd);
>> +       else if (data)
>> +               msdc_data_xfer_done(host, events, mrq, data);
>>
>>         return IRQ_HANDLED;
>>  }
>> @@ -1179,6 +1212,7 @@ static irqreturn_t msdc_irq(int irq, void *dev_id)
>>  static void msdc_init_hw(struct msdc_host *host)
>>  {
>>         u32 val;
>> +       unsigned long flags;
>>
>>         /* Configure to MMC/SD mode, clock free running */
>>         sdr_set_bits(host->base + MSDC_CFG, MSDC_CFG_MODE | MSDC_CFG_CKPDN);
>> @@ -1190,9 +1224,11 @@ static void msdc_init_hw(struct msdc_host *host)
>>         sdr_clr_bits(host->base + MSDC_PS, MSDC_PS_CDEN);
>>
>>         /* Disable and clear all interrupts */
>> +       spin_lock_irqsave(&host->irqlock, flags);
>>         writel(0, host->base + MSDC_INTEN);
>>         val = readl(host->base + MSDC_INT);
>>         writel(val, host->base + MSDC_INT);
>> +       spin_unlock_irqrestore(&host->irqlock, flags);
>>
>>         writel(0, host->base + MSDC_PAD_TUNE);
>>         writel(0, host->base + MSDC_IOCON);
>> @@ -1207,9 +1243,11 @@ static void msdc_init_hw(struct msdc_host *host)
>>          */
>>         sdr_set_bits(host->base + SDC_CFG, SDC_CFG_SDIO);
>>
>> -       /* disable detect SDIO device interrupt function */
>> -       sdr_clr_bits(host->base + SDC_CFG, SDC_CFG_SDIOIDE);
>> -
>> +       if (host->mmc->caps & MMC_CAP_SDIO_IRQ)
>> +               sdr_set_bits(host->base + SDC_CFG, SDC_CFG_SDIOIDE);
>> +       else
>> +               /* disable detect SDIO device interrupt function */
>> +               sdr_clr_bits(host->base + SDC_CFG, SDC_CFG_SDIOIDE);
>>         /* Configure to default data timeout */
>>         sdr_set_field(host->base + SDC_CFG, SDC_CFG_DTOC, 3);
>>
>> @@ -1221,11 +1259,15 @@ static void msdc_init_hw(struct msdc_host *host)
>>  static void msdc_deinit_hw(struct msdc_host *host)
>>  {
>>         u32 val;
>> +       unsigned long flags;
>> +
>>         /* Disable and clear all interrupts */
>> +       spin_lock_irqsave(&host->irqlock, flags);
>>         writel(0, host->base + MSDC_INTEN);
>>
>>         val = readl(host->base + MSDC_INT);
>>         writel(val, host->base + MSDC_INT);
>> +       spin_unlock_irqrestore(&host->irqlock, flags);
>>  }
>>
>>  /* init gpd and bd list in msdc_drv_probe */
>> @@ -1493,6 +1535,52 @@ static void msdc_hw_reset(struct mmc_host *mmc)
>>         sdr_clr_bits(host->base + EMMC_IOCON, 1);
>>  }
>>
>> +/**
>> + * msdc_recheck_sdio_irq - recheck whether the SDIO IRQ is lost
>> + * @host: The host to check.
>> + *
>> + * Host controller may lost interrupt in some special case.
>> + * Add sdio IRQ recheck mechanism to make sure all interrupts
>> + * can be processed immediately
>> + *
>> + */
>> +static void msdc_recheck_sdio_irq(struct msdc_host *host)
>> +{
>> +       u32 reg_int, reg_ps;
>> +
>> +       if (host->clock_on &&
>> +           (host->mmc->caps & MMC_CAP_SDIO_IRQ) &&
>> +           host->irq_thread_alive) {
>> +               reg_int = readl(host->base + MSDC_INT);
>> +               reg_ps  = readl(host->base + MSDC_PS);
>> +               if (!((reg_int & MSDC_INT_SDIOIRQ) ||
>> +                     (reg_ps & MSDC_PS_DATA1))) {
>> +                       mmc_signal_sdio_irq(host->mmc);
>> +               }
>> +       }
>> +}
>> +
>> +static void msdc_enable_sdio_irq(struct mmc_host *mmc, int enable)
>> +{
>> +       unsigned long flags;
>> +       struct msdc_host *host = mmc_priv(mmc);
>> +
>> +       host->irq_thread_alive = true;
>> +       if (enable) {
>> +               pm_runtime_get_sync(host->dev);\
> This cause problems in kernel >= 3.19. Since pm_runtime_get_sync calls
> __might_sleep, and you are not suppose to sleep in a IRQ handler.
Ignore above, apparently I don't know what I'm talking about ...
It's actually because the sdio_irq_thread set task state to
TASK_INTERRUPTABLE before calling enable_sdio_irq:

    set_current_state(TASK_INTERRUPTIBLE);
    if (host->caps & MMC_CAP_SDIO_IRQ)
      host->ops->enable_sdio_irq(host, 1);
    if (!kthread_should_stop())
      schedule_timeout(period);
    set_current_state(TASK_RUNNING);

so we are not suppose to call __might_sleep in the enable_sdio_irq callback.
>
> 2017-01-20T00:32:49.855603-08:00 WARNING kernel: [   11.068860] do not
> call blocking ops when !TASK_RUNNING; state=1 set at
> [<ffffffc0007a350c>] sdio_irq_thread+0x11c/0x1dc
> ...
> 2017-01-20T00:32:49.856042-08:00 EMERG kernel: [   12.136156] Call
> trace:
> 2017-01-20T00:32:49.856044-08:00 WARNING kernel: [   12.138584]
> [<ffffffc000249d84>] __might_sleep+0x64/0x90
> 2017-01-20T00:32:49.856047-08:00 WARNING kernel: [   12.143855]
> [<ffffffc000630f54>] __pm_runtime_resume+0x40/0x9c
> 2017-01-20T00:32:49.856049-08:00 WARNING kernel: [   12.149644]
> [<ffffffc0007ae994>] msdc_enable_sdio_irq+0x44/0xd0
> 2017-01-20T00:32:49.856051-08:00 WARNING kernel: [   12.155516]
> [<ffffffc0007a3544>] sdio_irq_thread+0x154/0x1dc
> 2017-01-20T00:32:49.856053-08:00 WARNING kernel: [   12.161131]
> [<ffffffc00023f30c>] kthread+0x10c/0x114
> 2017-01-20T00:32:49.856055-08:00 WARNING kernel: [   12.166056]
> [<ffffffc000203dd0>] ret_from_fork+0x10/0x40
> 2017-01-20T00:32:49.856057-08:00 WARNING kernel: [   12.171368] sched:
> RT throttling activated for rt_rq ffffffc0fff5a5c0 (cpu 1)
> 2017-01-20T00:32:49.856059-08:00 WARNING kernel: [   12.171368]
> potential CPU hogs:
> 2017-01-20T00:32:49.856061-08:00 WARNING kernel: [   12.171368]
>  ksdioirqd/mmc2 (1772)
>
> dw_mmc also enables autosuspend but it didn't call any pm_runtime_get*
> function in it's enable_sdio_irq function, I don't really know how it
> worked around this.
> Ulf, any suggestions on how we can do this?
>
> Wei-Ning
>
>> +               msdc_recheck_sdio_irq(host);
>> +
>> +               spin_lock_irqsave(&host->irqlock, flags);
>> +               sdr_set_bits(host->base + SDC_CFG, SDC_CFG_SDIOIDE);
>> +               sdr_set_bits(host->base + MSDC_INTEN, MSDC_INTEN_SDIOIRQ);
>> +               spin_unlock_irqrestore(&host->irqlock, flags);
>> +       } else {
>> +               spin_lock_irqsave(&host->irqlock, flags);
>> +               sdr_clr_bits(host->base + MSDC_INTEN, MSDC_INTEN_SDIOIRQ);
>> +               spin_unlock_irqrestore(&host->irqlock, flags);
>> +       }
>> +}
>> +
>>  static struct mmc_host_ops mt_msdc_ops = {
>>         .post_req = msdc_post_req,
>>         .pre_req = msdc_pre_req,
>> @@ -1504,6 +1592,7 @@ static void msdc_hw_reset(struct mmc_host *mmc)
>>         .execute_tuning = msdc_execute_tuning,
>>         .prepare_hs400_tuning = msdc_prepare_hs400_tuning,
>>         .hw_reset = msdc_hw_reset,
>> +       .enable_sdio_irq = msdc_enable_sdio_irq,
>>  };
>>
>>  static int msdc_drv_probe(struct platform_device *pdev)
>> @@ -1600,6 +1689,7 @@ static int msdc_drv_probe(struct platform_device *pdev)
>>         mmc_dev(mmc)->dma_mask = &host->dma_mask;
>>
>>         host->timeout_clks = 3 * 1048576;
>> +       host->irq_thread_alive = false;
>>         host->dma.gpd = dma_alloc_coherent(&pdev->dev,
>>                                 2 * sizeof(struct mt_gpdma_desc),
>>                                 &host->dma.gpd_addr, GFP_KERNEL);
>> @@ -1613,6 +1703,7 @@ static int msdc_drv_probe(struct platform_device *pdev)
>>         msdc_init_gpd_bd(host, &host->dma);
>>         INIT_DELAYED_WORK(&host->req_timeout, msdc_request_timeout);
>>         spin_lock_init(&host->lock);
>> +       spin_lock_init(&host->irqlock);
>>
>>         platform_set_drvdata(pdev, mmc);
>>         msdc_ungate_clock(host);
>> --
>> 1.7.9.5
>>
>
>
>
> --
> Wei-Ning Huang, 黃偉寧 | Software Engineer, Google Inc., Taiwan |
> wnhuang@google.com | Cell: +886 910-380678



-- 
Wei-Ning Huang, 黃偉寧 | Software Engineer, Google Inc., Taiwan |
wnhuang@google.com | Cell: +886 910-380678

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-01-23 11:34 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-08  6:08 [PATCH 0/4] Support sdio feature Yong Mao
2016-11-08  6:08 ` [PATCH 1/4] mmc: mediatek: Fix CMD6 timeout issue Yong Mao
2016-12-01  9:51   ` Ulf Hansson
2017-01-03  9:14     ` Yong Mao
2016-11-08  6:08 ` [PATCH 2/4] sdio: mediatek: Support sdio feature Yong Mao
2017-01-23 11:24   ` Wei-Ning Huang
2017-01-23 11:34     ` Wei-Ning Huang
2016-11-08  6:09 ` [PATCH 3/4] sdio: mediatek: support sdr104_clk_delay in sdio Yong Mao
2016-11-08  6:09 ` [PATCH 4/4] dts: arm64: enable mmc3 for supporting sdio feature Yong Mao
2016-11-10  2:08   ` Yingjoe Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).