All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC 0/8] mmc: sdhci-msm: Add additional support to sdhci-msm driver
@ 2016-06-29 11:20 Ritesh Harjani
  2016-06-29 11:20 ` [PATCH RFC 1/8] mmc: sdhci-msm: Reset vendor specific func register on probe Ritesh Harjani
                   ` (7 more replies)
  0 siblings, 8 replies; 18+ messages in thread
From: Ritesh Harjani @ 2016-06-29 11:20 UTC (permalink / raw)
  To: ulf.hansson, linux-mmc
  Cc: linux-arm-msm, adrian.hunter, asutoshd, kdorfman, david.griego,
	stummala, venkatg, Ritesh Harjani

Hi All,

This patch series aims to add additional support to upstream
sdhci-msm driver. It adds regulator DT parsing, pwr_irq support
(Qualcomm specific) & additional changes required to work with
HS200 mode on latest controllers.


Asutosh Das (1):
  mmc: sdhci-msm: Add pwr_irq support to sdhci-msm

Ritesh Harjani (4):
  mmc: sdhci-msm: Fix the regulator binding name
  mmc: sdhci-msm: Add DT parsing for regulator support
  mmc: sdhci-msm: Add regulator DT props to sdhci-msm bindings
  mmc: sdhci-msm: Add check_power_status to sdhci-msm

Sahitya Tummala (1):
  mmc: sdhci: Add check_power_status host operation

Venkat Gopalakrishnan (2):
  mmc: sdhci-msm: Reset vendor specific func register on probe
  mmc: sdhci-msm: Update DLL reset sequence

 .../devicetree/bindings/mmc/sdhci-msm.txt          |  27 +-
 drivers/mmc/host/sdhci-msm.c                       | 726 ++++++++++++++++++++-
 drivers/mmc/host/sdhci.c                           |  24 +-
 drivers/mmc/host/sdhci.h                           |   5 +
 4 files changed, 762 insertions(+), 20 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

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

* [PATCH RFC 1/8] mmc: sdhci-msm: Reset vendor specific func register on probe
  2016-06-29 11:20 [PATCH RFC 0/8] mmc: sdhci-msm: Add additional support to sdhci-msm driver Ritesh Harjani
@ 2016-06-29 11:20 ` Ritesh Harjani
  2016-06-29 11:20 ` [PATCH RFC 2/8] mmc: sdhci-msm: Fix the regulator binding name Ritesh Harjani
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Ritesh Harjani @ 2016-06-29 11:20 UTC (permalink / raw)
  To: ulf.hansson, linux-mmc
  Cc: linux-arm-msm, adrian.hunter, asutoshd, kdorfman, david.griego,
	stummala, venkatg, Ritesh Harjani

From: Venkat Gopalakrishnan <venkatg@codeaurora.org>

The vendor specific func register doesn't get reset when using the
software reset register. The various bootloader's could leave this
in an unknown state, hence reset this register to it's power on reset
value during probe.

Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 drivers/mmc/host/sdhci-msm.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 0653fe7..6e56f2b 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -45,6 +45,7 @@
 
 #define CORE_VENDOR_SPEC	0x10c
 #define CORE_CLK_PWRSAVE	BIT(1)
+#define CORE_VENDOR_SPEC_POR_VAL	0xa1c
 
 #define CORE_VENDOR_SPEC_CAPABILITIES0	0x11c
 
@@ -507,17 +508,9 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 		goto clk_disable;
 	}
 
-	/* Reset the core and Enable SDHC mode */
-	writel_relaxed(readl_relaxed(msm_host->core_mem + CORE_POWER) |
-		       CORE_SW_RST, msm_host->core_mem + CORE_POWER);
-
-	/* SW reset can take upto 10HCLK + 15MCLK cycles. (min 40us) */
-	usleep_range(1000, 5000);
-	if (readl(msm_host->core_mem + CORE_POWER) & CORE_SW_RST) {
-		dev_err(&pdev->dev, "Stuck in reset\n");
-		ret = -ETIMEDOUT;
-		goto clk_disable;
-	}
+	/* Reset the vendor spec register to power on reset state. */
+	writel_relaxed(CORE_VENDOR_SPEC_POR_VAL,
+			host->ioaddr + CORE_VENDOR_SPEC);
 
 	/* Set HC_MODE_EN bit in HC_MODE register */
 	writel_relaxed(HC_MODE_EN, (msm_host->core_mem + CORE_HC_MODE));
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

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

* [PATCH RFC 2/8] mmc: sdhci-msm: Fix the regulator binding name
  2016-06-29 11:20 [PATCH RFC 0/8] mmc: sdhci-msm: Add additional support to sdhci-msm driver Ritesh Harjani
  2016-06-29 11:20 ` [PATCH RFC 1/8] mmc: sdhci-msm: Reset vendor specific func register on probe Ritesh Harjani
@ 2016-06-29 11:20 ` Ritesh Harjani
  2016-06-29 21:48   ` Andy Gross
  2016-06-29 11:20 ` [PATCH RFC 3/8] mmc: sdhci-msm: Add DT parsing for regulator support Ritesh Harjani
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Ritesh Harjani @ 2016-06-29 11:20 UTC (permalink / raw)
  To: ulf.hansson, linux-mmc
  Cc: linux-arm-msm, adrian.hunter, asutoshd, kdorfman, david.griego,
	stummala, venkatg, Ritesh Harjani

QUALCOMM sdhci-msm driver follows a pwr_irq method
(deviation from regular standard) for any pwr related
operations (which also involves regulator settings changes).

vmmc/vqmmc regulator names are used by core layer,
so change the names to vdd/vdd-io so that it can
be used by sdhci-msm driver alone.

Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
index 485483a..851e66d 100644
--- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
+++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
@@ -27,8 +27,8 @@ Example:
 		bus-width = <8>;
 		non-removable;
 
-		vmmc-supply = <&pm8941_l20>;
-		vqmmc-supply = <&pm8941_s3>;
+		vdd-supply = <&pm8941_l20>;
+		vdd-io-supply = <&pm8941_s3>;
 
 		pinctrl-names = "default";
 		pinctrl-0 = <&sdc1_clk &sdc1_cmd &sdc1_data>;
@@ -44,8 +44,8 @@ Example:
 		bus-width = <4>;
 		cd-gpios = <&msmgpio 62 0x1>;
 
-		vmmc-supply = <&pm8941_l21>;
-		vqmmc-supply = <&pm8941_l13>;
+		vdd-supply = <&pm8941_l21>;
+		vdd-io-supply = <&pm8941_l13>;
 
 		pinctrl-names = "default";
 		pinctrl-0 = <&sdc2_clk &sdc2_cmd &sdc2_data>;
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

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

* [PATCH RFC 3/8] mmc: sdhci-msm: Add DT parsing for regulator support
  2016-06-29 11:20 [PATCH RFC 0/8] mmc: sdhci-msm: Add additional support to sdhci-msm driver Ritesh Harjani
  2016-06-29 11:20 ` [PATCH RFC 1/8] mmc: sdhci-msm: Reset vendor specific func register on probe Ritesh Harjani
  2016-06-29 11:20 ` [PATCH RFC 2/8] mmc: sdhci-msm: Fix the regulator binding name Ritesh Harjani
@ 2016-06-29 11:20 ` Ritesh Harjani
  2016-06-29 11:20 ` [PATCH RFC 4/8] mmc: sdhci-msm: Add regulator DT props to sdhci-msm bindings Ritesh Harjani
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Ritesh Harjani @ 2016-06-29 11:20 UTC (permalink / raw)
  To: ulf.hansson, linux-mmc
  Cc: linux-arm-msm, adrian.hunter, asutoshd, kdorfman, david.griego,
	stummala, venkatg, Ritesh Harjani

This adds the DT parsing for regulator support
to sdhci-msm platform driver.

Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 drivers/mmc/host/sdhci-msm.c | 386 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 384 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 6e56f2b..4964eda 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -19,6 +19,7 @@
 #include <linux/delay.h>
 #include <linux/mmc/mmc.h>
 #include <linux/slab.h>
+#include <linux/regulator/consumer.h>
 
 #include "sdhci-pltfm.h"
 
@@ -54,6 +55,39 @@
 #define CMUX_SHIFT_PHASE_SHIFT	24
 #define CMUX_SHIFT_PHASE_MASK	(7 << CMUX_SHIFT_PHASE_SHIFT)
 
+/* This structure keeps information per regulator */
+struct sdhci_msm_reg_data {
+	struct regulator *reg;	/* voltage regulator handle */
+	const char *name;	/* regulator name */
+
+	/* voltage level to be set */
+	u32 low_vol_level;
+	u32 high_vol_level;
+
+	/* Load values for low power and high power mode */
+	u32 lpm_uA;
+	u32 hpm_uA;
+
+	bool is_enabled;
+	bool is_always_on;
+
+	/* is low power mode setting required for this regulator? */
+	bool lpm_sup;
+};
+
+/*
+ * This structure keeps information for all the
+ * regulators required for a SDCC slot.
+ */
+struct sdhci_msm_slot_reg_data {
+	struct sdhci_msm_reg_data *vdd_data;
+	struct sdhci_msm_reg_data *vdd_io_data;
+};
+
+struct sdhci_msm_pltfm_data {
+	struct sdhci_msm_slot_reg_data *vreg_data;
+};
+
 struct sdhci_msm_host {
 	struct platform_device *pdev;
 	void __iomem *core_mem;	/* MSM SDCC mapped address */
@@ -61,8 +95,337 @@ struct sdhci_msm_host {
 	struct clk *pclk;	/* SDHC peripheral bus clock */
 	struct clk *bus_clk;	/* SDHC bus voter clock */
 	struct mmc_host *mmc;
+	struct sdhci_msm_pltfm_data *pdata;
 };
 
+#define MAX_PROP_SIZE 32
+static int sdhci_msm_dt_parse_vreg_info(struct device *dev,
+		struct sdhci_msm_reg_data **vreg_data, const char *vreg_name)
+{
+	int len, ret = 0;
+	const __be32 *prop;
+	char prop_name[MAX_PROP_SIZE];
+	struct sdhci_msm_reg_data *vreg;
+	struct device_node *np = dev->of_node;
+
+	snprintf(prop_name, MAX_PROP_SIZE, "%s-supply", vreg_name);
+	if (!of_parse_phandle(np, prop_name, 0)) {
+		dev_err(dev, "No vreg data found for %s\n", vreg_name);
+		ret = -EINVAL;
+		return ret;
+	}
+
+	vreg = devm_kzalloc(dev, sizeof(*vreg), GFP_KERNEL);
+	if (!vreg) {
+		ret = -ENOMEM;
+		return ret;
+	}
+
+	vreg->name = vreg_name;
+
+	snprintf(prop_name, MAX_PROP_SIZE,
+			"qcom,%s-always-on", vreg_name);
+	if (of_get_property(np, prop_name, NULL))
+		vreg->is_always_on = true;
+
+	snprintf(prop_name, MAX_PROP_SIZE,
+			"qcom,%s-lpm-sup", vreg_name);
+	if (of_get_property(np, prop_name, NULL))
+		vreg->lpm_sup = true;
+
+	snprintf(prop_name, MAX_PROP_SIZE,
+			"qcom,%s-voltage-level", vreg_name);
+	prop = of_get_property(np, prop_name, &len);
+	if (!prop || (len != (2 * sizeof(__be32)))) {
+		dev_warn(dev, "%s %s property\n",
+			prop ? "invalid format" : "no", prop_name);
+	} else {
+		vreg->low_vol_level = be32_to_cpup(&prop[0]);
+		vreg->high_vol_level = be32_to_cpup(&prop[1]);
+	}
+
+	snprintf(prop_name, MAX_PROP_SIZE,
+			"qcom,%s-current-level", vreg_name);
+	prop = of_get_property(np, prop_name, &len);
+	if (!prop || (len != (2 * sizeof(__be32)))) {
+		dev_warn(dev, "%s %s property\n",
+			prop ? "invalid format" : "no", prop_name);
+	} else {
+		vreg->lpm_uA = be32_to_cpup(&prop[0]);
+		vreg->hpm_uA = be32_to_cpup(&prop[1]);
+	}
+
+	*vreg_data = vreg;
+	dev_dbg(dev, "%s: %s %s vol=[%d %d]uV, curr=[%d %d]uA\n",
+		vreg->name, vreg->is_always_on ? "always_on," : "",
+		vreg->lpm_sup ? "lpm_sup," : "", vreg->low_vol_level,
+		vreg->high_vol_level, vreg->lpm_uA, vreg->hpm_uA);
+
+	return ret;
+}
+
+/* Parse platform data */
+static struct sdhci_msm_pltfm_data *sdhci_msm_populate_pdata(struct device *dev)
+{
+	struct sdhci_msm_pltfm_data *pdata = NULL;
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		goto out;
+
+	pdata->vreg_data = devm_kzalloc(dev, sizeof(struct
+						    sdhci_msm_slot_reg_data),
+					GFP_KERNEL);
+	if (!pdata->vreg_data)
+		goto out;
+
+	if (sdhci_msm_dt_parse_vreg_info(dev, &pdata->vreg_data->vdd_data,
+					 "vdd")) {
+		dev_err(dev, "failed parsing vdd data\n");
+		goto out;
+	}
+
+	if (sdhci_msm_dt_parse_vreg_info(dev,
+					 &pdata->vreg_data->vdd_io_data,
+					 "vdd-io")) {
+		dev_err(dev, "failed parsing vdd-io data\n");
+		goto out;
+	}
+
+	return pdata;
+out:
+	return NULL;
+}
+
+/* Regulator utility functions */
+static int sdhci_msm_vreg_init_reg(struct device *dev,
+					struct sdhci_msm_reg_data *vreg)
+{
+	int ret = 0;
+
+	/* check if regulator is already initialized? */
+	if (vreg->reg)
+		goto out;
+
+	/* Get the regulator handle */
+	vreg->reg = devm_regulator_get(dev, vreg->name);
+	if (IS_ERR(vreg->reg)) {
+		ret = PTR_ERR(vreg->reg);
+		pr_err("%s: devm_regulator_get(%s) failed. ret=%d\n",
+			__func__, vreg->name, ret);
+		goto out;
+	}
+
+	/* sanity check */
+	if (!vreg->high_vol_level || !vreg->hpm_uA) {
+		pr_err("%s: %s invalid constraints specified\n",
+		       __func__, vreg->name);
+		ret = -EINVAL;
+	}
+
+out:
+	return ret;
+}
+
+static void sdhci_msm_vreg_deinit_reg(struct sdhci_msm_reg_data *vreg)
+{
+	if (vreg->reg)
+		devm_regulator_put(vreg->reg);
+}
+
+static int sdhci_msm_vreg_set_optimum_mode(struct sdhci_msm_reg_data
+						  *vreg, int uA_load)
+{
+	int ret = 0;
+
+	/*
+	 * regulators that do not support regulator_set_voltage also
+	 * do not support regulator_set_load
+	 */
+	ret = regulator_set_load(vreg->reg, uA_load);
+	if (ret < 0)
+		pr_err("%s: regulator_set_load(reg=%s,uA_load=%d) failed. ret=%d\n",
+			       __func__, vreg->name, uA_load, ret);
+	else
+		/*
+		 * regulator_set_load() can return non zero
+		 * value even for success case.
+		 */
+		ret = 0;
+	return ret;
+}
+
+static int sdhci_msm_vreg_set_voltage(struct sdhci_msm_reg_data *vreg,
+					int min_uV, int max_uV)
+{
+	int ret = 0;
+
+	ret = regulator_set_voltage(vreg->reg, min_uV, max_uV);
+	if (ret)
+		pr_err("%s: regulator_set_voltage(%s)failed. min_uV=%d,max_uV=%d,ret=%d\n",
+			       __func__, vreg->name, min_uV, max_uV, ret);
+
+	return ret;
+}
+
+static int sdhci_msm_vreg_enable(struct sdhci_msm_reg_data *vreg)
+{
+	int ret = 0;
+
+	/* Put regulator in HPM (high power mode) */
+	ret = sdhci_msm_vreg_set_optimum_mode(vreg, vreg->hpm_uA);
+	if (ret < 0)
+		goto out;
+
+	if (!vreg->is_enabled) {
+		/* Set voltage level */
+		ret = sdhci_msm_vreg_set_voltage(vreg, vreg->high_vol_level,
+						vreg->high_vol_level);
+		if (ret)
+			goto out;
+	}
+	ret = regulator_enable(vreg->reg);
+	if (ret) {
+		pr_err("%s: regulator_enable(%s) failed. ret=%d\n",
+				__func__, vreg->name, ret);
+		goto out;
+	}
+	vreg->is_enabled = true;
+
+out:
+	return ret;
+}
+
+static int sdhci_msm_vreg_disable(struct sdhci_msm_reg_data *vreg)
+{
+	int ret = 0;
+
+	/* Never disable regulator marked as always_on */
+	if (vreg->is_enabled && !vreg->is_always_on) {
+		ret = regulator_disable(vreg->reg);
+		if (ret) {
+			pr_err("%s: regulator_disable(%s) failed. ret=%d\n",
+				__func__, vreg->name, ret);
+			goto out;
+		}
+		vreg->is_enabled = false;
+
+		ret = sdhci_msm_vreg_set_optimum_mode(vreg, 0);
+		if (ret < 0)
+			goto out;
+
+		/* Set min. voltage level to 0 */
+		ret = sdhci_msm_vreg_set_voltage(vreg, 0, vreg->high_vol_level);
+		if (ret)
+			goto out;
+	} else if (vreg->is_enabled && vreg->is_always_on) {
+		if (vreg->lpm_sup) {
+			/* Put always_on regulator in LPM (low power mode) */
+			ret = sdhci_msm_vreg_set_optimum_mode(vreg,
+							      vreg->lpm_uA);
+			if (ret < 0)
+				goto out;
+		}
+	}
+out:
+	return ret;
+}
+
+static int sdhci_msm_setup_vreg(struct sdhci_msm_pltfm_data *pdata,
+			bool enable, bool is_init)
+{
+	int ret = 0, i;
+	struct sdhci_msm_slot_reg_data *curr_slot;
+	struct sdhci_msm_reg_data *vreg_table[2];
+
+	curr_slot = pdata->vreg_data;
+	if (!curr_slot) {
+		pr_debug("%s: vreg info unavailable,assuming the slot is powered by always on domain\n",
+			 __func__);
+		goto out;
+	}
+
+	vreg_table[0] = curr_slot->vdd_data;
+	vreg_table[1] = curr_slot->vdd_io_data;
+
+	for (i = 0; i < ARRAY_SIZE(vreg_table); i++) {
+		if (vreg_table[i]) {
+			if (enable)
+				ret = sdhci_msm_vreg_enable(vreg_table[i]);
+			else
+				ret = sdhci_msm_vreg_disable(vreg_table[i]);
+			if (ret)
+				goto out;
+		}
+	}
+out:
+	return ret;
+}
+
+/*
+ * Reset vreg by ensuring it is off during probe. A call
+ * to enable vreg is needed to balance disable vreg
+ */
+static int sdhci_msm_vreg_reset(struct sdhci_msm_pltfm_data *pdata)
+{
+	int ret;
+
+	ret = sdhci_msm_setup_vreg(pdata, 1, true);
+	if (ret)
+		return ret;
+	ret = sdhci_msm_setup_vreg(pdata, 0, true);
+	return ret;
+}
+
+/* This init function should be called only once for each SDHC slot */
+static int sdhci_msm_vreg_init(struct device *dev,
+				struct sdhci_msm_pltfm_data *pdata,
+				bool is_init)
+{
+	int ret = 0;
+	struct sdhci_msm_slot_reg_data *curr_slot;
+	struct sdhci_msm_reg_data *curr_vdd_reg, *curr_vdd_io_reg;
+
+	curr_slot = pdata->vreg_data;
+	if (!curr_slot)
+		goto out;
+
+	curr_vdd_reg = curr_slot->vdd_data;
+	curr_vdd_io_reg = curr_slot->vdd_io_data;
+
+	if (!is_init)
+		/* Deregister all regulators from regulator framework */
+		goto vdd_io_reg_deinit;
+
+	/*
+	 * Get the regulator handle from voltage regulator framework
+	 * and then try to set the voltage level for the regulator
+	 */
+	if (curr_vdd_reg) {
+		ret = sdhci_msm_vreg_init_reg(dev, curr_vdd_reg);
+		if (ret)
+			goto out;
+	}
+	if (curr_vdd_io_reg) {
+		ret = sdhci_msm_vreg_init_reg(dev, curr_vdd_io_reg);
+		if (ret)
+			goto vdd_reg_deinit;
+	}
+	ret = sdhci_msm_vreg_reset(pdata);
+	if (ret)
+		dev_err(dev, "vreg reset failed (%d)\n", ret);
+	goto out;
+
+vdd_io_reg_deinit:
+	if (curr_vdd_io_reg)
+		sdhci_msm_vreg_deinit_reg(curr_vdd_io_reg);
+vdd_reg_deinit:
+	if (curr_vdd_reg)
+		sdhci_msm_vreg_deinit_reg(curr_vdd_reg);
+out:
+	return ret;
+}
+
 /* Platform specific tuning */
 static inline int msm_dll_poll_ck_out_en(struct sdhci_host *host, u8 poll)
 {
@@ -458,6 +821,15 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 
 	sdhci_get_of_property(pdev);
 
+	/* Extract platform data */
+	if (pdev->dev.of_node) {
+		msm_host->pdata = sdhci_msm_populate_pdata(&pdev->dev);
+		if (!msm_host->pdata) {
+			dev_err(&pdev->dev, "DT parsing error\n");
+			goto pltfm_free;
+		}
+	}
+
 	/* Setup SDCC bus voter clock. */
 	msm_host->bus_clk = devm_clk_get(&pdev->dev, "bus");
 	if (!IS_ERR(msm_host->bus_clk)) {
@@ -499,13 +871,20 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 	if (ret)
 		goto pclk_disable;
 
+	/* Setup regulators */
+	ret = sdhci_msm_vreg_init(&pdev->dev, msm_host->pdata, true);
+	if (ret) {
+		dev_err(&pdev->dev, "Regulator setup failed (%d)\n", ret);
+		goto clk_disable;
+	}
+
 	core_memres = platform_get_resource(pdev, IORESOURCE_MEM, 1);
 	msm_host->core_mem = devm_ioremap_resource(&pdev->dev, core_memres);
 
 	if (IS_ERR(msm_host->core_mem)) {
 		dev_err(&pdev->dev, "Failed to remap registers\n");
 		ret = PTR_ERR(msm_host->core_mem);
-		goto clk_disable;
+		goto vreg_deinit;
 	}
 
 	/* Reset the vendor spec register to power on reset state. */
@@ -540,10 +919,12 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 
 	ret = sdhci_add_host(host);
 	if (ret)
-		goto clk_disable;
+		goto vreg_deinit;
 
 	return 0;
 
+vreg_deinit:
+	sdhci_msm_vreg_init(&pdev->dev, msm_host->pdata, false);
 clk_disable:
 	clk_disable_unprepare(msm_host->clk);
 pclk_disable:
@@ -565,6 +946,7 @@ static int sdhci_msm_remove(struct platform_device *pdev)
 		    0xffffffff);
 
 	sdhci_remove_host(host, dead);
+	sdhci_msm_vreg_init(&pdev->dev, msm_host->pdata, false);
 	clk_disable_unprepare(msm_host->clk);
 	clk_disable_unprepare(msm_host->pclk);
 	if (!IS_ERR(msm_host->bus_clk))
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.


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

* [PATCH RFC 4/8] mmc: sdhci-msm: Add regulator DT props to sdhci-msm bindings
  2016-06-29 11:20 [PATCH RFC 0/8] mmc: sdhci-msm: Add additional support to sdhci-msm driver Ritesh Harjani
                   ` (2 preceding siblings ...)
  2016-06-29 11:20 ` [PATCH RFC 3/8] mmc: sdhci-msm: Add DT parsing for regulator support Ritesh Harjani
@ 2016-06-29 11:20 ` Ritesh Harjani
  2016-06-29 21:53   ` Andy Gross
  2016-06-29 11:20 ` [PATCH RFC 5/8] mmc: sdhci: Add check_power_status host operation Ritesh Harjani
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Ritesh Harjani @ 2016-06-29 11:20 UTC (permalink / raw)
  To: ulf.hansson, linux-mmc
  Cc: linux-arm-msm, adrian.hunter, asutoshd, kdorfman, david.griego,
	stummala, venkatg, Ritesh Harjani

This patch adds the DT properties for voltage regulator nodes
for Qualcomm SDHCI driver.

Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
index 851e66d..32cea75 100644
--- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
+++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
@@ -17,7 +17,15 @@ Required properties:
 	"iface" - Main peripheral bus clock (PCLK/HCLK - AHB Bus clock) (required)
 	"core"	- SDC MMC clock (MCLK) (required)
 	"bus"	- SDCC bus voter clock (optional)
+- qcom,<supply>-voltage_level - specifies voltage levels for supply. Should be
+					specified in pairs (min, max), units uV.
+- qcom,<supply>-current_level - specifies load levels for supply in lpm or
+					high power mode (hpm). Should be specified in
+					pairs (lpm, hpm), units uA.
 
+Optional Properties:
+	- qcom,<supply>-always-on - specifies whether supply should be kept "on" always.
+	- qcom,<supply>-lpm_sup - specifies whether supply can be kept in low power mode (lpm).
 Example:
 
 	sdhc_1: sdhci@f9824900 {
@@ -28,7 +36,13 @@ Example:
 		non-removable;
 
 		vdd-supply = <&pm8941_l20>;
+		qcom,vdd-voltage-level = <2950000 2950000>;
+		qcom,vdd-current-level = <200 570000>;
+
 		vdd-io-supply = <&pm8941_s3>;
+		qcom,vdd-io-always-on;
+		qcom,vdd-io-voltage-level = <1800000 1800000>;
+		qcom,vdd-io-current-level = <110 325000>;
 
 		pinctrl-names = "default";
 		pinctrl-0 = <&sdc1_clk &sdc1_cmd &sdc1_data>;
@@ -45,7 +59,12 @@ Example:
 		cd-gpios = <&msmgpio 62 0x1>;
 
 		vdd-supply = <&pm8941_l21>;
+		qcom,vdd-voltage-level = <2950000 2950000>;
+		qcom,vdd-current-level = <200 800000>;
+
 		vdd-io-supply = <&pm8941_l13>;
+		qcom,vdd-io-voltage-level = <1800000 2950000>;
+		qcom,vdd-io-current-level = <200 22000>;
 
 		pinctrl-names = "default";
 		pinctrl-0 = <&sdc2_clk &sdc2_cmd &sdc2_data>;
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

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

* [PATCH RFC 5/8] mmc: sdhci: Add check_power_status host operation
  2016-06-29 11:20 [PATCH RFC 0/8] mmc: sdhci-msm: Add additional support to sdhci-msm driver Ritesh Harjani
                   ` (3 preceding siblings ...)
  2016-06-29 11:20 ` [PATCH RFC 4/8] mmc: sdhci-msm: Add regulator DT props to sdhci-msm bindings Ritesh Harjani
@ 2016-06-29 11:20 ` Ritesh Harjani
  2016-06-30  6:00   ` Adrian Hunter
  2016-06-29 11:20 ` [PATCH RFC 6/8] mmc: sdhci-msm: Add pwr_irq support to sdhci-msm Ritesh Harjani
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Ritesh Harjani @ 2016-06-29 11:20 UTC (permalink / raw)
  To: ulf.hansson, linux-mmc
  Cc: linux-arm-msm, adrian.hunter, asutoshd, kdorfman, david.griego,
	stummala, venkatg, Ritesh Harjani

From: Sahitya Tummala <stummala@codeaurora.org>

MSM SDHCI doesn't control power as specified by the Standard
Host Controller 3.0 spec. Writing to power control register/
reset register/voltage bit of host control register would
trigger an IRQ with appropriate status bits set. Hence, use
host op check_power_status after writing to power control
register to check the status and wait until the IRQ is handled.

Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 drivers/mmc/host/sdhci.c | 24 +++++++++++++++++++++---
 drivers/mmc/host/sdhci.h |  5 +++++
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 0e3d7c0..12f74bd 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -176,6 +176,10 @@ void sdhci_reset(struct sdhci_host *host, u8 mask)
 	/* Wait max 100 ms */
 	timeout = 100;
 
+	if (host->ops->check_power_status && host->pwr &&
+	    (mask & SDHCI_RESET_ALL))
+		host->ops->check_power_status(host, REQ_BUS_OFF);
+
 	/* hw clears the bit when it's done */
 	while (sdhci_readb(host, SDHCI_SOFTWARE_RESET) & mask) {
 		if (timeout == 0) {
@@ -1306,6 +1310,8 @@ void sdhci_set_power(struct sdhci_host *host, unsigned char mode,
 
 	if (pwr == 0) {
 		sdhci_writeb(host, 0, SDHCI_POWER_CONTROL);
+		if (host->ops->check_power_status)
+			host->ops->check_power_status(host, REQ_BUS_OFF);
 		if (host->quirks2 & SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON)
 			sdhci_runtime_pm_bus_off(host);
 	} else {
@@ -1313,20 +1319,28 @@ void sdhci_set_power(struct sdhci_host *host, unsigned char mode,
 		 * Spec says that we should clear the power reg before setting
 		 * a new value. Some controllers don't seem to like this though.
 		 */
-		if (!(host->quirks & SDHCI_QUIRK_SINGLE_POWER_WRITE))
+		if (!(host->quirks & SDHCI_QUIRK_SINGLE_POWER_WRITE)) {
 			sdhci_writeb(host, 0, SDHCI_POWER_CONTROL);
-
+			if (host->ops->check_power_status)
+				host->ops->check_power_status(host,
+							REQ_BUS_OFF);
+		}
 		/*
 		 * At least the Marvell CaFe chip gets confused if we set the
 		 * voltage and set turn on power at the same time, so set the
 		 * voltage first.
 		 */
-		if (host->quirks & SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER)
+		if (host->quirks & SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER) {
 			sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL);
+			if (host->ops->check_power_status)
+				host->ops->check_power_status(host, REQ_BUS_ON);
+		}
 
 		pwr |= SDHCI_POWER_ON;
 
 		sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL);
+		if (host->ops->check_power_status)
+			host->ops->check_power_status(host, REQ_BUS_ON);
 
 		if (host->quirks2 & SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON)
 			sdhci_runtime_pm_bus_on(host);
@@ -1736,6 +1750,8 @@ static int sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
 		/* Set 1.8V Signal Enable in the Host Control2 register to 0 */
 		ctrl &= ~SDHCI_CTRL_VDD_180;
 		sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
+		if (host->ops->check_power_status)
+			host->ops->check_power_status(host, REQ_IO_HIGH);
 
 		if (!IS_ERR(mmc->supply.vqmmc)) {
 			ret = regulator_set_voltage(mmc->supply.vqmmc, 2700000,
@@ -1775,6 +1791,8 @@ static int sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
 		 */
 		ctrl |= SDHCI_CTRL_VDD_180;
 		sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
+		if (host->ops->check_power_status)
+			host->ops->check_power_status(host, REQ_IO_LOW);
 
 		/* Some controller need to do more when switching */
 		if (host->ops->voltage_switch)
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 609f87c..5758cca 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -549,6 +549,11 @@ struct sdhci_ops {
 					 struct mmc_card *card,
 					 unsigned int max_dtr, int host_drv,
 					 int card_drv, int *drv_type);
+#define REQ_BUS_OFF	BIT(0)
+#define REQ_BUS_ON	BIT(1)
+#define REQ_IO_LOW	BIT(2)
+#define REQ_IO_HIGH	BIT(3)
+	void    (*check_power_status)(struct sdhci_host *host, u32 req_type);
 };
 
 #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.


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

* [PATCH RFC 6/8] mmc: sdhci-msm: Add pwr_irq support to sdhci-msm
  2016-06-29 11:20 [PATCH RFC 0/8] mmc: sdhci-msm: Add additional support to sdhci-msm driver Ritesh Harjani
                   ` (4 preceding siblings ...)
  2016-06-29 11:20 ` [PATCH RFC 5/8] mmc: sdhci: Add check_power_status host operation Ritesh Harjani
@ 2016-06-29 11:20 ` Ritesh Harjani
  2016-07-01  3:57   ` Andy Gross
  2016-06-29 11:20 ` [PATCH RFC 7/8] mmc: sdhci-msm: Add check_power_status " Ritesh Harjani
  2016-06-29 11:20 ` [PATCH RFC 8/8] mmc: sdhci-msm: Update DLL reset sequence Ritesh Harjani
  7 siblings, 1 reply; 18+ messages in thread
From: Ritesh Harjani @ 2016-06-29 11:20 UTC (permalink / raw)
  To: ulf.hansson, linux-mmc
  Cc: linux-arm-msm, adrian.hunter, asutoshd, kdorfman, david.griego,
	stummala, venkatg, Ritesh Harjani

From: Asutosh Das <asutoshd@codeaurora.org>

MSM SDHCI doesn't control power as specified by the Standard
Host Controller 3.0 spec. Writing to power control register/
reset register/voltage bit of host control register would
trigger an IRQ with appropriate status bits set.

This adds the pwr_irq functionality to sdhci-msm.

Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 drivers/mmc/host/sdhci-msm.c | 217 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 217 insertions(+)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 4964eda..8badcf8 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -33,6 +33,23 @@
 #define CORE_POWER		0x0
 #define CORE_SW_RST		BIT(7)
 
+#define CORE_PWRCTL_STATUS	0xdc
+#define CORE_PWRCTL_MASK	0xe0
+#define CORE_PWRCTL_CLEAR	0xe4
+#define CORE_PWRCTL_CTL		0xe8
+
+#define CORE_PWRCTL_BUS_OFF	0x01
+#define CORE_PWRCTL_BUS_ON	BIT(1)
+#define CORE_PWRCTL_IO_LOW	BIT(2)
+#define CORE_PWRCTL_IO_HIGH	BIT(3)
+
+#define CORE_PWRCTL_BUS_SUCCESS	0x01
+#define CORE_PWRCTL_BUS_FAIL	BIT(1)
+#define CORE_PWRCTL_IO_SUCCESS	BIT(2)
+#define CORE_PWRCTL_IO_FAIL	BIT(3)
+
+#define INT_MASK		0xf
+
 #define MAX_PHASES		16
 #define CORE_DLL_LOCK		BIT(7)
 #define CORE_DLL_EN		BIT(16)
@@ -47,6 +64,7 @@
 #define CORE_VENDOR_SPEC	0x10c
 #define CORE_CLK_PWRSAVE	BIT(1)
 #define CORE_VENDOR_SPEC_POR_VAL	0xa1c
+#define CORE_IO_PAD_PWR_SWITCH	BIT(16)
 
 #define CORE_VENDOR_SPEC_CAPABILITIES0	0x11c
 
@@ -88,6 +106,18 @@ struct sdhci_msm_pltfm_data {
 	struct sdhci_msm_slot_reg_data *vreg_data;
 };
 
+enum vdd_io_level {
+	/* set vdd_io_data->low_vol_level */
+	VDD_IO_LOW,
+	/* set vdd_io_data->high_vol_level */
+	VDD_IO_HIGH,
+	/*
+	 * set whatever there in voltage_level (third argument) of
+	 * sdhci_msm_set_vdd_io_vol() function.
+	 */
+	VDD_IO_SET_LEVEL,
+};
+
 struct sdhci_msm_host {
 	struct platform_device *pdev;
 	void __iomem *core_mem;	/* MSM SDCC mapped address */
@@ -96,6 +126,9 @@ struct sdhci_msm_host {
 	struct clk *bus_clk;	/* SDHC bus voter clock */
 	struct mmc_host *mmc;
 	struct sdhci_msm_pltfm_data *pdata;
+	int pwr_irq;
+	u32 curr_pwr_state;
+	u32 curr_io_level;
 };
 
 #define MAX_PROP_SIZE 32
@@ -426,6 +459,150 @@ out:
 	return ret;
 }
 
+
+static int sdhci_msm_set_vdd_io_vol(struct sdhci_msm_pltfm_data *pdata,
+			enum vdd_io_level level,
+			unsigned int voltage_level)
+{
+	int ret = 0;
+	int set_level;
+	struct sdhci_msm_reg_data *vdd_io_reg;
+
+	if (!pdata->vreg_data)
+		return ret;
+
+	vdd_io_reg = pdata->vreg_data->vdd_io_data;
+	if (vdd_io_reg && vdd_io_reg->is_enabled) {
+		switch (level) {
+		case VDD_IO_LOW:
+			set_level = vdd_io_reg->low_vol_level;
+			break;
+		case VDD_IO_HIGH:
+			set_level = vdd_io_reg->high_vol_level;
+			break;
+		case VDD_IO_SET_LEVEL:
+			set_level = voltage_level;
+			break;
+		default:
+			pr_err("%s: invalid argument level = %d",
+					__func__, level);
+			ret = -EINVAL;
+			return ret;
+		}
+		ret = sdhci_msm_vreg_set_voltage(vdd_io_reg, set_level,
+				set_level);
+	}
+	return ret;
+}
+
+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);
+	u8 irq_status = 0;
+	u8 irq_ack = 0;
+	int ret = 0;
+	int pwr_state = 0, io_level = 0;
+	unsigned long flags;
+
+	irq_status = readb_relaxed(msm_host->core_mem + CORE_PWRCTL_STATUS);
+	pr_debug("%s: Received IRQ(%d), status=0x%x\n",
+		mmc_hostname(msm_host->mmc), irq, irq_status);
+
+	/* Clear the interrupt */
+	writeb_relaxed(irq_status, (msm_host->core_mem + CORE_PWRCTL_CLEAR));
+	/*
+	 * SDHC has core_mem and hc_mem device memory and these memory
+	 * addresses do not fall within 1KB region. Hence, any update to
+	 * core_mem address space would require an mb() to ensure this gets
+	 * completed before its next update to registers within hc_mem.
+	 */
+	mb();
+
+	/* Handle BUS ON/OFF*/
+	if (irq_status & CORE_PWRCTL_BUS_ON) {
+		ret = sdhci_msm_setup_vreg(msm_host->pdata, true, false);
+		if (!ret) {
+			ret = sdhci_msm_set_vdd_io_vol(msm_host->pdata,
+					VDD_IO_HIGH, 0);
+		}
+		if (ret)
+			irq_ack |= CORE_PWRCTL_BUS_FAIL;
+		else
+			irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
+
+		pwr_state = REQ_BUS_ON;
+		io_level = REQ_IO_HIGH;
+	}
+	if (irq_status & CORE_PWRCTL_BUS_OFF) {
+		ret = sdhci_msm_setup_vreg(msm_host->pdata, false, false);
+		if (!ret) {
+			ret = sdhci_msm_set_vdd_io_vol(msm_host->pdata,
+					VDD_IO_LOW, 0);
+		}
+		if (ret)
+			irq_ack |= CORE_PWRCTL_BUS_FAIL;
+		else
+			irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
+
+		pwr_state = REQ_BUS_OFF;
+		io_level = REQ_IO_LOW;
+	}
+	/* Handle IO LOW/HIGH */
+	if (irq_status & CORE_PWRCTL_IO_LOW) {
+		/* Switch voltage Low */
+		ret = sdhci_msm_set_vdd_io_vol(msm_host->pdata, VDD_IO_LOW, 0);
+		if (ret)
+			irq_ack |= CORE_PWRCTL_IO_FAIL;
+		else
+			irq_ack |= CORE_PWRCTL_IO_SUCCESS;
+
+		io_level = REQ_IO_LOW;
+	}
+	if (irq_status & CORE_PWRCTL_IO_HIGH) {
+		/* Switch voltage High */
+		ret = sdhci_msm_set_vdd_io_vol(msm_host->pdata, VDD_IO_HIGH, 0);
+		if (ret)
+			irq_ack |= CORE_PWRCTL_IO_FAIL;
+		else
+			irq_ack |= CORE_PWRCTL_IO_SUCCESS;
+
+		io_level = REQ_IO_HIGH;
+	}
+
+	/* ACK status to the core */
+	writeb_relaxed(irq_ack, (msm_host->core_mem + CORE_PWRCTL_CTL));
+	/*
+	 * SDHC has core_mem and hc_mem device memory and these memory
+	 * addresses do not fall within 1KB region. Hence, any update to
+	 * core_mem address space would require an mb() to ensure this gets
+	 * completed before its next update to registers within hc_mem.
+	 */
+	mb();
+
+	if (io_level & REQ_IO_HIGH)
+		writel_relaxed((readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC) &
+				~CORE_IO_PAD_PWR_SWITCH),
+				host->ioaddr + CORE_VENDOR_SPEC);
+	else if (io_level & REQ_IO_LOW)
+		writel_relaxed((readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC) |
+				CORE_IO_PAD_PWR_SWITCH),
+				host->ioaddr + CORE_VENDOR_SPEC);
+
+	/* Ensure before completion that above writes are propagated */
+	mb();
+
+	pr_debug("%s: Handled IRQ(%d), ret=%d, ack=0x%x\n",
+		mmc_hostname(msm_host->mmc), irq, ret, irq_ack);
+	if (pwr_state)
+		msm_host->curr_pwr_state = pwr_state;
+	if (io_level)
+		msm_host->curr_io_level = io_level;
+
+	return IRQ_HANDLED;
+}
+
 /* Platform specific tuning */
 static inline int msm_dll_poll_ck_out_en(struct sdhci_host *host, u8 poll)
 {
@@ -801,6 +978,7 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 	struct sdhci_pltfm_host *pltfm_host;
 	struct sdhci_msm_host *msm_host;
 	struct resource *core_memres;
+	u32 irq_status, irq_ctl;
 	int ret;
 	u16 host_version, core_minor;
 	u32 core_version, caps;
@@ -917,6 +1095,45 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 			       CORE_VENDOR_SPEC_CAPABILITIES0);
 	}
 
+	/*
+	 * POR reset of VENDOR_SPEC/CORE_SW_RST above 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.
+	 */
+	irq_status = readl_relaxed(msm_host->core_mem + CORE_PWRCTL_STATUS);
+	writel_relaxed(irq_status, (msm_host->core_mem + CORE_PWRCTL_CLEAR));
+	irq_ctl = readl_relaxed(msm_host->core_mem + CORE_PWRCTL_CTL);
+	if (irq_status & (CORE_PWRCTL_BUS_ON | CORE_PWRCTL_BUS_OFF))
+		irq_ctl |= CORE_PWRCTL_BUS_SUCCESS;
+	if (irq_status & (CORE_PWRCTL_IO_HIGH | CORE_PWRCTL_IO_LOW))
+		irq_ctl |= CORE_PWRCTL_IO_SUCCESS;
+	writel_relaxed(irq_ctl, (msm_host->core_mem + CORE_PWRCTL_CTL));
+
+	/*
+	 * Ensure that above writes are propogated before interrupt enablement
+	 * in GIC.
+	 */
+	mb();
+
+	/* Setup PWRCTL irq */
+	msm_host->pwr_irq = platform_get_irq_byname(pdev, "pwr_irq");
+	if (msm_host->pwr_irq < 0) {
+		dev_err(&pdev->dev, "Failed to get pwr_irq by name (%d)\n",
+				msm_host->pwr_irq);
+		goto vreg_deinit;
+	}
+	ret = devm_request_threaded_irq(&pdev->dev, msm_host->pwr_irq, NULL,
+					sdhci_msm_pwr_irq, IRQF_ONESHOT,
+					dev_name(&pdev->dev), host);
+	if (ret) {
+		dev_err(&pdev->dev, "Request threaded irq(%d) failed (%d)\n",
+				msm_host->pwr_irq, ret);
+		goto vreg_deinit;
+	}
+
 	ret = sdhci_add_host(host);
 	if (ret)
 		goto vreg_deinit;
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

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

* [PATCH RFC 7/8] mmc: sdhci-msm: Add check_power_status to sdhci-msm
  2016-06-29 11:20 [PATCH RFC 0/8] mmc: sdhci-msm: Add additional support to sdhci-msm driver Ritesh Harjani
                   ` (5 preceding siblings ...)
  2016-06-29 11:20 ` [PATCH RFC 6/8] mmc: sdhci-msm: Add pwr_irq support to sdhci-msm Ritesh Harjani
@ 2016-06-29 11:20 ` Ritesh Harjani
  2016-06-29 11:20 ` [PATCH RFC 8/8] mmc: sdhci-msm: Update DLL reset sequence Ritesh Harjani
  7 siblings, 0 replies; 18+ messages in thread
From: Ritesh Harjani @ 2016-06-29 11:20 UTC (permalink / raw)
  To: ulf.hansson, linux-mmc
  Cc: linux-arm-msm, adrian.hunter, asutoshd, kdorfman, david.griego,
	stummala, venkatg, Ritesh Harjani

This adds handler to check whether pwr_irq has
completed or not by adding sdhci_msm_check_power_status.

Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 drivers/mmc/host/sdhci-msm.c | 57 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 8badcf8..912ca6e 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -129,6 +129,8 @@ struct sdhci_msm_host {
 	int pwr_irq;
 	u32 curr_pwr_state;
 	u32 curr_io_level;
+	struct completion pwr_irq_completion;
+	spinlock_t pwr_irq_lock;
 };
 
 #define MAX_PROP_SIZE 32
@@ -595,14 +597,62 @@ static irqreturn_t sdhci_msm_pwr_irq(int irq, void *data)
 
 	pr_debug("%s: Handled IRQ(%d), ret=%d, ack=0x%x\n",
 		mmc_hostname(msm_host->mmc), irq, ret, irq_ack);
+	spin_lock_irqsave(&msm_host->pwr_irq_lock, flags);
 	if (pwr_state)
 		msm_host->curr_pwr_state = pwr_state;
 	if (io_level)
 		msm_host->curr_io_level = io_level;
+	complete(&msm_host->pwr_irq_completion);
+	spin_unlock_irqrestore(&msm_host->pwr_irq_lock, flags);
 
 	return IRQ_HANDLED;
 }
 
+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);
+	unsigned long flags;
+	bool locked = false;
+	bool done = false;
+
+	pr_debug("%s: %s: power status before waiting 0x%x\n",
+		mmc_hostname(host->mmc), __func__,
+		readb_relaxed(msm_host->core_mem + CORE_PWRCTL_CTL));
+
+	if (spin_is_locked(&host->lock))
+		locked = true;
+
+	spin_lock_irqsave(&msm_host->pwr_irq_lock, flags);
+	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);
+	if ((req_type & msm_host->curr_pwr_state) ||
+			(req_type & msm_host->curr_io_level))
+		done = true;
+	spin_unlock_irqrestore(&msm_host->pwr_irq_lock, flags);
+
+	if (locked)
+		spin_unlock_irq(&host->lock);
+	/*
+	 * 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)
+		init_completion(&msm_host->pwr_irq_completion);
+	else
+		wait_for_completion(&msm_host->pwr_irq_completion);
+
+	if (locked)
+		spin_lock_irq(&host->lock);
+
+	pr_debug("%s: %s: request %d done\n", mmc_hostname(host->mmc),
+			__func__, req_type);
+}
+
 /* Platform specific tuning */
 static inline int msm_dll_poll_ck_out_en(struct sdhci_host *host, u8 poll)
 {
@@ -964,6 +1014,7 @@ static const struct sdhci_ops sdhci_msm_ops = {
 	.set_clock = sdhci_set_clock,
 	.set_bus_width = sdhci_set_bus_width,
 	.set_uhs_signaling = sdhci_set_uhs_signaling,
+	.check_power_status = sdhci_msm_check_power_status,
 };
 
 static const struct sdhci_pltfm_data sdhci_msm_pdata = {
@@ -1134,6 +1185,12 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 		goto vreg_deinit;
 	}
 
+	init_completion(&msm_host->pwr_irq_completion);
+	spin_lock_init(&msm_host->pwr_irq_lock);
+
+	/* Enable pwr irq interrupts */
+	writel_relaxed(INT_MASK, (msm_host->core_mem + CORE_PWRCTL_MASK));
+
 	ret = sdhci_add_host(host);
 	if (ret)
 		goto vreg_deinit;
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

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

* [PATCH RFC 8/8] mmc: sdhci-msm: Update DLL reset sequence
  2016-06-29 11:20 [PATCH RFC 0/8] mmc: sdhci-msm: Add additional support to sdhci-msm driver Ritesh Harjani
                   ` (6 preceding siblings ...)
  2016-06-29 11:20 ` [PATCH RFC 7/8] mmc: sdhci-msm: Add check_power_status " Ritesh Harjani
@ 2016-06-29 11:20 ` Ritesh Harjani
  7 siblings, 0 replies; 18+ messages in thread
From: Ritesh Harjani @ 2016-06-29 11:20 UTC (permalink / raw)
  To: ulf.hansson, linux-mmc
  Cc: linux-arm-msm, adrian.hunter, asutoshd, kdorfman, david.griego,
	stummala, venkatg, Ritesh Harjani

From: Venkat Gopalakrishnan <venkatg@codeaurora.org>

The latest version of the SDCC core requires a change in the reset
sequence for DLL tuning. Make necessary changes as needed.

Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 drivers/mmc/host/sdhci-msm.c | 51 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 912ca6e..4a864cb 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -61,6 +61,10 @@
 #define CORE_DLL_CONFIG		0x100
 #define CORE_DLL_STATUS		0x108
 
+#define CORE_DLL_CONFIG_2	0x1b4
+#define CORE_FLL_CYCLE_CNT	BIT(18)
+#define CORE_DLL_CLOCK_DISABLE	BIT(21)
+
 #define CORE_VENDOR_SPEC	0x10c
 #define CORE_CLK_PWRSAVE	BIT(1)
 #define CORE_VENDOR_SPEC_POR_VAL	0xa1c
@@ -68,6 +72,8 @@
 
 #define CORE_VENDOR_SPEC_CAPABILITIES0	0x11c
 
+#define TCXO_FREQ		19200000
+
 #define CDR_SELEXT_SHIFT	20
 #define CDR_SELEXT_MASK		(0xf << CDR_SELEXT_SHIFT)
 #define CMUX_SHIFT_PHASE_SHIFT	24
@@ -131,6 +137,7 @@ struct sdhci_msm_host {
 	u32 curr_io_level;
 	struct completion pwr_irq_completion;
 	spinlock_t pwr_irq_lock;
+	bool use_updated_dll_reset;
 };
 
 #define MAX_PROP_SIZE 32
@@ -878,9 +885,14 @@ static inline void msm_cm_dll_set_freq(struct sdhci_host *host)
 static int msm_init_cm_dll(struct sdhci_host *host)
 {
 	struct mmc_host *mmc = host->mmc;
+	struct sdhci_pltfm_host *pltfm_host;
+	struct sdhci_msm_host *msm_host;
 	int wait_cnt = 50;
 	unsigned long flags;
 
+	pltfm_host = sdhci_priv(host);
+	msm_host = sdhci_pltfm_priv(pltfm_host);
+
 	spin_lock_irqsave(&host->lock, flags);
 
 	/*
@@ -891,6 +903,17 @@ static int msm_init_cm_dll(struct sdhci_host *host)
 	writel_relaxed((readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC)
 			& ~CORE_CLK_PWRSAVE), host->ioaddr + CORE_VENDOR_SPEC);
 
+	if (msm_host->use_updated_dll_reset) {
+		writel_relaxed((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG)
+				& ~CORE_CK_OUT_EN),
+				host->ioaddr + CORE_DLL_CONFIG);
+
+		writel_relaxed((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG_2)
+				| CORE_DLL_CLOCK_DISABLE),
+				host->ioaddr + CORE_DLL_CONFIG_2);
+	}
+
+
 	/* Write 1 to DLL_RST bit of DLL_CONFIG register */
 	writel_relaxed((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG)
 			| CORE_DLL_RST), host->ioaddr + CORE_DLL_CONFIG);
@@ -900,6 +923,23 @@ static int msm_init_cm_dll(struct sdhci_host *host)
 			| CORE_DLL_PDN), host->ioaddr + CORE_DLL_CONFIG);
 	msm_cm_dll_set_freq(host);
 
+	if (msm_host->use_updated_dll_reset) {
+		u32 mclk_freq = 0;
+
+		if ((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG_2)
+					& CORE_FLL_CYCLE_CNT))
+			mclk_freq = (u32) ((host->clock / TCXO_FREQ) * 8);
+		else
+			mclk_freq = (u32) ((host->clock / TCXO_FREQ) * 4);
+
+		writel_relaxed(((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG_2)
+				& ~(0xFF << 10)) | (mclk_freq << 10)),
+				host->ioaddr + CORE_DLL_CONFIG_2);
+		/* wait for 5us before enabling DLL clock */
+		udelay(5);
+	}
+
+
 	/* Write 0 to DLL_RST bit of DLL_CONFIG register */
 	writel_relaxed((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG)
 			& ~CORE_DLL_RST), host->ioaddr + CORE_DLL_CONFIG);
@@ -908,6 +948,14 @@ static int msm_init_cm_dll(struct sdhci_host *host)
 	writel_relaxed((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG)
 			& ~CORE_DLL_PDN), host->ioaddr + CORE_DLL_CONFIG);
 
+	if (msm_host->use_updated_dll_reset) {
+		msm_cm_dll_set_freq(host);
+		/* Enable the DLL clock */
+		writel_relaxed((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG_2)
+				& ~CORE_DLL_CLOCK_DISABLE),
+				host->ioaddr + CORE_DLL_CONFIG_2);
+	}
+
 	/* Set DLL_EN bit to 1. */
 	writel_relaxed((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG)
 			| CORE_DLL_EN), host->ioaddr + CORE_DLL_CONFIG);
@@ -1135,6 +1183,9 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 	dev_dbg(&pdev->dev, "MCI Version: 0x%08x, major: 0x%04x, minor: 0x%02x\n",
 		core_version, core_major, core_minor);
 
+	if ((core_major == 1) && (core_minor >= 0x42))
+		msm_host->use_updated_dll_reset = true;
+
 	/*
 	 * Support for some capabilities is not advertised by newer
 	 * controller versions and must be explicitly enabled.
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

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

* Re: [PATCH RFC 2/8] mmc: sdhci-msm: Fix the regulator binding name
  2016-06-29 11:20 ` [PATCH RFC 2/8] mmc: sdhci-msm: Fix the regulator binding name Ritesh Harjani
@ 2016-06-29 21:48   ` Andy Gross
  2016-06-30 13:05     ` Ritesh Harjani
  0 siblings, 1 reply; 18+ messages in thread
From: Andy Gross @ 2016-06-29 21:48 UTC (permalink / raw)
  To: Ritesh Harjani
  Cc: ulf.hansson, linux-mmc, linux-arm-msm, adrian.hunter, asutoshd,
	kdorfman, david.griego, stummala, venkatg

On Wed, Jun 29, 2016 at 04:50:27PM +0530, Ritesh Harjani wrote:
> QUALCOMM sdhci-msm driver follows a pwr_irq method
> (deviation from regular standard) for any pwr related
> operations (which also involves regulator settings changes).
> 
> vmmc/vqmmc regulator names are used by core layer,
> so change the names to vdd/vdd-io so that it can
> be used by sdhci-msm driver alone.
> 
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> index 485483a..851e66d 100644
> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> @@ -27,8 +27,8 @@ Example:
>  		bus-width = <8>;
>  		non-removable;
>  
> -		vmmc-supply = <&pm8941_l20>;
> -		vqmmc-supply = <&pm8941_s3>;
> +		vdd-supply = <&pm8941_l20>;
> +		vdd-io-supply = <&pm8941_s3>;

If you are adding supplies, you need to specify them in the required properties.
And if these properties are required by a specific compat you need to add the
compat.  Because this binding has consumers, you need to think about them as
well.

Is the pwr_irq stuff still part of the v4 controller?  If not and this is a new
revision, perhaps a new binding/compat is required.  This would make dealing
with it in the driver easier as well.

>  
>  		pinctrl-names = "default";
>  		pinctrl-0 = <&sdc1_clk &sdc1_cmd &sdc1_data>;
> @@ -44,8 +44,8 @@ Example:
>  		bus-width = <4>;
>  		cd-gpios = <&msmgpio 62 0x1>;
>  
> -		vmmc-supply = <&pm8941_l21>;
> -		vqmmc-supply = <&pm8941_l13>;
> +		vdd-supply = <&pm8941_l21>;
> +		vdd-io-supply = <&pm8941_l13>;

ditto above

>  
>  		pinctrl-names = "default";
>  		pinctrl-0 = <&sdc2_clk &sdc2_cmd &sdc2_data>;

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

* Re: [PATCH RFC 4/8] mmc: sdhci-msm: Add regulator DT props to sdhci-msm bindings
  2016-06-29 11:20 ` [PATCH RFC 4/8] mmc: sdhci-msm: Add regulator DT props to sdhci-msm bindings Ritesh Harjani
@ 2016-06-29 21:53   ` Andy Gross
  2016-06-30 13:30     ` Ritesh Harjani
  0 siblings, 1 reply; 18+ messages in thread
From: Andy Gross @ 2016-06-29 21:53 UTC (permalink / raw)
  To: Ritesh Harjani
  Cc: ulf.hansson, linux-mmc, linux-arm-msm, adrian.hunter, asutoshd,
	kdorfman, david.griego, stummala, venkatg

On Wed, Jun 29, 2016 at 04:50:29PM +0530, Ritesh Harjani wrote:
> This patch adds the DT properties for voltage regulator nodes
> for Qualcomm SDHCI driver.
> 
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> index 851e66d..32cea75 100644
> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> @@ -17,7 +17,15 @@ Required properties:
>  	"iface" - Main peripheral bus clock (PCLK/HCLK - AHB Bus clock) (required)
>  	"core"	- SDC MMC clock (MCLK) (required)
>  	"bus"	- SDCC bus voter clock (optional)
> +- qcom,<supply>-voltage_level - specifies voltage levels for supply. Should be
> +					specified in pairs (min, max), units uV.
> +- qcom,<supply>-current_level - specifies load levels for supply in lpm or
> +					high power mode (hpm). Should be specified in
> +					pairs (lpm, hpm), units uA.

These seem like OPPs to me.  Why use something non-standard?

Check out Documentation/devicetree/bindings/opp/opp.txt

>  
> +Optional Properties:
> +	- qcom,<supply>-always-on - specifies whether supply should be kept "on" always.

Would this only be the base if mmc is used on this platform?  You could specify
this in the regulator binding itself if this is more of a global thing.

> +	- qcom,<supply>-lpm_sup - specifies whether supply can be kept in low power mode (lpm).
>  Example:
>  
>  	sdhc_1: sdhci@f9824900 {
> @@ -28,7 +36,13 @@ Example:
>  		non-removable;
>  
>  		vdd-supply = <&pm8941_l20>;
> +		qcom,vdd-voltage-level = <2950000 2950000>;
> +		qcom,vdd-current-level = <200 570000>;
> +
>  		vdd-io-supply = <&pm8941_s3>;
> +		qcom,vdd-io-always-on;
> +		qcom,vdd-io-voltage-level = <1800000 1800000>;
> +		qcom,vdd-io-current-level = <110 325000>;
>  
>  		pinctrl-names = "default";
>  		pinctrl-0 = <&sdc1_clk &sdc1_cmd &sdc1_data>;
> @@ -45,7 +59,12 @@ Example:
>  		cd-gpios = <&msmgpio 62 0x1>;
>  
>  		vdd-supply = <&pm8941_l21>;
> +		qcom,vdd-voltage-level = <2950000 2950000>;
> +		qcom,vdd-current-level = <200 800000>;
> +
>  		vdd-io-supply = <&pm8941_l13>;
> +		qcom,vdd-io-voltage-level = <1800000 2950000>;
> +		qcom,vdd-io-current-level = <200 22000>;
>  
>  		pinctrl-names = "default";
>  		pinctrl-0 = <&sdc2_clk &sdc2_cmd &sdc2_data>;

Regards,

Andy

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

* Re: [PATCH RFC 5/8] mmc: sdhci: Add check_power_status host operation
  2016-06-29 11:20 ` [PATCH RFC 5/8] mmc: sdhci: Add check_power_status host operation Ritesh Harjani
@ 2016-06-30  6:00   ` Adrian Hunter
  2016-06-30 13:32     ` Ritesh Harjani
  0 siblings, 1 reply; 18+ messages in thread
From: Adrian Hunter @ 2016-06-30  6:00 UTC (permalink / raw)
  To: Ritesh Harjani, ulf.hansson, linux-mmc
  Cc: linux-arm-msm, asutoshd, kdorfman, david.griego, stummala, venkatg

On 29/06/16 14:20, Ritesh Harjani wrote:
> From: Sahitya Tummala <stummala@codeaurora.org>
> 
> MSM SDHCI doesn't control power as specified by the Standard
> Host Controller 3.0 spec. Writing to power control register/
> reset register/voltage bit of host control register would
> trigger an IRQ with appropriate status bits set. Hence, use
> host op check_power_status after writing to power control
> register to check the status and wait until the IRQ is handled.

Did you consider using the SDHCI I/O Accessors for this? i.e.
CONFIG_MMC_SDHCI_IO_ACCESSORS

> 
> Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
> ---
>  drivers/mmc/host/sdhci.c | 24 +++++++++++++++++++++---
>  drivers/mmc/host/sdhci.h |  5 +++++
>  2 files changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 0e3d7c0..12f74bd 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -176,6 +176,10 @@ void sdhci_reset(struct sdhci_host *host, u8 mask)
>  	/* Wait max 100 ms */
>  	timeout = 100;
>  
> +	if (host->ops->check_power_status && host->pwr &&
> +	    (mask & SDHCI_RESET_ALL))
> +		host->ops->check_power_status(host, REQ_BUS_OFF);
> +
>  	/* hw clears the bit when it's done */
>  	while (sdhci_readb(host, SDHCI_SOFTWARE_RESET) & mask) {
>  		if (timeout == 0) {
> @@ -1306,6 +1310,8 @@ void sdhci_set_power(struct sdhci_host *host, unsigned char mode,
>  
>  	if (pwr == 0) {
>  		sdhci_writeb(host, 0, SDHCI_POWER_CONTROL);
> +		if (host->ops->check_power_status)
> +			host->ops->check_power_status(host, REQ_BUS_OFF);
>  		if (host->quirks2 & SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON)
>  			sdhci_runtime_pm_bus_off(host);
>  	} else {
> @@ -1313,20 +1319,28 @@ void sdhci_set_power(struct sdhci_host *host, unsigned char mode,
>  		 * Spec says that we should clear the power reg before setting
>  		 * a new value. Some controllers don't seem to like this though.
>  		 */
> -		if (!(host->quirks & SDHCI_QUIRK_SINGLE_POWER_WRITE))
> +		if (!(host->quirks & SDHCI_QUIRK_SINGLE_POWER_WRITE)) {
>  			sdhci_writeb(host, 0, SDHCI_POWER_CONTROL);
> -
> +			if (host->ops->check_power_status)
> +				host->ops->check_power_status(host,
> +							REQ_BUS_OFF);
> +		}
>  		/*
>  		 * At least the Marvell CaFe chip gets confused if we set the
>  		 * voltage and set turn on power at the same time, so set the
>  		 * voltage first.
>  		 */
> -		if (host->quirks & SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER)
> +		if (host->quirks & SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER) {
>  			sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL);
> +			if (host->ops->check_power_status)
> +				host->ops->check_power_status(host, REQ_BUS_ON);
> +		}
>  
>  		pwr |= SDHCI_POWER_ON;
>  
>  		sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL);
> +		if (host->ops->check_power_status)
> +			host->ops->check_power_status(host, REQ_BUS_ON);
>  
>  		if (host->quirks2 & SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON)
>  			sdhci_runtime_pm_bus_on(host);
> @@ -1736,6 +1750,8 @@ static int sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
>  		/* Set 1.8V Signal Enable in the Host Control2 register to 0 */
>  		ctrl &= ~SDHCI_CTRL_VDD_180;
>  		sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
> +		if (host->ops->check_power_status)
> +			host->ops->check_power_status(host, REQ_IO_HIGH);
>  
>  		if (!IS_ERR(mmc->supply.vqmmc)) {
>  			ret = regulator_set_voltage(mmc->supply.vqmmc, 2700000,
> @@ -1775,6 +1791,8 @@ static int sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
>  		 */
>  		ctrl |= SDHCI_CTRL_VDD_180;
>  		sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
> +		if (host->ops->check_power_status)
> +			host->ops->check_power_status(host, REQ_IO_LOW);
>  
>  		/* Some controller need to do more when switching */
>  		if (host->ops->voltage_switch)
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index 609f87c..5758cca 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -549,6 +549,11 @@ struct sdhci_ops {
>  					 struct mmc_card *card,
>  					 unsigned int max_dtr, int host_drv,
>  					 int card_drv, int *drv_type);
> +#define REQ_BUS_OFF	BIT(0)
> +#define REQ_BUS_ON	BIT(1)
> +#define REQ_IO_LOW	BIT(2)
> +#define REQ_IO_HIGH	BIT(3)
> +	void    (*check_power_status)(struct sdhci_host *host, u32 req_type);
>  };
>  
>  #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
> 

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

* Re: [PATCH RFC 2/8] mmc: sdhci-msm: Fix the regulator binding name
  2016-06-29 21:48   ` Andy Gross
@ 2016-06-30 13:05     ` Ritesh Harjani
  0 siblings, 0 replies; 18+ messages in thread
From: Ritesh Harjani @ 2016-06-30 13:05 UTC (permalink / raw)
  To: Andy Gross
  Cc: ulf.hansson, linux-mmc, linux-arm-msm, adrian.hunter, asutoshd,
	kdorfman, david.griego, stummala, venkatg

Hi Andy,

Thanks for the review -

On 6/30/2016 3:18 AM, Andy Gross wrote:
> On Wed, Jun 29, 2016 at 04:50:27PM +0530, Ritesh Harjani wrote:
>> QUALCOMM sdhci-msm driver follows a pwr_irq method
>> (deviation from regular standard) for any pwr related
>> operations (which also involves regulator settings changes).
>>
>> vmmc/vqmmc regulator names are used by core layer,
>> so change the names to vdd/vdd-io so that it can
>> be used by sdhci-msm driver alone.
>>
>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>> ---
>>  Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>> index 485483a..851e66d 100644
>> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>> @@ -27,8 +27,8 @@ Example:
>>  		bus-width = <8>;
>>  		non-removable;
>>
>> -		vmmc-supply = <&pm8941_l20>;
>> -		vqmmc-supply = <&pm8941_s3>;
>> +		vdd-supply = <&pm8941_l20>;
>> +		vdd-io-supply = <&pm8941_s3>;
>
> If you are adding supplies, you need to specify them in the required properties.
Ok sure will add this in required properties.

> And if these properties are required by a specific compat you need to add the
> compat.  Because this binding has consumers, you need to think about them as
> well.
I think adding it in required properties should be fine, this is not 
specific to any compat.

>
> Is the pwr_irq stuff still part of the v4 controller?  If not and this is a new
> revision, perhaps a new binding/compat is required.  This would make dealing
> with it in the driver easier as well.
Pwr_irq is part of v4 controller itself.

>
>>
>>  		pinctrl-names = "default";
>>  		pinctrl-0 = <&sdc1_clk &sdc1_cmd &sdc1_data>;
>> @@ -44,8 +44,8 @@ Example:
>>  		bus-width = <4>;
>>  		cd-gpios = <&msmgpio 62 0x1>;
>>
>> -		vmmc-supply = <&pm8941_l21>;
>> -		vqmmc-supply = <&pm8941_l13>;
>> +		vdd-supply = <&pm8941_l21>;
>> +		vdd-io-supply = <&pm8941_l13>;
>
> ditto above
Done.

>
>>
>>  		pinctrl-names = "default";
>>  		pinctrl-0 = <&sdc2_clk &sdc2_cmd &sdc2_data>;

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

* Re: [PATCH RFC 4/8] mmc: sdhci-msm: Add regulator DT props to sdhci-msm bindings
  2016-06-29 21:53   ` Andy Gross
@ 2016-06-30 13:30     ` Ritesh Harjani
  2016-07-01  4:22       ` Andy Gross
  0 siblings, 1 reply; 18+ messages in thread
From: Ritesh Harjani @ 2016-06-30 13:30 UTC (permalink / raw)
  To: Andy Gross
  Cc: ulf.hansson, linux-mmc, linux-arm-msm, adrian.hunter, asutoshd,
	kdorfman, david.griego, stummala, venkatg

Hi Andy,

On 6/30/2016 3:23 AM, Andy Gross wrote:
> On Wed, Jun 29, 2016 at 04:50:29PM +0530, Ritesh Harjani wrote:
>> This patch adds the DT properties for voltage regulator nodes
>> for Qualcomm SDHCI driver.
>>
>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>> ---
>>  Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 19 +++++++++++++++++++
>>  1 file changed, 19 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>> index 851e66d..32cea75 100644
>> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>> @@ -17,7 +17,15 @@ Required properties:
>>  	"iface" - Main peripheral bus clock (PCLK/HCLK - AHB Bus clock) (required)
>>  	"core"	- SDC MMC clock (MCLK) (required)
>>  	"bus"	- SDCC bus voter clock (optional)
>> +- qcom,<supply>-voltage_level - specifies voltage levels for supply. Should be
>> +					specified in pairs (min, max), units uV.
>> +- qcom,<supply>-current_level - specifies load levels for supply in lpm or
>> +					high power mode (hpm). Should be specified in
>> +					pairs (lpm, hpm), units uA.
>
> These seem like OPPs to me.  Why use something non-standard?
>
> Check out Documentation/devicetree/bindings/opp/opp.txt
Isn't OPP used w.r.t. DVFS?
This is voltage/load regulator supplies used to provide vdd/vdd-io to 
card. We require this to switch the I/O voltage while switching to UHS 
card mode or to turn on/off the regulators while not in use, (but not 
tied to voltage switching while different frequency transitions).

This is mentioned in the same way how other drivers uses vmmc/vqmmc, but 
the deviation in the name is since Qcom follows pwr_irq 
deviation(different from standard) to switch the I/O voltage, bus on/off.

>
>>
>> +Optional Properties:
>> +	- qcom,<supply>-always-on - specifies whether supply should be kept "on" always.
>
> Would this only be the base if mmc is used on this platform?  You could specify
> this in the regulator binding itself if this is more of a global thing.
This could be specific to SDHC nodes, like for emmc - we may need only 
vdd-io to be on. So we need this per sdhc node.

>
>> +	- qcom,<supply>-lpm_sup - specifies whether supply can be kept in low power mode (lpm).
>>  Example:
>>
>>  	sdhc_1: sdhci@f9824900 {
>> @@ -28,7 +36,13 @@ Example:
>>  		non-removable;
>>
>>  		vdd-supply = <&pm8941_l20>;
>> +		qcom,vdd-voltage-level = <2950000 2950000>;
>> +		qcom,vdd-current-level = <200 570000>;
>> +
>>  		vdd-io-supply = <&pm8941_s3>;
>> +		qcom,vdd-io-always-on;
>> +		qcom,vdd-io-voltage-level = <1800000 1800000>;
>> +		qcom,vdd-io-current-level = <110 325000>;
>>
>>  		pinctrl-names = "default";
>>  		pinctrl-0 = <&sdc1_clk &sdc1_cmd &sdc1_data>;
>> @@ -45,7 +59,12 @@ Example:
>>  		cd-gpios = <&msmgpio 62 0x1>;
>>
>>  		vdd-supply = <&pm8941_l21>;
>> +		qcom,vdd-voltage-level = <2950000 2950000>;
>> +		qcom,vdd-current-level = <200 800000>;
>> +
>>  		vdd-io-supply = <&pm8941_l13>;
>> +		qcom,vdd-io-voltage-level = <1800000 2950000>;
>> +		qcom,vdd-io-current-level = <200 22000>;
>>
>>  		pinctrl-names = "default";
>>  		pinctrl-0 = <&sdc2_clk &sdc2_cmd &sdc2_data>;
>
> Regards,
>
> Andy
>

--
Regards
Ritesh

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

* Re: [PATCH RFC 5/8] mmc: sdhci: Add check_power_status host operation
  2016-06-30  6:00   ` Adrian Hunter
@ 2016-06-30 13:32     ` Ritesh Harjani
  2016-08-05  4:48       ` Ritesh Harjani
  0 siblings, 1 reply; 18+ messages in thread
From: Ritesh Harjani @ 2016-06-30 13:32 UTC (permalink / raw)
  To: Adrian Hunter, ulf.hansson, linux-mmc
  Cc: linux-arm-msm, asutoshd, kdorfman, david.griego, stummala, venkatg

Hi Adrian,


On 6/30/2016 11:30 AM, Adrian Hunter wrote:
> On 29/06/16 14:20, Ritesh Harjani wrote:
>> From: Sahitya Tummala <stummala@codeaurora.org>
>>
>> MSM SDHCI doesn't control power as specified by the Standard
>> Host Controller 3.0 spec. Writing to power control register/
>> reset register/voltage bit of host control register would
>> trigger an IRQ with appropriate status bits set. Hence, use
>> host op check_power_status after writing to power control
>> register to check the status and wait until the IRQ is handled.
>
> Did you consider using the SDHCI I/O Accessors for this? i.e.
> CONFIG_MMC_SDHCI_IO_ACCESSORS
Thanks for the suggestion. I will check and get back.

>
>>
>> Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>> ---
>>  drivers/mmc/host/sdhci.c | 24 +++++++++++++++++++++---
>>  drivers/mmc/host/sdhci.h |  5 +++++
>>  2 files changed, 26 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index 0e3d7c0..12f74bd 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -176,6 +176,10 @@ void sdhci_reset(struct sdhci_host *host, u8 mask)
>>  	/* Wait max 100 ms */
>>  	timeout = 100;
>>
>> +	if (host->ops->check_power_status && host->pwr &&
>> +	    (mask & SDHCI_RESET_ALL))
>> +		host->ops->check_power_status(host, REQ_BUS_OFF);
>> +
>>  	/* hw clears the bit when it's done */
>>  	while (sdhci_readb(host, SDHCI_SOFTWARE_RESET) & mask) {
>>  		if (timeout == 0) {
>> @@ -1306,6 +1310,8 @@ void sdhci_set_power(struct sdhci_host *host, unsigned char mode,
>>
>>  	if (pwr == 0) {
>>  		sdhci_writeb(host, 0, SDHCI_POWER_CONTROL);
>> +		if (host->ops->check_power_status)
>> +			host->ops->check_power_status(host, REQ_BUS_OFF);
>>  		if (host->quirks2 & SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON)
>>  			sdhci_runtime_pm_bus_off(host);
>>  	} else {
>> @@ -1313,20 +1319,28 @@ void sdhci_set_power(struct sdhci_host *host, unsigned char mode,
>>  		 * Spec says that we should clear the power reg before setting
>>  		 * a new value. Some controllers don't seem to like this though.
>>  		 */
>> -		if (!(host->quirks & SDHCI_QUIRK_SINGLE_POWER_WRITE))
>> +		if (!(host->quirks & SDHCI_QUIRK_SINGLE_POWER_WRITE)) {
>>  			sdhci_writeb(host, 0, SDHCI_POWER_CONTROL);
>> -
>> +			if (host->ops->check_power_status)
>> +				host->ops->check_power_status(host,
>> +							REQ_BUS_OFF);
>> +		}
>>  		/*
>>  		 * At least the Marvell CaFe chip gets confused if we set the
>>  		 * voltage and set turn on power at the same time, so set the
>>  		 * voltage first.
>>  		 */
>> -		if (host->quirks & SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER)
>> +		if (host->quirks & SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER) {
>>  			sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL);
>> +			if (host->ops->check_power_status)
>> +				host->ops->check_power_status(host, REQ_BUS_ON);
>> +		}
>>
>>  		pwr |= SDHCI_POWER_ON;
>>
>>  		sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL);
>> +		if (host->ops->check_power_status)
>> +			host->ops->check_power_status(host, REQ_BUS_ON);
>>
>>  		if (host->quirks2 & SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON)
>>  			sdhci_runtime_pm_bus_on(host);
>> @@ -1736,6 +1750,8 @@ static int sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
>>  		/* Set 1.8V Signal Enable in the Host Control2 register to 0 */
>>  		ctrl &= ~SDHCI_CTRL_VDD_180;
>>  		sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
>> +		if (host->ops->check_power_status)
>> +			host->ops->check_power_status(host, REQ_IO_HIGH);
>>
>>  		if (!IS_ERR(mmc->supply.vqmmc)) {
>>  			ret = regulator_set_voltage(mmc->supply.vqmmc, 2700000,
>> @@ -1775,6 +1791,8 @@ static int sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
>>  		 */
>>  		ctrl |= SDHCI_CTRL_VDD_180;
>>  		sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
>> +		if (host->ops->check_power_status)
>> +			host->ops->check_power_status(host, REQ_IO_LOW);
>>
>>  		/* Some controller need to do more when switching */
>>  		if (host->ops->voltage_switch)
>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>> index 609f87c..5758cca 100644
>> --- a/drivers/mmc/host/sdhci.h
>> +++ b/drivers/mmc/host/sdhci.h
>> @@ -549,6 +549,11 @@ struct sdhci_ops {
>>  					 struct mmc_card *card,
>>  					 unsigned int max_dtr, int host_drv,
>>  					 int card_drv, int *drv_type);
>> +#define REQ_BUS_OFF	BIT(0)
>> +#define REQ_BUS_ON	BIT(1)
>> +#define REQ_IO_LOW	BIT(2)
>> +#define REQ_IO_HIGH	BIT(3)
>> +	void    (*check_power_status)(struct sdhci_host *host, u32 req_type);
>>  };
>>
>>  #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

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

* Re: [PATCH RFC 6/8] mmc: sdhci-msm: Add pwr_irq support to sdhci-msm
  2016-06-29 11:20 ` [PATCH RFC 6/8] mmc: sdhci-msm: Add pwr_irq support to sdhci-msm Ritesh Harjani
@ 2016-07-01  3:57   ` Andy Gross
  0 siblings, 0 replies; 18+ messages in thread
From: Andy Gross @ 2016-07-01  3:57 UTC (permalink / raw)
  To: Ritesh Harjani
  Cc: ulf.hansson, linux-mmc, linux-arm-msm, adrian.hunter, asutoshd,
	kdorfman, david.griego, stummala, venkatg

On Wed, Jun 29, 2016 at 04:50:31PM +0530, Ritesh Harjani wrote:
> From: Asutosh Das <asutoshd@codeaurora.org>

<snip>

> +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);
> +	u8 irq_status = 0;
> +	u8 irq_ack = 0;
> +	int ret = 0;
> +	int pwr_state = 0, io_level = 0;
> +	unsigned long flags;
> +
> +	irq_status = readb_relaxed(msm_host->core_mem + CORE_PWRCTL_STATUS);
> +	pr_debug("%s: Received IRQ(%d), status=0x%x\n",
> +		mmc_hostname(msm_host->mmc), irq, irq_status);
> +
> +	/* Clear the interrupt */
> +	writeb_relaxed(irq_status, (msm_host->core_mem + CORE_PWRCTL_CLEAR));
> +	/*
> +	 * SDHC has core_mem and hc_mem device memory and these memory
> +	 * addresses do not fall within 1KB region. Hence, any update to
> +	 * core_mem address space would require an mb() to ensure this gets
> +	 * completed before its next update to registers within hc_mem.
> +	 */
> +	mb();

mb is a little heavy handed.  In general, these days we don't want to introduce
_relaxed usage anyway.  I know internally you guys force the use of _relaxed,
but in mainline we generally don't.  Wouldn't a wmb() suffice?

Also this isn't about completing before anything else, this is about enforcing
order and making sure this writeb doesn't get put behind the following writel.

Lastly, writeb?  Isn't this a 32 bit wide register?

> +
> +	/* Handle BUS ON/OFF*/
> +	if (irq_status & CORE_PWRCTL_BUS_ON) {
> +		ret = sdhci_msm_setup_vreg(msm_host->pdata, true, false);
> +		if (!ret) {
> +			ret = sdhci_msm_set_vdd_io_vol(msm_host->pdata,
> +					VDD_IO_HIGH, 0);
> +		}
> +		if (ret)
> +			irq_ack |= CORE_PWRCTL_BUS_FAIL;
> +		else
> +			irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
> +
> +		pwr_state = REQ_BUS_ON;
> +		io_level = REQ_IO_HIGH;
> +	}
> +	if (irq_status & CORE_PWRCTL_BUS_OFF) {
> +		ret = sdhci_msm_setup_vreg(msm_host->pdata, false, false);
> +		if (!ret) {
> +			ret = sdhci_msm_set_vdd_io_vol(msm_host->pdata,
> +					VDD_IO_LOW, 0);
> +		}
> +		if (ret)
> +			irq_ack |= CORE_PWRCTL_BUS_FAIL;
> +		else
> +			irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
> +
> +		pwr_state = REQ_BUS_OFF;
> +		io_level = REQ_IO_LOW;
> +	}
> +	/* Handle IO LOW/HIGH */
> +	if (irq_status & CORE_PWRCTL_IO_LOW) {
> +		/* Switch voltage Low */
> +		ret = sdhci_msm_set_vdd_io_vol(msm_host->pdata, VDD_IO_LOW, 0);
> +		if (ret)
> +			irq_ack |= CORE_PWRCTL_IO_FAIL;
> +		else
> +			irq_ack |= CORE_PWRCTL_IO_SUCCESS;
> +
> +		io_level = REQ_IO_LOW;
> +	}
> +	if (irq_status & CORE_PWRCTL_IO_HIGH) {
> +		/* Switch voltage High */
> +		ret = sdhci_msm_set_vdd_io_vol(msm_host->pdata, VDD_IO_HIGH, 0);
> +		if (ret)
> +			irq_ack |= CORE_PWRCTL_IO_FAIL;
> +		else
> +			irq_ack |= CORE_PWRCTL_IO_SUCCESS;
> +
> +		io_level = REQ_IO_HIGH;
> +	}
> +
> +	/* ACK status to the core */
> +	writeb_relaxed(irq_ack, (msm_host->core_mem + CORE_PWRCTL_CTL));
> +	/*
> +	 * SDHC has core_mem and hc_mem device memory and these memory
> +	 * addresses do not fall within 1KB region. Hence, any update to
> +	 * core_mem address space would require an mb() to ensure this gets
> +	 * completed before its next update to registers within hc_mem.
> +	 */
> +	mb();

ditto from above

> +
> +	if (io_level & REQ_IO_HIGH)
> +		writel_relaxed((readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC) &
> +				~CORE_IO_PAD_PWR_SWITCH),
> +				host->ioaddr + CORE_VENDOR_SPEC);
> +	else if (io_level & REQ_IO_LOW)
> +		writel_relaxed((readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC) |
> +				CORE_IO_PAD_PWR_SWITCH),
> +				host->ioaddr + CORE_VENDOR_SPEC);
> +
> +	/* Ensure before completion that above writes are propagated */
> +	mb();

ditto.  And to make matters worse, this is a double mb() possibly without
anything else going on.

> +
> +	pr_debug("%s: Handled IRQ(%d), ret=%d, ack=0x%x\n",
> +		mmc_hostname(msm_host->mmc), irq, ret, irq_ack);
> +	if (pwr_state)
> +		msm_host->curr_pwr_state = pwr_state;
> +	if (io_level)
> +		msm_host->curr_io_level = io_level;
> +
> +	return IRQ_HANDLED;
> +}
> +
>  /* Platform specific tuning */
>  static inline int msm_dll_poll_ck_out_en(struct sdhci_host *host, u8 poll)
>  {
> @@ -801,6 +978,7 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  	struct sdhci_pltfm_host *pltfm_host;
>  	struct sdhci_msm_host *msm_host;
>  	struct resource *core_memres;
> +	u32 irq_status, irq_ctl;
>  	int ret;
>  	u16 host_version, core_minor;
>  	u32 core_version, caps;
> @@ -917,6 +1095,45 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  			       CORE_VENDOR_SPEC_CAPABILITIES0);
>  	}
>  
> +	/*
> +	 * POR reset of VENDOR_SPEC/CORE_SW_RST above 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.
> +	 */
> +	irq_status = readl_relaxed(msm_host->core_mem + CORE_PWRCTL_STATUS);
> +	writel_relaxed(irq_status, (msm_host->core_mem + CORE_PWRCTL_CLEAR));
> +	irq_ctl = readl_relaxed(msm_host->core_mem + CORE_PWRCTL_CTL);
> +	if (irq_status & (CORE_PWRCTL_BUS_ON | CORE_PWRCTL_BUS_OFF))
> +		irq_ctl |= CORE_PWRCTL_BUS_SUCCESS;
> +	if (irq_status & (CORE_PWRCTL_IO_HIGH | CORE_PWRCTL_IO_LOW))
> +		irq_ctl |= CORE_PWRCTL_IO_SUCCESS;
> +	writel_relaxed(irq_ctl, (msm_host->core_mem + CORE_PWRCTL_CTL));
> +
> +	/*
> +	 * Ensure that above writes are propogated before interrupt enablement
> +	 * in GIC.
> +	 */
> +	mb();

this comment is better.  but again, wmb?

> +	/* Setup PWRCTL irq */
> +	msm_host->pwr_irq = platform_get_irq_byname(pdev, "pwr_irq");
> +	if (msm_host->pwr_irq < 0) {
> +		dev_err(&pdev->dev, "Failed to get pwr_irq by name (%d)\n",
> +				msm_host->pwr_irq);
> +		goto vreg_deinit;
> +	}
> +	ret = devm_request_threaded_irq(&pdev->dev, msm_host->pwr_irq, NULL,
> +					sdhci_msm_pwr_irq, IRQF_ONESHOT,
> +					dev_name(&pdev->dev), host);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Request threaded irq(%d) failed (%d)\n",
> +				msm_host->pwr_irq, ret);
> +		goto vreg_deinit;
> +	}
> +
>  	ret = sdhci_add_host(host);
>  	if (ret)
>  		goto vreg_deinit;

Regards,

Andy

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

* Re: [PATCH RFC 4/8] mmc: sdhci-msm: Add regulator DT props to sdhci-msm bindings
  2016-06-30 13:30     ` Ritesh Harjani
@ 2016-07-01  4:22       ` Andy Gross
  0 siblings, 0 replies; 18+ messages in thread
From: Andy Gross @ 2016-07-01  4:22 UTC (permalink / raw)
  To: Ritesh Harjani
  Cc: ulf.hansson, linux-mmc, linux-arm-msm, adrian.hunter, asutoshd,
	kdorfman, david.griego, stummala, venkatg

On Thu, Jun 30, 2016 at 07:00:54PM +0530, Ritesh Harjani wrote:
> Hi Andy,
> 
> On 6/30/2016 3:23 AM, Andy Gross wrote:
> >On Wed, Jun 29, 2016 at 04:50:29PM +0530, Ritesh Harjani wrote:
> >>This patch adds the DT properties for voltage regulator nodes
> >>for Qualcomm SDHCI driver.
> >>
> >>Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
> >>---
> >> Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 19 +++++++++++++++++++
> >> 1 file changed, 19 insertions(+)
> >>
> >>diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> >>index 851e66d..32cea75 100644
> >>--- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> >>+++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> >>@@ -17,7 +17,15 @@ Required properties:
> >> 	"iface" - Main peripheral bus clock (PCLK/HCLK - AHB Bus clock) (required)
> >> 	"core"	- SDC MMC clock (MCLK) (required)
> >> 	"bus"	- SDCC bus voter clock (optional)
> >>+- qcom,<supply>-voltage_level - specifies voltage levels for supply. Should be
> >>+					specified in pairs (min, max), units uV.
> >>+- qcom,<supply>-current_level - specifies load levels for supply in lpm or
> >>+					high power mode (hpm). Should be specified in
> >>+					pairs (lpm, hpm), units uA.
> >
> >These seem like OPPs to me.  Why use something non-standard?
> >
> >Check out Documentation/devicetree/bindings/opp/opp.txt
> Isn't OPP used w.r.t. DVFS?
> This is voltage/load regulator supplies used to provide vdd/vdd-io to card.
> We require this to switch the I/O voltage while switching to UHS card mode
> or to turn on/off the regulators while not in use, (but not tied to voltage
> switching while different frequency transitions).

It isn't necessarily tied to DVFS.  Someone else can chime in if I am wrong
here.

I was expecting these different voltage points to be tied to the speed/mode.  As
such it would kind of fall under the normal OPP definition.  Same with the load.

> 
> This is mentioned in the same way how other drivers uses vmmc/vqmmc, but the
> deviation in the name is since Qcom follows pwr_irq deviation(different from
> standard) to switch the I/O voltage, bus on/off.

Ok.  Fair enough.

> 
> >
> >>
> >>+Optional Properties:
> >>+	- qcom,<supply>-always-on - specifies whether supply should be kept "on" always.
> >
> >Would this only be the base if mmc is used on this platform?  You could specify
> >this in the regulator binding itself if this is more of a global thing.
> This could be specific to SDHC nodes, like for emmc - we may need only
> vdd-io to be on. So we need this per sdhc node.

Ok.  thanks for the clarification


Regards,

Andy

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

* Re: [PATCH RFC 5/8] mmc: sdhci: Add check_power_status host operation
  2016-06-30 13:32     ` Ritesh Harjani
@ 2016-08-05  4:48       ` Ritesh Harjani
  0 siblings, 0 replies; 18+ messages in thread
From: Ritesh Harjani @ 2016-08-05  4:48 UTC (permalink / raw)
  To: Adrian Hunter, ulf.hansson, linux-mmc
  Cc: linux-arm-msm, asutoshd, kdorfman, david.griego, stummala, venkatg

Hi,

Kindly drop this patch series, since recently there was another version 
of pwr_irq support added to upstream.

https://lkml.org/lkml/2016/6/24/416


Regards
Ritesh

On 6/30/2016 7:02 PM, Ritesh Harjani wrote:
> Hi Adrian,
>
>
> On 6/30/2016 11:30 AM, Adrian Hunter wrote:
>> On 29/06/16 14:20, Ritesh Harjani wrote:
>>> From: Sahitya Tummala <stummala@codeaurora.org>
>>>
>>> MSM SDHCI doesn't control power as specified by the Standard
>>> Host Controller 3.0 spec. Writing to power control register/
>>> reset register/voltage bit of host control register would
>>> trigger an IRQ with appropriate status bits set. Hence, use
>>> host op check_power_status after writing to power control
>>> register to check the status and wait until the IRQ is handled.
>>
>> Did you consider using the SDHCI I/O Accessors for this? i.e.
>> CONFIG_MMC_SDHCI_IO_ACCESSORS
> Thanks for the suggestion. I will check and get back.
>
>>
>>>
>>> Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
>>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>>> ---
>>>  drivers/mmc/host/sdhci.c | 24 +++++++++++++++++++++---
>>>  drivers/mmc/host/sdhci.h |  5 +++++
>>>  2 files changed, 26 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index 0e3d7c0..12f74bd 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -176,6 +176,10 @@ void sdhci_reset(struct sdhci_host *host, u8 mask)
>>>      /* Wait max 100 ms */
>>>      timeout = 100;
>>>
>>> +    if (host->ops->check_power_status && host->pwr &&
>>> +        (mask & SDHCI_RESET_ALL))
>>> +        host->ops->check_power_status(host, REQ_BUS_OFF);
>>> +
>>>      /* hw clears the bit when it's done */
>>>      while (sdhci_readb(host, SDHCI_SOFTWARE_RESET) & mask) {
>>>          if (timeout == 0) {
>>> @@ -1306,6 +1310,8 @@ void sdhci_set_power(struct sdhci_host *host,
>>> unsigned char mode,
>>>
>>>      if (pwr == 0) {
>>>          sdhci_writeb(host, 0, SDHCI_POWER_CONTROL);
>>> +        if (host->ops->check_power_status)
>>> +            host->ops->check_power_status(host, REQ_BUS_OFF);
>>>          if (host->quirks2 & SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON)
>>>              sdhci_runtime_pm_bus_off(host);
>>>      } else {
>>> @@ -1313,20 +1319,28 @@ void sdhci_set_power(struct sdhci_host *host,
>>> unsigned char mode,
>>>           * Spec says that we should clear the power reg before setting
>>>           * a new value. Some controllers don't seem to like this
>>> though.
>>>           */
>>> -        if (!(host->quirks & SDHCI_QUIRK_SINGLE_POWER_WRITE))
>>> +        if (!(host->quirks & SDHCI_QUIRK_SINGLE_POWER_WRITE)) {
>>>              sdhci_writeb(host, 0, SDHCI_POWER_CONTROL);
>>> -
>>> +            if (host->ops->check_power_status)
>>> +                host->ops->check_power_status(host,
>>> +                            REQ_BUS_OFF);
>>> +        }
>>>          /*
>>>           * At least the Marvell CaFe chip gets confused if we set the
>>>           * voltage and set turn on power at the same time, so set the
>>>           * voltage first.
>>>           */
>>> -        if (host->quirks & SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER)
>>> +        if (host->quirks & SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER) {
>>>              sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL);
>>> +            if (host->ops->check_power_status)
>>> +                host->ops->check_power_status(host, REQ_BUS_ON);
>>> +        }
>>>
>>>          pwr |= SDHCI_POWER_ON;
>>>
>>>          sdhci_writeb(host, pwr, SDHCI_POWER_CONTROL);
>>> +        if (host->ops->check_power_status)
>>> +            host->ops->check_power_status(host, REQ_BUS_ON);
>>>
>>>          if (host->quirks2 & SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON)
>>>              sdhci_runtime_pm_bus_on(host);
>>> @@ -1736,6 +1750,8 @@ static int
>>> sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
>>>          /* Set 1.8V Signal Enable in the Host Control2 register to 0 */
>>>          ctrl &= ~SDHCI_CTRL_VDD_180;
>>>          sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
>>> +        if (host->ops->check_power_status)
>>> +            host->ops->check_power_status(host, REQ_IO_HIGH);
>>>
>>>          if (!IS_ERR(mmc->supply.vqmmc)) {
>>>              ret = regulator_set_voltage(mmc->supply.vqmmc, 2700000,
>>> @@ -1775,6 +1791,8 @@ static int
>>> sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
>>>           */
>>>          ctrl |= SDHCI_CTRL_VDD_180;
>>>          sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
>>> +        if (host->ops->check_power_status)
>>> +            host->ops->check_power_status(host, REQ_IO_LOW);
>>>
>>>          /* Some controller need to do more when switching */
>>>          if (host->ops->voltage_switch)
>>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>>> index 609f87c..5758cca 100644
>>> --- a/drivers/mmc/host/sdhci.h
>>> +++ b/drivers/mmc/host/sdhci.h
>>> @@ -549,6 +549,11 @@ struct sdhci_ops {
>>>                       struct mmc_card *card,
>>>                       unsigned int max_dtr, int host_drv,
>>>                       int card_drv, int *drv_type);
>>> +#define REQ_BUS_OFF    BIT(0)
>>> +#define REQ_BUS_ON    BIT(1)
>>> +#define REQ_IO_LOW    BIT(2)
>>> +#define REQ_IO_HIGH    BIT(3)
>>> +    void    (*check_power_status)(struct sdhci_host *host, u32
>>> req_type);
>>>  };
>>>
>>>  #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>

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

end of thread, other threads:[~2016-08-05  4:48 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-29 11:20 [PATCH RFC 0/8] mmc: sdhci-msm: Add additional support to sdhci-msm driver Ritesh Harjani
2016-06-29 11:20 ` [PATCH RFC 1/8] mmc: sdhci-msm: Reset vendor specific func register on probe Ritesh Harjani
2016-06-29 11:20 ` [PATCH RFC 2/8] mmc: sdhci-msm: Fix the regulator binding name Ritesh Harjani
2016-06-29 21:48   ` Andy Gross
2016-06-30 13:05     ` Ritesh Harjani
2016-06-29 11:20 ` [PATCH RFC 3/8] mmc: sdhci-msm: Add DT parsing for regulator support Ritesh Harjani
2016-06-29 11:20 ` [PATCH RFC 4/8] mmc: sdhci-msm: Add regulator DT props to sdhci-msm bindings Ritesh Harjani
2016-06-29 21:53   ` Andy Gross
2016-06-30 13:30     ` Ritesh Harjani
2016-07-01  4:22       ` Andy Gross
2016-06-29 11:20 ` [PATCH RFC 5/8] mmc: sdhci: Add check_power_status host operation Ritesh Harjani
2016-06-30  6:00   ` Adrian Hunter
2016-06-30 13:32     ` Ritesh Harjani
2016-08-05  4:48       ` Ritesh Harjani
2016-06-29 11:20 ` [PATCH RFC 6/8] mmc: sdhci-msm: Add pwr_irq support to sdhci-msm Ritesh Harjani
2016-07-01  3:57   ` Andy Gross
2016-06-29 11:20 ` [PATCH RFC 7/8] mmc: sdhci-msm: Add check_power_status " Ritesh Harjani
2016-06-29 11:20 ` [PATCH RFC 8/8] mmc: sdhci-msm: Update DLL reset sequence Ritesh Harjani

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.