All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/5] mmc: sdhci-msm: Corrections to implementation of power irq
@ 2017-08-30 12:51 ` Vijay Viswanath
  0 siblings, 0 replies; 33+ messages in thread
From: Vijay Viswanath @ 2017-08-30 12:51 UTC (permalink / raw)
  To: adrian.hunter, ulf.hansson, will.deacon
  Cc: riteshh, linux-arm-msm, linux-mmc, linux-kernel, Vijay Viswanath,
	stummala, subhashj, linux-arm-kernel, asutoshd

Register writes which change voltage of IO lines or turn the IO bus on/off
require sdhc controller to be ready before progressing further. Once a
register write which affects IO lines is done, the driver should wait for
power irq from controller. Once the irq comes, the driver should acknowledge
the irq by writing to power control register. If the acknowledgement is not
given to controller, the controller may not complete the corresponding
register write action and this can mess up the controller if drivers proceeds
without power irq completing.

Changes since RFC:
	wait_for_completion_timeout replaced with wait_event_timeout when
	waiting for power irq.
	Removed the spinlock within power irq handler and API which waits
	for power irq.
	Added comments to sdhci msm register write functions, warning that they
	can sleep.
	Sdhci msm register write functions will do a memory barrier before
	writing to the register if the particular register can trigger
	power irq.
	Instead of enabling SDHCI IO ACCESSORS config in arm64/defconfig, it
	will be selected in mmc/host/Kconfig if the platform is MMC_SDHCI_MSM.


Sahitya Tummala (2):
  mmc: sdhci-msm: Fix HW issue with power IRQ handling during reset
  mmc: sdhci-msm: Add support to wait for power irq

Subhash Jadavani (1):
  mmc: sdhci-msm: fix issue with power irq

Vijay Viswanath (2):
  mmc: sdhci-msm: Add ops to do sdhc register write
  mmc: Kconfig: Enable CONFIG_MMC_SDHCI_IO_ACCESSORS

 drivers/mmc/host/Kconfig     |   1 +
 drivers/mmc/host/sdhci-msm.c | 253 ++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 249 insertions(+), 5 deletions(-)

-- 
 Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

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

* [PATCH v1 0/5] mmc: sdhci-msm: Corrections to implementation of power irq
@ 2017-08-30 12:51 ` Vijay Viswanath
  0 siblings, 0 replies; 33+ messages in thread
From: Vijay Viswanath @ 2017-08-30 12:51 UTC (permalink / raw)
  To: adrian.hunter, ulf.hansson, will.deacon
  Cc: linux-arm-kernel, linux-mmc, linux-kernel, linux-arm-msm,
	asutoshd, stummala, riteshh, subhashj, Vijay Viswanath

Register writes which change voltage of IO lines or turn the IO bus on/off
require sdhc controller to be ready before progressing further. Once a
register write which affects IO lines is done, the driver should wait for
power irq from controller. Once the irq comes, the driver should acknowledge
the irq by writing to power control register. If the acknowledgement is not
given to controller, the controller may not complete the corresponding
register write action and this can mess up the controller if drivers proceeds
without power irq completing.

Changes since RFC:
	wait_for_completion_timeout replaced with wait_event_timeout when
	waiting for power irq.
	Removed the spinlock within power irq handler and API which waits
	for power irq.
	Added comments to sdhci msm register write functions, warning that they
	can sleep.
	Sdhci msm register write functions will do a memory barrier before
	writing to the register if the particular register can trigger
	power irq.
	Instead of enabling SDHCI IO ACCESSORS config in arm64/defconfig, it
	will be selected in mmc/host/Kconfig if the platform is MMC_SDHCI_MSM.


Sahitya Tummala (2):
  mmc: sdhci-msm: Fix HW issue with power IRQ handling during reset
  mmc: sdhci-msm: Add support to wait for power irq

Subhash Jadavani (1):
  mmc: sdhci-msm: fix issue with power irq

Vijay Viswanath (2):
  mmc: sdhci-msm: Add ops to do sdhc register write
  mmc: Kconfig: Enable CONFIG_MMC_SDHCI_IO_ACCESSORS

 drivers/mmc/host/Kconfig     |   1 +
 drivers/mmc/host/sdhci-msm.c | 253 ++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 249 insertions(+), 5 deletions(-)

-- 
 Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

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

* [PATCH v1 0/5] mmc: sdhci-msm: Corrections to implementation of power irq
@ 2017-08-30 12:51 ` Vijay Viswanath
  0 siblings, 0 replies; 33+ messages in thread
From: Vijay Viswanath @ 2017-08-30 12:51 UTC (permalink / raw)
  To: linux-arm-kernel

Register writes which change voltage of IO lines or turn the IO bus on/off
require sdhc controller to be ready before progressing further. Once a
register write which affects IO lines is done, the driver should wait for
power irq from controller. Once the irq comes, the driver should acknowledge
the irq by writing to power control register. If the acknowledgement is not
given to controller, the controller may not complete the corresponding
register write action and this can mess up the controller if drivers proceeds
without power irq completing.

Changes since RFC:
	wait_for_completion_timeout replaced with wait_event_timeout when
	waiting for power irq.
	Removed the spinlock within power irq handler and API which waits
	for power irq.
	Added comments to sdhci msm register write functions, warning that they
	can sleep.
	Sdhci msm register write functions will do a memory barrier before
	writing to the register if the particular register can trigger
	power irq.
	Instead of enabling SDHCI IO ACCESSORS config in arm64/defconfig, it
	will be selected in mmc/host/Kconfig if the platform is MMC_SDHCI_MSM.


Sahitya Tummala (2):
  mmc: sdhci-msm: Fix HW issue with power IRQ handling during reset
  mmc: sdhci-msm: Add support to wait for power irq

Subhash Jadavani (1):
  mmc: sdhci-msm: fix issue with power irq

Vijay Viswanath (2):
  mmc: sdhci-msm: Add ops to do sdhc register write
  mmc: Kconfig: Enable CONFIG_MMC_SDHCI_IO_ACCESSORS

 drivers/mmc/host/Kconfig     |   1 +
 drivers/mmc/host/sdhci-msm.c | 253 ++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 249 insertions(+), 5 deletions(-)

-- 
 Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

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

* [PATCH v1 1/5] mmc: sdhci-msm: fix issue with power irq
  2017-08-30 12:51 ` Vijay Viswanath
  (?)
@ 2017-08-30 12:51   ` Vijay Viswanath
  -1 siblings, 0 replies; 33+ messages in thread
From: Vijay Viswanath @ 2017-08-30 12:51 UTC (permalink / raw)
  To: adrian.hunter, ulf.hansson, will.deacon
  Cc: riteshh, linux-arm-msm, linux-mmc, linux-kernel, Vijay Viswanath,
	stummala, subhashj, linux-arm-kernel, asutoshd

From: Subhash Jadavani <subhashj@codeaurora.org>

SDCC controller reset (SW_RST) during probe may trigger power irq if
previous status of PWRCTL was either BUS_ON or IO_HIGH_V. So before we
enable the power irq interrupt in GIC (by registering the interrupt
handler), we need to ensure that any pending power irq interrupt status
is acknowledged otherwise power irq interrupt handler would be fired
prematurely.

Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>
---
 drivers/mmc/host/sdhci-msm.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 9d601dc..d636251 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -1250,6 +1250,21 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 			       CORE_VENDOR_SPEC_CAPABILITIES0);
 	}
 
+	/*
+	 * Power on reset state may trigger power irq if previous status of
+	 * PWRCTL was either BUS_ON or IO_HIGH_V. So before enabling pwr irq
+	 * interrupt in GIC, any pending power irq interrupt should be
+	 * acknowledged. Otherwise power irq interrupt handler would be
+	 * fired prematurely.
+	 */
+	sdhci_msm_voltage_switch(host);
+
+	/*
+	 * Ensure that above writes are propogated before interrupt enablement
+	 * in GIC.
+	 */
+	mb();
+
 	/* Setup IRQ for handling power/voltage tasks with PMIC */
 	msm_host->pwr_irq = platform_get_irq_byname(pdev, "pwr_irq");
 	if (msm_host->pwr_irq < 0) {
@@ -1259,6 +1274,9 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 		goto clk_disable;
 	}
 
+	/* Enable pwr irq interrupts */
+	writel_relaxed(INT_MASK, msm_host->core_mem + CORE_PWRCTL_MASK);
+
 	ret = devm_request_threaded_irq(&pdev->dev, msm_host->pwr_irq, NULL,
 					sdhci_msm_pwr_irq, IRQF_ONESHOT,
 					dev_name(&pdev->dev), host);
-- 
 Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

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

* [PATCH v1 1/5] mmc: sdhci-msm: fix issue with power irq
@ 2017-08-30 12:51   ` Vijay Viswanath
  0 siblings, 0 replies; 33+ messages in thread
From: Vijay Viswanath @ 2017-08-30 12:51 UTC (permalink / raw)
  To: adrian.hunter, ulf.hansson, will.deacon
  Cc: linux-arm-kernel, linux-mmc, linux-kernel, linux-arm-msm,
	asutoshd, stummala, riteshh, subhashj, Vijay Viswanath

From: Subhash Jadavani <subhashj@codeaurora.org>

SDCC controller reset (SW_RST) during probe may trigger power irq if
previous status of PWRCTL was either BUS_ON or IO_HIGH_V. So before we
enable the power irq interrupt in GIC (by registering the interrupt
handler), we need to ensure that any pending power irq interrupt status
is acknowledged otherwise power irq interrupt handler would be fired
prematurely.

Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>
---
 drivers/mmc/host/sdhci-msm.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 9d601dc..d636251 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -1250,6 +1250,21 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 			       CORE_VENDOR_SPEC_CAPABILITIES0);
 	}
 
+	/*
+	 * Power on reset state may trigger power irq if previous status of
+	 * PWRCTL was either BUS_ON or IO_HIGH_V. So before enabling pwr irq
+	 * interrupt in GIC, any pending power irq interrupt should be
+	 * acknowledged. Otherwise power irq interrupt handler would be
+	 * fired prematurely.
+	 */
+	sdhci_msm_voltage_switch(host);
+
+	/*
+	 * Ensure that above writes are propogated before interrupt enablement
+	 * in GIC.
+	 */
+	mb();
+
 	/* Setup IRQ for handling power/voltage tasks with PMIC */
 	msm_host->pwr_irq = platform_get_irq_byname(pdev, "pwr_irq");
 	if (msm_host->pwr_irq < 0) {
@@ -1259,6 +1274,9 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 		goto clk_disable;
 	}
 
+	/* Enable pwr irq interrupts */
+	writel_relaxed(INT_MASK, msm_host->core_mem + CORE_PWRCTL_MASK);
+
 	ret = devm_request_threaded_irq(&pdev->dev, msm_host->pwr_irq, NULL,
 					sdhci_msm_pwr_irq, IRQF_ONESHOT,
 					dev_name(&pdev->dev), host);
-- 
 Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

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

* [PATCH v1 1/5] mmc: sdhci-msm: fix issue with power irq
@ 2017-08-30 12:51   ` Vijay Viswanath
  0 siblings, 0 replies; 33+ messages in thread
From: Vijay Viswanath @ 2017-08-30 12:51 UTC (permalink / raw)
  To: linux-arm-kernel

From: Subhash Jadavani <subhashj@codeaurora.org>

SDCC controller reset (SW_RST) during probe may trigger power irq if
previous status of PWRCTL was either BUS_ON or IO_HIGH_V. So before we
enable the power irq interrupt in GIC (by registering the interrupt
handler), we need to ensure that any pending power irq interrupt status
is acknowledged otherwise power irq interrupt handler would be fired
prematurely.

Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>
---
 drivers/mmc/host/sdhci-msm.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 9d601dc..d636251 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -1250,6 +1250,21 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 			       CORE_VENDOR_SPEC_CAPABILITIES0);
 	}
 
+	/*
+	 * Power on reset state may trigger power irq if previous status of
+	 * PWRCTL was either BUS_ON or IO_HIGH_V. So before enabling pwr irq
+	 * interrupt in GIC, any pending power irq interrupt should be
+	 * acknowledged. Otherwise power irq interrupt handler would be
+	 * fired prematurely.
+	 */
+	sdhci_msm_voltage_switch(host);
+
+	/*
+	 * Ensure that above writes are propogated before interrupt enablement
+	 * in GIC.
+	 */
+	mb();
+
 	/* Setup IRQ for handling power/voltage tasks with PMIC */
 	msm_host->pwr_irq = platform_get_irq_byname(pdev, "pwr_irq");
 	if (msm_host->pwr_irq < 0) {
@@ -1259,6 +1274,9 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 		goto clk_disable;
 	}
 
+	/* Enable pwr irq interrupts */
+	writel_relaxed(INT_MASK, msm_host->core_mem + CORE_PWRCTL_MASK);
+
 	ret = devm_request_threaded_irq(&pdev->dev, msm_host->pwr_irq, NULL,
 					sdhci_msm_pwr_irq, IRQF_ONESHOT,
 					dev_name(&pdev->dev), host);
-- 
 Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

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

* [PATCH v1 2/5] mmc: sdhci-msm: Fix HW issue with power IRQ handling during reset
  2017-08-30 12:51 ` Vijay Viswanath
@ 2017-08-30 12:51   ` Vijay Viswanath
  -1 siblings, 0 replies; 33+ messages in thread
From: Vijay Viswanath @ 2017-08-30 12:51 UTC (permalink / raw)
  To: adrian.hunter, ulf.hansson, will.deacon
  Cc: linux-arm-kernel, linux-mmc, linux-kernel, linux-arm-msm,
	asutoshd, stummala, riteshh, subhashj, Vijay Viswanath

From: Sahitya Tummala <stummala@codeaurora.org>

There is a rare scenario in HW, where the first clear pulse could
be lost when the actual reset and clear/read of status register
are happening at the same time. Fix this by retrying upto 10 times
to ensure the status register gets cleared. Otherwise, this will
lead to a spurious power IRQ which results in system instability.

Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>
---
 drivers/mmc/host/sdhci-msm.c | 46 ++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 42 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index d636251..42a65ab 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -995,17 +995,52 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
 		sdhci_msm_hs400(host, &mmc->ios);
 }
 
-static void sdhci_msm_voltage_switch(struct sdhci_host *host)
+static void sdhci_msm_dump_pwr_ctrl_regs(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+
+	pr_err("%s: PWRCTL_STATUS: 0x%08x | PWRCTL_MASK: 0x%08x | PWRCTL_CTL: 0x%08x\n",
+			mmc_hostname(host->mmc),
+			readl_relaxed(msm_host->core_mem + CORE_PWRCTL_STATUS),
+			readl_relaxed(msm_host->core_mem + CORE_PWRCTL_MASK),
+			readl_relaxed(msm_host->core_mem + CORE_PWRCTL_CTL));
+}
+
+static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
 	u32 irq_status, irq_ack = 0;
+	int retry = 10;
 
 	irq_status = readl_relaxed(msm_host->core_mem + CORE_PWRCTL_STATUS);
 	irq_status &= INT_MASK;
 
 	writel_relaxed(irq_status, msm_host->core_mem + CORE_PWRCTL_CLEAR);
 
+	/*
+	 * There is a rare HW scenario where the first clear pulse could be
+	 * lost when actual reset and clear/read of status register is
+	 * happening at a time. Hence, retry for at least 10 times to make
+	 * sure status register is cleared. Otherwise, this will result in
+	 * a spurious power IRQ resulting in system instability.
+	 */
+	while (irq_status & readl_relaxed(msm_host->core_mem +
+				CORE_PWRCTL_STATUS)) {
+		if (retry == 0) {
+			pr_err("%s: Timedout clearing (0x%x) pwrctl status register\n",
+					mmc_hostname(host->mmc), irq_status);
+			sdhci_msm_dump_pwr_ctrl_regs(host);
+			WARN_ON(1);
+			break;
+		}
+		writel_relaxed(irq_status,
+				msm_host->core_mem + CORE_PWRCTL_CLEAR);
+		retry--;
+		udelay(10);
+	}
+
 	if (irq_status & (CORE_PWRCTL_BUS_ON | CORE_PWRCTL_BUS_OFF))
 		irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
 	if (irq_status & (CORE_PWRCTL_IO_LOW | CORE_PWRCTL_IO_HIGH))
@@ -1017,13 +1052,17 @@ static void sdhci_msm_voltage_switch(struct sdhci_host *host)
 	 * switches are handled by the sdhci core, so just report success.
 	 */
 	writel_relaxed(irq_ack, msm_host->core_mem + CORE_PWRCTL_CTL);
+
+	pr_debug("%s: %s: Handled IRQ(%d), irq_status=0x%x, ack=0x%x\n",
+		mmc_hostname(msm_host->mmc), __func__, irq, irq_status,
+		irq_ack);
 }
 
 static irqreturn_t sdhci_msm_pwr_irq(int irq, void *data)
 {
 	struct sdhci_host *host = (struct sdhci_host *)data;
 
-	sdhci_msm_voltage_switch(host);
+	sdhci_msm_handle_pwr_irq(host, irq);
 
 	return IRQ_HANDLED;
 }
@@ -1106,7 +1145,6 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
 	.get_max_clock = sdhci_msm_get_max_clock,
 	.set_bus_width = sdhci_set_bus_width,
 	.set_uhs_signaling = sdhci_msm_set_uhs_signaling,
-	.voltage_switch = sdhci_msm_voltage_switch,
 };
 
 static const struct sdhci_pltfm_data sdhci_msm_pdata = {
@@ -1257,7 +1295,7 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 	 * acknowledged. Otherwise power irq interrupt handler would be
 	 * fired prematurely.
 	 */
-	sdhci_msm_voltage_switch(host);
+	sdhci_msm_handle_pwr_irq(host, 0);
 
 	/*
 	 * Ensure that above writes are propogated before interrupt enablement
-- 
 Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

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

* [PATCH v1 2/5] mmc: sdhci-msm: Fix HW issue with power IRQ handling during reset
@ 2017-08-30 12:51   ` Vijay Viswanath
  0 siblings, 0 replies; 33+ messages in thread
From: Vijay Viswanath @ 2017-08-30 12:51 UTC (permalink / raw)
  To: linux-arm-kernel

From: Sahitya Tummala <stummala@codeaurora.org>

There is a rare scenario in HW, where the first clear pulse could
be lost when the actual reset and clear/read of status register
are happening at the same time. Fix this by retrying upto 10 times
to ensure the status register gets cleared. Otherwise, this will
lead to a spurious power IRQ which results in system instability.

Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>
---
 drivers/mmc/host/sdhci-msm.c | 46 ++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 42 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index d636251..42a65ab 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -995,17 +995,52 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
 		sdhci_msm_hs400(host, &mmc->ios);
 }
 
-static void sdhci_msm_voltage_switch(struct sdhci_host *host)
+static void sdhci_msm_dump_pwr_ctrl_regs(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+
+	pr_err("%s: PWRCTL_STATUS: 0x%08x | PWRCTL_MASK: 0x%08x | PWRCTL_CTL: 0x%08x\n",
+			mmc_hostname(host->mmc),
+			readl_relaxed(msm_host->core_mem + CORE_PWRCTL_STATUS),
+			readl_relaxed(msm_host->core_mem + CORE_PWRCTL_MASK),
+			readl_relaxed(msm_host->core_mem + CORE_PWRCTL_CTL));
+}
+
+static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
 	u32 irq_status, irq_ack = 0;
+	int retry = 10;
 
 	irq_status = readl_relaxed(msm_host->core_mem + CORE_PWRCTL_STATUS);
 	irq_status &= INT_MASK;
 
 	writel_relaxed(irq_status, msm_host->core_mem + CORE_PWRCTL_CLEAR);
 
+	/*
+	 * There is a rare HW scenario where the first clear pulse could be
+	 * lost when actual reset and clear/read of status register is
+	 * happening at a time. Hence, retry for at least 10 times to make
+	 * sure status register is cleared. Otherwise, this will result in
+	 * a spurious power IRQ resulting in system instability.
+	 */
+	while (irq_status & readl_relaxed(msm_host->core_mem +
+				CORE_PWRCTL_STATUS)) {
+		if (retry == 0) {
+			pr_err("%s: Timedout clearing (0x%x) pwrctl status register\n",
+					mmc_hostname(host->mmc), irq_status);
+			sdhci_msm_dump_pwr_ctrl_regs(host);
+			WARN_ON(1);
+			break;
+		}
+		writel_relaxed(irq_status,
+				msm_host->core_mem + CORE_PWRCTL_CLEAR);
+		retry--;
+		udelay(10);
+	}
+
 	if (irq_status & (CORE_PWRCTL_BUS_ON | CORE_PWRCTL_BUS_OFF))
 		irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
 	if (irq_status & (CORE_PWRCTL_IO_LOW | CORE_PWRCTL_IO_HIGH))
@@ -1017,13 +1052,17 @@ static void sdhci_msm_voltage_switch(struct sdhci_host *host)
 	 * switches are handled by the sdhci core, so just report success.
 	 */
 	writel_relaxed(irq_ack, msm_host->core_mem + CORE_PWRCTL_CTL);
+
+	pr_debug("%s: %s: Handled IRQ(%d), irq_status=0x%x, ack=0x%x\n",
+		mmc_hostname(msm_host->mmc), __func__, irq, irq_status,
+		irq_ack);
 }
 
 static irqreturn_t sdhci_msm_pwr_irq(int irq, void *data)
 {
 	struct sdhci_host *host = (struct sdhci_host *)data;
 
-	sdhci_msm_voltage_switch(host);
+	sdhci_msm_handle_pwr_irq(host, irq);
 
 	return IRQ_HANDLED;
 }
@@ -1106,7 +1145,6 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
 	.get_max_clock = sdhci_msm_get_max_clock,
 	.set_bus_width = sdhci_set_bus_width,
 	.set_uhs_signaling = sdhci_msm_set_uhs_signaling,
-	.voltage_switch = sdhci_msm_voltage_switch,
 };
 
 static const struct sdhci_pltfm_data sdhci_msm_pdata = {
@@ -1257,7 +1295,7 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 	 * acknowledged. Otherwise power irq interrupt handler would be
 	 * fired prematurely.
 	 */
-	sdhci_msm_voltage_switch(host);
+	sdhci_msm_handle_pwr_irq(host, 0);
 
 	/*
 	 * Ensure that above writes are propogated before interrupt enablement
-- 
 Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

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

* [PATCH v1 3/5] mmc: sdhci-msm: Add support to wait for power irq
  2017-08-30 12:51 ` Vijay Viswanath
@ 2017-08-30 12:51   ` Vijay Viswanath
  -1 siblings, 0 replies; 33+ messages in thread
From: Vijay Viswanath @ 2017-08-30 12:51 UTC (permalink / raw)
  To: adrian.hunter, ulf.hansson, will.deacon
  Cc: linux-arm-kernel, linux-mmc, linux-kernel, linux-arm-msm,
	asutoshd, stummala, riteshh, subhashj, Vijay Viswanath

From: Sahitya Tummala <stummala@codeaurora.org>

Add support API which will check if power irq is expected to be
generated and wait for the power irq to come and complete if the irq is
expected.

Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>
---
 drivers/mmc/host/sdhci-msm.c | 124 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 122 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 42a65ab..e3e385e 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -123,6 +123,10 @@
 #define CMUX_SHIFT_PHASE_MASK	(7 << CMUX_SHIFT_PHASE_SHIFT)
 
 #define MSM_MMC_AUTOSUSPEND_DELAY_MS	50
+
+/* Timeout value to avoid infinite waiting for pwr_irq */
+#define MSM_PWR_IRQ_TIMEOUT_MS 5000
+
 struct sdhci_msm_host {
 	struct platform_device *pdev;
 	void __iomem *core_mem;	/* MSM SDCC mapped address */
@@ -138,6 +142,12 @@ struct sdhci_msm_host {
 	bool calibration_done;
 	u8 saved_tuning_phase;
 	bool use_cdclp533;
+	u32 curr_pwr_state;
+	u32 curr_io_level;
+#ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
+	wait_queue_head_t pwr_irq_wait;
+	bool pwr_irq_flag;
+#endif
 };
 
 static unsigned int msm_get_clock_rate_for_bus_mode(struct sdhci_host *host,
@@ -995,6 +1005,87 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
 		sdhci_msm_hs400(host, &mmc->ios);
 }
 
+#ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
+static inline void sdhci_msm_init_pwr_irq_wait(struct sdhci_msm_host *msm_host)
+{
+	init_waitqueue_head(&msm_host->pwr_irq_wait);
+}
+
+static inline void sdhci_msm_complete_pwr_irq_wait(
+		struct sdhci_msm_host *msm_host)
+{
+	wake_up(&msm_host->pwr_irq_wait);
+}
+
+/*
+ * sdhci_msm_check_power_status API should be called when registers writes
+ * which can toggle sdhci IO bus ON/OFF or change IO lines HIGH/LOW happens.
+ * To what state the register writes will change the IO lines should be passed
+ * as the argument req_type. This API will check whether the IO line's state
+ * is already the expected state and will wait for power irq only if
+ * power irq is expected to be trigerred based on the current IO line state
+ * and expected IO line state.
+ */
+static void sdhci_msm_check_power_status(struct sdhci_host *host, u32 req_type)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+	bool done = false;
+
+	pr_debug("%s: %s: request %d curr_pwr_state %x curr_io_level %x\n",
+			mmc_hostname(host->mmc), __func__, req_type,
+			msm_host->curr_pwr_state, msm_host->curr_io_level);
+
+	/*
+	 * The IRQ for request type IO High/LOW will be generated when -
+	 * there is a state change in 1.8V enable bit (bit 3) of
+	 * SDHCI_HOST_CONTROL2 register. The reset state of that bit is 0
+	 * which indicates 3.3V IO voltage. So, when MMC core layer tries
+	 * to set it to 3.3V before card detection happens, the
+	 * IRQ doesn't get triggered as there is no state change in this bit.
+	 * The driver already handles this case by changing the IO voltage
+	 * level to high as part of controller power up sequence. Hence, check
+	 * for host->pwr to handle a case where IO voltage high request is
+	 * issued even before controller power up.
+	 */
+	if ((req_type & REQ_IO_HIGH) && !host->pwr) {
+		pr_debug("%s: do not wait for power IRQ that never comes, req_type: %d\n",
+				mmc_hostname(host->mmc), req_type);
+		return;
+	}
+	if ((req_type & msm_host->curr_pwr_state) ||
+			(req_type & msm_host->curr_io_level))
+		done = true;
+	/*
+	 * This is needed here to hanlde a case where IRQ gets
+	 * triggered even before this function is called so that
+	 * x->done counter of completion gets reset. Otherwise,
+	 * next call to wait_for_completion returns immediately
+	 * without actually waiting for the IRQ to be handled.
+	 */
+	if (!done) {
+		if (!wait_event_timeout(msm_host->pwr_irq_wait,
+				msm_host->pwr_irq_flag,
+				msecs_to_jiffies(MSM_PWR_IRQ_TIMEOUT_MS)))
+			__WARN_printf("%s: pwr_irq for req: (%d) timed out\n",
+					mmc_hostname(host->mmc), req_type);
+	}
+	msm_host->pwr_irq_flag = 0;
+	pr_debug("%s: %s: request %d done\n", mmc_hostname(host->mmc),
+			__func__, req_type);
+}
+#else
+static inline void sdhci_msm_init_pwr_irq_completion(
+		struct sdhci_msm_host *msm_host)
+{
+}
+
+static inline void sdhci_msm_complete_pwr_irq_completion(
+		struct sdhci_msm_host *msm_host)
+{
+}
+#endif
+
 static void sdhci_msm_dump_pwr_ctrl_regs(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -1013,6 +1104,8 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
 	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
 	u32 irq_status, irq_ack = 0;
 	int retry = 10;
+	int pwr_state = 0, io_level = 0;
+
 
 	irq_status = readl_relaxed(msm_host->core_mem + CORE_PWRCTL_STATUS);
 	irq_status &= INT_MASK;
@@ -1041,10 +1134,26 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
 		udelay(10);
 	}
 
-	if (irq_status & (CORE_PWRCTL_BUS_ON | CORE_PWRCTL_BUS_OFF))
+	/* Handle BUS ON/OFF*/
+	if (irq_status & CORE_PWRCTL_BUS_ON) {
+		pwr_state = REQ_BUS_ON;
+		io_level = REQ_IO_HIGH;
 		irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
-	if (irq_status & (CORE_PWRCTL_IO_LOW | CORE_PWRCTL_IO_HIGH))
+	}
+	if (irq_status & CORE_PWRCTL_BUS_OFF) {
+		pwr_state = REQ_BUS_OFF;
+		io_level = REQ_IO_LOW;
+		irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
+	}
+	/* Handle IO LOW/HIGH */
+	if (irq_status & CORE_PWRCTL_IO_LOW) {
+		io_level = REQ_IO_LOW;
+		irq_ack |= CORE_PWRCTL_IO_SUCCESS;
+	}
+	if (irq_status & CORE_PWRCTL_IO_HIGH) {
+		io_level = REQ_IO_HIGH;
 		irq_ack |= CORE_PWRCTL_IO_SUCCESS;
+	}
 
 	/*
 	 * The driver has to acknowledge the interrupt, switch voltages and
@@ -1053,6 +1162,11 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
 	 */
 	writel_relaxed(irq_ack, msm_host->core_mem + CORE_PWRCTL_CTL);
 
+	if (pwr_state)
+		msm_host->curr_pwr_state = pwr_state;
+	if (io_level)
+		msm_host->curr_io_level = io_level;
+
 	pr_debug("%s: %s: Handled IRQ(%d), irq_status=0x%x, ack=0x%x\n",
 		mmc_hostname(msm_host->mmc), __func__, irq, irq_status,
 		irq_ack);
@@ -1061,8 +1175,13 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
 static irqreturn_t sdhci_msm_pwr_irq(int irq, void *data)
 {
 	struct sdhci_host *host = (struct sdhci_host *)data;
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
 
 	sdhci_msm_handle_pwr_irq(host, irq);
+	msm_host->pwr_irq_flag = 1;
+	sdhci_msm_complete_pwr_irq_wait(msm_host);
+
 
 	return IRQ_HANDLED;
 }
@@ -1312,6 +1431,7 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 		goto clk_disable;
 	}
 
+	sdhci_msm_init_pwr_irq_wait(msm_host);
 	/* Enable pwr irq interrupts */
 	writel_relaxed(INT_MASK, msm_host->core_mem + CORE_PWRCTL_MASK);
 
-- 
 Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

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

* [PATCH v1 3/5] mmc: sdhci-msm: Add support to wait for power irq
@ 2017-08-30 12:51   ` Vijay Viswanath
  0 siblings, 0 replies; 33+ messages in thread
From: Vijay Viswanath @ 2017-08-30 12:51 UTC (permalink / raw)
  To: linux-arm-kernel

From: Sahitya Tummala <stummala@codeaurora.org>

Add support API which will check if power irq is expected to be
generated and wait for the power irq to come and complete if the irq is
expected.

Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>
---
 drivers/mmc/host/sdhci-msm.c | 124 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 122 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 42a65ab..e3e385e 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -123,6 +123,10 @@
 #define CMUX_SHIFT_PHASE_MASK	(7 << CMUX_SHIFT_PHASE_SHIFT)
 
 #define MSM_MMC_AUTOSUSPEND_DELAY_MS	50
+
+/* Timeout value to avoid infinite waiting for pwr_irq */
+#define MSM_PWR_IRQ_TIMEOUT_MS 5000
+
 struct sdhci_msm_host {
 	struct platform_device *pdev;
 	void __iomem *core_mem;	/* MSM SDCC mapped address */
@@ -138,6 +142,12 @@ struct sdhci_msm_host {
 	bool calibration_done;
 	u8 saved_tuning_phase;
 	bool use_cdclp533;
+	u32 curr_pwr_state;
+	u32 curr_io_level;
+#ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
+	wait_queue_head_t pwr_irq_wait;
+	bool pwr_irq_flag;
+#endif
 };
 
 static unsigned int msm_get_clock_rate_for_bus_mode(struct sdhci_host *host,
@@ -995,6 +1005,87 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
 		sdhci_msm_hs400(host, &mmc->ios);
 }
 
+#ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
+static inline void sdhci_msm_init_pwr_irq_wait(struct sdhci_msm_host *msm_host)
+{
+	init_waitqueue_head(&msm_host->pwr_irq_wait);
+}
+
+static inline void sdhci_msm_complete_pwr_irq_wait(
+		struct sdhci_msm_host *msm_host)
+{
+	wake_up(&msm_host->pwr_irq_wait);
+}
+
+/*
+ * sdhci_msm_check_power_status API should be called when registers writes
+ * which can toggle sdhci IO bus ON/OFF or change IO lines HIGH/LOW happens.
+ * To what state the register writes will change the IO lines should be passed
+ * as the argument req_type. This API will check whether the IO line's state
+ * is already the expected state and will wait for power irq only if
+ * power irq is expected to be trigerred based on the current IO line state
+ * and expected IO line state.
+ */
+static void sdhci_msm_check_power_status(struct sdhci_host *host, u32 req_type)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+	bool done = false;
+
+	pr_debug("%s: %s: request %d curr_pwr_state %x curr_io_level %x\n",
+			mmc_hostname(host->mmc), __func__, req_type,
+			msm_host->curr_pwr_state, msm_host->curr_io_level);
+
+	/*
+	 * The IRQ for request type IO High/LOW will be generated when -
+	 * there is a state change in 1.8V enable bit (bit 3) of
+	 * SDHCI_HOST_CONTROL2 register. The reset state of that bit is 0
+	 * which indicates 3.3V IO voltage. So, when MMC core layer tries
+	 * to set it to 3.3V before card detection happens, the
+	 * IRQ doesn't get triggered as there is no state change in this bit.
+	 * The driver already handles this case by changing the IO voltage
+	 * level to high as part of controller power up sequence. Hence, check
+	 * for host->pwr to handle a case where IO voltage high request is
+	 * issued even before controller power up.
+	 */
+	if ((req_type & REQ_IO_HIGH) && !host->pwr) {
+		pr_debug("%s: do not wait for power IRQ that never comes, req_type: %d\n",
+				mmc_hostname(host->mmc), req_type);
+		return;
+	}
+	if ((req_type & msm_host->curr_pwr_state) ||
+			(req_type & msm_host->curr_io_level))
+		done = true;
+	/*
+	 * This is needed here to hanlde a case where IRQ gets
+	 * triggered even before this function is called so that
+	 * x->done counter of completion gets reset. Otherwise,
+	 * next call to wait_for_completion returns immediately
+	 * without actually waiting for the IRQ to be handled.
+	 */
+	if (!done) {
+		if (!wait_event_timeout(msm_host->pwr_irq_wait,
+				msm_host->pwr_irq_flag,
+				msecs_to_jiffies(MSM_PWR_IRQ_TIMEOUT_MS)))
+			__WARN_printf("%s: pwr_irq for req: (%d) timed out\n",
+					mmc_hostname(host->mmc), req_type);
+	}
+	msm_host->pwr_irq_flag = 0;
+	pr_debug("%s: %s: request %d done\n", mmc_hostname(host->mmc),
+			__func__, req_type);
+}
+#else
+static inline void sdhci_msm_init_pwr_irq_completion(
+		struct sdhci_msm_host *msm_host)
+{
+}
+
+static inline void sdhci_msm_complete_pwr_irq_completion(
+		struct sdhci_msm_host *msm_host)
+{
+}
+#endif
+
 static void sdhci_msm_dump_pwr_ctrl_regs(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -1013,6 +1104,8 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
 	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
 	u32 irq_status, irq_ack = 0;
 	int retry = 10;
+	int pwr_state = 0, io_level = 0;
+
 
 	irq_status = readl_relaxed(msm_host->core_mem + CORE_PWRCTL_STATUS);
 	irq_status &= INT_MASK;
@@ -1041,10 +1134,26 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
 		udelay(10);
 	}
 
-	if (irq_status & (CORE_PWRCTL_BUS_ON | CORE_PWRCTL_BUS_OFF))
+	/* Handle BUS ON/OFF*/
+	if (irq_status & CORE_PWRCTL_BUS_ON) {
+		pwr_state = REQ_BUS_ON;
+		io_level = REQ_IO_HIGH;
 		irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
-	if (irq_status & (CORE_PWRCTL_IO_LOW | CORE_PWRCTL_IO_HIGH))
+	}
+	if (irq_status & CORE_PWRCTL_BUS_OFF) {
+		pwr_state = REQ_BUS_OFF;
+		io_level = REQ_IO_LOW;
+		irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
+	}
+	/* Handle IO LOW/HIGH */
+	if (irq_status & CORE_PWRCTL_IO_LOW) {
+		io_level = REQ_IO_LOW;
+		irq_ack |= CORE_PWRCTL_IO_SUCCESS;
+	}
+	if (irq_status & CORE_PWRCTL_IO_HIGH) {
+		io_level = REQ_IO_HIGH;
 		irq_ack |= CORE_PWRCTL_IO_SUCCESS;
+	}
 
 	/*
 	 * The driver has to acknowledge the interrupt, switch voltages and
@@ -1053,6 +1162,11 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
 	 */
 	writel_relaxed(irq_ack, msm_host->core_mem + CORE_PWRCTL_CTL);
 
+	if (pwr_state)
+		msm_host->curr_pwr_state = pwr_state;
+	if (io_level)
+		msm_host->curr_io_level = io_level;
+
 	pr_debug("%s: %s: Handled IRQ(%d), irq_status=0x%x, ack=0x%x\n",
 		mmc_hostname(msm_host->mmc), __func__, irq, irq_status,
 		irq_ack);
@@ -1061,8 +1175,13 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
 static irqreturn_t sdhci_msm_pwr_irq(int irq, void *data)
 {
 	struct sdhci_host *host = (struct sdhci_host *)data;
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
 
 	sdhci_msm_handle_pwr_irq(host, irq);
+	msm_host->pwr_irq_flag = 1;
+	sdhci_msm_complete_pwr_irq_wait(msm_host);
+
 
 	return IRQ_HANDLED;
 }
@@ -1312,6 +1431,7 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 		goto clk_disable;
 	}
 
+	sdhci_msm_init_pwr_irq_wait(msm_host);
 	/* Enable pwr irq interrupts */
 	writel_relaxed(INT_MASK, msm_host->core_mem + CORE_PWRCTL_MASK);
 
-- 
 Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

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

* [PATCH v1 4/5] mmc: sdhci-msm: Add ops to do sdhc register write
  2017-08-30 12:51 ` Vijay Viswanath
@ 2017-08-30 12:51   ` Vijay Viswanath
  -1 siblings, 0 replies; 33+ messages in thread
From: Vijay Viswanath @ 2017-08-30 12:51 UTC (permalink / raw)
  To: adrian.hunter, ulf.hansson, will.deacon
  Cc: linux-arm-kernel, linux-mmc, linux-kernel, linux-arm-msm,
	asutoshd, stummala, riteshh, subhashj, Vijay Viswanath

Register writes which change voltage of IO lines or turn the IO bus
on/off require controller to be ready before progressing further. When
the controller is ready, it will generate a power irq which needs to be
handled. The thread which initiated the register write should wait for
power irq to complete. This will be done through the new sdhc msm write
APIs which will check whether the particular write can trigger a power
irq and wait for it with a timeout if it is expected.
The SDHC core power control IRQ gets triggered when -
* There is a state change in power control bit (bit 0)
  of SDHCI_POWER_CONTROL register.
* There is a state change in 1.8V enable bit (bit 3) of
  SDHCI_HOST_CONTROL2 register.
* Bit 1 of SDHCI_SOFTWARE_RESET is set.

Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>
---
 drivers/mmc/host/sdhci-msm.c | 69 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 68 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index e3e385e..a4a78b5 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -1070,7 +1070,6 @@ static void sdhci_msm_check_power_status(struct sdhci_host *host, u32 req_type)
 			__WARN_printf("%s: pwr_irq for req: (%d) timed out\n",
 					mmc_hostname(host->mmc), req_type);
 	}
-	msm_host->pwr_irq_flag = 0;
 	pr_debug("%s: %s: request %d done\n", mmc_hostname(host->mmc),
 			__func__, req_type);
 }
@@ -1250,6 +1249,70 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
 	__sdhci_msm_set_clock(host, clock);
 }
 
+#ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
+/*
+ * Platform specific register write functions. This is so that, if any
+ * register write needs to be followed up by platform specific actions,
+ * they can be added here. These functions can go to sleep when writes
+ * to certain registers are done.
+ * These functions are relying on sdhci_set_ios not using spinlock.
+ */
+static int __sdhci_msm_check_write(struct sdhci_host *host, u16 val, int reg)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+	u32 req_type = 0;
+
+	switch (reg) {
+	case SDHCI_HOST_CONTROL2:
+		req_type = (val & SDHCI_CTRL_VDD_180) ? REQ_IO_LOW :
+			REQ_IO_HIGH;
+		break;
+	case SDHCI_SOFTWARE_RESET:
+		if (host->pwr && (val & SDHCI_RESET_ALL))
+			req_type = REQ_BUS_OFF;
+		break;
+	case SDHCI_POWER_CONTROL:
+		req_type = !val ? REQ_BUS_OFF : REQ_BUS_ON;
+		break;
+	}
+
+	if (req_type) {
+		msm_host->pwr_irq_flag = 0;
+		/*
+		 * Since this register write may trigger a power irq, ensure
+		 * all previous register writes are complete by this point.
+		 */
+		mb();
+	}
+	return req_type;
+}
+
+/* This function may sleep*/
+static void sdhci_msm_writew(struct sdhci_host *host, u16 val, int reg)
+{
+	u32 req_type = 0;
+
+	req_type = __sdhci_msm_check_write(host, val, reg);
+	writew_relaxed(val, host->ioaddr + reg);
+
+	if (req_type)
+		sdhci_msm_check_power_status(host, req_type);
+}
+
+/* This function may sleep*/
+static void sdhci_msm_writeb(struct sdhci_host *host, u8 val, int reg)
+{
+	u32 req_type = 0;
+
+	req_type = __sdhci_msm_check_write(host, val, reg);
+
+	writeb_relaxed(val, host->ioaddr + reg);
+
+	if (req_type)
+		sdhci_msm_check_power_status(host, req_type);
+}
+#endif
 static const struct of_device_id sdhci_msm_dt_match[] = {
 	{ .compatible = "qcom,sdhci-msm-v4" },
 	{},
@@ -1264,6 +1327,10 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
 	.get_max_clock = sdhci_msm_get_max_clock,
 	.set_bus_width = sdhci_set_bus_width,
 	.set_uhs_signaling = sdhci_msm_set_uhs_signaling,
+#ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
+	.write_w = sdhci_msm_writew,
+	.write_b = sdhci_msm_writeb,
+#endif
 };
 
 static const struct sdhci_pltfm_data sdhci_msm_pdata = {
-- 
 Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

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

* [PATCH v1 4/5] mmc: sdhci-msm: Add ops to do sdhc register write
@ 2017-08-30 12:51   ` Vijay Viswanath
  0 siblings, 0 replies; 33+ messages in thread
From: Vijay Viswanath @ 2017-08-30 12:51 UTC (permalink / raw)
  To: linux-arm-kernel

Register writes which change voltage of IO lines or turn the IO bus
on/off require controller to be ready before progressing further. When
the controller is ready, it will generate a power irq which needs to be
handled. The thread which initiated the register write should wait for
power irq to complete. This will be done through the new sdhc msm write
APIs which will check whether the particular write can trigger a power
irq and wait for it with a timeout if it is expected.
The SDHC core power control IRQ gets triggered when -
* There is a state change in power control bit (bit 0)
  of SDHCI_POWER_CONTROL register.
* There is a state change in 1.8V enable bit (bit 3) of
  SDHCI_HOST_CONTROL2 register.
* Bit 1 of SDHCI_SOFTWARE_RESET is set.

Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>
---
 drivers/mmc/host/sdhci-msm.c | 69 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 68 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index e3e385e..a4a78b5 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -1070,7 +1070,6 @@ static void sdhci_msm_check_power_status(struct sdhci_host *host, u32 req_type)
 			__WARN_printf("%s: pwr_irq for req: (%d) timed out\n",
 					mmc_hostname(host->mmc), req_type);
 	}
-	msm_host->pwr_irq_flag = 0;
 	pr_debug("%s: %s: request %d done\n", mmc_hostname(host->mmc),
 			__func__, req_type);
 }
@@ -1250,6 +1249,70 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
 	__sdhci_msm_set_clock(host, clock);
 }
 
+#ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
+/*
+ * Platform specific register write functions. This is so that, if any
+ * register write needs to be followed up by platform specific actions,
+ * they can be added here. These functions can go to sleep when writes
+ * to certain registers are done.
+ * These functions are relying on sdhci_set_ios not using spinlock.
+ */
+static int __sdhci_msm_check_write(struct sdhci_host *host, u16 val, int reg)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+	u32 req_type = 0;
+
+	switch (reg) {
+	case SDHCI_HOST_CONTROL2:
+		req_type = (val & SDHCI_CTRL_VDD_180) ? REQ_IO_LOW :
+			REQ_IO_HIGH;
+		break;
+	case SDHCI_SOFTWARE_RESET:
+		if (host->pwr && (val & SDHCI_RESET_ALL))
+			req_type = REQ_BUS_OFF;
+		break;
+	case SDHCI_POWER_CONTROL:
+		req_type = !val ? REQ_BUS_OFF : REQ_BUS_ON;
+		break;
+	}
+
+	if (req_type) {
+		msm_host->pwr_irq_flag = 0;
+		/*
+		 * Since this register write may trigger a power irq, ensure
+		 * all previous register writes are complete by this point.
+		 */
+		mb();
+	}
+	return req_type;
+}
+
+/* This function may sleep*/
+static void sdhci_msm_writew(struct sdhci_host *host, u16 val, int reg)
+{
+	u32 req_type = 0;
+
+	req_type = __sdhci_msm_check_write(host, val, reg);
+	writew_relaxed(val, host->ioaddr + reg);
+
+	if (req_type)
+		sdhci_msm_check_power_status(host, req_type);
+}
+
+/* This function may sleep*/
+static void sdhci_msm_writeb(struct sdhci_host *host, u8 val, int reg)
+{
+	u32 req_type = 0;
+
+	req_type = __sdhci_msm_check_write(host, val, reg);
+
+	writeb_relaxed(val, host->ioaddr + reg);
+
+	if (req_type)
+		sdhci_msm_check_power_status(host, req_type);
+}
+#endif
 static const struct of_device_id sdhci_msm_dt_match[] = {
 	{ .compatible = "qcom,sdhci-msm-v4" },
 	{},
@@ -1264,6 +1327,10 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
 	.get_max_clock = sdhci_msm_get_max_clock,
 	.set_bus_width = sdhci_set_bus_width,
 	.set_uhs_signaling = sdhci_msm_set_uhs_signaling,
+#ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
+	.write_w = sdhci_msm_writew,
+	.write_b = sdhci_msm_writeb,
+#endif
 };
 
 static const struct sdhci_pltfm_data sdhci_msm_pdata = {
-- 
 Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

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

* [PATCH v1 5/5] mmc: Kconfig: Enable CONFIG_MMC_SDHCI_IO_ACCESSORS
  2017-08-30 12:51 ` Vijay Viswanath
@ 2017-08-30 12:51   ` Vijay Viswanath
  -1 siblings, 0 replies; 33+ messages in thread
From: Vijay Viswanath @ 2017-08-30 12:51 UTC (permalink / raw)
  To: adrian.hunter, ulf.hansson, will.deacon
  Cc: linux-arm-kernel, linux-mmc, linux-kernel, linux-arm-msm,
	asutoshd, stummala, riteshh, subhashj, Vijay Viswanath

Enable CONFIG_MMC_SDHCI_IO_ACCESSORS so that SDHC controller specific
register read and write APIs, if registered, can be used.
---
 drivers/mmc/host/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 2db84dd..64a9298 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -420,6 +420,7 @@ config MMC_SDHCI_MSM
 	tristate "Qualcomm SDHCI Controller Support"
 	depends on ARCH_QCOM || (ARM && COMPILE_TEST)
 	depends on MMC_SDHCI_PLTFM
+	select CONFIG_MMC_SDHCI_IO_ACCESSORS
 	help
 	  This selects the Secure Digital Host Controller Interface (SDHCI)
 	  support present in Qualcomm SOCs. The controller supports
-- 
 Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

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

* [PATCH v1 5/5] mmc: Kconfig: Enable CONFIG_MMC_SDHCI_IO_ACCESSORS
@ 2017-08-30 12:51   ` Vijay Viswanath
  0 siblings, 0 replies; 33+ messages in thread
From: Vijay Viswanath @ 2017-08-30 12:51 UTC (permalink / raw)
  To: linux-arm-kernel

Enable CONFIG_MMC_SDHCI_IO_ACCESSORS so that SDHC controller specific
register read and write APIs, if registered, can be used.
---
 drivers/mmc/host/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 2db84dd..64a9298 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -420,6 +420,7 @@ config MMC_SDHCI_MSM
 	tristate "Qualcomm SDHCI Controller Support"
 	depends on ARCH_QCOM || (ARM && COMPILE_TEST)
 	depends on MMC_SDHCI_PLTFM
+	select CONFIG_MMC_SDHCI_IO_ACCESSORS
 	help
 	  This selects the Secure Digital Host Controller Interface (SDHCI)
 	  support present in Qualcomm SOCs. The controller supports
-- 
 Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

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

* Re: [PATCH v1 0/5] mmc: sdhci-msm: Corrections to implementation of power irq
  2017-08-30 12:51 ` Vijay Viswanath
  (?)
@ 2017-09-12  9:27   ` Vijay Viswanath
  -1 siblings, 0 replies; 33+ messages in thread
From: Vijay Viswanath @ 2017-09-12  9:27 UTC (permalink / raw)
  To: adrian.hunter, ulf.hansson, will.deacon
  Cc: riteshh, linux-arm-msm, linux-mmc, linux-kernel, stummala,
	subhashj, linux-arm-kernel, asutoshd

Hi Adrian, Ulf,

I have addressed the comments on previous patch series. Can you please 
tell how I should proceed from here?

Thanks,
Vijay

On 8/30/2017 6:21 PM, Vijay Viswanath wrote:
> Register writes which change voltage of IO lines or turn the IO bus on/off
> require sdhc controller to be ready before progressing further. Once a
> register write which affects IO lines is done, the driver should wait for
> power irq from controller. Once the irq comes, the driver should acknowledge
> the irq by writing to power control register. If the acknowledgement is not
> given to controller, the controller may not complete the corresponding
> register write action and this can mess up the controller if drivers proceeds
> without power irq completing.
> 
> Changes since RFC:
> 	wait_for_completion_timeout replaced with wait_event_timeout when
> 	waiting for power irq.
> 	Removed the spinlock within power irq handler and API which waits
> 	for power irq.
> 	Added comments to sdhci msm register write functions, warning that they
> 	can sleep.
> 	Sdhci msm register write functions will do a memory barrier before
> 	writing to the register if the particular register can trigger
> 	power irq.
> 	Instead of enabling SDHCI IO ACCESSORS config in arm64/defconfig, it
> 	will be selected in mmc/host/Kconfig if the platform is MMC_SDHCI_MSM.
> 
> 
> Sahitya Tummala (2):
>    mmc: sdhci-msm: Fix HW issue with power IRQ handling during reset
>    mmc: sdhci-msm: Add support to wait for power irq
> 
> Subhash Jadavani (1):
>    mmc: sdhci-msm: fix issue with power irq
> 
> Vijay Viswanath (2):
>    mmc: sdhci-msm: Add ops to do sdhc register write
>    mmc: Kconfig: Enable CONFIG_MMC_SDHCI_IO_ACCESSORS
> 
>   drivers/mmc/host/Kconfig     |   1 +
>   drivers/mmc/host/sdhci-msm.c | 253 ++++++++++++++++++++++++++++++++++++++++++-
>   2 files changed, 249 insertions(+), 5 deletions(-)
> 

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

* Re: [PATCH v1 0/5] mmc: sdhci-msm: Corrections to implementation of power irq
@ 2017-09-12  9:27   ` Vijay Viswanath
  0 siblings, 0 replies; 33+ messages in thread
From: Vijay Viswanath @ 2017-09-12  9:27 UTC (permalink / raw)
  To: adrian.hunter, ulf.hansson, will.deacon
  Cc: linux-arm-kernel, linux-mmc, linux-kernel, linux-arm-msm,
	asutoshd, stummala, riteshh, subhashj

Hi Adrian, Ulf,

I have addressed the comments on previous patch series. Can you please 
tell how I should proceed from here?

Thanks,
Vijay

On 8/30/2017 6:21 PM, Vijay Viswanath wrote:
> Register writes which change voltage of IO lines or turn the IO bus on/off
> require sdhc controller to be ready before progressing further. Once a
> register write which affects IO lines is done, the driver should wait for
> power irq from controller. Once the irq comes, the driver should acknowledge
> the irq by writing to power control register. If the acknowledgement is not
> given to controller, the controller may not complete the corresponding
> register write action and this can mess up the controller if drivers proceeds
> without power irq completing.
> 
> Changes since RFC:
> 	wait_for_completion_timeout replaced with wait_event_timeout when
> 	waiting for power irq.
> 	Removed the spinlock within power irq handler and API which waits
> 	for power irq.
> 	Added comments to sdhci msm register write functions, warning that they
> 	can sleep.
> 	Sdhci msm register write functions will do a memory barrier before
> 	writing to the register if the particular register can trigger
> 	power irq.
> 	Instead of enabling SDHCI IO ACCESSORS config in arm64/defconfig, it
> 	will be selected in mmc/host/Kconfig if the platform is MMC_SDHCI_MSM.
> 
> 
> Sahitya Tummala (2):
>    mmc: sdhci-msm: Fix HW issue with power IRQ handling during reset
>    mmc: sdhci-msm: Add support to wait for power irq
> 
> Subhash Jadavani (1):
>    mmc: sdhci-msm: fix issue with power irq
> 
> Vijay Viswanath (2):
>    mmc: sdhci-msm: Add ops to do sdhc register write
>    mmc: Kconfig: Enable CONFIG_MMC_SDHCI_IO_ACCESSORS
> 
>   drivers/mmc/host/Kconfig     |   1 +
>   drivers/mmc/host/sdhci-msm.c | 253 ++++++++++++++++++++++++++++++++++++++++++-
>   2 files changed, 249 insertions(+), 5 deletions(-)
> 

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

* [PATCH v1 0/5] mmc: sdhci-msm: Corrections to implementation of power irq
@ 2017-09-12  9:27   ` Vijay Viswanath
  0 siblings, 0 replies; 33+ messages in thread
From: Vijay Viswanath @ 2017-09-12  9:27 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Adrian, Ulf,

I have addressed the comments on previous patch series. Can you please 
tell how I should proceed from here?

Thanks,
Vijay

On 8/30/2017 6:21 PM, Vijay Viswanath wrote:
> Register writes which change voltage of IO lines or turn the IO bus on/off
> require sdhc controller to be ready before progressing further. Once a
> register write which affects IO lines is done, the driver should wait for
> power irq from controller. Once the irq comes, the driver should acknowledge
> the irq by writing to power control register. If the acknowledgement is not
> given to controller, the controller may not complete the corresponding
> register write action and this can mess up the controller if drivers proceeds
> without power irq completing.
> 
> Changes since RFC:
> 	wait_for_completion_timeout replaced with wait_event_timeout when
> 	waiting for power irq.
> 	Removed the spinlock within power irq handler and API which waits
> 	for power irq.
> 	Added comments to sdhci msm register write functions, warning that they
> 	can sleep.
> 	Sdhci msm register write functions will do a memory barrier before
> 	writing to the register if the particular register can trigger
> 	power irq.
> 	Instead of enabling SDHCI IO ACCESSORS config in arm64/defconfig, it
> 	will be selected in mmc/host/Kconfig if the platform is MMC_SDHCI_MSM.
> 
> 
> Sahitya Tummala (2):
>    mmc: sdhci-msm: Fix HW issue with power IRQ handling during reset
>    mmc: sdhci-msm: Add support to wait for power irq
> 
> Subhash Jadavani (1):
>    mmc: sdhci-msm: fix issue with power irq
> 
> Vijay Viswanath (2):
>    mmc: sdhci-msm: Add ops to do sdhc register write
>    mmc: Kconfig: Enable CONFIG_MMC_SDHCI_IO_ACCESSORS
> 
>   drivers/mmc/host/Kconfig     |   1 +
>   drivers/mmc/host/sdhci-msm.c | 253 ++++++++++++++++++++++++++++++++++++++++++-
>   2 files changed, 249 insertions(+), 5 deletions(-)
> 

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

* Re: [PATCH v1 0/5] mmc: sdhci-msm: Corrections to implementation of power irq
  2017-09-12  9:27   ` Vijay Viswanath
@ 2017-09-12 10:41     ` Adrian Hunter
  -1 siblings, 0 replies; 33+ messages in thread
From: Adrian Hunter @ 2017-09-12 10:41 UTC (permalink / raw)
  To: Vijay Viswanath, ulf.hansson, will.deacon
  Cc: linux-arm-kernel, linux-mmc, linux-kernel, linux-arm-msm,
	asutoshd, stummala, riteshh, subhashj

On 12/09/17 12:27, Vijay Viswanath wrote:
> I have addressed the comments on previous patch series. Can you please tell
> how I should proceed from here?

I will look at it soon.

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

* [PATCH v1 0/5] mmc: sdhci-msm: Corrections to implementation of power irq
@ 2017-09-12 10:41     ` Adrian Hunter
  0 siblings, 0 replies; 33+ messages in thread
From: Adrian Hunter @ 2017-09-12 10:41 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/09/17 12:27, Vijay Viswanath wrote:
> I have addressed the comments on previous patch series. Can you please tell
> how I should proceed from here?

I will look at it soon.

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

* Re: [PATCH v1 1/5] mmc: sdhci-msm: fix issue with power irq
  2017-08-30 12:51   ` Vijay Viswanath
@ 2017-09-14  6:20     ` Adrian Hunter
  -1 siblings, 0 replies; 33+ messages in thread
From: Adrian Hunter @ 2017-09-14  6:20 UTC (permalink / raw)
  To: Vijay Viswanath, ulf.hansson, will.deacon
  Cc: linux-arm-kernel, linux-mmc, linux-kernel, linux-arm-msm,
	asutoshd, stummala, riteshh, subhashj

On 30/08/17 15:51, Vijay Viswanath wrote:
> From: Subhash Jadavani <subhashj@codeaurora.org>
> 
> SDCC controller reset (SW_RST) during probe may trigger power irq if
> previous status of PWRCTL was either BUS_ON or IO_HIGH_V. So before we
> enable the power irq interrupt in GIC (by registering the interrupt
> handler), we need to ensure that any pending power irq interrupt status
> is acknowledged otherwise power irq interrupt handler would be fired
> prematurely.
> 
> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
> Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-msm.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 9d601dc..d636251 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -1250,6 +1250,21 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  			       CORE_VENDOR_SPEC_CAPABILITIES0);
>  	}
>  
> +	/*
> +	 * Power on reset state may trigger power irq if previous status of
> +	 * PWRCTL was either BUS_ON or IO_HIGH_V. So before enabling pwr irq
> +	 * interrupt in GIC, any pending power irq interrupt should be
> +	 * acknowledged. Otherwise power irq interrupt handler would be
> +	 * fired prematurely.
> +	 */
> +	sdhci_msm_voltage_switch(host);
> +
> +	/*
> +	 * Ensure that above writes are propogated before interrupt enablement
> +	 * in GIC.
> +	 */
> +	mb();
> +
>  	/* Setup IRQ for handling power/voltage tasks with PMIC */
>  	msm_host->pwr_irq = platform_get_irq_byname(pdev, "pwr_irq");
>  	if (msm_host->pwr_irq < 0) {
> @@ -1259,6 +1274,9 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  		goto clk_disable;
>  	}
>  
> +	/* Enable pwr irq interrupts */
> +	writel_relaxed(INT_MASK, msm_host->core_mem + CORE_PWRCTL_MASK);
> +
>  	ret = devm_request_threaded_irq(&pdev->dev, msm_host->pwr_irq, NULL,
>  					sdhci_msm_pwr_irq, IRQF_ONESHOT,
>  					dev_name(&pdev->dev), host);
> 

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

* [PATCH v1 1/5] mmc: sdhci-msm: fix issue with power irq
@ 2017-09-14  6:20     ` Adrian Hunter
  0 siblings, 0 replies; 33+ messages in thread
From: Adrian Hunter @ 2017-09-14  6:20 UTC (permalink / raw)
  To: linux-arm-kernel

On 30/08/17 15:51, Vijay Viswanath wrote:
> From: Subhash Jadavani <subhashj@codeaurora.org>
> 
> SDCC controller reset (SW_RST) during probe may trigger power irq if
> previous status of PWRCTL was either BUS_ON or IO_HIGH_V. So before we
> enable the power irq interrupt in GIC (by registering the interrupt
> handler), we need to ensure that any pending power irq interrupt status
> is acknowledged otherwise power irq interrupt handler would be fired
> prematurely.
> 
> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
> Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-msm.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 9d601dc..d636251 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -1250,6 +1250,21 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  			       CORE_VENDOR_SPEC_CAPABILITIES0);
>  	}
>  
> +	/*
> +	 * Power on reset state may trigger power irq if previous status of
> +	 * PWRCTL was either BUS_ON or IO_HIGH_V. So before enabling pwr irq
> +	 * interrupt in GIC, any pending power irq interrupt should be
> +	 * acknowledged. Otherwise power irq interrupt handler would be
> +	 * fired prematurely.
> +	 */
> +	sdhci_msm_voltage_switch(host);
> +
> +	/*
> +	 * Ensure that above writes are propogated before interrupt enablement
> +	 * in GIC.
> +	 */
> +	mb();
> +
>  	/* Setup IRQ for handling power/voltage tasks with PMIC */
>  	msm_host->pwr_irq = platform_get_irq_byname(pdev, "pwr_irq");
>  	if (msm_host->pwr_irq < 0) {
> @@ -1259,6 +1274,9 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  		goto clk_disable;
>  	}
>  
> +	/* Enable pwr irq interrupts */
> +	writel_relaxed(INT_MASK, msm_host->core_mem + CORE_PWRCTL_MASK);
> +
>  	ret = devm_request_threaded_irq(&pdev->dev, msm_host->pwr_irq, NULL,
>  					sdhci_msm_pwr_irq, IRQF_ONESHOT,
>  					dev_name(&pdev->dev), host);
> 

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

* Re: [PATCH v1 2/5] mmc: sdhci-msm: Fix HW issue with power IRQ handling during reset
  2017-08-30 12:51   ` Vijay Viswanath
@ 2017-09-14  6:20     ` Adrian Hunter
  -1 siblings, 0 replies; 33+ messages in thread
From: Adrian Hunter @ 2017-09-14  6:20 UTC (permalink / raw)
  To: Vijay Viswanath, ulf.hansson, will.deacon
  Cc: linux-arm-kernel, linux-mmc, linux-kernel, linux-arm-msm,
	asutoshd, stummala, riteshh, subhashj

On 30/08/17 15:51, Vijay Viswanath wrote:
> From: Sahitya Tummala <stummala@codeaurora.org>
> 
> There is a rare scenario in HW, where the first clear pulse could
> be lost when the actual reset and clear/read of status register
> are happening at the same time. Fix this by retrying upto 10 times
> to ensure the status register gets cleared. Otherwise, this will
> lead to a spurious power IRQ which results in system instability.
> 
> Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
> Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-msm.c | 46 ++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 42 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index d636251..42a65ab 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -995,17 +995,52 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
>  		sdhci_msm_hs400(host, &mmc->ios);
>  }
>  
> -static void sdhci_msm_voltage_switch(struct sdhci_host *host)
> +static void sdhci_msm_dump_pwr_ctrl_regs(struct sdhci_host *host)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +
> +	pr_err("%s: PWRCTL_STATUS: 0x%08x | PWRCTL_MASK: 0x%08x | PWRCTL_CTL: 0x%08x\n",
> +			mmc_hostname(host->mmc),
> +			readl_relaxed(msm_host->core_mem + CORE_PWRCTL_STATUS),
> +			readl_relaxed(msm_host->core_mem + CORE_PWRCTL_MASK),
> +			readl_relaxed(msm_host->core_mem + CORE_PWRCTL_CTL));
> +}
> +
> +static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>  	u32 irq_status, irq_ack = 0;
> +	int retry = 10;
>  
>  	irq_status = readl_relaxed(msm_host->core_mem + CORE_PWRCTL_STATUS);
>  	irq_status &= INT_MASK;
>  
>  	writel_relaxed(irq_status, msm_host->core_mem + CORE_PWRCTL_CLEAR);
>  
> +	/*
> +	 * There is a rare HW scenario where the first clear pulse could be
> +	 * lost when actual reset and clear/read of status register is
> +	 * happening at a time. Hence, retry for at least 10 times to make
> +	 * sure status register is cleared. Otherwise, this will result in
> +	 * a spurious power IRQ resulting in system instability.
> +	 */
> +	while (irq_status & readl_relaxed(msm_host->core_mem +
> +				CORE_PWRCTL_STATUS)) {
> +		if (retry == 0) {
> +			pr_err("%s: Timedout clearing (0x%x) pwrctl status register\n",
> +					mmc_hostname(host->mmc), irq_status);
> +			sdhci_msm_dump_pwr_ctrl_regs(host);
> +			WARN_ON(1);
> +			break;
> +		}
> +		writel_relaxed(irq_status,
> +				msm_host->core_mem + CORE_PWRCTL_CLEAR);
> +		retry--;
> +		udelay(10);
> +	}
> +
>  	if (irq_status & (CORE_PWRCTL_BUS_ON | CORE_PWRCTL_BUS_OFF))
>  		irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
>  	if (irq_status & (CORE_PWRCTL_IO_LOW | CORE_PWRCTL_IO_HIGH))
> @@ -1017,13 +1052,17 @@ static void sdhci_msm_voltage_switch(struct sdhci_host *host)
>  	 * switches are handled by the sdhci core, so just report success.
>  	 */
>  	writel_relaxed(irq_ack, msm_host->core_mem + CORE_PWRCTL_CTL);
> +
> +	pr_debug("%s: %s: Handled IRQ(%d), irq_status=0x%x, ack=0x%x\n",
> +		mmc_hostname(msm_host->mmc), __func__, irq, irq_status,
> +		irq_ack);
>  }
>  
>  static irqreturn_t sdhci_msm_pwr_irq(int irq, void *data)
>  {
>  	struct sdhci_host *host = (struct sdhci_host *)data;
>  
> -	sdhci_msm_voltage_switch(host);
> +	sdhci_msm_handle_pwr_irq(host, irq);
>  
>  	return IRQ_HANDLED;
>  }
> @@ -1106,7 +1145,6 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>  	.get_max_clock = sdhci_msm_get_max_clock,
>  	.set_bus_width = sdhci_set_bus_width,
>  	.set_uhs_signaling = sdhci_msm_set_uhs_signaling,
> -	.voltage_switch = sdhci_msm_voltage_switch,
>  };
>  
>  static const struct sdhci_pltfm_data sdhci_msm_pdata = {
> @@ -1257,7 +1295,7 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  	 * acknowledged. Otherwise power irq interrupt handler would be
>  	 * fired prematurely.
>  	 */
> -	sdhci_msm_voltage_switch(host);
> +	sdhci_msm_handle_pwr_irq(host, 0);
>  
>  	/*
>  	 * Ensure that above writes are propogated before interrupt enablement
> 

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

* [PATCH v1 2/5] mmc: sdhci-msm: Fix HW issue with power IRQ handling during reset
@ 2017-09-14  6:20     ` Adrian Hunter
  0 siblings, 0 replies; 33+ messages in thread
From: Adrian Hunter @ 2017-09-14  6:20 UTC (permalink / raw)
  To: linux-arm-kernel

On 30/08/17 15:51, Vijay Viswanath wrote:
> From: Sahitya Tummala <stummala@codeaurora.org>
> 
> There is a rare scenario in HW, where the first clear pulse could
> be lost when the actual reset and clear/read of status register
> are happening at the same time. Fix this by retrying upto 10 times
> to ensure the status register gets cleared. Otherwise, this will
> lead to a spurious power IRQ which results in system instability.
> 
> Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
> Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-msm.c | 46 ++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 42 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index d636251..42a65ab 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -995,17 +995,52 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
>  		sdhci_msm_hs400(host, &mmc->ios);
>  }
>  
> -static void sdhci_msm_voltage_switch(struct sdhci_host *host)
> +static void sdhci_msm_dump_pwr_ctrl_regs(struct sdhci_host *host)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +
> +	pr_err("%s: PWRCTL_STATUS: 0x%08x | PWRCTL_MASK: 0x%08x | PWRCTL_CTL: 0x%08x\n",
> +			mmc_hostname(host->mmc),
> +			readl_relaxed(msm_host->core_mem + CORE_PWRCTL_STATUS),
> +			readl_relaxed(msm_host->core_mem + CORE_PWRCTL_MASK),
> +			readl_relaxed(msm_host->core_mem + CORE_PWRCTL_CTL));
> +}
> +
> +static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>  	u32 irq_status, irq_ack = 0;
> +	int retry = 10;
>  
>  	irq_status = readl_relaxed(msm_host->core_mem + CORE_PWRCTL_STATUS);
>  	irq_status &= INT_MASK;
>  
>  	writel_relaxed(irq_status, msm_host->core_mem + CORE_PWRCTL_CLEAR);
>  
> +	/*
> +	 * There is a rare HW scenario where the first clear pulse could be
> +	 * lost when actual reset and clear/read of status register is
> +	 * happening at a time. Hence, retry for at least 10 times to make
> +	 * sure status register is cleared. Otherwise, this will result in
> +	 * a spurious power IRQ resulting in system instability.
> +	 */
> +	while (irq_status & readl_relaxed(msm_host->core_mem +
> +				CORE_PWRCTL_STATUS)) {
> +		if (retry == 0) {
> +			pr_err("%s: Timedout clearing (0x%x) pwrctl status register\n",
> +					mmc_hostname(host->mmc), irq_status);
> +			sdhci_msm_dump_pwr_ctrl_regs(host);
> +			WARN_ON(1);
> +			break;
> +		}
> +		writel_relaxed(irq_status,
> +				msm_host->core_mem + CORE_PWRCTL_CLEAR);
> +		retry--;
> +		udelay(10);
> +	}
> +
>  	if (irq_status & (CORE_PWRCTL_BUS_ON | CORE_PWRCTL_BUS_OFF))
>  		irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
>  	if (irq_status & (CORE_PWRCTL_IO_LOW | CORE_PWRCTL_IO_HIGH))
> @@ -1017,13 +1052,17 @@ static void sdhci_msm_voltage_switch(struct sdhci_host *host)
>  	 * switches are handled by the sdhci core, so just report success.
>  	 */
>  	writel_relaxed(irq_ack, msm_host->core_mem + CORE_PWRCTL_CTL);
> +
> +	pr_debug("%s: %s: Handled IRQ(%d), irq_status=0x%x, ack=0x%x\n",
> +		mmc_hostname(msm_host->mmc), __func__, irq, irq_status,
> +		irq_ack);
>  }
>  
>  static irqreturn_t sdhci_msm_pwr_irq(int irq, void *data)
>  {
>  	struct sdhci_host *host = (struct sdhci_host *)data;
>  
> -	sdhci_msm_voltage_switch(host);
> +	sdhci_msm_handle_pwr_irq(host, irq);
>  
>  	return IRQ_HANDLED;
>  }
> @@ -1106,7 +1145,6 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>  	.get_max_clock = sdhci_msm_get_max_clock,
>  	.set_bus_width = sdhci_set_bus_width,
>  	.set_uhs_signaling = sdhci_msm_set_uhs_signaling,
> -	.voltage_switch = sdhci_msm_voltage_switch,
>  };
>  
>  static const struct sdhci_pltfm_data sdhci_msm_pdata = {
> @@ -1257,7 +1295,7 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  	 * acknowledged. Otherwise power irq interrupt handler would be
>  	 * fired prematurely.
>  	 */
> -	sdhci_msm_voltage_switch(host);
> +	sdhci_msm_handle_pwr_irq(host, 0);
>  
>  	/*
>  	 * Ensure that above writes are propogated before interrupt enablement
> 

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

* Re: [PATCH v1 3/5] mmc: sdhci-msm: Add support to wait for power irq
  2017-08-30 12:51   ` Vijay Viswanath
@ 2017-09-14  6:34     ` Adrian Hunter
  -1 siblings, 0 replies; 33+ messages in thread
From: Adrian Hunter @ 2017-09-14  6:34 UTC (permalink / raw)
  To: Vijay Viswanath, ulf.hansson, will.deacon
  Cc: linux-arm-kernel, linux-mmc, linux-kernel, linux-arm-msm,
	asutoshd, stummala, riteshh, subhashj

On 30/08/17 15:51, Vijay Viswanath wrote:
> From: Sahitya Tummala <stummala@codeaurora.org>
> 
> Add support API which will check if power irq is expected to be
> generated and wait for the power irq to come and complete if the irq is
> expected.
> 
> Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
> Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>

One comment below.

> ---
>  drivers/mmc/host/sdhci-msm.c | 124 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 122 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 42a65ab..e3e385e 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -123,6 +123,10 @@
>  #define CMUX_SHIFT_PHASE_MASK	(7 << CMUX_SHIFT_PHASE_SHIFT)
>  
>  #define MSM_MMC_AUTOSUSPEND_DELAY_MS	50
> +
> +/* Timeout value to avoid infinite waiting for pwr_irq */
> +#define MSM_PWR_IRQ_TIMEOUT_MS 5000
> +
>  struct sdhci_msm_host {
>  	struct platform_device *pdev;
>  	void __iomem *core_mem;	/* MSM SDCC mapped address */
> @@ -138,6 +142,12 @@ struct sdhci_msm_host {
>  	bool calibration_done;
>  	u8 saved_tuning_phase;
>  	bool use_cdclp533;
> +	u32 curr_pwr_state;
> +	u32 curr_io_level;
> +#ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
> +	wait_queue_head_t pwr_irq_wait;
> +	bool pwr_irq_flag;
> +#endif
>  };
>  
>  static unsigned int msm_get_clock_rate_for_bus_mode(struct sdhci_host *host,
> @@ -995,6 +1005,87 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
>  		sdhci_msm_hs400(host, &mmc->ios);
>  }
>  
> +#ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
> +static inline void sdhci_msm_init_pwr_irq_wait(struct sdhci_msm_host *msm_host)
> +{
> +	init_waitqueue_head(&msm_host->pwr_irq_wait);
> +}
> +
> +static inline void sdhci_msm_complete_pwr_irq_wait(
> +		struct sdhci_msm_host *msm_host)
> +{
> +	wake_up(&msm_host->pwr_irq_wait);
> +}
> +
> +/*
> + * sdhci_msm_check_power_status API should be called when registers writes
> + * which can toggle sdhci IO bus ON/OFF or change IO lines HIGH/LOW happens.
> + * To what state the register writes will change the IO lines should be passed
> + * as the argument req_type. This API will check whether the IO line's state
> + * is already the expected state and will wait for power irq only if
> + * power irq is expected to be trigerred based on the current IO line state
> + * and expected IO line state.
> + */
> +static void sdhci_msm_check_power_status(struct sdhci_host *host, u32 req_type)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +	bool done = false;
> +
> +	pr_debug("%s: %s: request %d curr_pwr_state %x curr_io_level %x\n",
> +			mmc_hostname(host->mmc), __func__, req_type,
> +			msm_host->curr_pwr_state, msm_host->curr_io_level);
> +
> +	/*
> +	 * The IRQ for request type IO High/LOW will be generated when -
> +	 * there is a state change in 1.8V enable bit (bit 3) of
> +	 * SDHCI_HOST_CONTROL2 register. The reset state of that bit is 0
> +	 * which indicates 3.3V IO voltage. So, when MMC core layer tries
> +	 * to set it to 3.3V before card detection happens, the
> +	 * IRQ doesn't get triggered as there is no state change in this bit.
> +	 * The driver already handles this case by changing the IO voltage
> +	 * level to high as part of controller power up sequence. Hence, check
> +	 * for host->pwr to handle a case where IO voltage high request is
> +	 * issued even before controller power up.
> +	 */
> +	if ((req_type & REQ_IO_HIGH) && !host->pwr) {
> +		pr_debug("%s: do not wait for power IRQ that never comes, req_type: %d\n",
> +				mmc_hostname(host->mmc), req_type);
> +		return;
> +	}
> +	if ((req_type & msm_host->curr_pwr_state) ||
> +			(req_type & msm_host->curr_io_level))
> +		done = true;
> +	/*
> +	 * This is needed here to hanlde a case where IRQ gets
> +	 * triggered even before this function is called so that
> +	 * x->done counter of completion gets reset. Otherwise,
> +	 * next call to wait_for_completion returns immediately
> +	 * without actually waiting for the IRQ to be handled.
> +	 */

This isn't true anymore.  If there is always an interrupt following the
register write, then you can always call wait_event_timeout() because
if the interrupt has already happened msm_host->pwr_irq_flag will already be
true.

However if the interrupt only happens if the pwr_state of io_level changes,
then you do need to check those, to avoid waiting for an interrupt that is
not coming.

> +	if (!done) {
> +		if (!wait_event_timeout(msm_host->pwr_irq_wait,
> +				msm_host->pwr_irq_flag,
> +				msecs_to_jiffies(MSM_PWR_IRQ_TIMEOUT_MS)))
> +			__WARN_printf("%s: pwr_irq for req: (%d) timed out\n",
> +					mmc_hostname(host->mmc), req_type);
> +	}
> +	msm_host->pwr_irq_flag = 0;
> +	pr_debug("%s: %s: request %d done\n", mmc_hostname(host->mmc),
> +			__func__, req_type);
> +}
> +#else
> +static inline void sdhci_msm_init_pwr_irq_completion(
> +		struct sdhci_msm_host *msm_host)
> +{
> +}
> +
> +static inline void sdhci_msm_complete_pwr_irq_completion(
> +		struct sdhci_msm_host *msm_host)
> +{
> +}
> +#endif
> +
>  static void sdhci_msm_dump_pwr_ctrl_regs(struct sdhci_host *host)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> @@ -1013,6 +1104,8 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
>  	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>  	u32 irq_status, irq_ack = 0;
>  	int retry = 10;
> +	int pwr_state = 0, io_level = 0;
> +
>  
>  	irq_status = readl_relaxed(msm_host->core_mem + CORE_PWRCTL_STATUS);
>  	irq_status &= INT_MASK;
> @@ -1041,10 +1134,26 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
>  		udelay(10);
>  	}
>  
> -	if (irq_status & (CORE_PWRCTL_BUS_ON | CORE_PWRCTL_BUS_OFF))
> +	/* Handle BUS ON/OFF*/
> +	if (irq_status & CORE_PWRCTL_BUS_ON) {
> +		pwr_state = REQ_BUS_ON;
> +		io_level = REQ_IO_HIGH;
>  		irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
> -	if (irq_status & (CORE_PWRCTL_IO_LOW | CORE_PWRCTL_IO_HIGH))
> +	}
> +	if (irq_status & CORE_PWRCTL_BUS_OFF) {
> +		pwr_state = REQ_BUS_OFF;
> +		io_level = REQ_IO_LOW;
> +		irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
> +	}
> +	/* Handle IO LOW/HIGH */
> +	if (irq_status & CORE_PWRCTL_IO_LOW) {
> +		io_level = REQ_IO_LOW;
> +		irq_ack |= CORE_PWRCTL_IO_SUCCESS;
> +	}
> +	if (irq_status & CORE_PWRCTL_IO_HIGH) {
> +		io_level = REQ_IO_HIGH;
>  		irq_ack |= CORE_PWRCTL_IO_SUCCESS;
> +	}
>  
>  	/*
>  	 * The driver has to acknowledge the interrupt, switch voltages and
> @@ -1053,6 +1162,11 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
>  	 */
>  	writel_relaxed(irq_ack, msm_host->core_mem + CORE_PWRCTL_CTL);
>  
> +	if (pwr_state)
> +		msm_host->curr_pwr_state = pwr_state;
> +	if (io_level)
> +		msm_host->curr_io_level = io_level;
> +
>  	pr_debug("%s: %s: Handled IRQ(%d), irq_status=0x%x, ack=0x%x\n",
>  		mmc_hostname(msm_host->mmc), __func__, irq, irq_status,
>  		irq_ack);
> @@ -1061,8 +1175,13 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
>  static irqreturn_t sdhci_msm_pwr_irq(int irq, void *data)
>  {
>  	struct sdhci_host *host = (struct sdhci_host *)data;
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>  
>  	sdhci_msm_handle_pwr_irq(host, irq);
> +	msm_host->pwr_irq_flag = 1;
> +	sdhci_msm_complete_pwr_irq_wait(msm_host);
> +
>  
>  	return IRQ_HANDLED;
>  }
> @@ -1312,6 +1431,7 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  		goto clk_disable;
>  	}
>  
> +	sdhci_msm_init_pwr_irq_wait(msm_host);
>  	/* Enable pwr irq interrupts */
>  	writel_relaxed(INT_MASK, msm_host->core_mem + CORE_PWRCTL_MASK);
>  
> 

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

* [PATCH v1 3/5] mmc: sdhci-msm: Add support to wait for power irq
@ 2017-09-14  6:34     ` Adrian Hunter
  0 siblings, 0 replies; 33+ messages in thread
From: Adrian Hunter @ 2017-09-14  6:34 UTC (permalink / raw)
  To: linux-arm-kernel

On 30/08/17 15:51, Vijay Viswanath wrote:
> From: Sahitya Tummala <stummala@codeaurora.org>
> 
> Add support API which will check if power irq is expected to be
> generated and wait for the power irq to come and complete if the irq is
> expected.
> 
> Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
> Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>

One comment below.

> ---
>  drivers/mmc/host/sdhci-msm.c | 124 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 122 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 42a65ab..e3e385e 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -123,6 +123,10 @@
>  #define CMUX_SHIFT_PHASE_MASK	(7 << CMUX_SHIFT_PHASE_SHIFT)
>  
>  #define MSM_MMC_AUTOSUSPEND_DELAY_MS	50
> +
> +/* Timeout value to avoid infinite waiting for pwr_irq */
> +#define MSM_PWR_IRQ_TIMEOUT_MS 5000
> +
>  struct sdhci_msm_host {
>  	struct platform_device *pdev;
>  	void __iomem *core_mem;	/* MSM SDCC mapped address */
> @@ -138,6 +142,12 @@ struct sdhci_msm_host {
>  	bool calibration_done;
>  	u8 saved_tuning_phase;
>  	bool use_cdclp533;
> +	u32 curr_pwr_state;
> +	u32 curr_io_level;
> +#ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
> +	wait_queue_head_t pwr_irq_wait;
> +	bool pwr_irq_flag;
> +#endif
>  };
>  
>  static unsigned int msm_get_clock_rate_for_bus_mode(struct sdhci_host *host,
> @@ -995,6 +1005,87 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
>  		sdhci_msm_hs400(host, &mmc->ios);
>  }
>  
> +#ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
> +static inline void sdhci_msm_init_pwr_irq_wait(struct sdhci_msm_host *msm_host)
> +{
> +	init_waitqueue_head(&msm_host->pwr_irq_wait);
> +}
> +
> +static inline void sdhci_msm_complete_pwr_irq_wait(
> +		struct sdhci_msm_host *msm_host)
> +{
> +	wake_up(&msm_host->pwr_irq_wait);
> +}
> +
> +/*
> + * sdhci_msm_check_power_status API should be called when registers writes
> + * which can toggle sdhci IO bus ON/OFF or change IO lines HIGH/LOW happens.
> + * To what state the register writes will change the IO lines should be passed
> + * as the argument req_type. This API will check whether the IO line's state
> + * is already the expected state and will wait for power irq only if
> + * power irq is expected to be trigerred based on the current IO line state
> + * and expected IO line state.
> + */
> +static void sdhci_msm_check_power_status(struct sdhci_host *host, u32 req_type)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +	bool done = false;
> +
> +	pr_debug("%s: %s: request %d curr_pwr_state %x curr_io_level %x\n",
> +			mmc_hostname(host->mmc), __func__, req_type,
> +			msm_host->curr_pwr_state, msm_host->curr_io_level);
> +
> +	/*
> +	 * The IRQ for request type IO High/LOW will be generated when -
> +	 * there is a state change in 1.8V enable bit (bit 3) of
> +	 * SDHCI_HOST_CONTROL2 register. The reset state of that bit is 0
> +	 * which indicates 3.3V IO voltage. So, when MMC core layer tries
> +	 * to set it to 3.3V before card detection happens, the
> +	 * IRQ doesn't get triggered as there is no state change in this bit.
> +	 * The driver already handles this case by changing the IO voltage
> +	 * level to high as part of controller power up sequence. Hence, check
> +	 * for host->pwr to handle a case where IO voltage high request is
> +	 * issued even before controller power up.
> +	 */
> +	if ((req_type & REQ_IO_HIGH) && !host->pwr) {
> +		pr_debug("%s: do not wait for power IRQ that never comes, req_type: %d\n",
> +				mmc_hostname(host->mmc), req_type);
> +		return;
> +	}
> +	if ((req_type & msm_host->curr_pwr_state) ||
> +			(req_type & msm_host->curr_io_level))
> +		done = true;
> +	/*
> +	 * This is needed here to hanlde a case where IRQ gets
> +	 * triggered even before this function is called so that
> +	 * x->done counter of completion gets reset. Otherwise,
> +	 * next call to wait_for_completion returns immediately
> +	 * without actually waiting for the IRQ to be handled.
> +	 */

This isn't true anymore.  If there is always an interrupt following the
register write, then you can always call wait_event_timeout() because
if the interrupt has already happened msm_host->pwr_irq_flag will already be
true.

However if the interrupt only happens if the pwr_state of io_level changes,
then you do need to check those, to avoid waiting for an interrupt that is
not coming.

> +	if (!done) {
> +		if (!wait_event_timeout(msm_host->pwr_irq_wait,
> +				msm_host->pwr_irq_flag,
> +				msecs_to_jiffies(MSM_PWR_IRQ_TIMEOUT_MS)))
> +			__WARN_printf("%s: pwr_irq for req: (%d) timed out\n",
> +					mmc_hostname(host->mmc), req_type);
> +	}
> +	msm_host->pwr_irq_flag = 0;
> +	pr_debug("%s: %s: request %d done\n", mmc_hostname(host->mmc),
> +			__func__, req_type);
> +}
> +#else
> +static inline void sdhci_msm_init_pwr_irq_completion(
> +		struct sdhci_msm_host *msm_host)
> +{
> +}
> +
> +static inline void sdhci_msm_complete_pwr_irq_completion(
> +		struct sdhci_msm_host *msm_host)
> +{
> +}
> +#endif
> +
>  static void sdhci_msm_dump_pwr_ctrl_regs(struct sdhci_host *host)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> @@ -1013,6 +1104,8 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
>  	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>  	u32 irq_status, irq_ack = 0;
>  	int retry = 10;
> +	int pwr_state = 0, io_level = 0;
> +
>  
>  	irq_status = readl_relaxed(msm_host->core_mem + CORE_PWRCTL_STATUS);
>  	irq_status &= INT_MASK;
> @@ -1041,10 +1134,26 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
>  		udelay(10);
>  	}
>  
> -	if (irq_status & (CORE_PWRCTL_BUS_ON | CORE_PWRCTL_BUS_OFF))
> +	/* Handle BUS ON/OFF*/
> +	if (irq_status & CORE_PWRCTL_BUS_ON) {
> +		pwr_state = REQ_BUS_ON;
> +		io_level = REQ_IO_HIGH;
>  		irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
> -	if (irq_status & (CORE_PWRCTL_IO_LOW | CORE_PWRCTL_IO_HIGH))
> +	}
> +	if (irq_status & CORE_PWRCTL_BUS_OFF) {
> +		pwr_state = REQ_BUS_OFF;
> +		io_level = REQ_IO_LOW;
> +		irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
> +	}
> +	/* Handle IO LOW/HIGH */
> +	if (irq_status & CORE_PWRCTL_IO_LOW) {
> +		io_level = REQ_IO_LOW;
> +		irq_ack |= CORE_PWRCTL_IO_SUCCESS;
> +	}
> +	if (irq_status & CORE_PWRCTL_IO_HIGH) {
> +		io_level = REQ_IO_HIGH;
>  		irq_ack |= CORE_PWRCTL_IO_SUCCESS;
> +	}
>  
>  	/*
>  	 * The driver has to acknowledge the interrupt, switch voltages and
> @@ -1053,6 +1162,11 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
>  	 */
>  	writel_relaxed(irq_ack, msm_host->core_mem + CORE_PWRCTL_CTL);
>  
> +	if (pwr_state)
> +		msm_host->curr_pwr_state = pwr_state;
> +	if (io_level)
> +		msm_host->curr_io_level = io_level;
> +
>  	pr_debug("%s: %s: Handled IRQ(%d), irq_status=0x%x, ack=0x%x\n",
>  		mmc_hostname(msm_host->mmc), __func__, irq, irq_status,
>  		irq_ack);
> @@ -1061,8 +1175,13 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
>  static irqreturn_t sdhci_msm_pwr_irq(int irq, void *data)
>  {
>  	struct sdhci_host *host = (struct sdhci_host *)data;
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>  
>  	sdhci_msm_handle_pwr_irq(host, irq);
> +	msm_host->pwr_irq_flag = 1;
> +	sdhci_msm_complete_pwr_irq_wait(msm_host);
> +
>  
>  	return IRQ_HANDLED;
>  }
> @@ -1312,6 +1431,7 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  		goto clk_disable;
>  	}
>  
> +	sdhci_msm_init_pwr_irq_wait(msm_host);
>  	/* Enable pwr irq interrupts */
>  	writel_relaxed(INT_MASK, msm_host->core_mem + CORE_PWRCTL_MASK);
>  
> 

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

* Re: [PATCH v1 4/5] mmc: sdhci-msm: Add ops to do sdhc register write
  2017-08-30 12:51   ` Vijay Viswanath
@ 2017-09-14  6:34     ` Adrian Hunter
  -1 siblings, 0 replies; 33+ messages in thread
From: Adrian Hunter @ 2017-09-14  6:34 UTC (permalink / raw)
  To: Vijay Viswanath, ulf.hansson, will.deacon
  Cc: linux-arm-kernel, linux-mmc, linux-kernel, linux-arm-msm,
	asutoshd, stummala, riteshh, subhashj

On 30/08/17 15:51, Vijay Viswanath wrote:
> Register writes which change voltage of IO lines or turn the IO bus
> on/off require controller to be ready before progressing further. When
> the controller is ready, it will generate a power irq which needs to be
> handled. The thread which initiated the register write should wait for
> power irq to complete. This will be done through the new sdhc msm write
> APIs which will check whether the particular write can trigger a power
> irq and wait for it with a timeout if it is expected.
> The SDHC core power control IRQ gets triggered when -
> * There is a state change in power control bit (bit 0)
>   of SDHCI_POWER_CONTROL register.
> * There is a state change in 1.8V enable bit (bit 3) of
>   SDHCI_HOST_CONTROL2 register.
> * Bit 1 of SDHCI_SOFTWARE_RESET is set.
> 
> Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-msm.c | 69 +++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 68 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index e3e385e..a4a78b5 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -1070,7 +1070,6 @@ static void sdhci_msm_check_power_status(struct sdhci_host *host, u32 req_type)
>  			__WARN_printf("%s: pwr_irq for req: (%d) timed out\n",
>  					mmc_hostname(host->mmc), req_type);
>  	}
> -	msm_host->pwr_irq_flag = 0;
>  	pr_debug("%s: %s: request %d done\n", mmc_hostname(host->mmc),
>  			__func__, req_type);
>  }
> @@ -1250,6 +1249,70 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>  	__sdhci_msm_set_clock(host, clock);
>  }
>  
> +#ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
> +/*
> + * Platform specific register write functions. This is so that, if any
> + * register write needs to be followed up by platform specific actions,
> + * they can be added here. These functions can go to sleep when writes
> + * to certain registers are done.
> + * These functions are relying on sdhci_set_ios not using spinlock.
> + */
> +static int __sdhci_msm_check_write(struct sdhci_host *host, u16 val, int reg)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +	u32 req_type = 0;
> +
> +	switch (reg) {
> +	case SDHCI_HOST_CONTROL2:
> +		req_type = (val & SDHCI_CTRL_VDD_180) ? REQ_IO_LOW :
> +			REQ_IO_HIGH;
> +		break;
> +	case SDHCI_SOFTWARE_RESET:
> +		if (host->pwr && (val & SDHCI_RESET_ALL))
> +			req_type = REQ_BUS_OFF;
> +		break;
> +	case SDHCI_POWER_CONTROL:
> +		req_type = !val ? REQ_BUS_OFF : REQ_BUS_ON;
> +		break;
> +	}
> +
> +	if (req_type) {
> +		msm_host->pwr_irq_flag = 0;
> +		/*
> +		 * Since this register write may trigger a power irq, ensure
> +		 * all previous register writes are complete by this point.
> +		 */
> +		mb();
> +	}
> +	return req_type;
> +}
> +
> +/* This function may sleep*/
> +static void sdhci_msm_writew(struct sdhci_host *host, u16 val, int reg)
> +{
> +	u32 req_type = 0;
> +
> +	req_type = __sdhci_msm_check_write(host, val, reg);
> +	writew_relaxed(val, host->ioaddr + reg);
> +
> +	if (req_type)
> +		sdhci_msm_check_power_status(host, req_type);
> +}
> +
> +/* This function may sleep*/
> +static void sdhci_msm_writeb(struct sdhci_host *host, u8 val, int reg)
> +{
> +	u32 req_type = 0;
> +
> +	req_type = __sdhci_msm_check_write(host, val, reg);
> +
> +	writeb_relaxed(val, host->ioaddr + reg);
> +
> +	if (req_type)
> +		sdhci_msm_check_power_status(host, req_type);
> +}
> +#endif
>  static const struct of_device_id sdhci_msm_dt_match[] = {
>  	{ .compatible = "qcom,sdhci-msm-v4" },
>  	{},
> @@ -1264,6 +1327,10 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>  	.get_max_clock = sdhci_msm_get_max_clock,
>  	.set_bus_width = sdhci_set_bus_width,
>  	.set_uhs_signaling = sdhci_msm_set_uhs_signaling,
> +#ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
> +	.write_w = sdhci_msm_writew,
> +	.write_b = sdhci_msm_writeb,
> +#endif
>  };
>  
>  static const struct sdhci_pltfm_data sdhci_msm_pdata = {
> 

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

* [PATCH v1 4/5] mmc: sdhci-msm: Add ops to do sdhc register write
@ 2017-09-14  6:34     ` Adrian Hunter
  0 siblings, 0 replies; 33+ messages in thread
From: Adrian Hunter @ 2017-09-14  6:34 UTC (permalink / raw)
  To: linux-arm-kernel

On 30/08/17 15:51, Vijay Viswanath wrote:
> Register writes which change voltage of IO lines or turn the IO bus
> on/off require controller to be ready before progressing further. When
> the controller is ready, it will generate a power irq which needs to be
> handled. The thread which initiated the register write should wait for
> power irq to complete. This will be done through the new sdhc msm write
> APIs which will check whether the particular write can trigger a power
> irq and wait for it with a timeout if it is expected.
> The SDHC core power control IRQ gets triggered when -
> * There is a state change in power control bit (bit 0)
>   of SDHCI_POWER_CONTROL register.
> * There is a state change in 1.8V enable bit (bit 3) of
>   SDHCI_HOST_CONTROL2 register.
> * Bit 1 of SDHCI_SOFTWARE_RESET is set.
> 
> Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-msm.c | 69 +++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 68 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index e3e385e..a4a78b5 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -1070,7 +1070,6 @@ static void sdhci_msm_check_power_status(struct sdhci_host *host, u32 req_type)
>  			__WARN_printf("%s: pwr_irq for req: (%d) timed out\n",
>  					mmc_hostname(host->mmc), req_type);
>  	}
> -	msm_host->pwr_irq_flag = 0;
>  	pr_debug("%s: %s: request %d done\n", mmc_hostname(host->mmc),
>  			__func__, req_type);
>  }
> @@ -1250,6 +1249,70 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>  	__sdhci_msm_set_clock(host, clock);
>  }
>  
> +#ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
> +/*
> + * Platform specific register write functions. This is so that, if any
> + * register write needs to be followed up by platform specific actions,
> + * they can be added here. These functions can go to sleep when writes
> + * to certain registers are done.
> + * These functions are relying on sdhci_set_ios not using spinlock.
> + */
> +static int __sdhci_msm_check_write(struct sdhci_host *host, u16 val, int reg)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +	u32 req_type = 0;
> +
> +	switch (reg) {
> +	case SDHCI_HOST_CONTROL2:
> +		req_type = (val & SDHCI_CTRL_VDD_180) ? REQ_IO_LOW :
> +			REQ_IO_HIGH;
> +		break;
> +	case SDHCI_SOFTWARE_RESET:
> +		if (host->pwr && (val & SDHCI_RESET_ALL))
> +			req_type = REQ_BUS_OFF;
> +		break;
> +	case SDHCI_POWER_CONTROL:
> +		req_type = !val ? REQ_BUS_OFF : REQ_BUS_ON;
> +		break;
> +	}
> +
> +	if (req_type) {
> +		msm_host->pwr_irq_flag = 0;
> +		/*
> +		 * Since this register write may trigger a power irq, ensure
> +		 * all previous register writes are complete by this point.
> +		 */
> +		mb();
> +	}
> +	return req_type;
> +}
> +
> +/* This function may sleep*/
> +static void sdhci_msm_writew(struct sdhci_host *host, u16 val, int reg)
> +{
> +	u32 req_type = 0;
> +
> +	req_type = __sdhci_msm_check_write(host, val, reg);
> +	writew_relaxed(val, host->ioaddr + reg);
> +
> +	if (req_type)
> +		sdhci_msm_check_power_status(host, req_type);
> +}
> +
> +/* This function may sleep*/
> +static void sdhci_msm_writeb(struct sdhci_host *host, u8 val, int reg)
> +{
> +	u32 req_type = 0;
> +
> +	req_type = __sdhci_msm_check_write(host, val, reg);
> +
> +	writeb_relaxed(val, host->ioaddr + reg);
> +
> +	if (req_type)
> +		sdhci_msm_check_power_status(host, req_type);
> +}
> +#endif
>  static const struct of_device_id sdhci_msm_dt_match[] = {
>  	{ .compatible = "qcom,sdhci-msm-v4" },
>  	{},
> @@ -1264,6 +1327,10 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>  	.get_max_clock = sdhci_msm_get_max_clock,
>  	.set_bus_width = sdhci_set_bus_width,
>  	.set_uhs_signaling = sdhci_msm_set_uhs_signaling,
> +#ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
> +	.write_w = sdhci_msm_writew,
> +	.write_b = sdhci_msm_writeb,
> +#endif
>  };
>  
>  static const struct sdhci_pltfm_data sdhci_msm_pdata = {
> 

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

* Re: [PATCH v1 5/5] mmc: Kconfig: Enable CONFIG_MMC_SDHCI_IO_ACCESSORS
  2017-08-30 12:51   ` Vijay Viswanath
@ 2017-09-14  6:34     ` Adrian Hunter
  -1 siblings, 0 replies; 33+ messages in thread
From: Adrian Hunter @ 2017-09-14  6:34 UTC (permalink / raw)
  To: Vijay Viswanath, ulf.hansson, will.deacon
  Cc: linux-arm-kernel, linux-mmc, linux-kernel, linux-arm-msm,
	asutoshd, stummala, riteshh, subhashj

On 30/08/17 15:51, Vijay Viswanath wrote:
> Enable CONFIG_MMC_SDHCI_IO_ACCESSORS so that SDHC controller specific
> register read and write APIs, if registered, can be used.

Missing signed-off

Why don't you put this patch earlier in the patch set and then you don't
need all the ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS ?

> ---
>  drivers/mmc/host/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 2db84dd..64a9298 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -420,6 +420,7 @@ config MMC_SDHCI_MSM
>  	tristate "Qualcomm SDHCI Controller Support"
>  	depends on ARCH_QCOM || (ARM && COMPILE_TEST)
>  	depends on MMC_SDHCI_PLTFM
> +	select CONFIG_MMC_SDHCI_IO_ACCESSORS

CONFIG_MMC_SDHCI_IO_ACCESSORS -> MMC_SDHCI_IO_ACCESSORS

>  	help
>  	  This selects the Secure Digital Host Controller Interface (SDHCI)
>  	  support present in Qualcomm SOCs. The controller supports
> 

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

* [PATCH v1 5/5] mmc: Kconfig: Enable CONFIG_MMC_SDHCI_IO_ACCESSORS
@ 2017-09-14  6:34     ` Adrian Hunter
  0 siblings, 0 replies; 33+ messages in thread
From: Adrian Hunter @ 2017-09-14  6:34 UTC (permalink / raw)
  To: linux-arm-kernel

On 30/08/17 15:51, Vijay Viswanath wrote:
> Enable CONFIG_MMC_SDHCI_IO_ACCESSORS so that SDHC controller specific
> register read and write APIs, if registered, can be used.

Missing signed-off

Why don't you put this patch earlier in the patch set and then you don't
need all the ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS ?

> ---
>  drivers/mmc/host/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 2db84dd..64a9298 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -420,6 +420,7 @@ config MMC_SDHCI_MSM
>  	tristate "Qualcomm SDHCI Controller Support"
>  	depends on ARCH_QCOM || (ARM && COMPILE_TEST)
>  	depends on MMC_SDHCI_PLTFM
> +	select CONFIG_MMC_SDHCI_IO_ACCESSORS

CONFIG_MMC_SDHCI_IO_ACCESSORS -> MMC_SDHCI_IO_ACCESSORS

>  	help
>  	  This selects the Secure Digital Host Controller Interface (SDHCI)
>  	  support present in Qualcomm SOCs. The controller supports
> 

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

* Re: [PATCH v1 5/5] mmc: Kconfig: Enable CONFIG_MMC_SDHCI_IO_ACCESSORS
  2017-09-14  6:34     ` Adrian Hunter
@ 2017-09-19  4:03       ` Vijay Viswanath
  -1 siblings, 0 replies; 33+ messages in thread
From: Vijay Viswanath @ 2017-09-19  4:03 UTC (permalink / raw)
  To: Adrian Hunter, ulf.hansson, will.deacon
  Cc: linux-arm-kernel, linux-mmc, linux-kernel, linux-arm-msm,
	asutoshd, stummala, riteshh, subhashj



On 9/14/2017 12:04 PM, Adrian Hunter wrote:
> On 30/08/17 15:51, Vijay Viswanath wrote:
>> Enable CONFIG_MMC_SDHCI_IO_ACCESSORS so that SDHC controller specific
>> register read and write APIs, if registered, can be used.
> 
> Missing signed-off
> 
> Why don't you put this patch earlier in the patch set and then you don't
> need all the ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS ?
> 

This will involve merging the current patches 3 & 4 into one (Some 
functions defined under ifdef in patch 3 are used only in patch 4). Will 
that be fine ?

>> ---
>>   drivers/mmc/host/Kconfig | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>> index 2db84dd..64a9298 100644
>> --- a/drivers/mmc/host/Kconfig
>> +++ b/drivers/mmc/host/Kconfig
>> @@ -420,6 +420,7 @@ config MMC_SDHCI_MSM
>>   	tristate "Qualcomm SDHCI Controller Support"
>>   	depends on ARCH_QCOM || (ARM && COMPILE_TEST)
>>   	depends on MMC_SDHCI_PLTFM
>> +	select CONFIG_MMC_SDHCI_IO_ACCESSORS
> 
> CONFIG_MMC_SDHCI_IO_ACCESSORS -> MMC_SDHCI_IO_ACCESSORS
> 
>>   	help
>>   	  This selects the Secure Digital Host Controller Interface (SDHCI)
>>   	  support present in Qualcomm SOCs. The controller supports
>>
> 

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

* [PATCH v1 5/5] mmc: Kconfig: Enable CONFIG_MMC_SDHCI_IO_ACCESSORS
@ 2017-09-19  4:03       ` Vijay Viswanath
  0 siblings, 0 replies; 33+ messages in thread
From: Vijay Viswanath @ 2017-09-19  4:03 UTC (permalink / raw)
  To: linux-arm-kernel



On 9/14/2017 12:04 PM, Adrian Hunter wrote:
> On 30/08/17 15:51, Vijay Viswanath wrote:
>> Enable CONFIG_MMC_SDHCI_IO_ACCESSORS so that SDHC controller specific
>> register read and write APIs, if registered, can be used.
> 
> Missing signed-off
> 
> Why don't you put this patch earlier in the patch set and then you don't
> need all the ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS ?
> 

This will involve merging the current patches 3 & 4 into one (Some 
functions defined under ifdef in patch 3 are used only in patch 4). Will 
that be fine ?

>> ---
>>   drivers/mmc/host/Kconfig | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>> index 2db84dd..64a9298 100644
>> --- a/drivers/mmc/host/Kconfig
>> +++ b/drivers/mmc/host/Kconfig
>> @@ -420,6 +420,7 @@ config MMC_SDHCI_MSM
>>   	tristate "Qualcomm SDHCI Controller Support"
>>   	depends on ARCH_QCOM || (ARM && COMPILE_TEST)
>>   	depends on MMC_SDHCI_PLTFM
>> +	select CONFIG_MMC_SDHCI_IO_ACCESSORS
> 
> CONFIG_MMC_SDHCI_IO_ACCESSORS -> MMC_SDHCI_IO_ACCESSORS
> 
>>   	help
>>   	  This selects the Secure Digital Host Controller Interface (SDHCI)
>>   	  support present in Qualcomm SOCs. The controller supports
>>
> 

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

* Re: [PATCH v1 5/5] mmc: Kconfig: Enable CONFIG_MMC_SDHCI_IO_ACCESSORS
  2017-09-19  4:03       ` Vijay Viswanath
@ 2017-09-19  5:44         ` Adrian Hunter
  -1 siblings, 0 replies; 33+ messages in thread
From: Adrian Hunter @ 2017-09-19  5:44 UTC (permalink / raw)
  To: Vijay Viswanath, ulf.hansson, will.deacon
  Cc: linux-arm-kernel, linux-mmc, linux-kernel, linux-arm-msm,
	asutoshd, stummala, riteshh, subhashj

On 19/09/17 07:03, Vijay Viswanath wrote:
> 
> 
> On 9/14/2017 12:04 PM, Adrian Hunter wrote:
>> On 30/08/17 15:51, Vijay Viswanath wrote:
>>> Enable CONFIG_MMC_SDHCI_IO_ACCESSORS so that SDHC controller specific
>>> register read and write APIs, if registered, can be used.
>>
>> Missing signed-off
>>
>> Why don't you put this patch earlier in the patch set and then you don't
>> need all the ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS ?
>>
> 
> This will involve merging the current patches 3 & 4 into one (Some functions
> defined under ifdef in patch 3 are used only in patch 4). Will that be fine ?

Sure

> 
>>> ---
>>>   drivers/mmc/host/Kconfig | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>>> index 2db84dd..64a9298 100644
>>> --- a/drivers/mmc/host/Kconfig
>>> +++ b/drivers/mmc/host/Kconfig
>>> @@ -420,6 +420,7 @@ config MMC_SDHCI_MSM
>>>       tristate "Qualcomm SDHCI Controller Support"
>>>       depends on ARCH_QCOM || (ARM && COMPILE_TEST)
>>>       depends on MMC_SDHCI_PLTFM
>>> +    select CONFIG_MMC_SDHCI_IO_ACCESSORS
>>
>> CONFIG_MMC_SDHCI_IO_ACCESSORS -> MMC_SDHCI_IO_ACCESSORS
>>
>>>       help
>>>         This selects the Secure Digital Host Controller Interface (SDHCI)
>>>         support present in Qualcomm SOCs. The controller supports
>>>
>>
> 

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

* [PATCH v1 5/5] mmc: Kconfig: Enable CONFIG_MMC_SDHCI_IO_ACCESSORS
@ 2017-09-19  5:44         ` Adrian Hunter
  0 siblings, 0 replies; 33+ messages in thread
From: Adrian Hunter @ 2017-09-19  5:44 UTC (permalink / raw)
  To: linux-arm-kernel

On 19/09/17 07:03, Vijay Viswanath wrote:
> 
> 
> On 9/14/2017 12:04 PM, Adrian Hunter wrote:
>> On 30/08/17 15:51, Vijay Viswanath wrote:
>>> Enable CONFIG_MMC_SDHCI_IO_ACCESSORS so that SDHC controller specific
>>> register read and write APIs, if registered, can be used.
>>
>> Missing signed-off
>>
>> Why don't you put this patch earlier in the patch set and then you don't
>> need all the ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS ?
>>
> 
> This will involve merging the current patches 3 & 4 into one (Some functions
> defined under ifdef in patch 3 are used only in patch 4). Will that be fine ?

Sure

> 
>>> ---
>>>   drivers/mmc/host/Kconfig | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>>> index 2db84dd..64a9298 100644
>>> --- a/drivers/mmc/host/Kconfig
>>> +++ b/drivers/mmc/host/Kconfig
>>> @@ -420,6 +420,7 @@ config MMC_SDHCI_MSM
>>>       tristate "Qualcomm SDHCI Controller Support"
>>>       depends on ARCH_QCOM || (ARM && COMPILE_TEST)
>>>       depends on MMC_SDHCI_PLTFM
>>> +    select CONFIG_MMC_SDHCI_IO_ACCESSORS
>>
>> CONFIG_MMC_SDHCI_IO_ACCESSORS -> MMC_SDHCI_IO_ACCESSORS
>>
>>>       help
>>>         This selects the Secure Digital Host Controller Interface (SDHCI)
>>>         support present in Qualcomm SOCs. The controller supports
>>>
>>
> 

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

end of thread, other threads:[~2017-09-19  5:51 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-30 12:51 [PATCH v1 0/5] mmc: sdhci-msm: Corrections to implementation of power irq Vijay Viswanath
2017-08-30 12:51 ` Vijay Viswanath
2017-08-30 12:51 ` Vijay Viswanath
2017-08-30 12:51 ` [PATCH v1 1/5] mmc: sdhci-msm: fix issue with " Vijay Viswanath
2017-08-30 12:51   ` Vijay Viswanath
2017-08-30 12:51   ` Vijay Viswanath
2017-09-14  6:20   ` Adrian Hunter
2017-09-14  6:20     ` Adrian Hunter
2017-08-30 12:51 ` [PATCH v1 2/5] mmc: sdhci-msm: Fix HW issue with power IRQ handling during reset Vijay Viswanath
2017-08-30 12:51   ` Vijay Viswanath
2017-09-14  6:20   ` Adrian Hunter
2017-09-14  6:20     ` Adrian Hunter
2017-08-30 12:51 ` [PATCH v1 3/5] mmc: sdhci-msm: Add support to wait for power irq Vijay Viswanath
2017-08-30 12:51   ` Vijay Viswanath
2017-09-14  6:34   ` Adrian Hunter
2017-09-14  6:34     ` Adrian Hunter
2017-08-30 12:51 ` [PATCH v1 4/5] mmc: sdhci-msm: Add ops to do sdhc register write Vijay Viswanath
2017-08-30 12:51   ` Vijay Viswanath
2017-09-14  6:34   ` Adrian Hunter
2017-09-14  6:34     ` Adrian Hunter
2017-08-30 12:51 ` [PATCH v1 5/5] mmc: Kconfig: Enable CONFIG_MMC_SDHCI_IO_ACCESSORS Vijay Viswanath
2017-08-30 12:51   ` Vijay Viswanath
2017-09-14  6:34   ` Adrian Hunter
2017-09-14  6:34     ` Adrian Hunter
2017-09-19  4:03     ` Vijay Viswanath
2017-09-19  4:03       ` Vijay Viswanath
2017-09-19  5:44       ` Adrian Hunter
2017-09-19  5:44         ` Adrian Hunter
2017-09-12  9:27 ` [PATCH v1 0/5] mmc: sdhci-msm: Corrections to implementation of power irq Vijay Viswanath
2017-09-12  9:27   ` Vijay Viswanath
2017-09-12  9:27   ` Vijay Viswanath
2017-09-12 10:41   ` Adrian Hunter
2017-09-12 10:41     ` Adrian Hunter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.